Merge pull request openshift#7721 from r4f4/azure-vm-terms-plan-fix

OCPBUGS-22840: ic/azure: validate field Plan when for marketplace images
JM1 · Nov 24, 2023 · 1ea1a54 · 1ea1a54
2 parents 1488a19 + 082fcc4
commit 1ea1a54
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 11 deletions.
diff --git a/pkg/asset/installconfig/azure/validation.go b/pkg/asset/installconfig/azure/validation.go
@@ -827,17 +827,25 @@ func validateMarketplaceImage(client API, region string, instanceHyperVGenSet se
 		return field.Invalid(osImageFieldPath, osImage.SKU, errMsg)
 	}
 
-	// Images with no purchase plan have no terms to be accepted
-	if osImage.Plan == aztypes.ImageNoPurchasePlan {
-		return nil
-	}
-
-	termsAccepted, err := client.AreMarketplaceImageTermsAccepted(context.Background(), osImage.Publisher, osImage.Offer, osImage.SKU)
-	if err != nil {
-		return field.Invalid(osImageFieldPath, osImage, fmt.Sprintf("could not determine if the license terms for the marketplace image have been accepted: %v", err))
+	// Image has license terms to be accepted
+	osImagePlan := osImage.Plan
+	if len(osImagePlan) == 0 {
+		// Use the default if not set in the install-config
+		osImagePlan = aztypes.ImageWithPurchasePlan
 	}
-	if !termsAccepted {
-		return field.Invalid(osImageFieldPath, osImage, "the license terms for the marketplace image have not been accepted")
+	if plan := vmImage.Plan; plan != nil {
+		if osImagePlan == aztypes.ImageNoPurchasePlan {
+			return field.Invalid(osImageFieldPath, osImage, "marketplace image requires license terms to be accepted")
+		}
+		termsAccepted, err := client.AreMarketplaceImageTermsAccepted(context.Background(), osImage.Publisher, osImage.Offer, osImage.SKU)
+		if err != nil {
+			return field.Invalid(osImageFieldPath, osImage, fmt.Sprintf("could not determine if the license terms for the marketplace image have been accepted: %v", err))
+		}
+		if !termsAccepted {
+			return field.Invalid(osImageFieldPath, osImage, "the license terms for the marketplace image have not been accepted")
+		}
+	} else if osImagePlan == aztypes.ImageWithPurchasePlan {
+		return field.Invalid(osImageFieldPath, osImage, "image has no license terms. Set Plan to \"NoPurchasePlan\" to continue.")
 	}
 
 	return nil

diff --git a/pkg/asset/installconfig/azure/validation_test.go b/pkg/asset/installconfig/azure/validation_test.go
@@ -213,6 +213,14 @@ var (
 	}()
 
 	marketplaceImageAPIResult = azenc.VirtualMachineImage{
+		Name: to.StringPtr("VMImage"),
+		VirtualMachineImageProperties: &azenc.VirtualMachineImageProperties{
+			HyperVGeneration: azenc.HyperVGenerationTypesV1,
+			Plan:             &azenc.PurchasePlan{},
+		},
+	}
+
+	marketplaceImageAPIResultNoPlan = azenc.VirtualMachineImage{
 		Name: to.StringPtr("VMImage"),
 		VirtualMachineImageProperties: &azenc.VirtualMachineImageProperties{
 			HyperVGeneration: azenc.HyperVGenerationTypesV1,
@@ -1283,7 +1291,7 @@ func TestAzureMarketplaceImage(t *testing.T) {
 		},
 	}, nil).AnyTimes()
 	azureClient.EXPECT().AreMarketplaceImageTermsAccepted(gomock.Any(), validOSImagePublisher, validOSImageOffer, erroringOSImageSKU).Return(true, nil).AnyTimes()
-	azureClient.EXPECT().GetMarketplaceImage(gomock.Any(), validRegion, validOSImagePublisher, validOSImageOffer, noPlanOSImageSKU, validOSImageVersion).Return(marketplaceImageAPIResult, nil).AnyTimes()
+	azureClient.EXPECT().GetMarketplaceImage(gomock.Any(), validRegion, validOSImagePublisher, validOSImageOffer, noPlanOSImageSKU, validOSImageVersion).Return(marketplaceImageAPIResultNoPlan, nil).AnyTimes()
 	// Should not check terms of images with no purchase plan
 	azureClient.EXPECT().AreMarketplaceImageTermsAccepted(gomock.Any(), validOSImagePublisher, validOSImageOffer, noPlanOSImageSKU).MaxTimes(0)