diff --git a/pkg/utils/cloudconfig/azure/config.go b/pkg/utils/cloudconfig/azure/config.go index 3aeb6c625..605857c60 100644 --- a/pkg/utils/cloudconfig/azure/config.go +++ b/pkg/utils/cloudconfig/azure/config.go @@ -23,16 +23,13 @@ type CloudConfig struct { azclient.ARMClientConfig `json:",inline" mapstructure:",squash"` azclient.AzureAuthConfig `json:",inline" mapstructure:",squash"` ratelimit.Config `json:",inline" mapstructure:",squash"` - // name of cluster - ClusterName string `json:"clusterName,omitempty" mapstructure:"clusterName,omitempty"` + // azure resource location Location string `json:"location,omitempty" mapstructure:"location,omitempty"` // subscription ID SubscriptionID string `json:"subscriptionID,omitempty" mapstructure:"subscriptionID,omitempty"` // default resource group where the azure resources are deployed ResourceGroup string `json:"resourceGroup,omitempty" mapstructure:"resourceGroup,omitempty"` - // default resource group where the cluster is deployed - ClusterResourceGroup string `json:"clusterResourceGroup,omitempty" mapstructure:"resourceGroup,omitempty"` // name of the virtual network of cluster VnetName string `json:"vnetName,omitempty" mapstructure:"vnetName,omitempty"` // name of the resource group where the virtual network is deployed @@ -91,14 +88,6 @@ func (cfg *CloudConfig) validate() error { return fmt.Errorf("resource group is empty") } - if cfg.ClusterResourceGroup == "" { - return fmt.Errorf("cluster resource group is empty") - } - - if cfg.ClusterName == "" { - return fmt.Errorf("cluster name is empty") - } - if cfg.VnetName == "" { return fmt.Errorf("virtual network name is empty") } @@ -146,8 +135,6 @@ func (cfg *CloudConfig) trimSpace() { cfg.UserAssignedIdentityID = strings.TrimSpace(cfg.UserAssignedIdentityID) cfg.AADClientID = strings.TrimSpace(cfg.AADClientID) cfg.AADClientSecret = strings.TrimSpace(cfg.AADClientSecret) - cfg.ClusterResourceGroup = strings.TrimSpace(cfg.ClusterResourceGroup) cfg.VnetName = strings.TrimSpace(cfg.VnetName) cfg.VnetResourceGroup = strings.TrimSpace(cfg.VnetResourceGroup) - cfg.ClusterName = strings.TrimSpace(cfg.ClusterName) } diff --git a/pkg/utils/cloudconfig/azure/config_test.go b/pkg/utils/cloudconfig/azure/config_test.go index 321dc79bd..9c22a24ef 100644 --- a/pkg/utils/cloudconfig/azure/config_test.go +++ b/pkg/utils/cloudconfig/azure/config_test.go @@ -6,7 +6,6 @@ import ( "github.com/google/go-cmp/cmp" "sigs.k8s.io/cloud-provider-azure/pkg/azclient" "sigs.k8s.io/cloud-provider-azure/pkg/azclient/policy/ratelimit" - "sigs.k8s.io/cloud-provider-azure/pkg/consts" ) func TestTrimSpace(t *testing.T) { @@ -23,28 +22,24 @@ func TestTrimSpace(t *testing.T) { AADClientID: "\n test \n", AADClientSecret: " test \n", }, - Location: " test \n", - SubscriptionID: " test \n", - ResourceGroup: "\r\n test \n", - ClusterName: " test \n", - ClusterResourceGroup: "\r\n test \n", - VnetName: " test ", - VnetResourceGroup: " \t test ", + Location: " test \n", + SubscriptionID: " test \n", + ResourceGroup: "\r\n test \n", + VnetName: " test ", + VnetResourceGroup: " \t test ", } - expected := CloudConfig{ + want := CloudConfig{ ARMClientConfig: azclient.ARMClientConfig{ Cloud: "test", TenantID: "test", UserAgent: "test", }, - Location: "test", - SubscriptionID: "test", - ResourceGroup: "test", - ClusterName: "test", - ClusterResourceGroup: "test", - VnetName: "test", - VnetResourceGroup: "test", + Location: "test", + SubscriptionID: "test", + ResourceGroup: "test", + VnetName: "test", + VnetResourceGroup: "test", AzureAuthConfig: azclient.AzureAuthConfig{ UseManagedIdentityExtension: true, UserAssignedIdentityID: "test", @@ -53,7 +48,7 @@ func TestTrimSpace(t *testing.T) { }, } config.trimSpace() - if diff := cmp.Diff(config, expected); diff != "" { + if diff := cmp.Diff(config, want); diff != "" { t.Errorf("trimSpace() mismatch (-got +want):\n%s", diff) } }) @@ -70,7 +65,8 @@ func TestSetUserAgent(t *testing.T) { func TestValidate(t *testing.T) { tests := map[string]struct { config *CloudConfig - expectPass bool + wantConfig *CloudConfig + wantPass bool }{ "Cloud empty": { config: &CloudConfig{ @@ -81,14 +77,12 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: true, UserAssignedIdentityID: "a", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", }, - expectPass: false, + wantPass: false, }, "Location empty": { config: &CloudConfig{ @@ -99,34 +93,14 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: true, UserAssignedIdentityID: "a", }, - Location: "", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", }, - expectPass: false, + wantPass: false, }, - "ClusterName empty": { - config: &CloudConfig{ - ARMClientConfig: azclient.ARMClientConfig{ - Cloud: "c", - }, - AzureAuthConfig: azclient.AzureAuthConfig{ - UseManagedIdentityExtension: true, - UserAssignedIdentityID: "a", - }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "", - ClusterResourceGroup: "g", - VnetName: "vn", - }, - expectPass: false, - }, - "ClusterResourceGroup empty": { + "SubscriptionID empty": { config: &CloudConfig{ ARMClientConfig: azclient.ARMClientConfig{ Cloud: "c", @@ -135,16 +109,14 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: true, UserAssignedIdentityID: "a", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "", - VnetName: "vn", + Location: "l", + SubscriptionID: "", + ResourceGroup: "v", + VnetName: "vn", }, - expectPass: false, + wantPass: false, }, - "SubscriptionID empty": { + "ResourceGroup empty": { config: &CloudConfig{ ARMClientConfig: azclient.ARMClientConfig{ Cloud: "c", @@ -153,16 +125,14 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: true, UserAssignedIdentityID: "a", }, - Location: "l", - SubscriptionID: "", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "", + VnetName: "vn", }, - expectPass: false, + wantPass: false, }, - "ResourceGroup empty": { + "VnetName empty": { config: &CloudConfig{ ARMClientConfig: azclient.ARMClientConfig{ Cloud: "c", @@ -171,16 +141,14 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: true, UserAssignedIdentityID: "a", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "", }, - expectPass: false, + wantPass: false, }, - "VnetName empty": { + "VnetResourceGroup empty": { config: &CloudConfig{ ARMClientConfig: azclient.ARMClientConfig{ Cloud: "c", @@ -189,17 +157,13 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: true, UserAssignedIdentityID: "a", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", + VnetResourceGroup: "", }, - expectPass: false, - }, - "VnetResourceGroup empty": { - config: &CloudConfig{ + wantConfig: &CloudConfig{ ARMClientConfig: azclient.ARMClientConfig{ Cloud: "c", }, @@ -207,15 +171,13 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: true, UserAssignedIdentityID: "a", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", - VnetResourceGroup: "", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", + VnetResourceGroup: "v", }, - expectPass: true, + wantPass: true, }, "UserAssignedIdentityID not empty when UseManagedIdentityExtension is false": { config: &CloudConfig{ @@ -226,14 +188,12 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: false, UserAssignedIdentityID: "aaaa", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", }, - expectPass: false, + wantPass: false, }, "AADClientID empty": { config: &CloudConfig{ @@ -246,14 +206,12 @@ func TestValidate(t *testing.T) { AADClientID: "", AADClientSecret: "2", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", }, - expectPass: false, + wantPass: false, }, "AADClientSecret empty": { config: &CloudConfig{ @@ -266,14 +224,12 @@ func TestValidate(t *testing.T) { AADClientID: "1", AADClientSecret: "", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", }, - expectPass: false, + wantPass: false, }, "RateLimitConfig values are zero": { config: &CloudConfig{ @@ -286,12 +242,10 @@ func TestValidate(t *testing.T) { AADClientID: "1", AADClientSecret: "2", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", Config: ratelimit.Config{ CloudProviderRateLimit: true, CloudProviderRateLimitQPS: 0, @@ -300,7 +254,30 @@ func TestValidate(t *testing.T) { CloudProviderRateLimitBucketWrite: 0, }, }, - expectPass: true, + wantConfig: &CloudConfig{ + ARMClientConfig: azclient.ARMClientConfig{ + Cloud: "c", + }, + AzureAuthConfig: azclient.AzureAuthConfig{ + UseManagedIdentityExtension: false, + UserAssignedIdentityID: "", + AADClientID: "1", + AADClientSecret: "2", + }, + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", + VnetResourceGroup: "v", + Config: ratelimit.Config{ + CloudProviderRateLimit: true, + CloudProviderRateLimitQPS: 1.0, + CloudProviderRateLimitBucket: 5, + CloudProviderRateLimitQPSWrite: 1.0, + CloudProviderRateLimitBucketWrite: 5, + }, + }, + wantPass: true, }, "RateLimitConfig with non-zero values": { config: &CloudConfig{ @@ -313,12 +290,10 @@ func TestValidate(t *testing.T) { AADClientID: "1", AADClientSecret: "2", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", Config: ratelimit.Config{ CloudProviderRateLimit: true, CloudProviderRateLimitQPS: 2, @@ -327,7 +302,30 @@ func TestValidate(t *testing.T) { CloudProviderRateLimitBucketWrite: 4, }, }, - expectPass: true, + wantConfig: &CloudConfig{ + ARMClientConfig: azclient.ARMClientConfig{ + Cloud: "c", + }, + AzureAuthConfig: azclient.AzureAuthConfig{ + UseManagedIdentityExtension: false, + UserAssignedIdentityID: "", + AADClientID: "1", + AADClientSecret: "2", + }, + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", + VnetResourceGroup: "v", + Config: ratelimit.Config{ + CloudProviderRateLimit: true, + CloudProviderRateLimitQPS: 2, + CloudProviderRateLimitBucket: 4, + CloudProviderRateLimitQPSWrite: 2, + CloudProviderRateLimitBucketWrite: 4, + }, + }, + wantPass: true, }, "CloudProviderRateLimit is false": { config: &CloudConfig{ @@ -340,17 +338,34 @@ func TestValidate(t *testing.T) { AADClientID: "1", AADClientSecret: "2", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", + Config: ratelimit.Config{ + CloudProviderRateLimit: false, + }, + }, + wantConfig: &CloudConfig{ + ARMClientConfig: azclient.ARMClientConfig{ + Cloud: "c", + }, + AzureAuthConfig: azclient.AzureAuthConfig{ + UseManagedIdentityExtension: false, + UserAssignedIdentityID: "", + AADClientID: "1", + AADClientSecret: "2", + }, + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", + VnetResourceGroup: "v", Config: ratelimit.Config{ CloudProviderRateLimit: false, }, }, - expectPass: true, + wantPass: true, }, "has all required properties with secret and default values": { config: &CloudConfig{ @@ -363,14 +378,28 @@ func TestValidate(t *testing.T) { AADClientID: "1", AADClientSecret: "2", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", }, - expectPass: true, + wantConfig: &CloudConfig{ + ARMClientConfig: azclient.ARMClientConfig{ + Cloud: "c", + }, + AzureAuthConfig: azclient.AzureAuthConfig{ + UseManagedIdentityExtension: false, + UserAssignedIdentityID: "", + AADClientID: "1", + AADClientSecret: "2", + }, + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", + VnetResourceGroup: "v", + }, + wantPass: true, }, "has all required properties with msi and specified values": { config: &CloudConfig{ @@ -381,43 +410,39 @@ func TestValidate(t *testing.T) { UseManagedIdentityExtension: true, UserAssignedIdentityID: "u", }, - Location: "l", - SubscriptionID: "s", - ResourceGroup: "v", - ClusterName: "c", - ClusterResourceGroup: "g", - VnetName: "vn", + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", }, - expectPass: true, + wantConfig: &CloudConfig{ + ARMClientConfig: azclient.ARMClientConfig{ + Cloud: "c", + }, + AzureAuthConfig: azclient.AzureAuthConfig{ + UseManagedIdentityExtension: true, + UserAssignedIdentityID: "u", + }, + Location: "l", + SubscriptionID: "s", + ResourceGroup: "v", + VnetName: "vn", + VnetResourceGroup: "v", + }, + wantPass: true, }, } for name, test := range tests { t.Run(name, func(t *testing.T) { err := test.config.validate() - if got := err == nil; got != test.expectPass { - t.Errorf("validate() = got %v, want %v", got, test.expectPass) + if got := err == nil; got != test.wantPass { + t.Fatalf("validate() = got %v, want %v", got, test.wantPass) } if err == nil { - if test.config.VnetResourceGroup != test.config.ResourceGroup { - t.Errorf("validate() = got %v, want %v", test.config.VnetResourceGroup, test.config.ResourceGroup) - } - - rateLimitConfig := test.config.Config - if rateLimitConfig.CloudProviderRateLimit { - if rateLimitConfig.CloudProviderRateLimitQPS == 0 { - t.Errorf("validate() = got %v, want default %v", rateLimitConfig.CloudProviderRateLimitQPS, consts.RateLimitQPSDefault) - } - if test.config.Config.CloudProviderRateLimitBucket == 0 { - t.Errorf("validate() = got %v, want default %v", rateLimitConfig.CloudProviderRateLimitBucket, consts.RateLimitBucketDefault) - } - if test.config.Config.CloudProviderRateLimitQPSWrite == 0 { - t.Errorf("validate() = got %v, want default %v", rateLimitConfig.CloudProviderRateLimitQPSWrite, rateLimitConfig.CloudProviderRateLimitQPS) - } - if test.config.Config.CloudProviderRateLimitBucketWrite == 0 { - t.Errorf("validate() = got %v, want default %v", rateLimitConfig.CloudProviderRateLimitBucketWrite, rateLimitConfig.CloudProviderRateLimitBucket) - } + if diff := cmp.Diff(test.config, test.wantConfig); diff != "" { + t.Errorf("validate() mismatch (-got +want):\n%s", diff) } } }) @@ -426,29 +451,29 @@ func TestValidate(t *testing.T) { func TestNewCloudConfigFromFile(t *testing.T) { tests := map[string]struct { - filePath string - expectErr bool - expectedConfig *CloudConfig + filePath string + wantErr bool + wantConfig *CloudConfig }{ "file path is empty": { - filePath: "", - expectErr: true, + filePath: "", + wantErr: true, }, "failed to open file": { - filePath: "./test/not_exist.json", - expectErr: true, + filePath: "./test/not_exist.json", + wantErr: true, }, "failed to unmarshal file": { - filePath: "./test/azure_config_nojson.txt", - expectErr: true, + filePath: "./test/azure_config_nojson.txt", + wantErr: true, }, "failed to validate config": { - filePath: "./test/azure_invalid_config.json", - expectErr: true, + filePath: "./test/azure_invalid_config.json", + wantErr: true, }, "succeeded to load config": { filePath: "./test/azure_valid_config.json", - expectedConfig: &CloudConfig{ + wantConfig: &CloudConfig{ ARMClientConfig: azclient.ARMClientConfig{ Cloud: "AzurePublicCloud", TenantID: "00000000-0000-0000-0000-000000000000", @@ -459,27 +484,25 @@ func TestNewCloudConfigFromFile(t *testing.T) { AADClientID: "", AADClientSecret: "", }, - Location: "eastus", - SubscriptionID: "00000000-0000-0000-0000-000000000000", - ResourceGroup: "test-rg", - VnetName: "test-vnet", - VnetResourceGroup: "test-rg", - ClusterName: "test-cluster", - ClusterResourceGroup: "test-rg", + Location: "eastus", + SubscriptionID: "00000000-0000-0000-0000-000000000000", + ResourceGroup: "test-rg", + VnetName: "test-vnet", + VnetResourceGroup: "test-rg", Config: ratelimit.Config{ CloudProviderRateLimit: false, }, }, - expectErr: false, + wantErr: false, }, } for name, test := range tests { t.Run(name, func(t *testing.T) { config, err := NewCloudConfigFromFile(test.filePath) - if got := err != nil; got != test.expectErr { - t.Fatalf("Failed to run NewCloudConfigFromFile(%s): got %v, want %v", test.filePath, got, test.expectErr) + if got := err != nil; got != test.wantErr { + t.Fatalf("Failed to run NewCloudConfigFromFile(%s): got %v, want %v", test.filePath, got, test.wantErr) } - if diff := cmp.Diff(config, test.expectedConfig); diff != "" { + if diff := cmp.Diff(config, test.wantConfig); diff != "" { t.Errorf("NewCloudConfigFromFile(%s) mismatch (-got +want):\n%s", test.filePath, diff) } }) diff --git a/pkg/utils/cloudconfig/azure/test/azure_invalid_config.json b/pkg/utils/cloudconfig/azure/test/azure_invalid_config.json index 5a50d257c..0fd9aa60d 100644 --- a/pkg/utils/cloudconfig/azure/test/azure_invalid_config.json +++ b/pkg/utils/cloudconfig/azure/test/azure_invalid_config.json @@ -6,7 +6,5 @@ "aadClientId": "00000000-0000-0000-0000-000000000000", "resourceGroup": " test-rg ", "location": " eastus ", - "clusterName": " test-cluster", - "vnetName": "test -vnet", - "clusterResourceGroup": "test-rg " + "vnetName": "test -vnet" } diff --git a/pkg/utils/cloudconfig/azure/test/azure_valid_config.json b/pkg/utils/cloudconfig/azure/test/azure_valid_config.json index a8c803acc..4c938e78c 100644 --- a/pkg/utils/cloudconfig/azure/test/azure_valid_config.json +++ b/pkg/utils/cloudconfig/azure/test/azure_valid_config.json @@ -6,8 +6,6 @@ "userAssignedIdentityID": "11111111-1111-1111-1111-111111111111", "resourceGroup": "test-rg", "location": "eastus", - "clusterResourceGroup": "test-rg", - "clusterName": "test-cluster", "vnetName": "test-vnet", - "vnetResourceGroup": "test-rg", + "vnetResourceGroup": "test-rg" }