Make cloud instance that is connected to configurable (#148)

* Make cloud instance that is connected to configurable * address review comments - add consts for known good Azure cloud names - add validation of CloudConfig * add unit tests * follow latest changes on provider-ext * address review comments * minor refactoring and corrections * correction in validation test * optimized method parameters for creating connect config --------- Co-authored-by: Rishabh Patel <[email protected]>
gardener · Jun 17, 2024 · fe4c7e8 · fe4c7e8
1 parent ea7c1d9
commit fe4c7e8
Show file tree

Hide file tree

Showing 8 changed files with 144 additions and 10 deletions.
diff --git a/pkg/azure/access/factory.go b/pkg/azure/access/factory.go
@@ -41,7 +41,12 @@ func NewAccessFactoryWithOptions(clientOptions *arm.ClientOptions, tokenCredenti
 
 // GetDefaultTokenCredentials provides the azure token credentials using the ConnectConfig passed as an argument.
 func GetDefaultTokenCredentials(connectConfig ConnectConfig) (azcore.TokenCredential, error) {
-	return azidentity.NewClientSecretCredential(connectConfig.TenantID, connectConfig.ClientID, connectConfig.ClientSecret, nil)
+	return azidentity.NewClientSecretCredential(
+		connectConfig.TenantID,
+		connectConfig.ClientID,
+		connectConfig.ClientSecret,
+		&azidentity.ClientSecretCredentialOptions{ClientOptions: connectConfig.ClientOptions},
+	)
 }
 
 func (f defaultFactory) GetResourceGroupsAccess(connectConfig ConnectConfig) (*armresources.ResourceGroupsClient, error) {

diff --git a/pkg/azure/access/types.go b/pkg/azure/access/types.go
@@ -5,6 +5,7 @@
 package access
 
 import (
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/marketplaceordering/armmarketplaceordering"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4"
@@ -22,6 +23,8 @@ type ConnectConfig struct {
 	ClientID string
 	// ClientSecret is a certificate issues for the ClientID.
 	ClientSecret string
+	// ClientOptions are the options to use when connecting with clients.
+	ClientOptions policy.ClientOptions
 }
 
 // Factory is an access factory providing methods to get facade/access for different resources.

diff --git a/pkg/azure/api/providerspec.go b/pkg/azure/api/providerspec.go
@@ -69,6 +69,8 @@ type AzureProviderSpec struct {
 	ResourceGroup string `json:"resourceGroup,omitempty"`
 	// SubnetInfo contains the configuration for an existing subnet.
 	SubnetInfo AzureSubnetInfo `json:"subnetInfo,omitempty"`
+	// CloudConfiguration contains config that controls which cloud to connect to
+	CloudConfiguration *CloudConfiguration `json:"cloudConfiguration,omitempty"`
 }
 
 // AzureVirtualMachineProperties describes the properties of a Virtual Machine.
@@ -309,3 +311,17 @@ type AzureDiagnosticsProfile struct {
 	// If not specified azure managed storage will be used.
 	StorageURI *string `json:"storageURI,omitempty"`
 }
+
+// The (currently) supported values for the names of clouds to use in the CloudConfiguration.
+const (
+	CloudNameChina  string = "AzureChina"
+	CloudNameGov    string = "AzureGovernment"
+	CloudNamePublic string = "AzurePublic"
+)
+
+// CloudConfiguration contains detailed config for the cloud to connect to. Currently we only support selection of well-
+// known Azure-instances by name, but this could be extended in future to support private clouds.
+type CloudConfiguration struct {
+	// Name is the name of the cloud to connect to, e.g. "AzurePublic" or "AzureChina".
+	Name string `json:"name"`
+}
diff --git a/pkg/azure/api/validation/validation.go b/pkg/azure/api/validation/validation.go
@@ -9,6 +9,7 @@ package validation
 
 import (
 	"fmt"
+	"slices"
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
@@ -50,6 +51,7 @@ func ValidateProviderSpec(spec api.AzureProviderSpec) field.ErrorList {
 	allErrs = append(allErrs, validateSubnetInfo(spec.SubnetInfo, specPath.Child("subnetInfo"))...)
 	allErrs = append(allErrs, validateProperties(spec.Properties, specPath.Child("properties"))...)
 	allErrs = append(allErrs, validateTags(spec.Tags, specPath.Child("tags"))...)
+	allErrs = append(allErrs, validateCloudConfiguration(spec.CloudConfiguration, specPath.Child("cloudConfiguration"))...)
 
 	return allErrs
 }
@@ -121,6 +123,22 @@ func validateProperties(properties api.AzureVirtualMachineProperties, fldPath *f
 	return allErrs
 }
 
+func validateCloudConfiguration(cloudConfiguration *api.CloudConfiguration, fldPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	if cloudConfiguration == nil {
+		return allErrs
+	}
+
+	knownCloudInstances := []string{api.CloudNamePublic, api.CloudNameChina, api.CloudNameGov}
+
+	if cloudName := cloudConfiguration.Name; !slices.Contains(knownCloudInstances, cloudName) {
+		allErrs = append(allErrs, field.NotSupported(fldPath.Child("name"), cloudName, knownCloudInstances))
+	}
+
+	return allErrs
+}
+
 func validateSecurityProfile(securityProfile *api.AzureSecurityProfile, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 	if securityProfile == nil {

diff --git a/pkg/azure/api/validation/validation_test.go b/pkg/azure/api/validation/validation_test.go
@@ -476,6 +476,37 @@ func TestValidateStorageImageRef(t *testing.T) {
 	}
 }
 
+func TestValidateCloudConfiguration(t *testing.T) {
+	fldPath := field.NewPath("providerSpec.properties.cloudConfiguration")
+	table := []struct {
+		description        string
+		cloudConfiguration *api.CloudConfiguration
+		matcher            gomegatypes.GomegaMatcher
+	}{
+		{description: "No cloud configuration set"},
+		{description: "cloud configuration is set to AzureGov", cloudConfiguration: &api.CloudConfiguration{Name: api.CloudNameGov}},
+		{description: "cloud configuration is set to AzureChina", cloudConfiguration: &api.CloudConfiguration{Name: api.CloudNameChina}},
+		{description: "cloud configuration is set to AzurePublic", cloudConfiguration: &api.CloudConfiguration{Name: api.CloudNamePublic}},
+		{
+			description:        "cloud configuration is set to an unsupported name",
+			cloudConfiguration: &api.CloudConfiguration{Name: "foo"},
+			matcher:            ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{"Type": Equal(field.ErrorTypeNotSupported), "Field": Equal("providerSpec.properties.cloudConfiguration.name")}))),
+		},
+	}
+	g := NewWithT(t)
+	t.Parallel()
+	for _, entry := range table {
+		t.Run(entry.description, func(t *testing.T) {
+			errList := validateCloudConfiguration(entry.cloudConfiguration, fldPath)
+			if entry.matcher != nil {
+				g.Expect(errList).To(entry.matcher)
+			} else {
+				g.Expect(len(errList)).To(BeZero())
+			}
+		})
+	}
+}
+
 func TestValidateSecurityProfile(t *testing.T) {
 	fldPath := field.NewPath("providerSpec.properties.securityProfile")
 	table := []struct {

diff --git a/pkg/azure/provider/helpers/secret.go → pkg/azure/provider/helpers/connectconfig.go b/pkg/azure/provider/helpers/secret.go → pkg/azure/provider/helpers/connectconfig.go
@@ -6,6 +6,8 @@ package helpers
 
 import (
 	"fmt"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
 	"strings"
 
 	"github.com/gardener/machine-controller-manager-provider-azure/pkg/azure/api/validation"
@@ -18,22 +20,25 @@ import (
 )
 
 // ValidateSecretAndCreateConnectConfig validates the secret and creates an instance of azure.ConnectConfig out of it.
-func ValidateSecretAndCreateConnectConfig(secret *corev1.Secret) (access.ConnectConfig, error) {
+func ValidateSecretAndCreateConnectConfig(secret *corev1.Secret, cloudConfiguration *api.CloudConfiguration) (access.ConnectConfig, error) {
 	if err := validation.ValidateProviderSecret(secret); err != nil {
 		return access.ConnectConfig{}, status.Error(codes.InvalidArgument, fmt.Sprintf("error in validating secret: %v", err))
 	}
 
 	var (
-		subscriptionID = ExtractCredentialsFromData(secret.Data, api.SubscriptionID, api.AzureSubscriptionID)
-		tenantID       = ExtractCredentialsFromData(secret.Data, api.TenantID, api.AzureTenantID)
-		clientID       = ExtractCredentialsFromData(secret.Data, api.ClientID, api.AzureClientID)
-		clientSecret   = ExtractCredentialsFromData(secret.Data, api.ClientSecret, api.AzureClientSecret)
+		subscriptionID       = ExtractCredentialsFromData(secret.Data, api.SubscriptionID, api.AzureSubscriptionID)
+		tenantID             = ExtractCredentialsFromData(secret.Data, api.TenantID, api.AzureTenantID)
+		clientID             = ExtractCredentialsFromData(secret.Data, api.ClientID, api.AzureClientID)
+		clientSecret         = ExtractCredentialsFromData(secret.Data, api.ClientSecret, api.AzureClientSecret)
+		azCloudConfiguration = DetermineAzureCloudConfiguration(cloudConfiguration)
 	)
+
 	return access.ConnectConfig{
 		SubscriptionID: subscriptionID,
 		TenantID:       tenantID,
 		ClientID:       clientID,
 		ClientSecret:   clientSecret,
+		ClientOptions:  azcore.ClientOptions{Cloud: azCloudConfiguration},
 	}, nil
 }
 
@@ -47,3 +52,22 @@ func ExtractCredentialsFromData(data map[string][]byte, keys ...string) string {
 	}
 	return ""
 }
+
+// DetermineAzureCloudConfiguration returns the Azure cloud.Configuration corresponding to the instance given by the provided api.Configuration.
+func DetermineAzureCloudConfiguration(cloudConfiguration *api.CloudConfiguration) cloud.Configuration {
+	if cloudConfiguration != nil {
+		cloudConfigurationName := cloudConfiguration.Name
+		switch {
+		case strings.EqualFold(cloudConfigurationName, api.CloudNamePublic):
+			return cloud.AzurePublic
+		case strings.EqualFold(cloudConfigurationName, api.CloudNameGov):
+			return cloud.AzureGovernment
+		case strings.EqualFold(cloudConfigurationName, api.CloudNameChina):
+			return cloud.AzureChina
+		default:
+			return cloud.AzurePublic
+		}
+	}
+	// Fallback
+	return cloud.AzurePublic
+}
diff --git a/pkg/azure/provider/helpers/connectconfig_test.go b/pkg/azure/provider/helpers/connectconfig_test.go
@@ -0,0 +1,36 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package helpers
+
+import (
+	"testing"
+
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
+	"github.com/gardener/machine-controller-manager-provider-azure/pkg/azure/api"
+	. "github.com/onsi/gomega"
+)
+
+func TestDetermineAzureCloudConfiguration(t *testing.T) {
+	type testData struct {
+		description       string
+		testConfiguration *api.CloudConfiguration
+		expectedOutput    *cloud.Configuration
+	}
+
+	tests := []testData{
+		{description: "cloud configuration name set to AzurePublic", testConfiguration: &api.CloudConfiguration{Name: api.CloudNamePublic}, expectedOutput: &cloud.AzurePublic},
+		{description: "cloud configuration name set to AzureChina", testConfiguration: &api.CloudConfiguration{Name: api.CloudNameChina}, expectedOutput: &cloud.AzureChina},
+		{description: "cloud configuration name set to AzureGov", testConfiguration: &api.CloudConfiguration{Name: api.CloudNameGov}, expectedOutput: &cloud.AzureGovernment},
+		{description: "cloud configuration not set", testConfiguration: nil, expectedOutput: &cloud.AzurePublic},
+	}
+	g := NewWithT(t)
+	t.Parallel()
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			cloudConfiguration := DetermineAzureCloudConfiguration(test.testConfiguration)
+			g.Expect(cloudConfiguration).To(Equal(*test.expectedOutput))
+		})
+	}
+}
diff --git a/pkg/azure/provider/helpers/driver.go b/pkg/azure/provider/helpers/driver.go
@@ -39,9 +39,9 @@ import (
 // ExtractProviderSpecAndConnectConfig extracts api.AzureProviderSpec from mcc and access.ConnectConfig from secret.
 func ExtractProviderSpecAndConnectConfig(mcc *v1alpha1.MachineClass, secret *corev1.Secret) (api.AzureProviderSpec, access.ConnectConfig, error) {
 	var (
-		err           error
-		providerSpec  api.AzureProviderSpec
-		connectConfig access.ConnectConfig
+		err                  error
+		providerSpec         api.AzureProviderSpec
+		connectConfig        access.ConnectConfig
 	)
 	// validate provider Spec provider. Exit early if it is not azure.
 	if err = validation.ValidateMachineClassProvider(mcc); err != nil {
@@ -52,9 +52,10 @@ func ExtractProviderSpecAndConnectConfig(mcc *v1alpha1.MachineClass, secret *cor
 		return api.AzureProviderSpec{}, access.ConnectConfig{}, err
 	}
 	// validate secret and extract connect config required to create clients.
-	if connectConfig, err = ValidateSecretAndCreateConnectConfig(secret); err != nil {
+	if connectConfig, err = ValidateSecretAndCreateConnectConfig(secret, providerSpec.CloudConfiguration); err != nil {
 		return api.AzureProviderSpec{}, access.ConnectConfig{}, err
 	}
+
 	return providerSpec, connectConfig, nil
 }