Merge pull request #1265 from chiragkyal/add-validation

CFE-1065: Add range validation for placementGroupPartition

Author: openshift-merge-bot[bot]

pkg/webhooks/machine_webhook.go

@@ -746,6 +746,19 @@ func validateAWS(m *machinev1beta1.Machine, config *admissionConfig) (bool, []st
 		)
 	}
 
+	// placementGroupPartition must be between 1 and 7 if it's not 0 (default).
+	if providerSpec.PlacementGroupPartition < 0 || providerSpec.PlacementGroupPartition > 7 {
+		errs = append(
+			errs,
+			field.Invalid(
+				field.NewPath("providerSpec", "placementGroupPartition"),
+				providerSpec.PlacementGroupPartition,
+				"placementGroupPartition must be between 1 and 7",
+			),
+		)
+	}
+
+	// If placementGroupPartition is not 0 (default), placementGroupName must be set.
 	if providerSpec.PlacementGroupName == "" && providerSpec.PlacementGroupPartition != 0 {
 		errs = append(
 			errs,

pkg/webhooks/machine_webhook_test.go

@@ -2142,6 +2142,24 @@ func TestValidateAWSProviderSpec(t *testing.T) {
 			expectedOk:    false,
 			expectedError: "providerSpec.placementGroupPartition: Invalid value: 2: providerSpec.placementGroupPartition is set but providerSpec.placementGroupName is empty",
 		},
+		{
+			testCase: "fail if placementGroupPartition is outside 1-7 range (lower)",
+			modifySpec: func(p *machinev1beta1.AWSMachineProviderConfig) {
+				p.PlacementGroupName = "placement-group"
+				p.PlacementGroupPartition = -1
+			},
+			expectedOk:    false,
+			expectedError: "providerSpec.placementGroupPartition: Invalid value: -1: placementGroupPartition must be between 1 and 7",
+		},
+		{
+			testCase: "fail if placementGroupPartition is outside 1-7 range (upper)",
+			modifySpec: func(p *machinev1beta1.AWSMachineProviderConfig) {
+				p.PlacementGroupName = "placement-group"
+				p.PlacementGroupPartition = 8
+			},
+			expectedOk:    false,
+			expectedError: "providerSpec.placementGroupPartition: Invalid value: 8: placementGroupPartition must be between 1 and 7",
+		},
 		{
 			testCase: "allow if only placementGroupName is set",
 			modifySpec: func(p *machinev1beta1.AWSMachineProviderConfig) {
@@ -2150,7 +2168,7 @@ func TestValidateAWSProviderSpec(t *testing.T) {
 			expectedOk: true,
 		},
 		{
-			testCase: "allow if both placementGroupName and placementGroupPartition are set",
+			testCase: "allow if correct placementGroupName and placementGroupPartition are set",
 			modifySpec: func(p *machinev1beta1.AWSMachineProviderConfig) {
 				p.PlacementGroupName = "placement-group"
 				p.PlacementGroupPartition = 2