From 23340d7522c115a42e4c0e7a24cbc0e4f9ac6fb3 Mon Sep 17 00:00:00 2001 From: bowan Date: Wed, 26 Feb 2020 01:23:48 +0800 Subject: [PATCH] [UseNetworkResourceInDifferentTenant] Fix bug of setting incorrect subscription id on azure network resource clients. --- .../legacy-cloud-providers/azure/azure.go | 9 +- .../azure/azure_standard.go | 14 +++- .../azure/azure_standard_test.go | 84 +++++++++++++++++++ 3 files changed, 103 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go index a2654fd7d88..da1e98f3e68 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -570,7 +570,7 @@ func (az *Cloud) configAzureClients( vmssVMClientConfig.Authorizer = multiTenantServicePrincipalTokenAuthorizer } - // If uses network resources in different AAD Tenant, update Authorizer for network resources client config + // If uses network resources in different AAD Tenant, update SubscriptionID and Authorizer for network resources client config if networkResourceServicePrincipalToken != nil { networkResourceServicePrincipalTokenAuthorizer := autorest.NewBearerAuthorizer(networkResourceServicePrincipalToken) routeClientConfig.Authorizer = networkResourceServicePrincipalTokenAuthorizer @@ -579,6 +579,13 @@ func (az *Cloud) configAzureClients( loadBalancerClientConfig.Authorizer = networkResourceServicePrincipalTokenAuthorizer securityGroupClientConfig.Authorizer = networkResourceServicePrincipalTokenAuthorizer publicIPClientConfig.Authorizer = networkResourceServicePrincipalTokenAuthorizer + + routeClientConfig.SubscriptionID = az.Config.NetworkResourceSubscriptionID + subnetClientConfig.SubscriptionID = az.Config.NetworkResourceSubscriptionID + routeTableClientConfig.SubscriptionID = az.Config.NetworkResourceSubscriptionID + loadBalancerClientConfig.SubscriptionID = az.Config.NetworkResourceSubscriptionID + securityGroupClientConfig.SubscriptionID = az.Config.NetworkResourceSubscriptionID + publicIPClientConfig.SubscriptionID = az.Config.NetworkResourceSubscriptionID } // Initialize all azure clients based on client config diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard.go index 6c7293200ef..9c89aa5fb56 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard.go @@ -98,7 +98,7 @@ func (az *Cloud) getAvailabilitySetID(resourceGroup, availabilitySetName string) func (az *Cloud) getFrontendIPConfigID(lbName, rgName, fipConfigName string) string { return fmt.Sprintf( frontendIPConfigIDTemplate, - az.SubscriptionID, + az.getNetworkResourceSubscriptionID(), rgName, lbName, fipConfigName) @@ -108,7 +108,7 @@ func (az *Cloud) getFrontendIPConfigID(lbName, rgName, fipConfigName string) str func (az *Cloud) getBackendPoolID(lbName, rgName, backendPoolName string) string { return fmt.Sprintf( backendPoolIDTemplate, - az.SubscriptionID, + az.getNetworkResourceSubscriptionID(), rgName, lbName, backendPoolName) @@ -118,12 +118,20 @@ func (az *Cloud) getBackendPoolID(lbName, rgName, backendPoolName string) string func (az *Cloud) getLoadBalancerProbeID(lbName, rgName, lbRuleName string) string { return fmt.Sprintf( loadBalancerProbeIDTemplate, - az.SubscriptionID, + az.getNetworkResourceSubscriptionID(), rgName, lbName, lbRuleName) } +// getNetworkResourceSubscriptionID returns the subscription id which hosts network resources +func (az *Cloud) getNetworkResourceSubscriptionID() string { + if az.Config.UsesNetworkResourceInDifferentTenant() { + return az.NetworkResourceSubscriptionID + } + return az.SubscriptionID +} + func (az *Cloud) mapLoadBalancerNameToVMSet(lbName string, clusterName string) (vmSetName string) { vmSetName = strings.TrimSuffix(lbName, InternalLoadBalancerNameSuffix) if strings.EqualFold(clusterName, vmSetName) { diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard_test.go index 2869533def7..0393170ef2b 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_standard_test.go @@ -19,6 +19,7 @@ limitations under the License. package azure import ( + "fmt" "strconv" "testing" @@ -29,6 +30,11 @@ import ( meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + networkResourceTenantID = "networkResourceTenantID" + networkResourceSubscriptionID = "networkResourceSubscriptionID" +) + func TestIsMasterNode(t *testing.T) { if isMasterNode(&v1.Node{}) { t.Errorf("Empty node should not be master!") @@ -416,3 +422,81 @@ func TestGetFrontendIPConfigName(t *testing.T) { assert.Equal(t, c.expected, ipconfigName, c.description) } } + +func TestGetFrontendIPConfigID(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + az := GetTestCloud(ctrl) + + testGetLoadBalancerSubResourceID(t, az, az.getFrontendIPConfigID, frontendIPConfigIDTemplate) +} + +func TestGetBackendPoolID(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + az := GetTestCloud(ctrl) + + testGetLoadBalancerSubResourceID(t, az, az.getBackendPoolID, backendPoolIDTemplate) +} + +func TestGetLoadBalancerProbeID(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + az := GetTestCloud(ctrl) + + testGetLoadBalancerSubResourceID(t, az, az.getLoadBalancerProbeID, loadBalancerProbeIDTemplate) +} + +func testGetLoadBalancerSubResourceID( + t *testing.T, + az *Cloud, + getLoadBalancerSubResourceID func(string, string, string) string, + expectedResourceIDTemplate string) { + cases := []struct { + description string + loadBalancerName string + resourceGroupName string + subResourceName string + useNetworkResourceInDifferentTenant bool + expected string + }{ + { + description: "resource id should contain NetworkResourceSubscriptionID when using network resources in different subscription", + loadBalancerName: "lbName", + resourceGroupName: "rgName", + subResourceName: "subResourceName", + useNetworkResourceInDifferentTenant: true, + }, + { + description: "resource id should contain SubscriptionID when not using network resources in different subscription", + loadBalancerName: "lbName", + resourceGroupName: "rgName", + subResourceName: "subResourceName", + useNetworkResourceInDifferentTenant: false, + }, + } + + for _, c := range cases { + if c.useNetworkResourceInDifferentTenant { + az.NetworkResourceTenantID = networkResourceTenantID + az.NetworkResourceSubscriptionID = networkResourceSubscriptionID + c.expected = fmt.Sprintf( + expectedResourceIDTemplate, + az.NetworkResourceSubscriptionID, + c.resourceGroupName, + c.loadBalancerName, + c.subResourceName) + } else { + az.NetworkResourceTenantID = "" + az.NetworkResourceSubscriptionID = "" + c.expected = fmt.Sprintf( + expectedResourceIDTemplate, + az.SubscriptionID, + c.resourceGroupName, + c.loadBalancerName, + c.subResourceName) + } + subResourceID := getLoadBalancerSubResourceID(c.loadBalancerName, c.resourceGroupName, c.subResourceName) + assert.Equal(t, c.expected, subResourceID, c.description) + } +}