OCPBUGS-25483: Adds CloudConfigTransformer for Azure

pkg/cloud/azure/azure.go

@@ -2,12 +2,20 @@ package azure
 
 import (
 	"embed"
+	"encoding/json"
 	"fmt"
+	"slices"
+	"strings"
 
 	"github.com/asaskevich/govalidator"
+	configv1 "github.com/openshift/api/config/v1"
 	appsv1 "k8s.io/api/apps/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	azureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
+	azure "sigs.k8s.io/cloud-provider-azure/pkg/provider"
+
 	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/common"
 	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config"
 )
@@ -23,6 +31,25 @@ var (
 	}
 )
 
+var (
+	validAzureCloudNames = map[configv1.AzureCloudEnvironment]struct{}{
+		configv1.AzurePublicCloud:       struct{}{},
+		configv1.AzureUSGovernmentCloud: struct{}{},
+		configv1.AzureChinaCloud:        struct{}{},
+		configv1.AzureGermanCloud:       struct{}{},
+		configv1.AzureStackCloud:        struct{}{},
+	}
+
+	validAzureCloudNameValues = func() []string {
+		v := make([]string, 0, len(validAzureCloudNames))
+		for n := range validAzureCloudNames {
+			v = append(v, string(n))
+		}
+		slices.Sort(v)
+		return v
+	}()
+)
+
 type imagesReference struct {
 	CloudControllerManager         string `valid:"required"`
 	CloudControllerManagerOperator string `valid:"required"`
@@ -85,3 +112,70 @@ func NewProviderAssets(config config.OperatorConfig) (common.CloudProviderAssets
 	}
 	return assets, nil
 }
+
+// IsAzure ensures that the underlying platform is Azure. It will fail if the
+// CloudName is AzureStack as we handle it separately with it's own
+// CloudConfigTransformer.
+func IsAzure(infra *configv1.Infrastructure) bool {
+	if infra.Status.PlatformStatus != nil {
+		if infra.Status.PlatformStatus.Type == configv1.AzurePlatformType &&
+			(infra.Status.PlatformStatus.Azure.CloudName != configv1.AzureStackCloud) {
+			return true
+		}
+	}
+	return false
+}
+
+func CloudConfigTransformer(source string, infra *configv1.Infrastructure, network *configv1.Network) (string, error) {
+	if !IsAzure(infra) {
+		return "", fmt.Errorf("invalid platform, expected CloudName to be %s", configv1.AzurePublicCloud)
+	}
+
+	var cfg azure.Config
+	if err := json.Unmarshal([]byte(source), &cfg); err != nil {
+		return "", fmt.Errorf("failed to unmarshal the cloud.conf: %w", err)
+	}
+
+	// We are copying the behaviour from CCO's transformer we need to:
+	// 1. Ensure that the Cloud is set in the cloud.conf
+	//   i. If it is set, verify that it is valid and does not conflict with the
+	//      infrastructure config. If it conflicts, we want to error
+	//  ii. If it is not set, default to public cloud (configv1.AzurePublicCloud)
+	//
+	// 2. Verify the cloud name set in the infra config is valid, if it is not
+	// bail with an informative error
+
+	// Verify the cloud name set in the infra config is valid
+	cloud := configv1.AzurePublicCloud
+	if azurePlatform := infra.Status.PlatformStatus.Azure; azurePlatform != nil {
+		if c := azurePlatform.CloudName; c != "" {
+			if _, ok := validAzureCloudNames[c]; !ok {
+				return "", field.NotSupported(field.NewPath("status", "platformStatus", "azure", "cloudName"), c, validAzureCloudNameValues)
+			}
+			cloud = c
+		}
+	}
+
+	// Ensure cloud set in cloud.conf matches infra
+	if cfg.Cloud != "" {
+		if !strings.EqualFold(string(cloud), cfg.Cloud) {
+			return "",
+				fmt.Errorf(`invalid user-provided cloud.conf: \"cloud\" field in user-provided
+				cloud.conf conflicts with infrastructure object`)
+		}
+	}
+	cfg.Cloud = string(cloud)
+
+	// If the virtual machine type is not set we need to make sure it uses the
+	// "standard" instance type. See OCPBUGS-25483 and OCPBUGS-20213 for more
+	// information
+	if cfg.VMType == "" {
+		cfg.VMType = azureconsts.VMTypeStandard
+	}
+
+	cfgbytes, err := json.Marshal(cfg)
+	if err != nil {
+		return "", fmt.Errorf("failed to marshal the cloud.conf: %w", err)
+	}
+	return string(cfgbytes), nil
+}

pkg/cloud/azure/azure_test.go

@@ -1,14 +1,26 @@
 package azure
 
 import (
+	"encoding/json"
+	"fmt"
 	"testing"
 
+	. "github.com/onsi/gomega"
 	configv1 "github.com/openshift/api/config/v1"
 	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	azure "sigs.k8s.io/cloud-provider-azure/pkg/provider"
+
+	ratelimitconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
 
 	"github.com/openshift/cluster-cloud-controller-manager-operator/pkg/config"
 )
 
+const (
+	infraCloudConfName = "test-config"
+	infraCloudConfKey  = "foo"
+)
+
 func TestResourcesRenderingSmoke(t *testing.T) {
 
 	tc := []struct {
@@ -79,3 +91,155 @@ func TestResourcesRenderingSmoke(t *testing.T) {
 		})
 	}
 }
+
+func makeInfrastructureResource(platform configv1.PlatformType, cloudName configv1.AzureCloudEnvironment) *configv1.Infrastructure {
+	cfg := configv1.Infrastructure{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "cluster",
+		},
+		Status: configv1.InfrastructureStatus{
+			PlatformStatus: &configv1.PlatformStatus{
+				Type: platform,
+			},
+		},
+		Spec: configv1.InfrastructureSpec{
+			CloudConfig: configv1.ConfigMapFileReference{
+				Name: infraCloudConfName,
+				Key:  infraCloudConfKey,
+			},
+			PlatformSpec: configv1.PlatformSpec{
+				Type: platform,
+			},
+		},
+	}
+
+	if platform == configv1.AzurePlatformType {
+		cfg.Status.PlatformStatus.Azure = &configv1.AzurePlatformStatus{
+			CloudName: cloudName,
+		}
+	}
+
+	return &cfg
+}
+
+// This test is a little complicated with all the JSON marshalling and
+// unmarshalling, but it is necessary due to the nature of how this data
+// is stored in Kuberenetes. The ConfigMaps containing the cloud config
+// will have string encoded JSON objects in them, due to the non-deterministic
+// natue of map object in Go we will need to examine the data instead of
+// comparing strings.
+func TestCloudConfigTransformer(t *testing.T) {
+	tc := []struct {
+		name     string
+		source   azure.Config
+		expected azure.Config
+		infra    *configv1.Infrastructure
+		errMsg   string
+	}{
+		{
+			name:   "Non Azure returns an error",
+			source: azure.Config{},
+			infra:  makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureStackCloud),
+			errMsg: fmt.Sprintf("invalid platform, expected CloudName to be %s", configv1.AzurePublicCloud),
+		},
+		{
+			name:     "Azure sets the vmType to standard and cloud to AzurePublicCloud when neither is set",
+			source:   azure.Config{},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
+		},
+		{
+			name:     "Azure doesn't modify vmType if user set",
+			source:   azure.Config{VMType: "vmss"},
+			expected: azure.Config{VMType: "vmss", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
+		},
+		{
+			name: "Azure sets the cloud to AzurePublicCloud and keeps existing fields",
+			source: azure.Config{
+				ResourceGroup: "test-rg",
+			},
+			expected: azure.Config{VMType: "standard", ResourceGroup: "test-rg", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
+		},
+		{
+			name:     "Azure keeps the cloud set to AzurePublicCloud",
+			source:   azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
+		},
+		{
+			name:     "Azure keeps the cloud set to US Gov cloud",
+			source:   azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzureUSGovernmentCloud)}},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzureUSGovernmentCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureUSGovernmentCloud),
+		},
+		{
+			name:     "Azure keeps the cloud set to China cloud",
+			source:   azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzureChinaCloud)}},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzureChinaCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureChinaCloud),
+		},
+		{
+			name:     "Azure keeps the cloud set to German cloud",
+			source:   azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzureGermanCloud)}},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzureGermanCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureGermanCloud),
+		},
+		{
+			name:   "Azure throws an error if the infra has an invalid cloud",
+			source: azure.Config{},
+			infra:  makeInfrastructureResource(configv1.AzurePlatformType, "AzureAnotherCloud"),
+			errMsg: "status.platformStatus.azure.cloudName: Unsupported value: \"AzureAnotherCloud\": supported values: \"AzureChinaCloud\", \"AzureGermanCloud\", \"AzurePublicCloud\", \"AzureStackCloud\", \"AzureUSGovernmentCloud\"",
+		},
+		{
+			name:     "Azure keeps the cloud set in the source when there is not one set in infrastructure",
+			source:   azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, ""),
+		},
+		{
+			name:     "Azure sets the cloud to match the infrastructure if an empty string is provided in source",
+			source:   azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: ""}},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
+		},
+		{
+			name:     "Azure sets the cloud to match the infrastructure if an empty string is provided in source and the infrastructure is non standard",
+			source:   azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: ""}},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzureUSGovernmentCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureUSGovernmentCloud),
+		},
+		{
+			name:   "Azure returns an error if the source config conflicts with the infrastructure",
+			source: azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			infra:  makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzureUSGovernmentCloud),
+			errMsg: "invalid user-provided cloud.conf: \\\"cloud\\\" field in user-provided\n\t\t\t\tcloud.conf conflicts with infrastructure object",
+		},
+		{
+			name:     "Azure keeps the cloud set to AzurePublicCloud if the source is upper case",
+			source:   azure.Config{AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: "AZUREPUBLICCLOUD"}},
+			expected: azure.Config{VMType: "standard", AzureAuthConfig: ratelimitconfig.AzureAuthConfig{Cloud: string(configv1.AzurePublicCloud)}},
+			infra:    makeInfrastructureResource(configv1.AzurePlatformType, configv1.AzurePublicCloud),
+		},
+	}
+
+	for _, tc := range tc {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			src, err := json.Marshal(tc.source)
+			g.Expect(err).NotTo(HaveOccurred(), "Marshal of source data should succeed")
+
+			actual, err := CloudConfigTransformer(string(src), tc.infra, nil)
+			if tc.errMsg != "" {
+				g.Expect(err).Should(MatchError(tc.errMsg))
+				g.Expect(actual).Should(Equal(""))
+			} else {
+				var observed azure.Config
+				g.Expect(json.Unmarshal([]byte(actual), &observed)).To(Succeed(), "Unmarshal of observed data should succeed")
+				g.Expect(observed).Should(Equal(tc.expected))
+			}
+		})
+	}
+}

pkg/cloud/cloud.go

@@ -53,7 +53,7 @@ func GetCloudConfigTransformer(platformStatus *configv1.PlatformStatus) (cloudCo
 		if azurestack.IsAzureStackHub(platformStatus) {
 			return azurestack.CloudConfigTransformer, true, nil
 		}
-		return nil, true, nil
+		return azure.CloudConfigTransformer, true, nil
 	case configv1.GCPPlatformType:
 		return common.NoOpTransformer, false, nil
 	case configv1.IBMCloudPlatformType: