diff --git a/cmd/vsphere/main.go b/cmd/vsphere/main.go index 5b30fa4838..5b5771a290 100644 --- a/cmd/vsphere/main.go +++ b/cmd/vsphere/main.go @@ -173,7 +173,7 @@ func main() { EventRecorder: mgr.GetEventRecorderFor("vspherecontroller"), TaskIDCache: taskIDCache, FeatureGates: defaultMutableGate, - OpenshiftConfigNamespace: vsphere.OpenshiftConfigNamespace, + OpenshiftConfigNamespace: vsphere.OpenshiftConfigManagedNamespace, }) if err := configv1.Install(mgr.GetScheme()); err != nil { diff --git a/go.mod b/go.mod index 2245eede48..aab45bc764 100644 --- a/go.mod +++ b/go.mod @@ -369,3 +369,5 @@ require ( sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.6.0 // indirect ) + +replace github.com/openshift/api => github.com/vr4manta/api v0.0.0-20260914125654-86c3aa9e7ae5 diff --git a/go.sum b/go.sum index 1d98f7e0d4..9d36d068eb 100644 --- a/go.sum +++ b/go.sum @@ -457,8 +457,6 @@ github.com/opencontainers/selinux v1.11.1 h1:nHFvthhM0qY8/m+vfhJylliSshm8G1jJ2jD github.com/opencontainers/selinux v1.11.1/go.mod h1:E5dMC3VPuVvVHDYmi78qvhJp8+M586T4DlDRYpFkyec= github.com/openshift-eng/openshift-tests-extension v0.0.0-20250711173707-dc2a20e5a5f8 h1:D+Qga9nujuIcrAjcAuKPukoUcVBl6ZDEbtgNLgKKlgY= github.com/openshift-eng/openshift-tests-extension v0.0.0-20250711173707-dc2a20e5a5f8/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M= -github.com/openshift/api v0.0.0-20260605122244-2c1c5b39566d h1:Djz4aG2W0ypAHQEtubqbXpxeWGwdCds05w6dfz1GVWk= -github.com/openshift/api v0.0.0-20260605122244-2c1c5b39566d/go.mod h1:SPLf21TYPipzCO67BURkCfK6dcIIxx0oNRVWaOyRcXM= github.com/openshift/client-go v0.0.0-20250710075018-396b36f983ee h1:tOtrrxfDEW8hK3eEsHqxsXurq/D6LcINGfprkQC3hqY= github.com/openshift/client-go v0.0.0-20250710075018-396b36f983ee/go.mod h1:zhRiYyNMk89llof2qEuGPWPD+joQPhCRUc2IK0SB510= github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20250718085303-e712b1ebf374 h1:ldUi0e64kdYJC2+ucB24GRXIXfMnI3NpSkcnalPqBGo= @@ -625,6 +623,8 @@ github.com/uudashr/iface v1.3.1 h1:bA51vmVx1UIhiIsQFSNq6GZ6VPTk3WNMZgRiCe9R29U= github.com/uudashr/iface v1.3.1/go.mod h1:4QvspiRd3JLPAEXBQ9AiZpLbJlrWWgRChOKDJEuQTdg= github.com/vmware/govmomi v0.46.3 h1:zBn42Rl0WZBFhGao8Dy0MFRkbE4YNPqOu0OBd+ww6VM= github.com/vmware/govmomi v0.46.3/go.mod h1:uoLVU9zlXC4p4GmLVG+ZJmBC0Gn3Q7mytOJvi39OhxA= +github.com/vr4manta/api v0.0.0-20260914125654-86c3aa9e7ae5 h1:C+HzTIb9DqFgnJwZ7ax8zq3WAyFGaRrEKPwk19HHGDo= +github.com/vr4manta/api v0.0.0-20260914125654-86c3aa9e7ae5/go.mod h1:SPLf21TYPipzCO67BURkCfK6dcIIxx0oNRVWaOyRcXM= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xen0n/gosmopolitan v1.2.2 h1:/p2KTnMzwRexIW8GlKawsTWOxn7UHA+jCMF/V8HHtvU= diff --git a/install/0000_30_machine-api-operator_09_rbac.yaml b/install/0000_30_machine-api-operator_09_rbac.yaml index ae24aa41fe..5df41bcae7 100644 --- a/install/0000_30_machine-api-operator_09_rbac.yaml +++ b/install/0000_30_machine-api-operator_09_rbac.yaml @@ -450,6 +450,8 @@ rules: resources: - validatingwebhookconfigurations - mutatingwebhookconfigurations + - validatingadmissionpolicies + - validatingadmissionpolicybindings verbs: - get - list @@ -457,6 +459,20 @@ rules: - create - update + # The machine-api-operator service account must be able to read Machines, + # ControlPlaneMachineSets, and MachineSets so the kube-apiserver can resolve + # them as VAP params when validating Infrastructure/cluster updates. + - apiGroups: + - machine.openshift.io + resources: + - machines + - controlplanemachinesets + - machinesets + verbs: + - get + - list + - watch + --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/pkg/controller/vsphere/actuator_test.go b/pkg/controller/vsphere/actuator_test.go index 3f10129b23..53135b9379 100644 --- a/pkg/controller/vsphere/actuator_test.go +++ b/pkg/controller/vsphere/actuator_test.go @@ -137,11 +137,11 @@ func TestMachineEvents(t *testing.T) { testConfig := fmt.Sprintf(testConfigFmt, port, credentialsSecretName, testNamespaceName) configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "testname", + Name: OpenshiftConfigManagedConfigMap, Namespace: openshiftConfigNamespaceForTest, }, Data: map[string]string{ - "testkey": testConfig, + OpenshiftConfigManagedCloudConfigKey: testConfig, }, } @@ -150,22 +150,6 @@ func TestMachineEvents(t *testing.T) { g.Expect(k8sClient.Delete(context.Background(), configMap)).To(Succeed()) }() - infra := &configv1.Infrastructure{ - ObjectMeta: metav1.ObjectMeta{ - Name: globalInfrastuctureName, - }, - Spec: configv1.InfrastructureSpec{ - CloudConfig: configv1.ConfigMapFileReference{ - Name: "testname", - Key: "testkey", - }, - }, - } - g.Expect(k8sClient.Create(context.Background(), infra)).To(Succeed()) - defer func() { - g.Expect(k8sClient.Delete(context.Background(), infra)).To(Succeed()) - }() - userDataSecretName := "vsphere-ignition" userDataSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/controller/vsphere/machine_scope_test.go b/pkg/controller/vsphere/machine_scope_test.go index ffad8f885d..2337a7a110 100644 --- a/pkg/controller/vsphere/machine_scope_test.go +++ b/pkg/controller/vsphere/machine_scope_test.go @@ -10,7 +10,6 @@ import ( "time" . "github.com/onsi/gomega" - configv1 "github.com/openshift/api/config/v1" machinev1 "github.com/openshift/api/machine/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -355,11 +354,11 @@ func TestPatchMachine(t *testing.T) { testConfig := fmt.Sprintf(testConfigFmt, port, credentialsSecretName, testNamespaceName) configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "testname", + Name: OpenshiftConfigManagedConfigMap, Namespace: openshiftConfigNamespaceForTest, }, Data: map[string]string{ - "testkey": testConfig, + OpenshiftConfigManagedCloudConfigKey: testConfig, }, } g.Expect(k8sClient.Create(ctx, configMap)).To(Succeed()) @@ -367,22 +366,6 @@ func TestPatchMachine(t *testing.T) { g.Expect(k8sClient.Delete(ctx, configMap)).To(Succeed()) }() - infra := &configv1.Infrastructure{ - ObjectMeta: metav1.ObjectMeta{ - Name: globalInfrastuctureName, - }, - Spec: configv1.InfrastructureSpec{ - CloudConfig: configv1.ConfigMapFileReference{ - Name: "testname", - Key: "testkey", - }, - }, - } - g.Expect(k8sClient.Create(ctx, infra)).To(Succeed()) - defer func() { - g.Expect(k8sClient.Delete(ctx, infra)).To(Succeed()) - }() - failedPhase := "Failed" providerStatus := &machinev1.VSphereMachineProviderStatus{} diff --git a/pkg/controller/vsphere/reconciler_test.go b/pkg/controller/vsphere/reconciler_test.go index b60252f583..ef95403621 100644 --- a/pkg/controller/vsphere/reconciler_test.go +++ b/pkg/controller/vsphere/reconciler_test.go @@ -1948,7 +1948,6 @@ func TestConvertUUIDToProviderID(t *testing.T) { func TestDelete(t *testing.T) { type vCenterSimConfig struct { - infra *configv1.Infrastructure secret *corev1.Secret configMap *corev1.ConfigMap featureGate *configv1.FeatureGate @@ -2000,23 +1999,11 @@ func TestDelete(t *testing.T) { testConfig := fmt.Sprintf(testConfigFmt, port, credentialsSecretName, ns) configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "testName", + Name: OpenshiftConfigManagedConfigMap, Namespace: openshiftConfigNamespaceForTest, }, Data: map[string]string{ - "testKey": testConfig, - }, - } - - infra := &configv1.Infrastructure{ - ObjectMeta: metav1.ObjectMeta{ - Name: globalInfrastuctureName, - }, - Spec: configv1.InfrastructureSpec{ - CloudConfig: configv1.ConfigMapFileReference{ - Name: "testName", - Key: "testKey", - }, + OpenshiftConfigManagedCloudConfigKey: testConfig, }, } @@ -2027,7 +2014,6 @@ func TestDelete(t *testing.T) { } return &vCenterSimConfig{ - infra: infra, secret: secret, configMap: configMap, host: host, @@ -2179,7 +2165,6 @@ func TestDelete(t *testing.T) { simParams.secret, tc.machine(t, simParams.host), simParams.configMap, - simParams.infra, tc.node(t)).Build() machineScope, err := newMachineScope(machineScopeParams{ client: client, @@ -2459,7 +2444,6 @@ func TestDelete(t *testing.T) { simParams.secret, tc.machine(t, simParams.host), simParams.configMap, - simParams.infra, tc.node(t), ).Build() mScope, err := newMachineScope(machineScopeParams{ @@ -2529,23 +2513,11 @@ func TestCreate(t *testing.T) { testConfig := fmt.Sprintf(testConfigFmt, port, credentialsSecretName, namespace) configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "testName", + Name: OpenshiftConfigManagedConfigMap, Namespace: openshiftConfigNamespaceForTest, }, Data: map[string]string{ - "testKey": testConfig, - }, - } - - infra := &configv1.Infrastructure{ - ObjectMeta: metav1.ObjectMeta{ - Name: globalInfrastuctureName, - }, - Spec: configv1.InfrastructureSpec{ - CloudConfig: configv1.ConfigMapFileReference{ - Name: "testName", - Key: "testKey", - }, + OpenshiftConfigManagedCloudConfigKey: testConfig, }, } @@ -2888,7 +2860,6 @@ func TestCreate(t *testing.T) { builder := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects( credentialsSecret, configMap, - infra, userDataSecret, machine) if tc.ipAddressClaim != nil { @@ -3025,23 +2996,11 @@ func TestUpdate(t *testing.T) { testConfig := fmt.Sprintf(testConfigFmt, port, credentialsSecretName, namespace) configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "testName", + Name: OpenshiftConfigManagedConfigMap, Namespace: openshiftConfigNamespaceForTest, }, Data: map[string]string{ - "testKey": testConfig, - }, - } - - infra := &configv1.Infrastructure{ - ObjectMeta: metav1.ObjectMeta{ - Name: globalInfrastuctureName, - }, - Spec: configv1.InfrastructureSpec{ - CloudConfig: configv1.ConfigMapFileReference{ - Name: "testName", - Key: "testKey", - }, + OpenshiftConfigManagedCloudConfigKey: testConfig, }, } @@ -3111,8 +3070,7 @@ func TestUpdate(t *testing.T) { client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects( credentialsSecret, - configMap, - infra).Build() + configMap).Build() rawProviderSpec, err := RawExtensionFromProviderSpec(&tc.providerSpec) if err != nil { diff --git a/pkg/controller/vsphere/util.go b/pkg/controller/vsphere/util.go index 8b7f3da8a5..f2afe8b9c9 100644 --- a/pkg/controller/vsphere/util.go +++ b/pkg/controller/vsphere/util.go @@ -20,8 +20,10 @@ import ( ) const ( - globalInfrastuctureName = "cluster" - OpenshiftConfigNamespace = "openshift-config" + globalInfrastuctureName = "cluster" + OpenshiftConfigManagedNamespace = "openshift-config-managed" + OpenshiftConfigManagedConfigMap = "kube-cloud-config" + OpenshiftConfigManagedCloudConfigKey = "cloud.conf" ) // GetInfrastructure retrieves the Infrastructure object from the provided API reader. @@ -46,22 +48,13 @@ func getVSphereConfig(c runtimeclient.Reader, configNamespace string) (*vsphere. return nil, errors.New("no API reader -- will not fetch vSphere config") } - infra, err := GetInfrastructure(c) - if err != nil { - return nil, err - } - - if infra.Spec.CloudConfig.Name == "" { - return nil, fmt.Errorf("cluster infrastructure CloudConfig has empty name") - } - - if infra.Spec.CloudConfig.Key == "" { - return nil, fmt.Errorf("cluster infrastructure CloudConfig has empty key") + if configNamespace == "" { + configNamespace = OpenshiftConfigManagedNamespace } cm := &corev1.ConfigMap{} cmName := runtimeclient.ObjectKey{ - Name: infra.Spec.CloudConfig.Name, + Name: OpenshiftConfigManagedConfigMap, Namespace: configNamespace, } @@ -69,10 +62,10 @@ func getVSphereConfig(c runtimeclient.Reader, configNamespace string) (*vsphere. return nil, err } - cloudConfig, found := cm.Data[infra.Spec.CloudConfig.Key] + cloudConfig, found := cm.Data[OpenshiftConfigManagedCloudConfigKey] if !found { return nil, fmt.Errorf("cloud-config ConfigMap has no %q key", - infra.Spec.CloudConfig.Key, + OpenshiftConfigManagedCloudConfigKey, ) } diff --git a/pkg/controller/vsphere/util_test.go b/pkg/controller/vsphere/util_test.go index 042ab4b17f..b26e54f620 100644 --- a/pkg/controller/vsphere/util_test.go +++ b/pkg/controller/vsphere/util_test.go @@ -4,7 +4,6 @@ import ( "fmt" "testing" - configv1 "github.com/openshift/api/config/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" @@ -34,27 +33,15 @@ func TestGetVSphereConfig(t *testing.T) { testConfig := fmt.Sprintf(testConfigFmt, "443", "test", "test-namespace") configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "testName", + Name: OpenshiftConfigManagedConfigMap, Namespace: openshiftConfigNamespaceForTest, }, Data: map[string]string{ - "testKey": testConfig, + OpenshiftConfigManagedCloudConfigKey: testConfig, }, } - infra := &configv1.Infrastructure{ - ObjectMeta: metav1.ObjectMeta{ - Name: globalInfrastuctureName, - }, - Spec: configv1.InfrastructureSpec{ - CloudConfig: configv1.ConfigMapFileReference{ - Name: "testName", - Key: "testKey", - }, - }, - } - - client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(infra, configMap).Build() + client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(configMap).Build() vSphereConfig, err := getVSphereConfig(client, openshiftConfigNamespaceForTest) if err != nil { diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 95ffc58146..83cf235d4b 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -79,6 +79,7 @@ type Operator struct { mutatingWebhookListerSynced cache.InformerSynced featureGateAccessor featuregates.FeatureGateAccess + featureGates featuregates.FeatureGate // queue only ever has one item, but it has nice error handling backoff/retry semantics queue workqueue.TypedRateLimitingInterface[string] @@ -166,6 +167,7 @@ func New( recorder, ) featureGateAccessor.SetChangeHandler(func(featureChange featuregates.FeatureChange) { + optr.featureGates = featuregates.NewFeatureGate(featureChange.New.Enabled, featureChange.New.Disabled) if featureChange.Previous == nil { // When the initial featuregate is set, the previous version is nil. // Nothing to do in this case, it's handled by the 1st sync, which only runs after the initial feature set was received. @@ -175,7 +177,7 @@ func New( klog.V(4).InfoS("FeatureGates changed", "enabled", featureChange.New.Enabled, "disabled", featureChange.New.Disabled) prevDisableMHC := featuregates.NewFeatureGate(featureChange.Previous.Enabled, featureChange.Previous.Disabled). Enabled(apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController) - newDisableMHC := featuregates.NewFeatureGate(featureChange.New.Enabled, featureChange.New.Disabled). + newDisableMHC := optr.featureGates. Enabled(apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController) if prevDisableMHC != newDisableMHC { diff --git a/pkg/operator/operator_test.go b/pkg/operator/operator_test.go index d913df759f..db2b478b3b 100644 --- a/pkg/operator/operator_test.go +++ b/pkg/operator/operator_test.go @@ -47,6 +47,11 @@ var ( {Name: apifeatures.FeatureGateVSphereHostVMGroupZonal}, } + disabledFeatureGates = []openshiftv1.FeatureGateAttributes{ + {Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}, + {Name: apifeatures.FeatureGateVSphereMultiVCenterDay2}, + } + enabledFeatureMap = map[string]bool{ "MachineAPIMigration": true, "AzureWorkloadIdentity": true, @@ -78,7 +83,7 @@ func newFakeOperator(kubeObjects, osObjects, machineObjects []runtime.Object, im { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -89,6 +94,11 @@ func newFakeOperator(kubeObjects, osObjects, machineObjects []runtime.Object, im return nil, fmt.Errorf("error adding event handler to deployments informer: %v", err) } + featureGates, err := featureGateAccessor.CurrentFeatureGates() + if err != nil { + return nil, fmt.Errorf("error getting current feature gates: %v", err) + } + optr := &Operator{ kubeClient: kubeClient, osClient: osClient, @@ -100,6 +110,7 @@ func newFakeOperator(kubeObjects, osObjects, machineObjects []runtime.Object, im mutatingWebhookLister: mutatingWebhookInformer.Lister(), validatingWebhookLister: validatingWebhookInformer.Lister(), featureGateAccessor: featureGateAccessor, + featureGates: featureGates, imagesFile: imagesFile, namespace: targetNamespace, eventRecorder: record.NewFakeRecorder(50), @@ -371,7 +382,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -405,7 +416,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -439,7 +450,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -473,7 +484,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -507,7 +518,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -541,7 +552,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -575,7 +586,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -609,7 +620,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -645,7 +656,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, @@ -679,7 +690,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: append(enabledFeatureGates, openshiftv1.FeatureGateAttributes{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}), - Disabled: []openshiftv1.FeatureGateAttributes{}, + Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateVSphereMultiVCenterDay2}}, }, }, }, @@ -713,7 +724,7 @@ func TestMAOConfigFromInfrastructure(t *testing.T) { { Version: "", Enabled: enabledFeatureGates, - Disabled: []openshiftv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + Disabled: disabledFeatureGates, }, }, }, diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index c10c7ab9d5..278a67c95b 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -20,7 +20,8 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/reconcile" - v1 "github.com/openshift/api/config/v1" + configv1 "github.com/openshift/api/config/v1" + apifeatures "github.com/openshift/api/features" machinev1beta1 "github.com/openshift/api/machine/v1beta1" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourceapply" @@ -237,7 +238,7 @@ func (optr *Operator) syncWebhookConfiguration(config *OperatorConfig) error { if err := optr.syncMachineMutatingWebhook(); err != nil { return err } - if config.PlatformType == v1.BareMetalPlatformType { + if config.PlatformType == configv1.BareMetalPlatformType { if err := optr.syncMetal3RemediationValidatingWebhook(); err != nil { return err } @@ -245,6 +246,57 @@ func (optr *Operator) syncWebhookConfiguration(config *OperatorConfig) error { return err } } + if config.PlatformType == configv1.VSpherePlatformType { + if err := optr.syncVSphereFailureDomainVAPs(); err != nil { + return err + } + } + return nil +} + +// syncVSphereFailureDomainVAPs ensures that the ValidatingAdmissionPolicies and their +// bindings for protecting vSphere failure domains are present and up to date. +// These policies prevent an administrator from removing a failure domain from the +// Infrastructure/cluster CR while it is still referenced by Machines or +// ControlPlaneMachineSets managed by the Machine API Operator. +func (optr *Operator) syncVSphereFailureDomainVAPs() error { + if !optr.featureGates.Enabled(apifeatures.FeatureGateVSphereMultiVCenterDay2) { + return nil + } + + recorder := events.NewLoggingEventRecorder(optr.name, clock.RealClock{}) + + if _, _, err := resourceapply.ApplyValidatingAdmissionPolicyV1(context.TODO(), + optr.kubeClient.AdmissionregistrationV1(), recorder, + mapiwebhooks.NewVSphereFailureDomainMachineVAP(), optr.cache); err != nil { + return fmt.Errorf("failed to apply vSphere failure domain Machine ValidatingAdmissionPolicy: %w", err) + } + if _, _, err := resourceapply.ApplyValidatingAdmissionPolicyBindingV1(context.TODO(), + optr.kubeClient.AdmissionregistrationV1(), recorder, + mapiwebhooks.NewVSphereFailureDomainMachineVAPBinding(), optr.cache); err != nil { + return fmt.Errorf("failed to apply vSphere failure domain Machine ValidatingAdmissionPolicyBinding: %w", err) + } + if _, _, err := resourceapply.ApplyValidatingAdmissionPolicyV1(context.TODO(), + optr.kubeClient.AdmissionregistrationV1(), recorder, + mapiwebhooks.NewVSphereFailureDomainCPMSVAP(), optr.cache); err != nil { + return fmt.Errorf("failed to apply vSphere failure domain ControlPlaneMachineSet ValidatingAdmissionPolicy: %w", err) + } + if _, _, err := resourceapply.ApplyValidatingAdmissionPolicyBindingV1(context.TODO(), + optr.kubeClient.AdmissionregistrationV1(), recorder, + mapiwebhooks.NewVSphereFailureDomainCPMSVAPBinding(), optr.cache); err != nil { + return fmt.Errorf("failed to apply vSphere failure domain ControlPlaneMachineSet ValidatingAdmissionPolicyBinding: %w", err) + } + if _, _, err := resourceapply.ApplyValidatingAdmissionPolicyV1(context.TODO(), + optr.kubeClient.AdmissionregistrationV1(), recorder, + mapiwebhooks.NewVSphereFailureDomainMachineSetVAP(), optr.cache); err != nil { + return fmt.Errorf("failed to apply vSphere failure domain MachineSet ValidatingAdmissionPolicy: %w", err) + } + if _, _, err := resourceapply.ApplyValidatingAdmissionPolicyBindingV1(context.TODO(), + optr.kubeClient.AdmissionregistrationV1(), recorder, + mapiwebhooks.NewVSphereFailureDomainMachineSetVAPBinding(), optr.cache); err != nil { + return fmt.Errorf("failed to apply vSphere failure domain MachineSet ValidatingAdmissionPolicyBinding: %w", err) + } + return nil } @@ -673,7 +725,7 @@ func newContainers(config *OperatorConfig, features map[string]bool) []corev1.Co machineControllerArgs := append([]string{}, featureGateArgs...) switch config.PlatformType { - case v1.AzurePlatformType, v1.GCPPlatformType: + case configv1.AzurePlatformType, configv1.GCPPlatformType: machineControllerArgs = append(machineControllerArgs, "--max-concurrent-reconciles=10") } diff --git a/pkg/operator/sync_test.go b/pkg/operator/sync_test.go index a4f38d798d..d3f40d7767 100644 --- a/pkg/operator/sync_test.go +++ b/pkg/operator/sync_test.go @@ -7,7 +7,8 @@ import ( "time" . "github.com/onsi/gomega" - v1 "github.com/openshift/api/config/v1" + configv1 "github.com/openshift/api/config/v1" + apifeatures "github.com/openshift/api/features" machinev1beta1 "github.com/openshift/api/machine/v1beta1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -463,20 +464,20 @@ func TestSyncWebhookConfiguration(t *testing.T) { testCases := []struct { name string - platformType v1.PlatformType + platformType configv1.PlatformType expectedNrMutatingWebhooks int expectedNrValidatingWebhooks int }{ { name: "webhooks on non baremetal", // using AWS as random non baremetal platform - platformType: v1.AWSPlatformType, + platformType: configv1.AWSPlatformType, expectedNrMutatingWebhooks: 1, expectedNrValidatingWebhooks: 1, }, { name: "webhooks on baremetal", - platformType: v1.BareMetalPlatformType, + platformType: configv1.BareMetalPlatformType, expectedNrMutatingWebhooks: 2, expectedNrValidatingWebhooks: 2, }, @@ -511,3 +512,95 @@ func TestSyncWebhookConfiguration(t *testing.T) { }) } } + +func TestSyncVSphereFailureDomainVAPs(t *testing.T) { + vsphereMultiVCenterDay2Enabled := &configv1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.FeatureGateStatus{ + FeatureGates: []configv1.FeatureGateDetails{ + { + Enabled: append(enabledFeatureGates, configv1.FeatureGateAttributes{Name: apifeatures.FeatureGateVSphereMultiVCenterDay2}), + Disabled: []configv1.FeatureGateAttributes{{Name: apifeatures.FeatureGateMachineAPIOperatorDisableMachineHealthCheckController}}, + }, + }, + }, + } + + testCases := []struct { + name string + platformType configv1.PlatformType + featureGate *configv1.FeatureGate + expectedNrValidatingAdmPolicies int + expectedNrValidatingAdmPBindings int + expectedPolicyNames []string + }{ + { + name: "no VAPs on non-vSphere platform", + platformType: configv1.AWSPlatformType, + expectedNrValidatingAdmPolicies: 0, + expectedNrValidatingAdmPBindings: 0, + }, + { + name: "no VAPs on vSphere platform when VSphereMultiVCenterDay2 is disabled", + platformType: configv1.VSpherePlatformType, + expectedNrValidatingAdmPolicies: 0, + expectedNrValidatingAdmPBindings: 0, + }, + { + name: "three VAPs and three bindings on vSphere platform when VSphereMultiVCenterDay2 is enabled", + platformType: configv1.VSpherePlatformType, + featureGate: vsphereMultiVCenterDay2Enabled, + expectedNrValidatingAdmPolicies: 3, + expectedNrValidatingAdmPBindings: 3, + expectedPolicyNames: []string{ + "vsphere-failure-domain-in-use-by-machine", + "vsphere-failure-domain-in-use-by-cpms", + "vsphere-failure-domain-in-use-by-machineset", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + stopCh := make(chan struct{}) + defer close(stopCh) + optr, err := newFakeOperator(nil, nil, nil, "", tc.featureGate, stopCh) + if err != nil { + t.Fatal(err) + } + + err = optr.syncWebhookConfiguration(&OperatorConfig{PlatformType: tc.platformType}) + g.Expect(err).ToNot(HaveOccurred()) + + policies, err := optr.kubeClient.AdmissionregistrationV1(). + ValidatingAdmissionPolicies().List(t.Context(), metav1.ListOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(policies.Items).To(HaveLen(tc.expectedNrValidatingAdmPolicies), + "unexpected number of ValidatingAdmissionPolicies") + + bindings, err := optr.kubeClient.AdmissionregistrationV1(). + ValidatingAdmissionPolicyBindings().List(t.Context(), metav1.ListOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(bindings.Items).To(HaveLen(tc.expectedNrValidatingAdmPBindings), + "unexpected number of ValidatingAdmissionPolicyBindings") + + if len(tc.expectedPolicyNames) > 0 { + policyNames := make([]string, 0, len(policies.Items)) + for _, p := range policies.Items { + policyNames = append(policyNames, p.Name) + } + g.Expect(policyNames).To(ConsistOf(tc.expectedPolicyNames), + "unexpected ValidatingAdmissionPolicy names") + + bindingPolicyRefs := make([]string, 0, len(bindings.Items)) + for _, b := range bindings.Items { + bindingPolicyRefs = append(bindingPolicyRefs, b.Spec.PolicyName) + } + g.Expect(bindingPolicyRefs).To(ConsistOf(tc.expectedPolicyNames), + "binding PolicyNames must reference the created policies") + } + }) + } +} diff --git a/pkg/webhooks/vap.go b/pkg/webhooks/vap.go new file mode 100644 index 0000000000..e025928683 --- /dev/null +++ b/pkg/webhooks/vap.go @@ -0,0 +1,400 @@ +package webhooks + +import ( + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" +) + +const ( + // VAPMachineFailureDomainName is the name of the ValidatingAdmissionPolicy that guards + // against removing a vSphere failure domain that is still referenced by a Machine. + VAPMachineFailureDomainName = "vsphere-failure-domain-in-use-by-machine" + + // VAPCPMSFailureDomainName is the name of the ValidatingAdmissionPolicy that guards + // against removing a vSphere failure domain that is still referenced by a ControlPlaneMachineSet. + VAPCPMSFailureDomainName = "vsphere-failure-domain-in-use-by-cpms" + + // VAPMachineSetFailureDomainName is the name of the ValidatingAdmissionPolicy that guards + // against removing a vSphere failure domain that is still referenced by a MachineSet (including + // MachineSets with zero replicas that would have no running Machines to catch the check). + VAPMachineSetFailureDomainName = "vsphere-failure-domain-in-use-by-machineset" + + // machineRegionLabel is the label used to identify the region a Machine belongs to. + machineRegionLabel = "machine.openshift.io/region" + + // machineZoneLabel is the label used to identify the availability zone a Machine belongs to. + machineZoneLabel = "machine.openshift.io/zone" + + // vspherePlatformType is the platform type string as used in infrastructure.status.platformStatus.type. + vspherePlatformType = "VSphere" + + // openMachineAPINamespace is the namespace where Machines and CPMS live. + openMachineAPINamespace = "openshift-machine-api" +) + +var ( + // vapDenyAction is the enforcement action that denies the admission request. + vapDenyAction = admissionregistrationv1.Deny + + // vapParamNotFoundAllow means: if no param object exists (e.g. no Machines yet), allow the infra update. + vapParamNotFoundAllow = admissionregistrationv1.AllowAction +) + +func matchPolicyEquivalent() *admissionregistrationv1.MatchPolicyType { + p := admissionregistrationv1.Equivalent + return &p +} + +// NewVSphereFailureDomainMachineVAP returns a ValidatingAdmissionPolicy that prevents +// an infrastructure/cluster UPDATE from removing a vSphere failure domain that is still +// referenced by at least one Machine (identified via machine.openshift.io/region and +// machine.openshift.io/zone labels). +// +// The policy fires on every UPDATE of infrastructures.config.openshift.io. It is evaluated +// once per Machine that exists in the openshift-machine-api namespace (param binding). +// If any Machine's region+zone labels match a failure domain being removed, admission is denied. +func NewVSphereFailureDomainMachineVAP() *admissionregistrationv1.ValidatingAdmissionPolicy { + failurePolicy := admissionregistrationv1.Fail + + return &admissionregistrationv1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: VAPMachineFailureDomainName, + }, + Spec: admissionregistrationv1.ValidatingAdmissionPolicySpec{ + // The param object is a Machine (one evaluation per Machine). + ParamKind: &admissionregistrationv1.ParamKind{ + APIVersion: "machine.openshift.io/v1beta1", + Kind: "Machine", + }, + // Fire on UPDATE of the Infrastructure CR only. + MatchConstraints: &admissionregistrationv1.MatchResources{ + MatchPolicy: matchPolicyEquivalent(), + NamespaceSelector: &metav1.LabelSelector{}, + ObjectSelector: &metav1.LabelSelector{}, + ResourceRules: []admissionregistrationv1.NamedRuleWithOperations{ + { + RuleWithOperations: admissionregistrationv1.RuleWithOperations{ + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"config.openshift.io"}, + APIVersions: []string{"v1"}, + Resources: []string{"infrastructures"}, + }, + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Update, + }, + }, + }, + }, + }, + // Only evaluate when the cluster is running on vSphere. + MatchConditions: []admissionregistrationv1.MatchCondition{ + { + Name: "is-vsphere-platform", + Expression: `object.?status.platformStatus.type.orValue("") == "` + vspherePlatformType + `"`, + }, + }, + // Reusable sub-expressions. + Variables: []admissionregistrationv1.Variable{ + { + // fds: the failure domains list from the incoming (updated) Infrastructure spec. + Name: "fds", + Expression: `object.?spec.platformSpec.vsphere.failureDomains.orValue([])`, + }, + { + // oldFds: the failure domains list from the existing (pre-update) Infrastructure + // spec. Used to distinguish "this Machine's failure domain was removed" from + // "this Machine's failure domain never existed in the spec to begin with" + // (SPLAT-2826). + Name: "oldFds", + Expression: `oldObject.?spec.platformSpec.vsphere.failureDomains.orValue([])`, + }, + { + // machineRegion: the region label of the Machine param (empty string if absent). + Name: "machineRegion", + Expression: `params.?metadata.labels["` + machineRegionLabel + `"].orValue("")`, + }, + { + // machineZone: the zone label of the Machine param (empty string if absent). + Name: "machineZone", + Expression: `params.?metadata.labels["` + machineZoneLabel + `"].orValue("")`, + }, + }, + // Core validation: only deny when the Machine's failure domain existed in the OLD infra + // spec (something to protect) and is missing from the NEW spec (it was removed). + Validations: []admissionregistrationv1.Validation{ + { + // Pass when: + // - Machine has no region/zone label (not a failure-domain-managed Machine), OR + // - The Machine's region/zone never matched any failure domain in the OLD spec + // (nothing to protect — SPLAT-2826), OR + // - The failure domain is still present in the incoming spec. + Expression: `variables.machineRegion == "" || variables.machineZone == "" || +!variables.oldFds.exists(fd, + fd.region == variables.machineRegion && fd.zone == variables.machineZone +) || +variables.fds.exists(fd, + fd.region == variables.machineRegion && fd.zone == variables.machineZone +)`, + MessageExpression: `"Infrastructure update would remove vSphere failure domain (region=" + variables.machineRegion + ", zone=" + variables.machineZone + ") that is still in use by Machine '" + params.metadata.name + "'"`, + Reason: ptr.To(metav1.StatusReasonInvalid), + }, + }, + // Hard fail if the policy itself errors (e.g. param parse failure). + FailurePolicy: &failurePolicy, + }, + } +} + +// NewVSphereFailureDomainMachineVAPBinding returns the ValidatingAdmissionPolicyBinding that +// connects the Machine VAP to all Machines in the openshift-machine-api namespace. +// The policy is evaluated once per Machine (param). +func NewVSphereFailureDomainMachineVAPBinding() *admissionregistrationv1.ValidatingAdmissionPolicyBinding { + return &admissionregistrationv1.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: VAPMachineFailureDomainName, + }, + Spec: admissionregistrationv1.ValidatingAdmissionPolicyBindingSpec{ + // Reference the policy defined above. + PolicyName: VAPMachineFailureDomainName, + // Param: iterate over all Machines in openshift-machine-api. + ParamRef: &admissionregistrationv1.ParamRef{ + // Empty selector matches all Machines. + Selector: &metav1.LabelSelector{}, + Namespace: openMachineAPINamespace, + ParameterNotFoundAction: &vapParamNotFoundAllow, + }, + // Deny if any validation fails. + ValidationActions: []admissionregistrationv1.ValidationAction{ + vapDenyAction, + }, + }, + } +} + +// NewVSphereFailureDomainCPMSVAP returns a ValidatingAdmissionPolicy that prevents +// an infrastructure/cluster UPDATE from removing a vSphere failure domain that is still +// referenced by a ControlPlaneMachineSet (CPMS) by failure domain name. +// +// The CPMS references failure domains by the Name field of VSpherePlatformFailureDomainSpec. +// The policy fires on every UPDATE of infrastructures.config.openshift.io and is evaluated +// once per ControlPlaneMachineSet in the openshift-machine-api namespace. +// +// Unlike the Machine and MachineSet VAPs, this VAP intentionally does not compare against +// oldObject (see SPLAT-2826). CPMS matching is by failure domain Name, an unambiguous identity +// key, rather than by region/zone labels sourced from vCenter tags — so "is this name present +// in the current spec" is a correct check whether evaluated against the old or the new spec. +func NewVSphereFailureDomainCPMSVAP() *admissionregistrationv1.ValidatingAdmissionPolicy { + failurePolicy := admissionregistrationv1.Fail + + return &admissionregistrationv1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: VAPCPMSFailureDomainName, + }, + Spec: admissionregistrationv1.ValidatingAdmissionPolicySpec{ + // The param object is a ControlPlaneMachineSet. + ParamKind: &admissionregistrationv1.ParamKind{ + APIVersion: "machine.openshift.io/v1", + Kind: "ControlPlaneMachineSet", + }, + // Fire on UPDATE of the Infrastructure CR only. + MatchConstraints: &admissionregistrationv1.MatchResources{ + MatchPolicy: matchPolicyEquivalent(), + NamespaceSelector: &metav1.LabelSelector{}, + ObjectSelector: &metav1.LabelSelector{}, + ResourceRules: []admissionregistrationv1.NamedRuleWithOperations{ + { + RuleWithOperations: admissionregistrationv1.RuleWithOperations{ + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"config.openshift.io"}, + APIVersions: []string{"v1"}, + Resources: []string{"infrastructures"}, + }, + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Update, + }, + }, + }, + }, + }, + // Only evaluate when the cluster is running on vSphere. + MatchConditions: []admissionregistrationv1.MatchCondition{ + { + Name: "is-vsphere-platform", + Expression: `object.?status.platformStatus.type.orValue("") == "` + vspherePlatformType + `"`, + }, + }, + // Reusable sub-expressions. + Variables: []admissionregistrationv1.Variable{ + { + // fds: failure domains from the incoming (updated) Infrastructure spec. + Name: "fds", + Expression: `object.?spec.platformSpec.vsphere.failureDomains.orValue([])`, + }, + { + // cpmsFDs: the list of vSphere failure domain names referenced by the CPMS param. + // The CPMS template field path is: + // spec.template.machines_v1beta1_machine_openshift_io.failureDomains.vsphere[*].name + Name: "cpmsFDs", + Expression: `(has(params.spec.template.machines_v1beta1_machine_openshift_io) && + has(params.spec.template.machines_v1beta1_machine_openshift_io.failureDomains) && + has(params.spec.template.machines_v1beta1_machine_openshift_io.failureDomains.vsphere)) + ? params.spec.template.machines_v1beta1_machine_openshift_io.failureDomains.vsphere + : []`, + }, + }, + // Core validation: every CPMS failure domain name must still exist in the updated infra spec. + Validations: []admissionregistrationv1.Validation{ + { + // Pass when: + // - CPMS has no vSphere failure domains configured (empty list), OR + // - Every CPMS failure domain name is still present in the incoming infra spec. + Expression: `variables.cpmsFDs.size() == 0 || +variables.cpmsFDs.all(cpmsfd, + variables.fds.exists(infrafd, infrafd.name == cpmsfd.name) +)`, + MessageExpression: `"Infrastructure update would remove vSphere failure domain(s) still referenced by ControlPlaneMachineSet '" + params.metadata.name + "': [" + variables.cpmsFDs.filter(cpmsfd, !variables.fds.exists(infrafd, infrafd.name == cpmsfd.name)).map(cpmsfd, cpmsfd.name).join(", ") + "]"`, + Reason: ptr.To(metav1.StatusReasonInvalid), + }, + }, + // Hard fail if the policy itself errors. + FailurePolicy: &failurePolicy, + }, + } +} + +// NewVSphereFailureDomainCPMSVAPBinding returns the ValidatingAdmissionPolicyBinding that +// connects the CPMS VAP to all ControlPlaneMachineSets in the openshift-machine-api namespace. +func NewVSphereFailureDomainCPMSVAPBinding() *admissionregistrationv1.ValidatingAdmissionPolicyBinding { + return &admissionregistrationv1.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: VAPCPMSFailureDomainName, + }, + Spec: admissionregistrationv1.ValidatingAdmissionPolicyBindingSpec{ + // Reference the CPMS policy. + PolicyName: VAPCPMSFailureDomainName, + // Param: iterate over all ControlPlaneMachineSets in openshift-machine-api. + ParamRef: &admissionregistrationv1.ParamRef{ + Selector: &metav1.LabelSelector{}, + Namespace: openMachineAPINamespace, + ParameterNotFoundAction: &vapParamNotFoundAllow, + }, + // Deny if any validation fails. + ValidationActions: []admissionregistrationv1.ValidationAction{ + vapDenyAction, + }, + }, + } +} + +// NewVSphereFailureDomainMachineSetVAP returns a ValidatingAdmissionPolicy that prevents an +// Infrastructure update from removing a vSphere failure domain that is still referenced by at +// least one MachineSet (identified via machine.openshift.io/region and +// machine.openshift.io/zone labels on the MachineSet template). This covers MachineSets with +// zero replicas, which would otherwise have no running Machines for the Machine VAP to check. +func NewVSphereFailureDomainMachineSetVAP() *admissionregistrationv1.ValidatingAdmissionPolicy { + failurePolicy := admissionregistrationv1.Fail + return &admissionregistrationv1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: VAPMachineSetFailureDomainName, + }, + Spec: admissionregistrationv1.ValidatingAdmissionPolicySpec{ + // Param: each evaluation receives one MachineSet as a param object. + ParamKind: &admissionregistrationv1.ParamKind{ + APIVersion: "machine.openshift.io/v1beta1", + Kind: "MachineSet", + }, + // Trigger: UPDATE of the Infrastructure CR only. + MatchConstraints: &admissionregistrationv1.MatchResources{ + MatchPolicy: matchPolicyEquivalent(), + NamespaceSelector: &metav1.LabelSelector{}, + ObjectSelector: &metav1.LabelSelector{}, + ResourceRules: []admissionregistrationv1.NamedRuleWithOperations{ + { + RuleWithOperations: admissionregistrationv1.RuleWithOperations{ + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"config.openshift.io"}, + APIVersions: []string{"v1"}, + Resources: []string{"infrastructures"}, + }, + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Update, + }, + }, + }, + }, + }, + // Only evaluate when the cluster is running on vSphere. + MatchConditions: []admissionregistrationv1.MatchCondition{ + { + Name: "is-vsphere-platform", + Expression: `object.?status.platformStatus.type.orValue("") == "` + vspherePlatformType + `"`, + }, + }, + // Reusable sub-expressions. + Variables: []admissionregistrationv1.Variable{ + { + // fds: the failure domains list from the incoming (updated) Infrastructure spec. + Name: "fds", + Expression: `object.?spec.platformSpec.vsphere.failureDomains.orValue([])`, + }, + { + // oldFds: the failure domains list from the existing (pre-update) Infrastructure + // spec. See SPLAT-2826. + Name: "oldFds", + Expression: `oldObject.?spec.platformSpec.vsphere.failureDomains.orValue([])`, + }, + { + // msRegion: the region label of the MachineSet template (empty string if absent). + Name: "msRegion", + Expression: `params.?spec.template.metadata.labels["` + machineRegionLabel + `"].orValue("")`, + }, + { + // msZone: the zone label of the MachineSet template (empty string if absent). + Name: "msZone", + Expression: `params.?spec.template.metadata.labels["` + machineZoneLabel + `"].orValue("")`, + }, + }, + // Core validation: only deny when the MachineSet's failure domain existed in the OLD + // infra spec and is missing from the NEW spec (see SPLAT-2826). + Validations: []admissionregistrationv1.Validation{ + { + // MachineSet has no region/zone label in its template (not a failure-domain-managed + // MachineSet), OR its region/zone never matched any FD in the OLD spec (nothing to + // protect), OR the failure domain is still present in the incoming spec. + Expression: `variables.msRegion == "" || variables.msZone == "" || +!variables.oldFds.exists(fd, fd.region == variables.msRegion && fd.zone == variables.msZone) || +variables.fds.exists(fd, fd.region == variables.msRegion && fd.zone == variables.msZone)`, + MessageExpression: `"Infrastructure update would remove vSphere failure domain (region=" + variables.msRegion + ", zone=" + variables.msZone + ") that is still in use by MachineSet '" + params.metadata.name + "'"`, + Reason: ptr.To(metav1.StatusReasonInvalid), + }, + }, + // Hard fail if the VAP itself cannot be evaluated. + FailurePolicy: &failurePolicy, + }, + } +} + +// NewVSphereFailureDomainMachineSetVAPBinding returns a ValidatingAdmissionPolicyBinding that +// connects the MachineSet VAP to all MachineSets in the openshift-machine-api namespace. +func NewVSphereFailureDomainMachineSetVAPBinding() *admissionregistrationv1.ValidatingAdmissionPolicyBinding { + return &admissionregistrationv1.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: VAPMachineSetFailureDomainName, + }, + Spec: admissionregistrationv1.ValidatingAdmissionPolicyBindingSpec{ + // Reference the MachineSet policy. + PolicyName: VAPMachineSetFailureDomainName, + // Param: iterate over all MachineSets in openshift-machine-api. + ParamRef: &admissionregistrationv1.ParamRef{ + Selector: &metav1.LabelSelector{}, + Namespace: openMachineAPINamespace, + ParameterNotFoundAction: &vapParamNotFoundAllow, + }, + // Deny if any validation fails. + ValidationActions: []admissionregistrationv1.ValidationAction{ + vapDenyAction, + }, + }, + } +} diff --git a/pkg/webhooks/vap_test.go b/pkg/webhooks/vap_test.go new file mode 100644 index 0000000000..bc8152dadd --- /dev/null +++ b/pkg/webhooks/vap_test.go @@ -0,0 +1,313 @@ +package webhooks + +import ( + "testing" + + . "github.com/onsi/gomega" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestNewVSphereFailureDomainMachineVAP(t *testing.T) { + g := NewWithT(t) + + policy := NewVSphereFailureDomainMachineVAP() + g.Expect(policy).NotTo(BeNil()) + g.Expect(policy.Name).To(Equal(VAPMachineFailureDomainName)) + + spec := policy.Spec + + // ParamKind must reference Machine. + g.Expect(spec.ParamKind).NotTo(BeNil()) + g.Expect(spec.ParamKind.APIVersion).To(Equal("machine.openshift.io/v1beta1")) + g.Expect(spec.ParamKind.Kind).To(Equal("Machine")) + + // Must fire only on UPDATE of infrastructures. + g.Expect(spec.MatchConstraints).NotTo(BeNil()) + g.Expect(spec.MatchConstraints.ResourceRules).To(HaveLen(1)) + rule := spec.MatchConstraints.ResourceRules[0] + g.Expect(rule.APIGroups).To(ConsistOf("config.openshift.io")) + g.Expect(rule.APIVersions).To(ConsistOf("v1")) + g.Expect(rule.Resources).To(ConsistOf("infrastructures")) + g.Expect(rule.Operations).To(ConsistOf(admissionregistrationv1.Update)) + + // Must have the platform match condition. + g.Expect(spec.MatchConditions).To(HaveLen(1)) + g.Expect(spec.MatchConditions[0].Name).To(Equal("is-vsphere-platform")) + g.Expect(spec.MatchConditions[0].Expression).To(ContainSubstring(`"VSphere"`)) + + // Must define the four CEL variables. + varNames := make([]string, 0, len(spec.Variables)) + for _, v := range spec.Variables { + varNames = append(varNames, v.Name) + } + g.Expect(varNames).To(ConsistOf("fds", "oldFds", "machineRegion", "machineZone")) + + // The oldFds variable must read from oldObject so removal (not mere absence) is what's checked. + for _, v := range spec.Variables { + if v.Name == "oldFds" { + g.Expect(v.Expression).To(ContainSubstring("oldObject")) + g.Expect(v.Expression).To(ContainSubstring("failureDomains")) + } + } + + // Must have exactly one validation rule. + g.Expect(spec.Validations).To(HaveLen(1)) + validation := spec.Validations[0] + g.Expect(validation.Expression).To(ContainSubstring("variables.machineRegion")) + g.Expect(validation.Expression).To(ContainSubstring("variables.machineZone")) + g.Expect(validation.Expression).To(ContainSubstring("variables.fds.exists")) + g.Expect(validation.Expression).To(ContainSubstring("variables.oldFds.exists")) + g.Expect(validation.Expression).To(ContainSubstring("!variables.oldFds.exists")) + g.Expect(validation.MessageExpression).To(ContainSubstring("params.metadata.name")) + g.Expect(validation.Reason).NotTo(BeNil()) + g.Expect(*validation.Reason).To(Equal(metav1.StatusReasonInvalid)) + + // Failure policy must be Fail. + g.Expect(spec.FailurePolicy).NotTo(BeNil()) + g.Expect(*spec.FailurePolicy).To(Equal(admissionregistrationv1.Fail)) +} + +func TestNewVSphereFailureDomainMachineVAPBinding(t *testing.T) { + g := NewWithT(t) + + binding := NewVSphereFailureDomainMachineVAPBinding() + g.Expect(binding).NotTo(BeNil()) + g.Expect(binding.Name).To(Equal(VAPMachineFailureDomainName)) + + spec := binding.Spec + g.Expect(spec.PolicyName).To(Equal(VAPMachineFailureDomainName)) + + // ParamRef must select all Machines in openshift-machine-api. + g.Expect(spec.ParamRef).NotTo(BeNil()) + g.Expect(spec.ParamRef.Namespace).To(Equal(openMachineAPINamespace)) + g.Expect(spec.ParamRef.Selector).To(Equal(&metav1.LabelSelector{})) + g.Expect(spec.ParamRef.ParameterNotFoundAction).NotTo(BeNil()) + g.Expect(*spec.ParamRef.ParameterNotFoundAction).To(Equal(admissionregistrationv1.AllowAction)) + + // Enforcement must be Deny. + g.Expect(spec.ValidationActions).To(ConsistOf(admissionregistrationv1.Deny)) +} + +func TestNewVSphereFailureDomainCPMSVAP(t *testing.T) { + g := NewWithT(t) + + policy := NewVSphereFailureDomainCPMSVAP() + g.Expect(policy).NotTo(BeNil()) + g.Expect(policy.Name).To(Equal(VAPCPMSFailureDomainName)) + + spec := policy.Spec + + // ParamKind must reference ControlPlaneMachineSet. + g.Expect(spec.ParamKind).NotTo(BeNil()) + g.Expect(spec.ParamKind.APIVersion).To(Equal("machine.openshift.io/v1")) + g.Expect(spec.ParamKind.Kind).To(Equal("ControlPlaneMachineSet")) + + // Must fire only on UPDATE of infrastructures. + g.Expect(spec.MatchConstraints).NotTo(BeNil()) + g.Expect(spec.MatchConstraints.ResourceRules).To(HaveLen(1)) + rule := spec.MatchConstraints.ResourceRules[0] + g.Expect(rule.APIGroups).To(ConsistOf("config.openshift.io")) + g.Expect(rule.Resources).To(ConsistOf("infrastructures")) + g.Expect(rule.Operations).To(ConsistOf(admissionregistrationv1.Update)) + + // Must have the platform match condition. + g.Expect(spec.MatchConditions).To(HaveLen(1)) + g.Expect(spec.MatchConditions[0].Name).To(Equal("is-vsphere-platform")) + + // Must define the two CEL variables. + varNames := make([]string, 0, len(spec.Variables)) + for _, v := range spec.Variables { + varNames = append(varNames, v.Name) + } + g.Expect(varNames).To(ConsistOf("fds", "cpmsFDs")) + + // The cpmsFDs variable must reference the correct CPMS template field path. + for _, v := range spec.Variables { + if v.Name == "cpmsFDs" { + g.Expect(v.Expression).To(ContainSubstring("machines_v1beta1_machine_openshift_io")) + g.Expect(v.Expression).To(ContainSubstring("failureDomains")) + g.Expect(v.Expression).To(ContainSubstring("vsphere")) + } + } + + // Must have exactly one validation rule. + g.Expect(spec.Validations).To(HaveLen(1)) + validation := spec.Validations[0] + g.Expect(validation.Expression).To(ContainSubstring("variables.cpmsFDs")) + g.Expect(validation.Expression).To(ContainSubstring("variables.fds.exists")) + g.Expect(validation.MessageExpression).To(ContainSubstring("params.metadata.name")) + g.Expect(validation.Reason).NotTo(BeNil()) + g.Expect(*validation.Reason).To(Equal(metav1.StatusReasonInvalid)) + + // Failure policy must be Fail. + g.Expect(spec.FailurePolicy).NotTo(BeNil()) + g.Expect(*spec.FailurePolicy).To(Equal(admissionregistrationv1.Fail)) +} + +func TestNewVSphereFailureDomainCPMSVAPBinding(t *testing.T) { + g := NewWithT(t) + + binding := NewVSphereFailureDomainCPMSVAPBinding() + g.Expect(binding).NotTo(BeNil()) + g.Expect(binding.Name).To(Equal(VAPCPMSFailureDomainName)) + + spec := binding.Spec + g.Expect(spec.PolicyName).To(Equal(VAPCPMSFailureDomainName)) + + // ParamRef must select all CPMSes in openshift-machine-api. + g.Expect(spec.ParamRef).NotTo(BeNil()) + g.Expect(spec.ParamRef.Namespace).To(Equal(openMachineAPINamespace)) + g.Expect(spec.ParamRef.Selector).To(Equal(&metav1.LabelSelector{})) + g.Expect(spec.ParamRef.ParameterNotFoundAction).NotTo(BeNil()) + g.Expect(*spec.ParamRef.ParameterNotFoundAction).To(Equal(admissionregistrationv1.AllowAction)) + + // Enforcement must be Deny. + g.Expect(spec.ValidationActions).To(ConsistOf(admissionregistrationv1.Deny)) +} + +func TestNewVSphereFailureDomainMachineSetVAP(t *testing.T) { + g := NewWithT(t) + + policy := NewVSphereFailureDomainMachineSetVAP() + g.Expect(policy).NotTo(BeNil()) + g.Expect(policy.Name).To(Equal(VAPMachineSetFailureDomainName)) + + spec := policy.Spec + + // ParamKind must reference MachineSet. + g.Expect(spec.ParamKind).NotTo(BeNil()) + g.Expect(spec.ParamKind.APIVersion).To(Equal("machine.openshift.io/v1beta1")) + g.Expect(spec.ParamKind.Kind).To(Equal("MachineSet")) + + // Must fire only on UPDATE of infrastructures. + g.Expect(spec.MatchConstraints).NotTo(BeNil()) + g.Expect(spec.MatchConstraints.ResourceRules).To(HaveLen(1)) + rule := spec.MatchConstraints.ResourceRules[0] + g.Expect(rule.APIGroups).To(ConsistOf("config.openshift.io")) + g.Expect(rule.APIVersions).To(ConsistOf("v1")) + g.Expect(rule.Resources).To(ConsistOf("infrastructures")) + g.Expect(rule.Operations).To(ConsistOf(admissionregistrationv1.Update)) + + // Must have the platform match condition. + g.Expect(spec.MatchConditions).To(HaveLen(1)) + g.Expect(spec.MatchConditions[0].Name).To(Equal("is-vsphere-platform")) + g.Expect(spec.MatchConditions[0].Expression).To(ContainSubstring(`"VSphere"`)) + + // Must define the four CEL variables. + varNames := make([]string, 0, len(spec.Variables)) + for _, v := range spec.Variables { + varNames = append(varNames, v.Name) + } + g.Expect(varNames).To(ConsistOf("fds", "oldFds", "msRegion", "msZone")) + + // The oldFds, msRegion, and msZone variables must read from the expected paths. + for _, v := range spec.Variables { + switch v.Name { + case "oldFds": + g.Expect(v.Expression).To(ContainSubstring("oldObject")) + g.Expect(v.Expression).To(ContainSubstring("failureDomains")) + case "msRegion": + g.Expect(v.Expression).To(ContainSubstring("params.?spec.template.metadata.labels")) + g.Expect(v.Expression).To(ContainSubstring(machineRegionLabel)) + case "msZone": + g.Expect(v.Expression).To(ContainSubstring("params.?spec.template.metadata.labels")) + g.Expect(v.Expression).To(ContainSubstring(machineZoneLabel)) + } + } + + // Must have exactly one validation rule. + g.Expect(spec.Validations).To(HaveLen(1)) + validation := spec.Validations[0] + g.Expect(validation.Expression).To(ContainSubstring("variables.msRegion")) + g.Expect(validation.Expression).To(ContainSubstring("variables.msZone")) + g.Expect(validation.Expression).To(ContainSubstring("variables.fds.exists")) + g.Expect(validation.Expression).To(ContainSubstring("variables.oldFds.exists")) + g.Expect(validation.Expression).To(ContainSubstring("!variables.oldFds.exists")) + g.Expect(validation.MessageExpression).To(ContainSubstring("params.metadata.name")) + g.Expect(validation.Reason).NotTo(BeNil()) + g.Expect(*validation.Reason).To(Equal(metav1.StatusReasonInvalid)) + + // Failure policy must be Fail. + g.Expect(spec.FailurePolicy).NotTo(BeNil()) + g.Expect(*spec.FailurePolicy).To(Equal(admissionregistrationv1.Fail)) +} + +func TestNewVSphereFailureDomainMachineSetVAPBinding(t *testing.T) { + g := NewWithT(t) + + binding := NewVSphereFailureDomainMachineSetVAPBinding() + g.Expect(binding).NotTo(BeNil()) + g.Expect(binding.Name).To(Equal(VAPMachineSetFailureDomainName)) + + spec := binding.Spec + g.Expect(spec.PolicyName).To(Equal(VAPMachineSetFailureDomainName)) + + // ParamRef must select all MachineSets in openshift-machine-api. + g.Expect(spec.ParamRef).NotTo(BeNil()) + g.Expect(spec.ParamRef.Namespace).To(Equal(openMachineAPINamespace)) + g.Expect(spec.ParamRef.Selector).To(Equal(&metav1.LabelSelector{})) + g.Expect(spec.ParamRef.ParameterNotFoundAction).NotTo(BeNil()) + g.Expect(*spec.ParamRef.ParameterNotFoundAction).To(Equal(admissionregistrationv1.AllowAction)) + + // Enforcement must be Deny. + g.Expect(spec.ValidationActions).To(ConsistOf(admissionregistrationv1.Deny)) +} + +// TestVAPMatchConstraintsServerDefaultedFields ensures that all three VAPs explicitly set +// MatchPolicy, NamespaceSelector, and ObjectSelector on MatchConstraints. When these fields +// are nil, the API server defaults them on storage. On the next sync cycle, resourceapply +// sees a diff between the desired spec (nil) and the stored spec (defaulted), triggering a +// spurious UPDATE every cycle. See SPLAT-2854. +func TestVAPMatchConstraintsServerDefaultedFields(t *testing.T) { + expectedMatchPolicy := admissionregistrationv1.Equivalent + expectedSelector := &metav1.LabelSelector{} + + tests := []struct { + name string + policy *admissionregistrationv1.ValidatingAdmissionPolicy + }{ + {"Machine VAP", NewVSphereFailureDomainMachineVAP()}, + {"CPMS VAP", NewVSphereFailureDomainCPMSVAP()}, + {"MachineSet VAP", NewVSphereFailureDomainMachineSetVAP()}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + mc := tt.policy.Spec.MatchConstraints + g.Expect(mc).NotTo(BeNil(), "MatchConstraints must not be nil") + + g.Expect(mc.MatchPolicy).NotTo(BeNil(), + "MatchPolicy must be explicitly set to avoid spurious updates from API server defaulting") + g.Expect(*mc.MatchPolicy).To(Equal(expectedMatchPolicy)) + + g.Expect(mc.NamespaceSelector).To(Equal(expectedSelector), + "NamespaceSelector must be explicitly set to avoid spurious updates from API server defaulting") + + g.Expect(mc.ObjectSelector).To(Equal(expectedSelector), + "ObjectSelector must be explicitly set to avoid spurious updates from API server defaulting") + }) + } +} + +// TestVAPNamesAreConsistent ensures the binding policy names match the policy names. +func TestVAPNamesAreConsistent(t *testing.T) { + g := NewWithT(t) + + machineVAP := NewVSphereFailureDomainMachineVAP() + machineBinding := NewVSphereFailureDomainMachineVAPBinding() + g.Expect(machineBinding.Spec.PolicyName).To(Equal(machineVAP.Name), + "Machine binding PolicyName must match Machine VAP name") + + cpmsVAP := NewVSphereFailureDomainCPMSVAP() + cpmsBinding := NewVSphereFailureDomainCPMSVAPBinding() + g.Expect(cpmsBinding.Spec.PolicyName).To(Equal(cpmsVAP.Name), + "CPMS binding PolicyName must match CPMS VAP name") + + machineSetVAP := NewVSphereFailureDomainMachineSetVAP() + machineSetBinding := NewVSphereFailureDomainMachineSetVAPBinding() + g.Expect(machineSetBinding.Spec.PolicyName).To(Equal(machineSetVAP.Name), + "MachineSet binding PolicyName must match MachineSet VAP name") +} diff --git a/test/e2e/vsphere/failure_domain_vap.go b/test/e2e/vsphere/failure_domain_vap.go new file mode 100644 index 0000000000..ada21abbc7 --- /dev/null +++ b/test/e2e/vsphere/failure_domain_vap.go @@ -0,0 +1,699 @@ +package vsphere + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + configv1 "github.com/openshift/api/config/v1" + machinev1 "github.com/openshift/api/machine/v1" + machinev1beta1 "github.com/openshift/api/machine/v1beta1" + configclient "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" + machineclientv1 "github.com/openshift/client-go/machine/clientset/versioned/typed/machine/v1" + machineclientv1beta1 "github.com/openshift/client-go/machine/clientset/versioned/typed/machine/v1beta1" + mapiwebhooks "github.com/openshift/machine-api-operator/pkg/webhooks" + e2eutil "github.com/openshift/machine-api-operator/test/e2e" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + e2e "k8s.io/kubernetes/test/e2e/framework" +) + +const ( + // vapTestMachineSetSuffix is appended to the cluster infra name to form a unique MachineSet name for VAP tests. + vapTestMachineSetSuffix = "-vap-fd-test" + + // vapTestWaitTimeout is the maximum time to wait for a MachineSet to be deleted. + vapTestWaitTimeout = 2 * time.Minute + + // vapPollAttemptTimeout bounds each individual dry-run Update call made while polling for + // the VAP to catch up, so a single stuck attempt can't block Eventually past the point + // where it should give up and report a failure. + vapPollAttemptTimeout = 10 * time.Second + + // vapCacheSyncWindow bounds how long to keep re-checking a dry-run update after creating a + // param object (e.g. a MachineSet), so the assertion is exercised both before and after the + // VAP's informer-backed cache has observed the new object, rather than only immediately + // after creation while the cache may still be stale. + vapCacheSyncWindow = 30 * time.Second +) + +// infraWithFDRemoved returns a deep copy of the given Infrastructure with the named failure domain removed +// from spec.platformSpec.vsphere.failureDomains. If no such failure domain exists the original copy is +// returned unchanged. +func infraWithFDRemoved(infra *configv1.Infrastructure, fdName string) *configv1.Infrastructure { + copy := infra.DeepCopy() + fds := copy.Spec.PlatformSpec.VSphere.FailureDomains + filtered := fds[:0] + for _, fd := range fds { + if fd.Name != fdName { + filtered = append(filtered, fd) + } + } + copy.Spec.PlatformSpec.VSphere.FailureDomains = filtered + return copy +} + +// getCPMSFailureDomainNames returns the set of failure domain names referenced by the +// ControlPlaneMachineSet. Returns an empty (non-nil) map and nil error when the CPMS +// does not exist or has no vSphere failure domain entries. Non-NotFound API errors +// are propagated so callers can fail loudly instead of silently skipping CPMS exclusion. +func getCPMSFailureDomainNames(ctx context.Context, mcv1 *machineclientv1.MachineV1Client) (map[string]bool, error) { + names := make(map[string]bool) + cpms, err := mcv1.ControlPlaneMachineSets(e2eutil.MachineAPINamespace).Get(ctx, "cluster", metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + return names, nil + } + if err != nil { + return nil, fmt.Errorf("failed to get ControlPlaneMachineSet: %w", err) + } + t := cpms.Spec.Template.OpenShiftMachineV1Beta1Machine + if t == nil || t.FailureDomains == nil { + return names, nil + } + for _, vfd := range t.FailureDomains.VSphere { + names[vfd.Name] = true + } + return names, nil +} + +// isFDReferencedByMachine reports whether any Machine in the list has region/zone labels +// matching the given failure domain. +func isFDReferencedByMachine(fd configv1.VSpherePlatformFailureDomainSpec, machines *machinev1beta1.MachineList) bool { + for _, m := range machines.Items { + if m.Labels["machine.openshift.io/region"] == fd.Region && + m.Labels["machine.openshift.io/zone"] == fd.Zone { + return true + } + } + return false +} + +// isFDReferencedByMachineSet reports whether any MachineSet in the list has template +// region/zone labels matching the given failure domain. +func isFDReferencedByMachineSet(fd configv1.VSpherePlatformFailureDomainSpec, machineSets *machinev1beta1.MachineSetList) bool { + for _, ms := range machineSets.Items { + if ms.Spec.Template.ObjectMeta.Labels["machine.openshift.io/region"] == fd.Region && + ms.Spec.Template.ObjectMeta.Labels["machine.openshift.io/zone"] == fd.Zone { + return true + } + } + return false +} + +// findFDUsedByMachine returns a vSphere failure domain referenced by at least one Machine +// (matched via explicit region/zone labels). It prefers FDs NOT in excludeFDNames so the +// caller can avoid conflicts with other VAPs (e.g. the CPMS VAP). The exclusive flag +// indicates whether the returned FD is outside the exclude set. +func findFDUsedByMachine( + machines *machinev1beta1.MachineList, + infra *configv1.Infrastructure, + excludeFDNames map[string]bool, +) (fdName string, fdSpec configv1.VSpherePlatformFailureDomainSpec, found bool, exclusive bool) { + fds := infra.Spec.PlatformSpec.VSphere.FailureDomains + if len(machines.Items) == 0 || len(fds) == 0 { + return "", configv1.VSpherePlatformFailureDomainSpec{}, false, false + } + + var fallbackName string + var fallbackSpec configv1.VSpherePlatformFailureDomainSpec + for _, m := range machines.Items { + region := m.Labels["machine.openshift.io/region"] + zone := m.Labels["machine.openshift.io/zone"] + if region == "" || zone == "" { + continue + } + for _, fd := range fds { + if fd.Region == region && fd.Zone == zone { + if !excludeFDNames[fd.Name] { + return fd.Name, fd, true, true + } + if fallbackName == "" { + fallbackName = fd.Name + fallbackSpec = fd + } + } + } + } + if fallbackName != "" { + return fallbackName, fallbackSpec, true, false + } + return "", configv1.VSpherePlatformFailureDomainSpec{}, false, false +} + +// findFDUsedByCPMS returns the name of a vSphere failure domain that is referenced by the +// ControlPlaneMachineSet. It prefers an FD that is NOT also referenced by any worker Machine +// or MachineSet so that removing it triggers only the CPMS VAP. The returned exclusive flag +// indicates whether the match is CPMS-only (true) or shared with workers (false). When +// exclusive is false, removing the FD may trigger the Machine or MachineSet VAP first, +// producing an error that does not mention "ControlPlaneMachineSet". +// +// Returns ("", false, false) when the CPMS has no vsphere failure domain entries. +func findFDUsedByCPMS( + cpms *machinev1.ControlPlaneMachineSet, + infra *configv1.Infrastructure, + machines *machinev1beta1.MachineList, + machineSets *machinev1beta1.MachineSetList, +) (fdName string, found bool, exclusive bool) { + template := cpms.Spec.Template.OpenShiftMachineV1Beta1Machine + if template == nil { + return "", false, false + } + cpmsFDs := template.FailureDomains + if cpmsFDs == nil || len(cpmsFDs.VSphere) == 0 { + return "", false, false + } + + infraFDByName := make(map[string]configv1.VSpherePlatformFailureDomainSpec) + for _, fd := range infra.Spec.PlatformSpec.VSphere.FailureDomains { + infraFDByName[fd.Name] = fd + } + + var firstMatch string + for _, cpmsFD := range cpmsFDs.VSphere { + infraFD, ok := infraFDByName[cpmsFD.Name] + if !ok { + continue // CPMS references an FD not present in infra — skip. + } + if firstMatch == "" { + firstMatch = infraFD.Name // record the first valid match as fallback + } + // Prefer an FD not shared with any worker Machine or MachineSet. + if !isFDReferencedByMachine(infraFD, machines) && !isFDReferencedByMachineSet(infraFD, machineSets) { + return infraFD.Name, true, true + } + } + + // Fall back to the first CPMS FD that exists in infra, even if workers also use it. + if firstMatch != "" { + return firstMatch, true, false + } + return "", false, false +} + +// createVAPTestMachineSet creates a zero-replica MachineSet whose template carries +// region/zone labels matching the given failure domain. The MachineSet is named using +// the cluster infra name + vapTestMachineSetSuffix. It clones the provider spec from +// the first existing worker MachineSet. +func createVAPTestMachineSet( + ctx context.Context, + cfg *rest.Config, + mc *machineclientv1beta1.MachineV1beta1Client, + infra *configv1.Infrastructure, + fd configv1.VSpherePlatformFailureDomainSpec, +) (*machinev1beta1.MachineSet, error) { + machineSets, err := e2eutil.GetMachineSets(cfg) + if err != nil { + return nil, fmt.Errorf("could not list MachineSets: %w", err) + } + if len(machineSets.Items) == 0 { + return nil, fmt.Errorf("no MachineSets found on the cluster — cannot clone provider spec") + } + + clonedProvider := machineSets.Items[0].Spec.Template.Spec.ProviderSpec.Value + + msName := infra.Status.InfrastructureName + vapTestMachineSetSuffix + + replicas := int32(0) + ms := &machinev1beta1.MachineSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "MachineSet", + APIVersion: "machine.openshift.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: msName, + Namespace: e2eutil.MachineAPINamespace, + Labels: map[string]string{ + "machine.openshift.io/test": msName, + }, + }, + Spec: machinev1beta1.MachineSetSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "machine.openshift.io/cluster-api-cluster": infra.Status.InfrastructureName, + "machine.openshift.io/cluster-api-machineset": msName, + }, + }, + Replicas: &replicas, + Template: machinev1beta1.MachineTemplateSpec{ + ObjectMeta: machinev1beta1.ObjectMeta{ + Labels: map[string]string{ + "machine.openshift.io/cluster-api-machineset": msName, + "machine.openshift.io/cluster-api-cluster": infra.Status.InfrastructureName, + "machine.openshift.io/cluster-api-machine-role": "worker", + "machine.openshift.io/cluster-api-machine-type": "worker", + // The VAP inspects these labels to identify which FD the MachineSet references. + "machine.openshift.io/region": fd.Region, + "machine.openshift.io/zone": fd.Zone, + }, + }, + Spec: machinev1beta1.MachineSpec{ + ProviderSpec: machinev1beta1.ProviderSpec{ + Value: clonedProvider, + }, + }, + }, + }, + } + + return mc.MachineSets(e2eutil.MachineAPINamespace).Create(ctx, ms, metav1.CreateOptions{}) +} + +// waitForInfraUpdateToBeDenied polls the given Infrastructure update as a dry-run until it is +// denied. ValidatingAdmissionPolicyBinding resolves paramRef objects (e.g. a freshly created +// MachineSet) via an informer-backed cache rather than a live read, so there is a short window +// right after creating the param object during which the VAP has not yet observed it and would +// incorrectly allow the update. Using dry-run avoids mutating the Infrastructure while we wait +// for that cache to catch up. +func waitForInfraUpdateToBeDenied(ctx context.Context, cc *configclient.ConfigV1Client, updatedInfra *configv1.Infrastructure) { + Eventually(func() bool { + attemptCtx, cancel := context.WithTimeout(ctx, vapPollAttemptTimeout) + defer cancel() + _, err := cc.Infrastructures().Update(attemptCtx, updatedInfra, metav1.UpdateOptions{DryRun: []string{metav1.DryRunAll}}) + return apierrors.IsInvalid(err) || apierrors.IsForbidden(err) + }, vapTestWaitTimeout, time.Second).Should(BeTrue(), + "expected the VAP to eventually observe the newly created param object and deny a dry-run update with Invalid or Forbidden") +} + +// waitForInfraUpdateToSucceed polls a dry-run Infrastructure update removing the named failure +// domain until it succeeds, re-fetching the Infrastructure on every attempt so the dry-run always +// carries a current resourceVersion. This is the mirror image of waitForInfraUpdateToBeDenied: +// after a param MachineSet is deleted, the VAP's cached view of it may not have caught up yet, so +// an immediate update can be incorrectly denied. +func waitForInfraUpdateToSucceed(ctx context.Context, cc *configclient.ConfigV1Client, fdName string) { + Eventually(func() error { + attemptCtx, cancel := context.WithTimeout(ctx, vapPollAttemptTimeout) + defer cancel() + current, err := cc.Infrastructures().Get(attemptCtx, "cluster", metav1.GetOptions{}) + if err != nil { + return err + } + _, err = cc.Infrastructures().Update(attemptCtx, infraWithFDRemoved(current, fdName), metav1.UpdateOptions{DryRun: []string{metav1.DryRunAll}}) + return err + }, vapTestWaitTimeout, time.Second).Should(Succeed(), + "expected the VAP to eventually observe the MachineSet deletion and allow a dry-run update") +} + +var _ = Describe( + "[sig-cluster-lifecycle][OCPFeatureGate:VSphereMultiVCenterDay2][platform:vsphere] vSphere failure domain ValidatingAdmissionPolicies", + Label("Conformance"), Label("Serial"), + func() { + defer GinkgoRecover() + + ctx := context.Background() + + var ( + cfg *rest.Config + c *kubernetes.Clientset + dc *dynamic.DynamicClient + cc *configclient.ConfigV1Client + mc *machineclientv1beta1.MachineV1beta1Client + mcv1 *machineclientv1.MachineV1Client + infra *configv1.Infrastructure + err error + ) + + BeforeEach(func() { + cfg, err = e2e.LoadConfig() + Expect(err).NotTo(HaveOccurred()) + + c, err = e2e.LoadClientset() + Expect(err).NotTo(HaveOccurred()) + + dc, err = dynamic.NewForConfig(cfg) + Expect(err).NotTo(HaveOccurred()) + + mc, err = machineclientv1beta1.NewForConfig(cfg) + Expect(err).NotTo(HaveOccurred()) + + mcv1, err = machineclientv1.NewForConfig(cfg) + Expect(err).NotTo(HaveOccurred()) + + cc, err = configclient.NewForConfig(cfg) + Expect(err).NotTo(HaveOccurred()) + + infra, err = cc.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + // Ensure Machine API is running on this cluster. + e2eutil.SkipUnlessMachineAPIOperator(dc, c.CoreV1().Namespaces()) + + // All tests in this suite require at least one vSphere failure domain in the infra spec. + Expect(infra.Spec.PlatformSpec.VSphere).NotTo(BeNil(), "expected vSphere platform spec on Infrastructure/cluster") + if len(infra.Spec.PlatformSpec.VSphere.FailureDomains) == 0 { + Skip("skipping — Infrastructure/cluster has no vSphere failure domains configured") + } + }) + + It("should have three VAPs and three bindings deployed by the operator [apigroup:machine.openshift.io][Suite:openshift/conformance/serial]", func() { + By("verifying the Machine VAP exists") + _, err := c.AdmissionregistrationV1().ValidatingAdmissionPolicies().Get(ctx, mapiwebhooks.VAPMachineFailureDomainName, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected ValidatingAdmissionPolicy %q to exist", mapiwebhooks.VAPMachineFailureDomainName) + + By("verifying the CPMS VAP exists") + _, err = c.AdmissionregistrationV1().ValidatingAdmissionPolicies().Get(ctx, mapiwebhooks.VAPCPMSFailureDomainName, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected ValidatingAdmissionPolicy %q to exist", mapiwebhooks.VAPCPMSFailureDomainName) + + By("verifying the MachineSet VAP exists") + _, err = c.AdmissionregistrationV1().ValidatingAdmissionPolicies().Get(ctx, mapiwebhooks.VAPMachineSetFailureDomainName, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected ValidatingAdmissionPolicy %q to exist", mapiwebhooks.VAPMachineSetFailureDomainName) + + By("verifying the Machine VAP binding exists") + _, err = c.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Get(ctx, mapiwebhooks.VAPMachineFailureDomainName, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected ValidatingAdmissionPolicyBinding %q to exist", mapiwebhooks.VAPMachineFailureDomainName) + + By("verifying the CPMS VAP binding exists") + _, err = c.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Get(ctx, mapiwebhooks.VAPCPMSFailureDomainName, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected ValidatingAdmissionPolicyBinding %q to exist", mapiwebhooks.VAPCPMSFailureDomainName) + + By("verifying the MachineSet VAP binding exists") + _, err = c.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Get(ctx, mapiwebhooks.VAPMachineSetFailureDomainName, metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected ValidatingAdmissionPolicyBinding %q to exist", mapiwebhooks.VAPMachineSetFailureDomainName) + }) + + It("should allow removing an unused failure domain from Infrastructure [apigroup:machine.openshift.io][Suite:openshift/conformance/serial]", func() { + if len(infra.Spec.PlatformSpec.VSphere.FailureDomains) < 2 { + Skip("skipping — need at least two failure domains to remove one while keeping another") + } + + // Find a failure domain not referenced by any Machine, MachineSet, or CPMS. + machines, err := mc.Machines(e2eutil.MachineAPINamespace).List(ctx, metav1.ListOptions{}) + Expect(err).NotTo(HaveOccurred()) + + machineSets, err := e2eutil.GetMachineSets(cfg) + Expect(err).NotTo(HaveOccurred()) + + cpmsFDNames, cpmsErr := getCPMSFailureDomainNames(ctx, mcv1) + Expect(cpmsErr).NotTo(HaveOccurred()) + + var unusedFDName string + for _, fd := range infra.Spec.PlatformSpec.VSphere.FailureDomains { + if cpmsFDNames[fd.Name] { + continue + } + if isFDReferencedByMachine(fd, machines) || isFDReferencedByMachineSet(fd, machineSets) { + continue + } + unusedFDName = fd.Name + break + } + + if unusedFDName == "" { + Skip("skipping — all failure domains are in use by Machines, MachineSets, or ControlPlaneMachineSets") + } + + By(fmt.Sprintf("removing unused failure domain %q from Infrastructure", unusedFDName)) + updatedInfra := infraWithFDRemoved(infra, unusedFDName) + _, err = cc.Infrastructures().Update(ctx, updatedInfra, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected infra update removing unused FD %q to succeed", unusedFDName) + + By("restoring original Infrastructure failure domain list") + // Re-fetch to get the latest resource version, then restore. + current, err := cc.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + current.Spec.PlatformSpec.VSphere.FailureDomains = infra.Spec.PlatformSpec.VSphere.FailureDomains + _, err = cc.Infrastructures().Update(ctx, current, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected infra restore to succeed") + }) + + It("should block removing a failure domain referenced by a Machine [apigroup:machine.openshift.io][Suite:openshift/conformance/serial]", func() { + machines, err := mc.Machines(e2eutil.MachineAPINamespace).List(ctx, metav1.ListOptions{}) + Expect(err).NotTo(HaveOccurred()) + + cpmsFDNames, cpmsErr := getCPMSFailureDomainNames(ctx, mcv1) + Expect(cpmsErr).NotTo(HaveOccurred()) + fdName, _, found, exclusive := findFDUsedByMachine(machines, infra, cpmsFDNames) + if !found { + Skip("skipping — no existing Machine with region/zone labels matching a known failure domain") + } + + By(fmt.Sprintf("attempting to remove failure domain %q that is still in use by a Machine (exclusive=%v)", fdName, exclusive)) + updatedInfra := infraWithFDRemoved(infra, fdName) + _, err = cc.Infrastructures().Update(ctx, updatedInfra, metav1.UpdateOptions{}) + Expect(err).To(HaveOccurred(), "expected infra update removing in-use FD %q to be denied", fdName) + Expect(apierrors.IsInvalid(err) || apierrors.IsForbidden(err)).To(BeTrue(), + "expected a 422/Invalid or 403/Forbidden response, got: %v", err) + + // VAP evaluation order is non-deterministic — when the failure domain is + // also referenced by a MachineSet or the ControlPlaneMachineSet, any of + // those VAPs may deny first. + Expect(err.Error()).To(Or( + ContainSubstring("in use by Machine '"), + ContainSubstring("in use by MachineSet '"), + ContainSubstring("referenced by ControlPlaneMachineSet '"), + ), "expected error to mention 'Machine', 'MachineSet', or 'ControlPlaneMachineSet' as the blocking resource") + }) + + It("should block removing a failure domain referenced by a MachineSet [apigroup:machine.openshift.io][Suite:openshift/conformance/serial]", func() { + machineSets, err := e2eutil.GetMachineSets(cfg) + Expect(err).NotTo(HaveOccurred()) + if len(machineSets.Items) == 0 { + Skip("skipping — no MachineSets available to clone for this test") + } + + cpmsFDNames, cpmsErr := getCPMSFailureDomainNames(ctx, mcv1) + Expect(cpmsErr).NotTo(HaveOccurred()) + + var fd configv1.VSpherePlatformFailureDomainSpec + fdExclusive := false + for _, candidate := range infra.Spec.PlatformSpec.VSphere.FailureDomains { + if !cpmsFDNames[candidate.Name] { + fd = candidate + fdExclusive = true + break + } + } + if !fdExclusive { + fd = infra.Spec.PlatformSpec.VSphere.FailureDomains[0] + } + + By(fmt.Sprintf("creating a zero-replica MachineSet referencing failure domain %q (region=%s, zone=%s)", fd.Name, fd.Region, fd.Zone)) + testMS, err := createVAPTestMachineSet(ctx, cfg, mc, infra, fd) + Expect(err).NotTo(HaveOccurred(), "expected test MachineSet creation to succeed") + + DeferCleanup(func() { + By("cleaning up test MachineSet") + if delErr := mc.MachineSets(e2eutil.MachineAPINamespace).Delete(ctx, testMS.Name, metav1.DeleteOptions{}); delErr != nil && !apierrors.IsNotFound(delErr) { + e2e.Logf("warning: could not delete test MachineSet %q: %v", testMS.Name, delErr) + } + }) + + By(fmt.Sprintf("attempting to remove failure domain %q while it is referenced by a MachineSet", fd.Name)) + updatedInfra := infraWithFDRemoved(infra, fd.Name) + waitForInfraUpdateToBeDenied(ctx, cc, updatedInfra) + _, err = cc.Infrastructures().Update(ctx, updatedInfra, metav1.UpdateOptions{}) + Expect(err).To(HaveOccurred(), "expected infra update removing in-use FD %q to be denied", fd.Name) + Expect(apierrors.IsInvalid(err) || apierrors.IsForbidden(err)).To(BeTrue(), + "expected a 422/Invalid or 403/Forbidden response, got: %v", err) + // VAP evaluation order is non-deterministic — when the failure domain is + // also referenced by the CPMS or existing Machines, any of those VAPs may deny first. + Expect(err.Error()).To(SatisfyAny( + ContainSubstring("in use by MachineSet '"), + ContainSubstring("in use by Machine '"), + ContainSubstring("referenced by ControlPlaneMachineSet '"), + ), "expected error to mention 'MachineSet', 'Machine', or 'ControlPlaneMachineSet' as the blocking resource") + }) + + It("should block removing a failure domain referenced by a ControlPlaneMachineSet [apigroup:machine.openshift.io][Suite:openshift/conformance/serial]", func() { + By("fetching the ControlPlaneMachineSet") + cpms, err := mcv1.ControlPlaneMachineSets(e2eutil.MachineAPINamespace).Get(ctx, "cluster", metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + Skip("skipping — no ControlPlaneMachineSet 'cluster' found on this cluster") + } + Expect(err).NotTo(HaveOccurred()) + + // Fetch worker Machines and MachineSets so findFDUsedByCPMS can prefer a FD that + // is only used by the CPMS (not also by workers), ensuring the CPMS VAP fires first. + machines, err := mc.Machines(e2eutil.MachineAPINamespace).List(ctx, metav1.ListOptions{}) + Expect(err).NotTo(HaveOccurred()) + machineSets, err := e2eutil.GetMachineSets(cfg) + Expect(err).NotTo(HaveOccurred()) + + fdName, found, exclusive := findFDUsedByCPMS(cpms, infra, machines, machineSets) + if !found { + Skip("skipping — ControlPlaneMachineSet has no vSphere failure domain entries that match Infrastructure") + } + + By(fmt.Sprintf("attempting to remove failure domain %q that is still referenced by ControlPlaneMachineSet (exclusive=%v)", fdName, exclusive)) + updatedInfra := infraWithFDRemoved(infra, fdName) + _, err = cc.Infrastructures().Update(ctx, updatedInfra, metav1.UpdateOptions{}) + Expect(err).To(HaveOccurred(), "expected infra update removing in-use FD %q to be denied", fdName) + Expect(apierrors.IsInvalid(err) || apierrors.IsForbidden(err)).To(BeTrue(), + "expected a 422/Invalid or 403/Forbidden response, got: %v", err) + if exclusive { + Expect(err.Error()).To(ContainSubstring("ControlPlaneMachineSet"), + "expected error to mention 'ControlPlaneMachineSet' as the blocking resource") + } + }) + + It("should allow removing a failure domain after the referencing MachineSet is deleted [apigroup:machine.openshift.io][Suite:openshift/conformance/serial]", func() { + machineSets, err := e2eutil.GetMachineSets(cfg) + Expect(err).NotTo(HaveOccurred()) + if len(machineSets.Items) == 0 { + Skip("skipping — no MachineSets available to clone for this test") + } + + if len(infra.Spec.PlatformSpec.VSphere.FailureDomains) < 2 { + Skip("skipping — need at least two failure domains so we can remove one without breaking the cluster") + } + + // Find an FD that has no existing Machines (of any role), is not referenced by the + // CPMS, and is not referenced by any existing MachineSet. Our test MachineSet will be + // the *only* thing referencing it, so once we delete it the infra update must succeed. + machines, err := mc.Machines(e2eutil.MachineAPINamespace).List(ctx, metav1.ListOptions{}) + Expect(err).NotTo(HaveOccurred()) + + cpmsFDNames, cpmsErr := getCPMSFailureDomainNames(ctx, mcv1) + Expect(cpmsErr).NotTo(HaveOccurred()) + + var fd *configv1.VSpherePlatformFailureDomainSpec + for i := range infra.Spec.PlatformSpec.VSphere.FailureDomains { + candidate := &infra.Spec.PlatformSpec.VSphere.FailureDomains[i] + if cpmsFDNames[candidate.Name] { + continue + } + if isFDReferencedByMachine(*candidate, machines) || isFDReferencedByMachineSet(*candidate, machineSets) { + continue + } + fd = candidate + break + } + if fd == nil { + Skip("skipping — every failure domain is referenced by an existing Machine, MachineSet, or ControlPlaneMachineSet; cannot isolate test MachineSet as the sole blocker") + } + + By(fmt.Sprintf("creating a zero-replica MachineSet referencing failure domain %q", fd.Name)) + testMS, err := createVAPTestMachineSet(ctx, cfg, mc, infra, *fd) + Expect(err).NotTo(HaveOccurred()) + + // Always attempt cleanup so the MachineSet doesn't leak if the test fails mid-way. + DeferCleanup(func() { + if delErr := mc.MachineSets(e2eutil.MachineAPINamespace).Delete(ctx, testMS.Name, metav1.DeleteOptions{}); delErr != nil && !apierrors.IsNotFound(delErr) { + e2e.Logf("warning: could not delete test MachineSet %q: %v", testMS.Name, delErr) + } + // Restore the infra in case the test succeeded and removed the FD. + current, getErr := cc.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}) + if getErr != nil { + e2e.Logf("warning: could not get Infrastructure for restore: %v", getErr) + return + } + alreadyPresent := false + for _, existingFD := range current.Spec.PlatformSpec.VSphere.FailureDomains { + if existingFD.Name == fd.Name { + alreadyPresent = true + break + } + } + if !alreadyPresent { + current.Spec.PlatformSpec.VSphere.FailureDomains = infra.Spec.PlatformSpec.VSphere.FailureDomains + if _, restoreErr := cc.Infrastructures().Update(ctx, current, metav1.UpdateOptions{}); restoreErr != nil { + e2e.Logf("warning: could not restore Infrastructure failure domains: %v", restoreErr) + } + } + }) + + By("verifying that the MachineSet VAP blocks removal of the failure domain") + updatedInfra := infraWithFDRemoved(infra, fd.Name) + waitForInfraUpdateToBeDenied(ctx, cc, updatedInfra) + _, err = cc.Infrastructures().Update(ctx, updatedInfra, metav1.UpdateOptions{}) + Expect(err).To(HaveOccurred(), "expected infra update to be denied while MachineSet references FD %q", fd.Name) + Expect(apierrors.IsInvalid(err) || apierrors.IsForbidden(err)).To(BeTrue(), + "expected a 422/Invalid or 403/Forbidden response, got: %v", err) + + By("deleting the test MachineSet") + Expect(mc.MachineSets(e2eutil.MachineAPINamespace).Delete(ctx, testMS.Name, metav1.DeleteOptions{})).To(Succeed()) + + By("waiting for the MachineSet to be fully deleted") + Eventually(func() bool { + _, getErr := mc.MachineSets(e2eutil.MachineAPINamespace).Get(ctx, testMS.Name, metav1.GetOptions{}) + return apierrors.IsNotFound(getErr) + }, vapTestWaitTimeout, 5*time.Second).Should(BeTrue(), "MachineSet %q should be deleted within %s", testMS.Name, vapTestWaitTimeout) + + By(fmt.Sprintf("retrying Infrastructure update to remove failure domain %q — should now succeed", fd.Name)) + waitForInfraUpdateToSucceed(ctx, cc, fd.Name) + // Re-fetch so we have the latest resource version. + freshInfra, err := cc.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + freshWithFDRemoved := infraWithFDRemoved(freshInfra, fd.Name) + _, err = cc.Infrastructures().Update(ctx, freshWithFDRemoved, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred(), "expected infra update to succeed after MachineSet referencing FD %q was deleted", fd.Name) + }) + + It("should allow an unrelated Infrastructure update when a MachineSet's region/zone labels match no failure domain [apigroup:machine.openshift.io][Suite:openshift/conformance/serial]", func() { + // Regression test for SPLAT-2826: a MachineSet whose region/zone labels never matched + // any failure domain — in the OLD spec or the NEW spec — must not block ANY + // Infrastructure update, including a no-op re-apply of the unchanged spec. + bogusFD := configv1.VSpherePlatformFailureDomainSpec{ + Region: "splat-2826-unmatched-region", + Zone: "splat-2826-unmatched-zone", + } + for _, fd := range infra.Spec.PlatformSpec.VSphere.FailureDomains { + Expect(fd.Region == bogusFD.Region && fd.Zone == bogusFD.Zone).To(BeFalse(), + "test precondition: bogus region+zone pair must not collide with a real failure domain") + } + + By("creating a zero-replica MachineSet whose region/zone labels match no known failure domain") + testMS, err := createVAPTestMachineSet(ctx, cfg, mc, infra, bogusFD) + Expect(err).NotTo(HaveOccurred(), "expected test MachineSet creation to succeed") + + DeferCleanup(func() { + By("cleaning up test MachineSet") + delErr := mc.MachineSets(e2eutil.MachineAPINamespace).Delete(ctx, testMS.Name, metav1.DeleteOptions{}) + if delErr != nil && !apierrors.IsNotFound(delErr) { + e2e.Logf("warning: could not delete test MachineSet %q: %v", testMS.Name, delErr) + return + } + // createVAPTestMachineSet always uses the same deterministic name, so a subsequent + // test's Create can collide if this one is still terminating — wait for confirmed + // absence before cleanup completes. + Eventually(func() bool { + _, getErr := mc.MachineSets(e2eutil.MachineAPINamespace).Get(ctx, testMS.Name, metav1.GetOptions{}) + return apierrors.IsNotFound(getErr) + }, vapTestWaitTimeout, 5*time.Second).Should(BeTrue(), "MachineSet %q should be deleted within %s", testMS.Name, vapTestWaitTimeout) + }) + + // The VAP's ParamRef resolves MachineSets via an informer-backed cache, so immediately + // after creating testMS the VAP may not yet be evaluating it — a single Update run right + // away could pass "by accident" before the fixed oldFds logic is ever exercised against + // testMS's mismatched labels. Poll dry-run updates with Consistently across a bounded + // window that comfortably covers cache propagation, so the assertion is proven both + // before and after testMS is actually observed by the VAP. + By("verifying dry-run no-op updates consistently succeed while the MachineSet is observed") + Consistently(func() error { + attemptCtx, cancel := context.WithTimeout(ctx, vapPollAttemptTimeout) + defer cancel() + for { + latest, getErr := cc.Infrastructures().Get(attemptCtx, "cluster", metav1.GetOptions{}) + if getErr != nil { + return getErr + } + _, updErr := cc.Infrastructures().Update(attemptCtx, latest, metav1.UpdateOptions{DryRun: []string{metav1.DryRunAll}}) + if updErr != nil && apierrors.IsConflict(updErr) { + // Someone else updated the Infrastructure object between our Get and dry-run + // Update (e.g. a status refresh bumping resourceVersion) — retry the whole + // sequence with a fresh object rather than treating this as a VAP denial. + select { + case <-attemptCtx.Done(): + return updErr + default: + continue + } + } + return updErr + } + }, vapCacheSyncWindow, time.Second).Should(Succeed(), + "expected the no-op Infrastructure update to remain allowed throughout the VAP cache-sync window, even once the MachineSet with mismatched region/zone labels is observed (SPLAT-2826 regression)") + + By("re-applying the unchanged Infrastructure spec (no-op update)") + current, err := cc.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + _, err = cc.Infrastructures().Update(ctx, current, metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred(), + "expected a no-op Infrastructure update to succeed even though a MachineSet exists whose region/zone labels match no failure domain (SPLAT-2826 regression)") + }) + }, +) diff --git a/vendor/github.com/openshift/api/config/v1/types_infrastructure.go b/vendor/github.com/openshift/api/config/v1/types_infrastructure.go index 4d911877e8..6906ec9b83 100644 --- a/vendor/github.com/openshift/api/config/v1/types_infrastructure.go +++ b/vendor/github.com/openshift/api/config/v1/types_infrastructure.go @@ -284,7 +284,8 @@ type ExternalPlatformSpec struct { // PlatformSpec holds the desired state specific to the underlying infrastructure provider // of the current cluster. Since these are used at spec-level for the underlying cluster, it // is supposed that only one of the spec structs is set. -// +kubebuilder:validation:XValidation:rule="!has(oldSelf.vsphere) && has(self.vsphere) ? size(self.vsphere.vcenters) < 2 : true",message="vcenters can have at most 1 item when configured post-install" +// +openshift:validation:FeatureGateAwareXValidation:featureGate="",rule="!has(oldSelf.vsphere) && has(self.vsphere) ? (has(self.vsphere.vcenters) && size(self.vsphere.vcenters) < 2) : true",message="vcenters can have at most 1 item when configured post-install" +// +openshift:validation:FeatureGateAwareXValidation:featureGate=VSphereMultiVCenterDay2,rule="oldSelf.?vsphere.vcenters.hasValue() ? self.?vsphere.vcenters.hasValue() : true",message="vcenters is required once set and cannot be removed" type PlatformSpec struct { // type is the underlying infrastructure provider for the cluster. This // value controls whether infrastructure automation such as service load @@ -1334,6 +1335,9 @@ type VSpherePlatformFailureDomainSpec struct { ZoneAffinity *VSphereFailureDomainZoneAffinity `json:"zoneAffinity,omitempty"` // server is the fully-qualified domain name or the IP address of the vCenter server. + // This must match the server field of an entry in the vcenters list. + // The match is case-sensitive; the value must be specified exactly as it appears in the vcenters entry. + // The value must be between 1 and 255 characters long. // +required // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=255 @@ -1568,27 +1572,32 @@ type VSpherePlatformNodeNetworking struct { // use these fields for configuration. // +kubebuilder:validation:XValidation:rule="!has(oldSelf.apiServerInternalIPs) || has(self.apiServerInternalIPs)",message="apiServerInternalIPs list is required once set" // +kubebuilder:validation:XValidation:rule="!has(oldSelf.ingressIPs) || has(self.ingressIPs)",message="ingressIPs list is required once set" -// +kubebuilder:validation:XValidation:rule="!has(oldSelf.vcenters) && has(self.vcenters) ? size(self.vcenters) < 2 : true",message="vcenters can have at most 1 item when configured post-install" +// +openshift:validation:FeatureGateAwareXValidation:featureGate=VSphereMultiVCenterDay2,rule="!has(self.failureDomains) || size(self.failureDomains) == 0 || (has(self.vcenters) && self.failureDomains.all(fd, self.vcenters.exists(vc, vc.server == fd.server)))",message="all failure domains must have a corresponding vCenter entry" type VSpherePlatformSpec struct { // vcenters holds the connection details for services to communicate with vCenter. - // Currently, only a single vCenter is supported, but in tech preview 3 vCenters are supported. + // Up to 3 vCenters are supported. // Once the cluster has been installed, you are unable to change the current number of defined - // vCenters except in the case where the cluster has been upgraded from a version of OpenShift - // where the vsphere platform spec was not present. You may make modifications to the existing + // vCenters except when 1.) the cluster has been upgraded from a version of OpenShift + // where the vsphere platform spec was not present or 2.) in TechPreview you are able to add and + // remove vCenters but may not remove all vCenters. You may make modifications to the existing // vCenters that are defined in the vcenters list in order to match with any added or modified // failure domains. // --- // + If VCenters is not defined use the existing cloud-config configmap defined // + in openshift-config. - // +kubebuilder:validation:MinItems=0 + // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=3 - // +kubebuilder:validation:XValidation:rule="size(self) != size(oldSelf) ? size(oldSelf) == 0 && size(self) < 2 : true",message="vcenters cannot be added or removed once set" + // +openshift:validation:FeatureGateAwareXValidation:featureGate="",rule="size(self) != size(oldSelf) ? size(oldSelf) == 0 && size(self) < 2 : true",message="vcenters cannot be added or removed once set" + // +openshift:validation:FeatureGateAwareXValidation:featureGate=VSphereMultiVCenterDay2,rule="size(self) >= size(oldSelf) ? oldSelf.all(x, self.exists(y, y.server == x.server)) : true",message="Cannot add and remove vCenters at the same time" + // +openshift:validation:FeatureGateAwareXValidation:featureGate=VSphereMultiVCenterDay2,rule="size(self) < size(oldSelf) ? self.all(x, oldSelf.exists(y, y.server == x.server)) : true",message="Cannot add and remove vCenters at the same time" + // +kubebuilder:validation:XValidation:rule="self.all(x, self.exists_one(y, y.server == x.server))",message="vcenters must have unique server values" // +listType=atomic // +optional VCenters []VSpherePlatformVCenterSpec `json:"vcenters,omitempty"` // failureDomains contains the definition of region, zone and the vCenter topology. // If this is omitted failure domains (regions and zones) will not be used. + // Each failure domain's server must match the server field of an entry in the vcenters list. // +listType=map // +listMapKey=name // +optional diff --git a/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml b/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml index b8d0b7b765..b9a6bb50a6 100644 --- a/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml +++ b/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-CustomNoUpgrade.crd.yaml @@ -686,6 +686,7 @@ spec: description: |- failureDomains contains the definition of region, zone and the vCenter topology. If this is omitted failure domains (regions and zones) will not be used. + Each failure domain's server must match the server field of an entry in the vcenters list. items: description: VSpherePlatformFailureDomainSpec holds the region and zone failure domain and the vCenter topology @@ -730,8 +731,11 @@ spec: - format: ipv4 - format: ipv6 - format: hostname - description: server is the fully-qualified domain name - or the IP address of the vCenter server. + description: |- + server is the fully-qualified domain name or the IP address of the vCenter server. + This must match the server field of an entry in the vcenters list. + The match is case-sensitive; the value must be specified exactly as it appears in the vcenters entry. + The value must be between 1 and 255 characters long. maxLength: 255 minLength: 1 type: string @@ -1030,10 +1034,11 @@ spec: vcenters: description: |- vcenters holds the connection details for services to communicate with vCenter. - Currently, only a single vCenter is supported, but in tech preview 3 vCenters are supported. + Up to 3 vCenters are supported. Once the cluster has been installed, you are unable to change the current number of defined - vCenters except in the case where the cluster has been upgraded from a version of OpenShift - where the vsphere platform spec was not present. You may make modifications to the existing + vCenters except when 1.) the cluster has been upgraded from a version of OpenShift + where the vsphere platform spec was not present or 2.) in TechPreview you are able to add and + remove vCenters but may not remove all vCenters. You may make modifications to the existing vCenters that are defined in the vcenters list in order to match with any added or modified failure domains. items: @@ -1078,27 +1083,34 @@ spec: - server type: object maxItems: 3 - minItems: 0 + minItems: 1 type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - - message: vcenters cannot be added or removed once set - rule: 'size(self) != size(oldSelf) ? size(oldSelf) == 0 - && size(self) < 2 : true' + - message: Cannot add and remove vCenters at the same time + rule: 'size(self) >= size(oldSelf) ? oldSelf.all(x, self.exists(y, + y.server == x.server)) : true' + - message: Cannot add and remove vCenters at the same time + rule: 'size(self) < size(oldSelf) ? self.all(x, oldSelf.exists(y, + y.server == x.server)) : true' + - message: vcenters must have unique server values + rule: self.all(x, self.exists_one(y, y.server == x.server)) type: object x-kubernetes-validations: + - message: all failure domains must have a corresponding vCenter + entry + rule: '!has(self.failureDomains) || size(self.failureDomains) + == 0 || (has(self.vcenters) && self.failureDomains.all(fd, + self.vcenters.exists(vc, vc.server == fd.server)))' - message: apiServerInternalIPs list is required once set rule: '!has(oldSelf.apiServerInternalIPs) || has(self.apiServerInternalIPs)' - message: ingressIPs list is required once set rule: '!has(oldSelf.ingressIPs) || has(self.ingressIPs)' - - message: vcenters can have at most 1 item when configured post-install - rule: '!has(oldSelf.vcenters) && has(self.vcenters) ? size(self.vcenters) - < 2 : true' type: object x-kubernetes-validations: - - message: vcenters can have at most 1 item when configured post-install - rule: '!has(oldSelf.vsphere) && has(self.vsphere) ? size(self.vsphere.vcenters) - < 2 : true' + - message: vcenters is required once set and cannot be removed + rule: 'oldSelf.?vsphere.vcenters.hasValue() ? self.?vsphere.vcenters.hasValue() + : true' type: object status: description: status holds observed values from the cluster. They may not diff --git a/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yaml b/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yaml index 1ae221ca45..948b87fdad 100644 --- a/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yaml +++ b/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yaml @@ -620,6 +620,7 @@ spec: description: |- failureDomains contains the definition of region, zone and the vCenter topology. If this is omitted failure domains (regions and zones) will not be used. + Each failure domain's server must match the server field of an entry in the vcenters list. items: description: VSpherePlatformFailureDomainSpec holds the region and zone failure domain and the vCenter topology @@ -664,8 +665,11 @@ spec: - format: ipv4 - format: ipv6 - format: hostname - description: server is the fully-qualified domain name - or the IP address of the vCenter server. + description: |- + server is the fully-qualified domain name or the IP address of the vCenter server. + This must match the server field of an entry in the vcenters list. + The match is case-sensitive; the value must be specified exactly as it appears in the vcenters entry. + The value must be between 1 and 255 characters long. maxLength: 255 minLength: 1 type: string @@ -953,10 +957,11 @@ spec: vcenters: description: |- vcenters holds the connection details for services to communicate with vCenter. - Currently, only a single vCenter is supported, but in tech preview 3 vCenters are supported. + Up to 3 vCenters are supported. Once the cluster has been installed, you are unable to change the current number of defined - vCenters except in the case where the cluster has been upgraded from a version of OpenShift - where the vsphere platform spec was not present. You may make modifications to the existing + vCenters except when 1.) the cluster has been upgraded from a version of OpenShift + where the vsphere platform spec was not present or 2.) in TechPreview you are able to add and + remove vCenters but may not remove all vCenters. You may make modifications to the existing vCenters that are defined in the vcenters list in order to match with any added or modified failure domains. items: @@ -1001,27 +1006,23 @@ spec: - server type: object maxItems: 3 - minItems: 0 + minItems: 1 type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - - message: vcenters cannot be added or removed once set - rule: 'size(self) != size(oldSelf) ? size(oldSelf) == 0 - && size(self) < 2 : true' + - message: vcenters must have unique server values + rule: self.all(x, self.exists_one(y, y.server == x.server)) type: object x-kubernetes-validations: - message: apiServerInternalIPs list is required once set rule: '!has(oldSelf.apiServerInternalIPs) || has(self.apiServerInternalIPs)' - message: ingressIPs list is required once set rule: '!has(oldSelf.ingressIPs) || has(self.ingressIPs)' - - message: vcenters can have at most 1 item when configured post-install - rule: '!has(oldSelf.vcenters) && has(self.vcenters) ? size(self.vcenters) - < 2 : true' type: object x-kubernetes-validations: - message: vcenters can have at most 1 item when configured post-install - rule: '!has(oldSelf.vsphere) && has(self.vsphere) ? size(self.vsphere.vcenters) - < 2 : true' + rule: '!has(oldSelf.vsphere) && has(self.vsphere) ? (has(self.vsphere.vcenters) + && size(self.vsphere.vcenters) < 2) : true' type: object status: description: status holds observed values from the cluster. They may not diff --git a/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml b/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml index bf96f8f2e5..88c18e794b 100644 --- a/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml +++ b/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-DevPreviewNoUpgrade.crd.yaml @@ -686,6 +686,7 @@ spec: description: |- failureDomains contains the definition of region, zone and the vCenter topology. If this is omitted failure domains (regions and zones) will not be used. + Each failure domain's server must match the server field of an entry in the vcenters list. items: description: VSpherePlatformFailureDomainSpec holds the region and zone failure domain and the vCenter topology @@ -730,8 +731,11 @@ spec: - format: ipv4 - format: ipv6 - format: hostname - description: server is the fully-qualified domain name - or the IP address of the vCenter server. + description: |- + server is the fully-qualified domain name or the IP address of the vCenter server. + This must match the server field of an entry in the vcenters list. + The match is case-sensitive; the value must be specified exactly as it appears in the vcenters entry. + The value must be between 1 and 255 characters long. maxLength: 255 minLength: 1 type: string @@ -1030,10 +1034,11 @@ spec: vcenters: description: |- vcenters holds the connection details for services to communicate with vCenter. - Currently, only a single vCenter is supported, but in tech preview 3 vCenters are supported. + Up to 3 vCenters are supported. Once the cluster has been installed, you are unable to change the current number of defined - vCenters except in the case where the cluster has been upgraded from a version of OpenShift - where the vsphere platform spec was not present. You may make modifications to the existing + vCenters except when 1.) the cluster has been upgraded from a version of OpenShift + where the vsphere platform spec was not present or 2.) in TechPreview you are able to add and + remove vCenters but may not remove all vCenters. You may make modifications to the existing vCenters that are defined in the vcenters list in order to match with any added or modified failure domains. items: @@ -1078,27 +1083,34 @@ spec: - server type: object maxItems: 3 - minItems: 0 + minItems: 1 type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - - message: vcenters cannot be added or removed once set - rule: 'size(self) != size(oldSelf) ? size(oldSelf) == 0 - && size(self) < 2 : true' + - message: Cannot add and remove vCenters at the same time + rule: 'size(self) >= size(oldSelf) ? oldSelf.all(x, self.exists(y, + y.server == x.server)) : true' + - message: Cannot add and remove vCenters at the same time + rule: 'size(self) < size(oldSelf) ? self.all(x, oldSelf.exists(y, + y.server == x.server)) : true' + - message: vcenters must have unique server values + rule: self.all(x, self.exists_one(y, y.server == x.server)) type: object x-kubernetes-validations: + - message: all failure domains must have a corresponding vCenter + entry + rule: '!has(self.failureDomains) || size(self.failureDomains) + == 0 || (has(self.vcenters) && self.failureDomains.all(fd, + self.vcenters.exists(vc, vc.server == fd.server)))' - message: apiServerInternalIPs list is required once set rule: '!has(oldSelf.apiServerInternalIPs) || has(self.apiServerInternalIPs)' - message: ingressIPs list is required once set rule: '!has(oldSelf.ingressIPs) || has(self.ingressIPs)' - - message: vcenters can have at most 1 item when configured post-install - rule: '!has(oldSelf.vcenters) && has(self.vcenters) ? size(self.vcenters) - < 2 : true' type: object x-kubernetes-validations: - - message: vcenters can have at most 1 item when configured post-install - rule: '!has(oldSelf.vsphere) && has(self.vsphere) ? size(self.vsphere.vcenters) - < 2 : true' + - message: vcenters is required once set and cannot be removed + rule: 'oldSelf.?vsphere.vcenters.hasValue() ? self.?vsphere.vcenters.hasValue() + : true' type: object status: description: status holds observed values from the cluster. They may not diff --git a/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml b/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml index 86dd58e48a..7dec7c8f0e 100644 --- a/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml +++ b/vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml @@ -686,6 +686,7 @@ spec: description: |- failureDomains contains the definition of region, zone and the vCenter topology. If this is omitted failure domains (regions and zones) will not be used. + Each failure domain's server must match the server field of an entry in the vcenters list. items: description: VSpherePlatformFailureDomainSpec holds the region and zone failure domain and the vCenter topology @@ -730,8 +731,11 @@ spec: - format: ipv4 - format: ipv6 - format: hostname - description: server is the fully-qualified domain name - or the IP address of the vCenter server. + description: |- + server is the fully-qualified domain name or the IP address of the vCenter server. + This must match the server field of an entry in the vcenters list. + The match is case-sensitive; the value must be specified exactly as it appears in the vcenters entry. + The value must be between 1 and 255 characters long. maxLength: 255 minLength: 1 type: string @@ -1030,10 +1034,11 @@ spec: vcenters: description: |- vcenters holds the connection details for services to communicate with vCenter. - Currently, only a single vCenter is supported, but in tech preview 3 vCenters are supported. + Up to 3 vCenters are supported. Once the cluster has been installed, you are unable to change the current number of defined - vCenters except in the case where the cluster has been upgraded from a version of OpenShift - where the vsphere platform spec was not present. You may make modifications to the existing + vCenters except when 1.) the cluster has been upgraded from a version of OpenShift + where the vsphere platform spec was not present or 2.) in TechPreview you are able to add and + remove vCenters but may not remove all vCenters. You may make modifications to the existing vCenters that are defined in the vcenters list in order to match with any added or modified failure domains. items: @@ -1078,27 +1083,34 @@ spec: - server type: object maxItems: 3 - minItems: 0 + minItems: 1 type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - - message: vcenters cannot be added or removed once set - rule: 'size(self) != size(oldSelf) ? size(oldSelf) == 0 - && size(self) < 2 : true' + - message: Cannot add and remove vCenters at the same time + rule: 'size(self) >= size(oldSelf) ? oldSelf.all(x, self.exists(y, + y.server == x.server)) : true' + - message: Cannot add and remove vCenters at the same time + rule: 'size(self) < size(oldSelf) ? self.all(x, oldSelf.exists(y, + y.server == x.server)) : true' + - message: vcenters must have unique server values + rule: self.all(x, self.exists_one(y, y.server == x.server)) type: object x-kubernetes-validations: + - message: all failure domains must have a corresponding vCenter + entry + rule: '!has(self.failureDomains) || size(self.failureDomains) + == 0 || (has(self.vcenters) && self.failureDomains.all(fd, + self.vcenters.exists(vc, vc.server == fd.server)))' - message: apiServerInternalIPs list is required once set rule: '!has(oldSelf.apiServerInternalIPs) || has(self.apiServerInternalIPs)' - message: ingressIPs list is required once set rule: '!has(oldSelf.ingressIPs) || has(self.ingressIPs)' - - message: vcenters can have at most 1 item when configured post-install - rule: '!has(oldSelf.vcenters) && has(self.vcenters) ? size(self.vcenters) - < 2 : true' type: object x-kubernetes-validations: - - message: vcenters can have at most 1 item when configured post-install - rule: '!has(oldSelf.vsphere) && has(self.vsphere) ? size(self.vsphere.vcenters) - < 2 : true' + - message: vcenters is required once set and cannot be removed + rule: 'oldSelf.?vsphere.vcenters.hasValue() ? self.?vsphere.vcenters.hasValue() + : true' type: object status: description: status holds observed values from the cluster. They may not diff --git a/vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml b/vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml index 6d756e8f90..006fdd729d 100644 --- a/vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml +++ b/vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml @@ -372,6 +372,7 @@ infrastructures.config.openshift.io: - NutanixMultiSubnets - VSphereHostVMGroupZonal - VSphereMultiNetworks + - VSphereMultiVCenterDay2 FilenameOperatorName: config-operator FilenameOperatorOrdering: "01" FilenameRunLevel: "0000_10" diff --git a/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go b/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go index 13ae075da9..2c727d2827 100644 --- a/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go +++ b/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go @@ -2007,7 +2007,7 @@ var map_VSpherePlatformFailureDomainSpec = map[string]string{ "zone": "zone defines the name of a zone tag that will be attached to a vCenter cluster. The tag category in vCenter must be named openshift-zone.", "regionAffinity": "regionAffinity holds the type of region, Datacenter or ComputeCluster. When set to Datacenter, this means the region is a vCenter Datacenter as defined in topology. When set to ComputeCluster, this means the region is a vCenter Cluster as defined in topology.", "zoneAffinity": "zoneAffinity holds the type of the zone and the hostGroup which vmGroup and the hostGroup names in vCenter corresponds to a vm-host group of type Virtual Machine and Host respectively. Is also contains the vmHostRule which is an affinity vm-host rule in vCenter.", - "server": "server is the fully-qualified domain name or the IP address of the vCenter server.", + "server": "server is the fully-qualified domain name or the IP address of the vCenter server. This must match the server field of an entry in the vcenters list. The match is case-sensitive; the value must be specified exactly as it appears in the vcenters entry. The value must be between 1 and 255 characters long.", "topology": "topology describes a given failure domain using vSphere constructs", } @@ -2047,8 +2047,8 @@ func (VSpherePlatformNodeNetworkingSpec) SwaggerDoc() map[string]string { var map_VSpherePlatformSpec = map[string]string{ "": "VSpherePlatformSpec holds the desired state of the vSphere infrastructure provider. In the future the cloud provider operator, storage operator and machine operator will use these fields for configuration.", - "vcenters": "vcenters holds the connection details for services to communicate with vCenter. Currently, only a single vCenter is supported, but in tech preview 3 vCenters are supported. Once the cluster has been installed, you are unable to change the current number of defined vCenters except in the case where the cluster has been upgraded from a version of OpenShift where the vsphere platform spec was not present. You may make modifications to the existing vCenters that are defined in the vcenters list in order to match with any added or modified failure domains.", - "failureDomains": "failureDomains contains the definition of region, zone and the vCenter topology. If this is omitted failure domains (regions and zones) will not be used.", + "vcenters": "vcenters holds the connection details for services to communicate with vCenter. Up to 3 vCenters are supported. Once the cluster has been installed, you are unable to change the current number of defined vCenters except when 1.) the cluster has been upgraded from a version of OpenShift where the vsphere platform spec was not present or 2.) in TechPreview you are able to add and remove vCenters but may not remove all vCenters. You may make modifications to the existing vCenters that are defined in the vcenters list in order to match with any added or modified failure domains.", + "failureDomains": "failureDomains contains the definition of region, zone and the vCenter topology. If this is omitted failure domains (regions and zones) will not be used. Each failure domain's server must match the server field of an entry in the vcenters list.", "nodeNetworking": "nodeNetworking contains the definition of internal and external network constraints for assigning the node's networking. If this field is omitted, networking defaults to the legacy address selection behavior which is to only support a single address and return the first one found.", "apiServerInternalIPs": "apiServerInternalIPs are the IP addresses to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. These are the IPs for a self-hosted load balancer in front of the API servers. In dual stack clusters this list contains two IP addresses, one from IPv4 family and one from IPv6. In single stack clusters a single IP address is expected. When omitted, values from the status.apiServerInternalIPs will be used. Once set, the list cannot be completely removed (but its second entry can).", "ingressIPs": "ingressIPs are the external IPs which route to the default ingress controller. The IPs are suitable targets of a wildcard DNS record used to resolve default route host names. In dual stack clusters this list contains two IP addresses, one from IPv4 family and one from IPv6. In single stack clusters a single IP address is expected. When omitted, values from the status.ingressIPs will be used. Once set, the list cannot be completely removed (but its second entry can).", diff --git a/vendor/github.com/openshift/api/features.md b/vendor/github.com/openshift/api/features.md index 792c55505b..9d1d91adf6 100644 --- a/vendor/github.com/openshift/api/features.md +++ b/vendor/github.com/openshift/api/features.md @@ -63,6 +63,7 @@ | TranslateStreamCloseWebsocketRequests| | | Enabled | Enabled | Enabled | Enabled | | VSphereConfigurableMaxAllowedBlockVolumesPerNode| | | Enabled | Enabled | Enabled | Enabled | | VSphereHostVMGroupZonal| | | Enabled | Enabled | Enabled | Enabled | +| VSphereMultiVCenterDay2| | | Enabled | Enabled | Enabled | Enabled | | VolumeAttributesClass| | | Enabled | Enabled | Enabled | Enabled | | VolumeGroupSnapshot| | | Enabled | Enabled | Enabled | Enabled | | AdditionalRoutingCapabilities| Enabled | Enabled | Enabled | Enabled | Enabled | Enabled | @@ -76,6 +77,7 @@ | ExternalOIDCWithUIDAndExtraClaimMappings| Enabled | Enabled | Enabled | Enabled | Enabled | Enabled | | GatewayAPI| Enabled | Enabled | Enabled | Enabled | Enabled | Enabled | | GatewayAPIController| Enabled | Enabled | Enabled | Enabled | Enabled | Enabled | +| GatewayAPIWithoutOLM| Enabled | Enabled | Enabled | Enabled | Enabled | Enabled | | HighlyAvailableArbiter| Enabled | Enabled | Enabled | Enabled | Enabled | Enabled | | ImageVolume| Enabled | Enabled | Enabled | Enabled | Enabled | Enabled | | IngressControllerLBSubnetsAWS| Enabled | Enabled | Enabled | Enabled | Enabled | Enabled | diff --git a/vendor/github.com/openshift/api/features/features.go b/vendor/github.com/openshift/api/features/features.go index c555d650fd..6bf0981795 100644 --- a/vendor/github.com/openshift/api/features/features.go +++ b/vendor/github.com/openshift/api/features/features.go @@ -783,6 +783,14 @@ var ( enableIn(configv1.DevPreviewNoUpgrade). mustRegister() + FeatureGateVSphereMultiVCenterDay2 = newFeatureGate("VSphereMultiVCenterDay2"). + reportProblemsToJiraComponent("splat"). + contactPerson("vr4manta"). + productScope(ocpSpecific). + enhancementPR("https://github.com/openshift/enhancements/pull/1961"). + enableIn(configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade). + mustRegister() + FeatureGatePreconfiguredUDNAddresses = newFeatureGate("PreconfiguredUDNAddresses"). reportProblemsToJiraComponent("Networking/ovn-kubernetes"). contactPerson("kyrtapz"). @@ -846,4 +854,12 @@ var ( enhancementPR("https://github.com/openshift/enhancements/pull/1785"). enableIn(configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade). mustRegister() + + FeatureGateGatewayAPIWithoutOLM = newFeatureGate("GatewayAPIWithoutOLM"). + reportProblemsToJiraComponent("Routing"). + contactPerson("miciah"). + productScope(ocpSpecific). + enhancementPR("https://github.com/openshift/enhancements/pull/1933"). + enableIn(configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade, configv1.Default). + mustRegister() ) diff --git a/vendor/github.com/openshift/api/security/v1/types.go b/vendor/github.com/openshift/api/security/v1/types.go index fb491480d7..09736e3eec 100644 --- a/vendor/github.com/openshift/api/security/v1/types.go +++ b/vendor/github.com/openshift/api/security/v1/types.go @@ -217,6 +217,7 @@ var ( FSTypeCSI FSType = "csi" FSTypeEphemeral FSType = "ephemeral" FSTypeImage FSType = "image" + FSTypeServiceAccountToken FSType = "serviceAccountToken" FSTypeAll FSType = "*" FSTypeNone FSType = "none" ) diff --git a/vendor/modules.txt b/vendor/modules.txt index 1e44303509..da62ca5124 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -995,7 +995,7 @@ github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo github.com/openshift-eng/openshift-tests-extension/pkg/junit github.com/openshift-eng/openshift-tests-extension/pkg/util/sets github.com/openshift-eng/openshift-tests-extension/pkg/version -# github.com/openshift/api v0.0.0-20260605122244-2c1c5b39566d +# github.com/openshift/api v0.0.0-20260605122244-2c1c5b39566d => github.com/vr4manta/api v0.0.0-20260914125654-86c3aa9e7ae5 ## explicit; go 1.24.0 github.com/openshift/api github.com/openshift/api/annotations @@ -3649,3 +3649,4 @@ sigs.k8s.io/yaml/goyaml.v3 # k8s.io/apiserver => github.com/openshift/kubernetes/staging/src/k8s.io/apiserver v0.0.0-20250716113245-b94367cabf3e # k8s.io/kubelet => github.com/openshift/kubernetes/staging/src/k8s.io/kubelet v0.0.0-20250716113245-b94367cabf3e # k8s.io/kubernetes => github.com/openshift/kubernetes v1.30.1-0.20250716113245-b94367cabf3e +# github.com/openshift/api => github.com/vr4manta/api v0.0.0-20260914125654-86c3aa9e7ae5