Merge pull request #1267 from chiragkyal/use-pointer

OCPBUGS-36469: Update validation for placementGroupPartition to honour it as pointer

Author: openshift-merge-bot[bot]

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/google/uuid v1.6.0
 	github.com/onsi/ginkgo/v2 v2.17.1
 	github.com/onsi/gomega v1.32.0
-	github.com/openshift/api v0.0.0-20240613141850-76a71dac36a0
+	github.com/openshift/api v0.0.0-20240708071937-c9a91940bf0f
 	github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87
 	github.com/openshift/library-go v0.0.0-20240116081341-964bcb3f545c
 	github.com/prometheus/client_golang v1.18.0

go.sum

@@ -476,8 +476,8 @@ github.com/onsi/ginkgo/v2 v2.17.1 h1:V++EzdbhI4ZV4ev0UTIj0PzhzOcReJFyJaLjtSF55M8
 github.com/onsi/ginkgo/v2 v2.17.1/go.mod h1:llBI3WDLL9Z6taip6f33H76YcWtJv+7R3HigUjbIBOs=
 github.com/onsi/gomega v1.32.0 h1:JRYU78fJ1LPxlckP6Txi/EYqJvjtMrDC04/MM5XRHPk=
 github.com/onsi/gomega v1.32.0/go.mod h1:a4x4gW6Pz2yK1MAmvluYme5lvYTn61afQ2ETw/8n4Lg=
-github.com/openshift/api v0.0.0-20240613141850-76a71dac36a0 h1:Kn16YZDBGwetg+pMQ0u0/3aOxRXQJi/1lu6rIlK+1So=
-github.com/openshift/api v0.0.0-20240613141850-76a71dac36a0/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
+github.com/openshift/api v0.0.0-20240708071937-c9a91940bf0f h1:NmJAlN2fPnL86aq5BbEQJ62v/D16LzIaaQ0Qn72s87E=
+github.com/openshift/api v0.0.0-20240708071937-c9a91940bf0f/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
 github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87 h1:JtLhaGpSEconE+1IKmIgCOof/Len5ceG6H1pk43yv5U=
 github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87/go.mod h1:3IPD4U0qyovZS4EFady2kqY32m8lGcbs/Wx+yprg9z8=
 github.com/openshift/library-go v0.0.0-20240116081341-964bcb3f545c h1:gLylEQQryG+A6nqWYIwE1wUzn1eFUmthjADvflMWKnM=

pkg/webhooks/machine_webhook.go

@@ -746,28 +746,30 @@ 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,
-			field.Invalid(
-				field.NewPath("providerSpec", "placementGroupPartition"),
-				providerSpec.PlacementGroupPartition,
-				"providerSpec.placementGroupPartition is set but providerSpec.placementGroupName is empty",
-			),
-		)
+	if providerSpec.PlacementGroupPartition != nil {
+		partition := *providerSpec.PlacementGroupPartition
+		// placementGroupPartition must be between 1 and 7
+		if partition < 1 || partition > 7 {
+			errs = append(
+				errs,
+				field.Invalid(
+					field.NewPath("providerSpec", "placementGroupPartition"),
+					partition,
+					"providerSpec.placementGroupPartition must be between 1 and 7",
+				),
+			)
+		}
+		// placementGroupName must be set
+		if providerSpec.PlacementGroupName == "" {
+			errs = append(
+				errs,
+				field.Invalid(
+					field.NewPath("providerSpec", "placementGroupPartition"),
+					partition,
+					"providerSpec.placementGroupPartition is set but providerSpec.placementGroupName is empty",
+				),
+			)
+		}
 	}
 
 	duplicatedTags := getDuplicatedTags(providerSpec.Tags)

pkg/webhooks/machine_webhook_test.go

@@ -2137,7 +2137,8 @@ func TestValidateAWSProviderSpec(t *testing.T) {
 			testCase: "fail if placementGroupPartition is set, but placementGroupName is empty",
 			modifySpec: func(p *machinev1beta1.AWSMachineProviderConfig) {
 				p.PlacementGroupName = ""
-				p.PlacementGroupPartition = 2
+				partition := int32(2)
+				p.PlacementGroupPartition = &partition
 			},
 			expectedOk:    false,
 			expectedError: "providerSpec.placementGroupPartition: Invalid value: 2: providerSpec.placementGroupPartition is set but providerSpec.placementGroupName is empty",
@@ -2146,19 +2147,21 @@ func TestValidateAWSProviderSpec(t *testing.T) {
 			testCase: "fail if placementGroupPartition is outside 1-7 range (lower)",
 			modifySpec: func(p *machinev1beta1.AWSMachineProviderConfig) {
 				p.PlacementGroupName = "placement-group"
-				p.PlacementGroupPartition = -1
+				partition := int32(0)
+				p.PlacementGroupPartition = &partition
 			},
 			expectedOk:    false,
-			expectedError: "providerSpec.placementGroupPartition: Invalid value: -1: placementGroupPartition must be between 1 and 7",
+			expectedError: "providerSpec.placementGroupPartition: Invalid value: 0: providerSpec.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
+				partition := int32(8)
+				p.PlacementGroupPartition = &partition
 			},
 			expectedOk:    false,
-			expectedError: "providerSpec.placementGroupPartition: Invalid value: 8: placementGroupPartition must be between 1 and 7",
+			expectedError: "providerSpec.placementGroupPartition: Invalid value: 8: providerSpec.placementGroupPartition must be between 1 and 7",
 		},
 		{
 			testCase: "allow if only placementGroupName is set",
@@ -2171,7 +2174,8 @@ func TestValidateAWSProviderSpec(t *testing.T) {
 			testCase: "allow if correct placementGroupName and placementGroupPartition are set",
 			modifySpec: func(p *machinev1beta1.AWSMachineProviderConfig) {
 				p.PlacementGroupName = "placement-group"
-				p.PlacementGroupPartition = 2
+				partition := int32(2)
+				p.PlacementGroupPartition = &partition
 			},
 			expectedOk: true,
 		},