Merge pull request #1477 from vr4manta/SPLAT-2679

SPLAT-2680: Fixed MAO to not allow dedicated host configurations for control plane nodes

Author: openshift-merge-bot[bot]

pkg/webhooks/machine_webhook.go

@@ -831,7 +831,7 @@ func validateAWS(m *machinev1beta1.Machine, config *admissionConfig) (bool, []st
 		}
 	}
 
-	errs = append(errs, processAWSPlacementTenancy(providerSpec.Placement)...)
+	errs = append(errs, processAWSPlacementTenancy(m, providerSpec.Placement)...)
 
 	if providerSpec.PlacementGroupPartition != nil {
 		partition := *providerSpec.PlacementGroupPartition
@@ -928,9 +928,26 @@ func validateAWS(m *machinev1beta1.Machine, config *admissionConfig) (bool, []st
 	return true, warnings, nil
 }
 
+const (
+	machineRoleLabel  = "machine.openshift.io/cluster-api-machine-role"
+	machineTypeLabel  = "machine.openshift.io/cluster-api-machine-type"
+	machineMasterRole = "master"
+)
+
+// isControlPlaneMachine checks if a machine is a control plane/master machine by examining its labels.
+func isControlPlaneMachine(m *machinev1beta1.Machine) bool {
+	if m.Labels == nil {
+		return false
+	}
+	role, hasRole := m.Labels[machineRoleLabel]
+	machineType, hasType := m.Labels[machineTypeLabel]
+
+	return (hasRole && role == machineMasterRole) || (hasType && machineType == machineMasterRole)
+}
+
 // processAWSPlacement analyzes the Placement field in relation to Tenancy and host placement.  These are analyzed
 // together based based on their relations to one another.
-func processAWSPlacementTenancy(placement machinev1beta1.Placement) field.ErrorList {
+func processAWSPlacementTenancy(m *machinev1beta1.Machine, placement machinev1beta1.Placement) field.ErrorList {
 	var errs field.ErrorList
 
 	switch placement.Tenancy {
@@ -940,6 +957,12 @@ func processAWSPlacementTenancy(placement machinev1beta1.Placement) field.ErrorL
 			errs = append(errs, field.Forbidden(field.NewPath("spec.placement.host"), "host may only be specified when tenancy is 'host'"))
 		}
 	case machinev1beta1.HostTenancy:
+		// Dedicated hosts are not supported for control plane machines
+		if isControlPlaneMachine(m) {
+			errs = append(errs, field.Forbidden(field.NewPath("spec.placement.tenancy"), "dedicated host tenancy is not supported for control plane machines"))
+			return errs
+		}
+
 		if placement.Host != nil {
 			klog.V(4).Infof("Validating AWS Host Placement")
 

pkg/webhooks/machine_webhook_test.go

@@ -123,6 +123,7 @@ func TestMachineCreation(t *testing.T) {
 		platformType      osconfigv1.PlatformType
 		clusterID         string
 		presetClusterID   bool
+		isMasterMachine   bool
 		expectedError     string
 		disconnected      bool
 		providerSpecValue *kruntime.RawExtension
@@ -1033,6 +1034,66 @@ func TestMachineCreation(t *testing.T) {
 			},
 			expectedError: "",
 		},
+		{
+			name:            "control plane machine with dedicated host should fail",
+			platformType:    osconfigv1.AWSPlatformType,
+			clusterID:       "aws-cluster",
+			isMasterMachine: true,
+			providerSpecValue: &kruntime.RawExtension{
+				Object: &machinev1beta1.AWSMachineProviderConfig{
+					AMI:          machinev1beta1.AWSResourceReference{ID: ptr.To[string]("ami")},
+					InstanceType: "test",
+					Placement: machinev1beta1.Placement{
+						Tenancy: machinev1beta1.HostTenancy,
+						Host: &machinev1beta1.HostPlacement{
+							Affinity: ptr.To(machinev1beta1.HostAffinityAnyAvailable),
+						},
+					},
+				},
+			},
+			expectedError: "admission webhook \"validation.machine.machine.openshift.io\" denied the request: spec.placement.tenancy: Forbidden: dedicated host tenancy is not supported for control plane machines",
+		},
+		{
+			name:            "control plane machine with dedicated host and ID should fail",
+			platformType:    osconfigv1.AWSPlatformType,
+			clusterID:       "aws-cluster",
+			isMasterMachine: true,
+			providerSpecValue: &kruntime.RawExtension{
+				Object: &machinev1beta1.AWSMachineProviderConfig{
+					AMI:          machinev1beta1.AWSResourceReference{ID: ptr.To[string]("ami")},
+					InstanceType: "test",
+					Placement: machinev1beta1.Placement{
+						Tenancy: machinev1beta1.HostTenancy,
+						Host: &machinev1beta1.HostPlacement{
+							Affinity: ptr.To(machinev1beta1.HostAffinityDedicatedHost),
+							DedicatedHost: &machinev1beta1.DedicatedHost{
+								ID: "h-1234567890abcdef0",
+							},
+						},
+					},
+				},
+			},
+			expectedError: "admission webhook \"validation.machine.machine.openshift.io\" denied the request: spec.placement.tenancy: Forbidden: dedicated host tenancy is not supported for control plane machines",
+		},
+		{
+			name:            "worker machine with dedicated host should succeed",
+			platformType:    osconfigv1.AWSPlatformType,
+			clusterID:       "aws-cluster",
+			isMasterMachine: false,
+			providerSpecValue: &kruntime.RawExtension{
+				Object: &machinev1beta1.AWSMachineProviderConfig{
+					AMI:          machinev1beta1.AWSResourceReference{ID: ptr.To[string]("ami")},
+					InstanceType: "test",
+					Placement: machinev1beta1.Placement{
+						Tenancy: machinev1beta1.HostTenancy,
+						Host: &machinev1beta1.HostPlacement{
+							Affinity: ptr.To(machinev1beta1.HostAffinityAnyAvailable),
+						},
+					},
+				},
+			},
+			expectedError: "",
+		},
 		{
 			name:         "with VolumeType set to gp3 and Throughput set under minium value",
 			platformType: osconfigv1.AWSPlatformType,
@@ -2243,6 +2304,13 @@ func TestMachineCreation(t *testing.T) {
 				m.Labels = make(map[string]string)
 				m.Labels[machinev1beta1.MachineClusterIDLabel] = presetClusterID
 			}
+			if tc.isMasterMachine {
+				if m.Labels == nil {
+					m.Labels = make(map[string]string)
+				}
+				m.Labels[machineRoleLabel] = "master"
+				m.Labels[machineTypeLabel] = "master"
+			}
 
 			err = c.Create(ctx, m)
 			if err == nil {