Add SKU availability and restriction checks to dynamic validation (#1790

)

* add sku filtering and restriction checks

* add install-time instance validation

pkg/api/validate/dynamic/dynamic.go

@@ -53,6 +53,7 @@ type Dynamic interface {
 	ValidateQuota(ctx context.Context, oc *api.OpenShiftCluster) error
 	ValidateDiskEncryptionSets(ctx context.Context, oc *api.OpenShiftCluster) error
 	ValidateEncryptionAtHost(ctx context.Context, oc *api.OpenShiftCluster) error
+	ValidateVMSku(ctx context.Context, location string, subscriptionID string, oc *api.OpenShiftCluster) error
 }
 
 type dynamic struct {
@@ -65,14 +66,17 @@ type dynamic struct {
 	providers          features.ProvidersClient
 	virtualNetworks    virtualNetworksGetClient
 	diskEncryptionSets compute.DiskEncryptionSetsClient
+	resourceSkusClient compute.ResourceSkusClient
 	spComputeUsage     compute.UsageClient
 	spNetworkUsage     network.UsageClient
 }
 
 type AuthorizerType string
 
-const AuthorizerFirstParty AuthorizerType = "resource provider"
-const AuthorizerClusterServicePrincipal AuthorizerType = "cluster"
+const (
+	AuthorizerFirstParty              AuthorizerType = "resource provider"
+	AuthorizerClusterServicePrincipal AuthorizerType = "cluster"
+)
 
 func NewValidator(log *logrus.Entry, env env.Interface, azEnv *azureclient.AROEnvironment, subscriptionID string, authorizer refreshable.Authorizer, authorizerType AuthorizerType) (Dynamic, error) {
 	return &dynamic{
@@ -87,6 +91,7 @@ func NewValidator(log *logrus.Entry, env env.Interface, azEnv *azureclient.AROEn
 		permissions:        authorization.NewPermissionsClient(azEnv, subscriptionID, authorizer),
 		virtualNetworks:    newVirtualNetworksCache(network.NewVirtualNetworksClient(azEnv, subscriptionID, authorizer)),
 		diskEncryptionSets: compute.NewDiskEncryptionSetsClient(azEnv, subscriptionID, authorizer),
+		resourceSkusClient: compute.NewResourceSkusClient(azEnv, subscriptionID, authorizer),
 	}, nil
 }
 

pkg/api/validate/dynamic/sku.go

@@ -0,0 +1,63 @@
+package dynamic
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+
+	mgmtcompute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-01/compute"
+
+	"github.com/Azure/ARO-RP/pkg/api"
+	"github.com/Azure/ARO-RP/pkg/util/computeskus"
+)
+
+// ValidateWorkerSku uses resourceSkusClient to ensure that the VM sizes listed in the cluster document are available for use in the target region.
+func (dv *dynamic) ValidateVMSku(ctx context.Context, location string, subscriptionID string, oc *api.OpenShiftCluster) error {
+	// Get a list of available worker SKUs, filtering by location. We initialize a new resourceSkusClient here instead of using the one in dv.env,
+	// so that we can determine SKU availability within target cluster subscription instead of within RP subscription.
+	filter := fmt.Sprintf("location eq %s", location)
+	skus, err := dv.resourceSkusClient.List(ctx, filter)
+	if err != nil {
+		return err
+	}
+
+	filteredSkus := computeskus.FilterVMSizes(skus, location)
+	masterProfileSku := string(oc.Properties.MasterProfile.VMSize)
+
+	err = checkSKUAvailability(filteredSkus, location, "properties.masterProfile.VMSize", masterProfileSku)
+	if err != nil {
+		return err
+	}
+
+	// In case there are multiple WorkerProfiles listed in the cluster document (such as post-install),
+	// compare VMSize in each WorkerProfile to the resourceSkusClient call above to ensure that the sku is available in region.
+	for i, workerprofile := range oc.Properties.WorkerProfiles {
+		workerProfileSku := string(workerprofile.VMSize)
+
+		err = checkSKUAvailability(filteredSkus, location, fmt.Sprintf("properties.workerProfiles[%d].VMSize", i), workerProfileSku)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func checkSKUAvailability(skus map[string]*mgmtcompute.ResourceSku, location, path, vmsize string) error {
+	// Ensure desired sku exists in target region
+	if skus[vmsize] == nil {
+		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path, "The selected SKU '%v' is unavailable in region '%v'", vmsize, location)
+	}
+
+	// Fail if sku is available, but restricted within the subscription. Restrictions are subscription-specific.
+	// https://docs.microsoft.com/en-us/azure/azure-resource-manager/templates/error-sku-not-available
+	isRestricted := computeskus.IsRestricted(skus, location, vmsize)
+	if isRestricted {
+		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path, "The selected SKU '%v' is restricted in region '%v' for selected subscription", vmsize, location)
+	}
+
+	return nil
+}

pkg/api/validate/dynamic/sku_test.go

@@ -0,0 +1,172 @@
+package dynamic
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"testing"
+
+	mgmtcompute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-01/compute"
+	"github.com/Azure/go-autorest/autorest/to"
+	"github.com/golang/mock/gomock"
+	"github.com/sirupsen/logrus"
+
+	"github.com/Azure/ARO-RP/pkg/api"
+	mock_compute "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/compute"
+)
+
+func TestValidateVMSku(t *testing.T) {
+	for _, tt := range []struct {
+		name                  string
+		restrictions          mgmtcompute.ResourceSkuRestrictionsReasonCode
+		restrictionLocation   *[]string
+		targetLocation        string
+		workerProfile1Sku     string
+		workerProfile2Sku     string
+		masterProfileSku      string
+		availableSku          string
+		restrictedSku         string
+		resourceSkusClientErr error
+		wantErr               string
+	}{
+		{
+			name:              "worker and master sku are valid",
+			workerProfile1Sku: "Standard_D4s_v2",
+			workerProfile2Sku: "Standard_D4s_v2",
+			masterProfileSku:  "Standard_D4s_v2",
+			availableSku:      "Standard_D4s_v2",
+		},
+		{
+			name:                  "unable to retrieve skus information",
+			workerProfile1Sku:     "Standard_D4s_v2",
+			workerProfile2Sku:     "Standard_D4s_v2",
+			resourceSkusClientErr: errors.New("unable to retrieve skus information"),
+			wantErr:               "unable to retrieve skus information",
+		},
+		{
+			name:              "desired worker sku doesn't exist in the target region",
+			workerProfile1Sku: "Standard_L80",
+			workerProfile2Sku: "Standard_L80",
+			masterProfileSku:  "Standard_D4s_v2",
+			availableSku:      "Standard_D4s_v2",
+			wantErr:           "400: InvalidParameter: properties.workerProfiles[0].VMSize: The selected SKU 'Standard_L80' is unavailable in region 'eastus'",
+		},
+		{
+			name:              "desired master sku doesn't exist in the target region",
+			workerProfile1Sku: "Standard_D4s_v2",
+			workerProfile2Sku: "Standard_D4s_v2",
+			masterProfileSku:  "Standard_L80",
+			availableSku:      "Standard_D4s_v2",
+			wantErr:           "400: InvalidParameter: properties.masterProfile.VMSize: The selected SKU 'Standard_L80' is unavailable in region 'eastus'",
+		},
+		{
+			name:              "one valid workerprofile and one invalid workerprofile",
+			workerProfile1Sku: "Standard_L80",
+			workerProfile2Sku: "Standard_D4s_v2",
+			masterProfileSku:  "Standard_D4s_v2",
+			availableSku:      "Standard_D4s_v2",
+			wantErr:           "400: InvalidParameter: properties.workerProfiles[0].VMSize: The selected SKU 'Standard_L80' is unavailable in region 'eastus'",
+		},
+		{
+			name:         "worker sku exists in region but is not available in subscription",
+			restrictions: mgmtcompute.NotAvailableForSubscription,
+			restrictionLocation: &[]string{
+				"eastus",
+			},
+			workerProfile1Sku: "Standard_L80",
+			workerProfile2Sku: "Standard_L80",
+			masterProfileSku:  "Standard_D4s_v2",
+			availableSku:      "Standard_D4s_v2",
+			restrictedSku:     "Standard_L80",
+			wantErr:           "400: InvalidParameter: properties.workerProfiles[0].VMSize: The selected SKU 'Standard_L80' is restricted in region 'eastus' for selected subscription",
+		},
+		{
+			name:         "master sku exists in region but is not available in subscription",
+			restrictions: mgmtcompute.NotAvailableForSubscription,
+			restrictionLocation: &[]string{
+				"eastus",
+			},
+			workerProfile1Sku: "Standard_D4s_v2",
+			workerProfile2Sku: "Standard_D4s_v2",
+			masterProfileSku:  "Standard_L80",
+			availableSku:      "Standard_D4s_v2",
+			restrictedSku:     "Standard_L80",
+			wantErr:           "400: InvalidParameter: properties.masterProfile.VMSize: The selected SKU 'Standard_L80' is restricted in region 'eastus' for selected subscription",
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.targetLocation == "" {
+				tt.targetLocation = "eastus"
+			}
+
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			oc := &api.OpenShiftCluster{
+				Properties: api.OpenShiftClusterProperties{
+					WorkerProfiles: []api.WorkerProfile{
+						{
+							VMSize: api.VMSize(tt.workerProfile1Sku),
+						},
+						{
+							VMSize: api.VMSize(tt.workerProfile2Sku),
+						},
+					},
+					MasterProfile: api.MasterProfile{
+						VMSize: api.VMSize(tt.masterProfileSku),
+					},
+				},
+			}
+
+			skus := []mgmtcompute.ResourceSku{
+				{
+					Name:      &tt.availableSku,
+					Locations: &[]string{"eastus"},
+					LocationInfo: &[]mgmtcompute.ResourceSkuLocationInfo{
+						{Zones: &[]string{"1, 2, 3"}},
+					},
+					Restrictions: &[]mgmtcompute.ResourceSkuRestrictions{},
+					Capabilities: &[]mgmtcompute.ResourceSkuCapabilities{},
+					ResourceType: to.StringPtr("virtualMachines"),
+				},
+				{
+					Name:      &tt.restrictedSku,
+					Locations: &[]string{tt.targetLocation},
+					LocationInfo: &[]mgmtcompute.ResourceSkuLocationInfo{
+						{Zones: &[]string{"1, 2, 3"}},
+					},
+					Restrictions: &[]mgmtcompute.ResourceSkuRestrictions{
+						{
+							ReasonCode: tt.restrictions,
+							RestrictionInfo: &mgmtcompute.ResourceSkuRestrictionInfo{
+								Locations: tt.restrictionLocation,
+							},
+						},
+					},
+					Capabilities: &[]mgmtcompute.ResourceSkuCapabilities{},
+					ResourceType: to.StringPtr("virtualMachines"),
+				},
+			}
+
+			resourceSkusClient := mock_compute.NewMockResourceSkusClient(controller)
+			resourceSkusClient.EXPECT().
+				List(gomock.Any(), fmt.Sprintf("location eq %v", tt.targetLocation)).
+				Return(skus, tt.resourceSkusClientErr)
+
+			dv := dynamic{
+				authorizerType:     AuthorizerClusterServicePrincipal,
+				log:                logrus.NewEntry(logrus.StandardLogger()),
+				resourceSkusClient: resourceSkusClient,
+			}
+
+			err := dv.ValidateVMSku(context.Background(), tt.targetLocation, subscriptionID, oc)
+			if err != nil && err.Error() != tt.wantErr ||
+				err == nil && tt.wantErr != "" {
+				t.Error(err)
+			}
+		})
+	}
+}

pkg/api/validate/openshiftcluster_validatedynamic.go

@@ -121,5 +121,10 @@ func (dv *openShiftClusterDynamicValidator) Dynamic(ctx context.Context) error {
 		return err
 	}
 
+	err = spDynamic.ValidateVMSku(ctx, dv.oc.Location, dv.subscriptionDoc.ID, dv.oc)
+	if err != nil {
+		return err
+	}
+
 	return nil
 }

pkg/env/prod.go

@@ -22,6 +22,7 @@ import (
 	"github.com/Azure/ARO-RP/pkg/proxy"
 	"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/compute"
 	"github.com/Azure/ARO-RP/pkg/util/clientauthorizer"
+	"github.com/Azure/ARO-RP/pkg/util/computeskus"
 	"github.com/Azure/ARO-RP/pkg/util/keyvault"
 	"github.com/Azure/ARO-RP/pkg/util/refreshable"
 	"github.com/Azure/ARO-RP/pkg/util/version"
@@ -274,29 +275,7 @@ func (p *prod) populateVMSkus(ctx context.Context, resourceSkusClient compute.Re
 		return err
 	}
 
-	p.vmskus = map[string]*mgmtcompute.ResourceSku{}
-	for _, sku := range skus {
-		// TODO(mjudeikis): At some point some SKU's stopped returning zones and
-		// locations. IcM is open with MSFT but this might take a while.
-		// Revert once we find out right behaviour.
-		// https://github.com/Azure/ARO-RP/issues/1515
-		if len(*sku.Locations) == 0 || !strings.EqualFold((*sku.Locations)[0], p.Location()) ||
-			*sku.ResourceType != "virtualMachines" {
-			continue
-		}
-
-		if len(*sku.LocationInfo) == 0 { // happened in eastus2euap
-			continue
-		}
-
-		// We copy only part of the object so we don't have to keep
-		// a lot of data in memory.
-		p.vmskus[*sku.Name] = &mgmtcompute.ResourceSku{
-			Name:         sku.Name,
-			LocationInfo: sku.LocationInfo,
-			Capabilities: sku.Capabilities,
-		}
-	}
+	p.vmskus = computeskus.FilterVMSizes(skus, p.Location())
 
 	return nil
 }

pkg/util/computeskus/computeskus.go

@@ -4,6 +4,8 @@ package computeskus
 // Licensed under the Apache License 2.0.
 
 import (
+	"strings"
+
 	mgmtcompute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-01/compute"
 )
 
@@ -18,7 +20,7 @@ func Zones(sku *mgmtcompute.ResourceSku) []string {
 	return *(*sku.LocationInfo)[0].Zones
 }
 
-// Capabilities checks whether given resource SKU has specific capability
+// HasCapability checks whether given resource SKU has specific capability
 func HasCapability(sku *mgmtcompute.ResourceSku, capabilityName string) bool {
 	if sku.Capabilities == nil {
 		return false
@@ -32,3 +34,46 @@ func HasCapability(sku *mgmtcompute.ResourceSku, capabilityName string) bool {
 
 	return false
 }
+
+// IsRestricted checks whether given resource SKU is restricted in a given location
+func IsRestricted(skus map[string]*mgmtcompute.ResourceSku, location, VMSize string) bool {
+	for _, restriction := range *skus[VMSize].Restrictions {
+		for _, restrictedLocation := range *restriction.RestrictionInfo.Locations {
+			if restrictedLocation == location {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
+// FilterVMSizes filters resource SKU by location and returns only virtual machines, their names, restrictions, location info, and capabilities.
+func FilterVMSizes(skus []mgmtcompute.ResourceSku, location string) map[string]*mgmtcompute.ResourceSku {
+	vmskus := map[string]*mgmtcompute.ResourceSku{}
+	for _, sku := range skus {
+		// TODO(mjudeikis): At some point some SKU's stopped returning zones and
+		// locations. IcM is open with MSFT but this might take a while.
+		// Revert once we find out right behaviour.
+		// https://github.com/Azure/ARO-RP/issues/1515
+		if len(*sku.Locations) == 0 || !strings.EqualFold((*sku.Locations)[0], location) ||
+			*sku.ResourceType != "virtualMachines" {
+			continue
+		}
+
+		if len(*sku.LocationInfo) == 0 { // happened in eastus2euap
+			continue
+		}
+
+		// We copy only part of the object so we don't have to keep
+		// a lot of data in memory.
+		vmskus[*sku.Name] = &mgmtcompute.ResourceSku{
+			Name:         sku.Name,
+			Restrictions: sku.Restrictions,
+			LocationInfo: sku.LocationInfo,
+			Capabilities: sku.Capabilities,
+		}
+	}
+
+	return vmskus
+}