Merge pull request #2309 from k8s-infra-cherrypick-robot/cherry-pick-2214-to-release-1.3

k8s-ci-robot · web-flow · commit e4bcb89a534b · 2022-05-17T14:02:30.000-07:00
[release-1.3] Use MSI ClientID as userAssignedIdentityID in azure.json
diff --git a/azure/services/identities/client.go b/azure/services/identities/client.go
@@ -0,0 +1,69 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package identities
+
+import (
+	"context"
+
+	"github.com/Azure/azure-sdk-for-go/services/msi/mgmt/2018-11-30/msi"
+	"github.com/Azure/go-autorest/autorest"
+	azuresdk "github.com/Azure/go-autorest/autorest/azure"
+	"sigs.k8s.io/cluster-api-provider-azure/azure"
+	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
+)
+
+// AzureClient contains the Azure go-sdk Client.
+type AzureClient struct {
+	userAssignedIdentities msi.UserAssignedIdentitiesClient
+}
+
+// NewClient creates a new MSI client from auth info.
+func NewClient(auth azure.Authorizer) *AzureClient {
+	c := newUserAssignedIdentitiesClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer())
+	return &AzureClient{c}
+}
+
+// newUserAssignedIdentitiesClient creates a new MSI client from subscription ID, base URI, and authorizer.
+func newUserAssignedIdentitiesClient(subscriptionID string, baseURI string, authorizer autorest.Authorizer) msi.UserAssignedIdentitiesClient {
+	userAssignedIdentitiesClient := msi.NewUserAssignedIdentitiesClientWithBaseURI(baseURI, subscriptionID)
+	azure.SetAutoRestClientDefaults(&userAssignedIdentitiesClient.Client, authorizer)
+	return userAssignedIdentitiesClient
+}
+
+// Get returns a managed service identity.
+func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, name string) (msi.Identity, error) {
+	ctx, _, done := tele.StartSpanWithLogger(ctx, "identities.AzureClient.Get")
+	defer done()
+
+	return ac.userAssignedIdentities.Get(ctx, resourceGroupName, name)
+}
+
+// GetClientID returns the client ID of a managed service identity, given its full URL identifier.
+func (ac *AzureClient) GetClientID(ctx context.Context, providerID string) (string, error) {
+	ctx, _, done := tele.StartSpanWithLogger(ctx, "identities.GetClientID")
+	defer done()
+
+	parsed, err := azuresdk.ParseResourceID(providerID)
+	if err != nil {
+		return "", err
+	}
+	ident, err := ac.Get(ctx, parsed.ResourceGroup, parsed.ResourceName)
+	if err != nil {
+		return "", err
+	}
+	return ident.ClientID.String(), nil
+}
diff --git a/controllers/azurejson_machine_controller.go b/controllers/azurejson_machine_controller.go
@@ -30,6 +30,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/identities"
 	"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -184,7 +185,13 @@ func (r *AzureJSONMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	// Construct secret for this machine
 	userAssignedIdentityIfExists := ""
 	if len(azureMachine.Spec.UserAssignedIdentities) > 0 {
-		userAssignedIdentityIfExists = azureMachine.Spec.UserAssignedIdentities[0].ProviderID
+		// TODO: remove this ClientID lookup code when the fixed cloud-provider-azure is default
+		idsClient := identities.NewClient(clusterScope)
+		userAssignedIdentityIfExists, err = idsClient.GetClientID(
+			ctx, azureMachine.Spec.UserAssignedIdentities[0].ProviderID)
+		if err != nil {
+			return reconcile.Result{}, errors.Wrap(err, "failed to get user-assigned identity ClientID")
+		}
 	}
 
 	if azureMachine.Spec.Identity == infrav1.VMIdentityNone {
diff --git a/controllers/azurejson_machinepool_controller.go b/controllers/azurejson_machinepool_controller.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/identities"
 	expv1 "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
@@ -131,12 +132,6 @@ func (r *AzureJSONMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl
 		return reconcile.Result{}, err
 	}
 
-	// Construct secret for this machine
-	userAssignedIdentityIfExists := ""
-	if len(azureMachinePool.Spec.UserAssignedIdentities) > 0 {
-		userAssignedIdentityIfExists = azureMachinePool.Spec.UserAssignedIdentities[0].ProviderID
-	}
-
 	// Create the scope.
 	clusterScope, err := scope.NewClusterScope(ctx, scope.ClusterScopeParams{
 		Client:       r.Client,
@@ -147,6 +142,18 @@ func (r *AzureJSONMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl
 		return reconcile.Result{}, errors.Wrap(err, "failed to create scope")
 	}
 
+	// Construct secret for this machine
+	userAssignedIdentityIfExists := ""
+	if len(azureMachinePool.Spec.UserAssignedIdentities) > 0 {
+		// TODO: remove this ClientID lookup code when the fixed cloud-provider-azure is default
+		idsClient := identities.NewClient(clusterScope)
+		userAssignedIdentityIfExists, err = idsClient.GetClientID(
+			ctx, azureMachinePool.Spec.UserAssignedIdentities[0].ProviderID)
+		if err != nil {
+			return reconcile.Result{}, errors.Wrap(err, "failed to get user-assigned identity ClientID")
+		}
+	}
+
 	apiVersion, kind := infrav1.GroupVersion.WithKind("AzureMachinePool").ToAPIVersionAndKind()
 	owner := metav1.OwnerReference{
 		APIVersion: apiVersion,
diff --git a/controllers/azurejson_machinetemplate_controller.go b/controllers/azurejson_machinetemplate_controller.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/identities"
 	"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
 	"sigs.k8s.io/cluster-api/util"
@@ -143,7 +144,13 @@ func (r *AzureJSONTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	// Construct secret for this machine template
 	userAssignedIdentityIfExists := ""
 	if len(azureMachineTemplate.Spec.Template.Spec.UserAssignedIdentities) > 0 {
-		userAssignedIdentityIfExists = azureMachineTemplate.Spec.Template.Spec.UserAssignedIdentities[0].ProviderID
+		// TODO: remove this ClientID lookup code when the fixed cloud-provider-azure is default
+		idsClient := identities.NewClient(clusterScope)
+		userAssignedIdentityIfExists, err = idsClient.GetClientID(
+			ctx, azureMachineTemplate.Spec.Template.Spec.UserAssignedIdentities[0].ProviderID)
+		if err != nil {
+			return reconcile.Result{}, errors.Wrap(err, "failed to get user-assigned identity ClientID")
+		}
 	}
 
 	if azureMachineTemplate.Spec.Template.Spec.Identity == infrav1.VMIdentityNone {
diff --git a/go.mod b/go.mod
@@ -97,6 +97,7 @@ require (
 	github.com/go-openapi/swag v0.19.14 // indirect
 	github.com/gobuffalo/flect v0.2.4 // indirect
 	github.com/gobwas/glob v0.2.3 // indirect
+	github.com/gofrs/uuid v4.2.0+incompatible // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang-jwt/jwt/v4 v4.0.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
diff --git a/go.sum b/go.sum
@@ -536,6 +536,8 @@ github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5x
 github.com/godror/godror v0.24.2/go.mod h1:wZv/9vPiUib6tkoDl+AZ/QLf5YZgMravZ7jxH2eQWAE=
 github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
 github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
+github.com/gofrs/uuid v4.2.0+incompatible h1:yyYWMnhkhrKwwr8gAOcOCYxOOscHgDS9yZgBrnJfGa0=
+github.com/gofrs/uuid v4.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
 github.com/gogo/googleapis v1.2.0/go.mod h1:Njal3psf3qN6dwBtQfUmBZh2ybovJ0tlu3o/AC7HYjU=
 github.com/gogo/googleapis v1.4.0/go.mod h1:5YRNX2z1oM5gXdAkurHa942MDgEJyk02w4OecKY87+c=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
diff --git a/templates/test/ci/cluster-template-prow-dual-stack.yaml b/templates/test/ci/cluster-template-prow-dual-stack.yaml
diff --git a/templates/test/ci/cluster-template-prow-external-cloud-provider.yaml b/templates/test/ci/cluster-template-prow-external-cloud-provider.yaml
diff --git a/templates/test/ci/prow-dual-stack/kustomization.yaml b/templates/test/ci/prow-dual-stack/kustomization.yaml
@@ -8,6 +8,8 @@ patchesStrategicMerge:
   - ../patches/tags.yaml
   - ../patches/controller-manager.yaml
   - ../patches/cluster-cni.yaml
+  - patches/azure-machine-template-control-plane.yaml
+  - patches/azure-machine-template.yaml
 configMapGenerator:
   - name: cni-${CLUSTER_NAME}-calico
     files:
diff --git a/templates/test/ci/prow-dual-stack/patches/azure-machine-template-control-plane.yaml b/templates/test/ci/prow-dual-stack/patches/azure-machine-template-control-plane.yaml
@@ -0,0 +1,11 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: AzureMachineTemplate
+metadata:
+  name: ${CLUSTER_NAME}-control-plane
+  namespace: default
+spec:
+  template:
+    spec:
+      identity: UserAssigned
+      userAssignedIdentities:
+      - providerID: /subscriptions/${AZURE_SUBSCRIPTION_ID}/resourceGroups/${CI_RG:=capz-ci}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/${USER_IDENTITY:=cloud-provider-user-identity}
diff --git a/templates/test/ci/prow-dual-stack/patches/azure-machine-template.yaml b/templates/test/ci/prow-dual-stack/patches/azure-machine-template.yaml
@@ -0,0 +1,11 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: AzureMachineTemplate
+metadata:
+  name: ${CLUSTER_NAME}-md-0
+  namespace: default
+spec:
+  template:
+    spec:
+      identity: UserAssigned
+      userAssignedIdentities:
+      - providerID: /subscriptions/${AZURE_SUBSCRIPTION_ID}/resourceGroups/${CI_RG:=capz-ci}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/${USER_IDENTITY:=cloud-provider-user-identity}
diff --git a/templates/test/ci/prow-external-cloud-provider/kustomization.yaml b/templates/test/ci/prow-external-cloud-provider/kustomization.yaml
@@ -9,6 +9,8 @@ patchesStrategicMerge:
   - ../patches/controller-manager.yaml
   - ../patches/cluster-cni.yaml
   - ../patches/apiserver.yaml
+  - patches/azure-machine-template.yaml
+  - patches/azure-machine-template-control-plane.yaml
 configMapGenerator:
   - name: cni-${CLUSTER_NAME}-calico
     files:
diff --git a/templates/test/ci/prow-external-cloud-provider/patches/azure-machine-template-control-plane.yaml b/templates/test/ci/prow-external-cloud-provider/patches/azure-machine-template-control-plane.yaml
@@ -0,0 +1,11 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: AzureMachineTemplate
+metadata:
+  name: ${CLUSTER_NAME}-control-plane
+  namespace: default
+spec:
+  template:
+    spec:
+      identity: UserAssigned
+      userAssignedIdentities:
+      - providerID: /subscriptions/${AZURE_SUBSCRIPTION_ID}/resourceGroups/${CI_RG:=capz-ci}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/${USER_IDENTITY:=cloud-provider-user-identity}
diff --git a/templates/test/ci/prow-external-cloud-provider/patches/azure-machine-template.yaml b/templates/test/ci/prow-external-cloud-provider/patches/azure-machine-template.yaml
@@ -0,0 +1,11 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
+kind: AzureMachineTemplate
+metadata:
+  name: ${CLUSTER_NAME}-md-0
+  namespace: default
+spec:
+  template:
+    spec:
+      identity: UserAssigned
+      userAssignedIdentities:
+      - providerID: /subscriptions/${AZURE_SUBSCRIPTION_ID}/resourceGroups/${CI_RG:=capz-ci}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/${USER_IDENTITY:=cloud-provider-user-identity}
diff --git a/test/e2e/azure_test.go b/test/e2e/azure_test.go
@@ -428,10 +428,12 @@ var _ = Describe("Workload cluster creation", func() {
 		})
 	})
 
-	// ci-e2e.sh and Prow CI skip this test by default.
-	// To include this test, set `GINKGO_SKIP=""`.
+	// ci-e2e.sh and Prow CI skip this test by default. To include this test, set `GINKGO_SKIP=""`.
+	// This spec expects a user-assigned identity named "cloud-provider-user-identity" in a "capz-ci"
+	// resource group. Override these defaults by setting the USER_IDENTITY and CI_RG environment variables.
 	Context("Creating a cluster that uses the external cloud provider [OPTIONAL]", func() {
 		It("with a 1 control plane nodes and 2 worker nodes", func() {
+			By("using user-assigned identity")
 			clusterName = getClusterName(clusterNamePrefix, "oot")
 			clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{
 				ClusterProxy: bootstrapClusterProxy,
@@ -499,6 +501,7 @@ var _ = Describe("Workload cluster creation", func() {
 		})
 	})
 
+	// ci-e2e.sh and Prow CI skip this test by default. To include this test, set `GINKGO_SKIP=""`.
 	Context("Creating a Windows Enabled cluster with dockershim [OPTIONAL]", func() {
 		// Requires 3 control planes due to https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/857
 		It("With 3 control-plane nodes and 1 Linux worker node and 1 Windows worker node", func() {
@@ -547,8 +550,12 @@ var _ = Describe("Workload cluster creation", func() {
 		})
 	})
 
+	// ci-e2e.sh and Prow CI skip this test by default. To include this test, set `GINKGO_SKIP=""`.
+	// This spec expects a user-assigned identity named "cloud-provider-user-identity" in a "capz-ci"
+	// resource group. Override these defaults by setting the USER_IDENTITY and CI_RG environment variables.
 	Context("Creating a dual-stack cluster [OPTIONAL]", func() {
 		It("With dual-stack worker node", func() {
+			By("using user-assigned identity")
 			clusterName = getClusterName(clusterNamePrefix, "dual-stack")
 			clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{
 				ClusterProxy: bootstrapClusterProxy,
