From 5f39919bc40e39922f75744cdbb86b9d494fdf1c Mon Sep 17 00:00:00 2001 From: Ashutosh Kumar Date: Sat, 20 May 2023 20:07:29 +0530 Subject: [PATCH] (feat): add workload identity in capz Signed-off-by: Ashutosh Kumar --- Makefile | 12 +- api/v1beta1/azureclusteridentity_webhook.go | 16 +- .../azureclusteridentity_webhook_test.go | 81 +++++++ api/v1beta1/types.go | 5 +- azure/scope/identity.go | 10 + azure/scope/workload_identity.go | 155 ++++++++++++ ...uster.x-k8s.io_azureclusteridentities.yaml | 1 + config/manager/manager.yaml | 16 ++ config/rbac/service_account.yaml | 3 +- scripts/kind-with-registry.sh | 61 ++++- ...uster-template-prow-workload-identity.yaml | 220 ++++++++++++++++++ .../ci/patches/azureclusteridentity-azwi.yaml | 11 + .../prow-workload-identity/kustomization.yaml | 12 + test/e2e/azure_test.go | 51 ++++ test/e2e/config/azure-dev.yaml | 2 + test/e2e/e2e_suite_test.go | 11 +- 16 files changed, 659 insertions(+), 8 deletions(-) create mode 100644 azure/scope/workload_identity.go create mode 100644 templates/test/ci/cluster-template-prow-workload-identity.yaml create mode 100644 templates/test/ci/patches/azureclusteridentity-azwi.yaml create mode 100644 templates/test/ci/prow-workload-identity/kustomization.yaml diff --git a/Makefile b/Makefile index 6335488c3f0..9fa44be0236 100644 --- a/Makefile +++ b/Makefile @@ -169,7 +169,11 @@ E2E_CONF_FILE ?= $(ROOT_DIR)/test/e2e/config/azure-dev.yaml E2E_CONF_FILE_ENVSUBST := $(ROOT_DIR)/test/e2e/config/azure-dev-envsubst.yaml SKIP_CLEANUP ?= false SKIP_LOG_COLLECTION ?= false -SKIP_CREATE_MGMT_CLUSTER ?= false +# @sonasingh46: Skip creating mgmt cluster for ci as workload identity needs kind cluster +# to be created with extra mounts for key pairs which is not yet supported +# by existing e2e framework. A mgmt cluster(kind) is created as part of e2e suite +# that meets workload identity pre-requisites. +SKIP_CREATE_MGMT_CLUSTER ?= true WIN_REPO_URL ?= # Build time versioning details. @@ -663,8 +667,12 @@ test-cover: test ## Run tests with code coverage and generate reports. ./hack/codecov-ignore.sh go tool cover -html=coverage.out -o coverage.html +.PHONY: kind-create-bootstrap +kind-create-bootstrap: $(KUBECTL) ## Create capz kind bootstrap cluster. + export AZWI=true KIND_CLUSTER_NAME=capz-e2e && ./scripts/kind-with-registry.sh + .PHONY: test-e2e-run -test-e2e-run: generate-e2e-templates install-tools ## Run e2e tests. +test-e2e-run: generate-e2e-templates install-tools kind-create-bootstrap ## Run e2e tests. $(ENVSUBST) < $(E2E_CONF_FILE) > $(E2E_CONF_FILE_ENVSUBST) && \ $(GINKGO) -v --trace --timeout=4h --tags=e2e --focus="$(GINKGO_FOCUS)" --skip="$(GINKGO_SKIP)" --nodes=$(GINKGO_NODES) --no-color=$(GINKGO_NOCOLOR) --output-dir="$(ARTIFACTS)" --junit-report="junit.e2e_suite.1.xml" $(GINKGO_ARGS) ./test/e2e -- \ -e2e.artifacts-folder="$(ARTIFACTS)" \ diff --git a/api/v1beta1/azureclusteridentity_webhook.go b/api/v1beta1/azureclusteridentity_webhook.go index 3301587b346..622b4f20d84 100644 --- a/api/v1beta1/azureclusteridentity_webhook.go +++ b/api/v1beta1/azureclusteridentity_webhook.go @@ -17,7 +17,10 @@ limitations under the License. package v1beta1 import ( + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + webhookutils "sigs.k8s.io/cluster-api-provider-azure/util/webhook" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" ) @@ -40,7 +43,18 @@ func (c *AzureClusterIdentity) ValidateCreate() error { // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. func (c *AzureClusterIdentity) ValidateUpdate(oldRaw runtime.Object) error { - return c.validateClusterIdentity() + var allErrs field.ErrorList + old := oldRaw.(*AzureClusterIdentity) + if err := webhookutils.ValidateImmutable( + field.NewPath("Spec", "Type"), + old.Spec.Type, + c.Spec.Type); err != nil { + allErrs = append(allErrs, err) + } + if len(allErrs) == 0 { + return c.validateClusterIdentity() + } + return apierrors.NewInvalid(GroupVersion.WithKind("AzureClusterIdentity").GroupKind(), c.Name, allErrs) } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. diff --git a/api/v1beta1/azureclusteridentity_webhook_test.go b/api/v1beta1/azureclusteridentity_webhook_test.go index 18b991088e2..b4dcae1d494 100644 --- a/api/v1beta1/azureclusteridentity_webhook_test.go +++ b/api/v1beta1/azureclusteridentity_webhook_test.go @@ -93,3 +93,84 @@ func TestAzureClusterIdentity_ValidateCreate(t *testing.T) { }) } } + +func TestAzureClusterIdentity_ValidateUpdate(t *testing.T) { + g := NewWithT(t) + + tests := []struct { + name string + oldClusterIdentity *AzureClusterIdentity + clusterIdentity *AzureClusterIdentity + wantErr bool + }{ + { + name: "azureclusteridentity with no change", + clusterIdentity: &AzureClusterIdentity{ + Spec: AzureClusterIdentitySpec{ + Type: ServicePrincipal, + ClientID: fakeClientID, + TenantID: fakeTenantID, + }, + }, + oldClusterIdentity: &AzureClusterIdentity{ + Spec: AzureClusterIdentitySpec{ + Type: ServicePrincipal, + ClientID: fakeClientID, + TenantID: fakeTenantID, + }, + }, + wantErr: false, + }, + { + name: "azureclusteridentity with a change in type", + clusterIdentity: &AzureClusterIdentity{ + Spec: AzureClusterIdentitySpec{ + Type: ServicePrincipal, + ClientID: fakeClientID, + TenantID: fakeTenantID, + ResourceID: fakeResourceID, + }, + }, + oldClusterIdentity: &AzureClusterIdentity{ + Spec: AzureClusterIdentitySpec{ + Type: WorkloadIdentity, + ClientID: fakeClientID, + TenantID: fakeTenantID, + ResourceID: fakeResourceID, + }, + }, + wantErr: true, + }, + { + name: "azureclusteridentity with a change in client ID", + clusterIdentity: &AzureClusterIdentity{ + Spec: AzureClusterIdentitySpec{ + Type: ServicePrincipal, + ClientID: fakeClientID, + TenantID: fakeTenantID, + ResourceID: fakeResourceID, + }, + }, + oldClusterIdentity: &AzureClusterIdentity{ + Spec: AzureClusterIdentitySpec{ + Type: WorkloadIdentity, + ClientID: "diff-fake-Client-ID", + TenantID: fakeTenantID, + ResourceID: fakeResourceID, + }, + }, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.clusterIdentity.ValidateUpdate(tc.oldClusterIdentity) + if tc.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 6ac084834e3..39c54b8c26a 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -557,7 +557,7 @@ const ( ) // IdentityType represents different types of identities. -// +kubebuilder:validation:Enum=ServicePrincipal;UserAssignedMSI;ManualServicePrincipal;ServicePrincipalCertificate +// +kubebuilder:validation:Enum=ServicePrincipal;UserAssignedMSI;ManualServicePrincipal;ServicePrincipalCertificate;WorkloadIdentity type IdentityType string const ( @@ -572,6 +572,9 @@ const ( // ServicePrincipalCertificate represents a service principal using a certificate as secret. ServicePrincipalCertificate IdentityType = "ServicePrincipalCertificate" + + // WorkloadIdentity represents a WorkloadIdentity. + WorkloadIdentity IdentityType = "WorkloadIdentity" ) // OSDisk defines the operating system disk for a VM. diff --git a/azure/scope/identity.go b/azure/scope/identity.go index e9ee0eb31b3..1ee6e2e8327 100644 --- a/azure/scope/identity.go +++ b/azure/scope/identity.go @@ -142,6 +142,16 @@ func (p *AzureCredentialsProvider) GetAuthorizer(ctx context.Context, resourceMa var authErr error var cred azcore.TokenCredential switch p.Identity.Spec.Type { + case infrav1.WorkloadIdentity: + azwiCredOptions, err := NewWorkloadIdentityCredentialOptions(). + WithTenantID(p.Identity.Spec.TenantID). + WithClientID(p.Identity.Spec.ClientID). + WithDefaults() + if err != nil { + return nil, errors.Wrapf(err, "failed to setup azwi options for identity %s", p.Identity.Name) + } + cred, authErr = NewWorkloadIdentityCredential(azwiCredOptions) + case infrav1.ServicePrincipal, infrav1.ServicePrincipalCertificate, infrav1.UserAssignedMSI: if err := createAzureIdentityWithBindings(ctx, p.Identity, resourceManagerEndpoint, activeDirectoryEndpoint, clusterMeta, p.Client); err != nil { return nil, err diff --git a/azure/scope/workload_identity.go b/azure/scope/workload_identity.go new file mode 100644 index 00000000000..745c1d302ab --- /dev/null +++ b/azure/scope/workload_identity.go @@ -0,0 +1,155 @@ +/* +Copyright 2023 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 scope + +import ( + "context" + "os" + "strings" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/pkg/errors" +) + +/* + +For workload identity to work we need the following. + +|-----------------------------------------------------------------------------------| +|AZURE_AUTHORITY_HOST | The Azure Active Directory (AAD) endpoint. | +|AZURE_CLIENT_ID | The client ID of the Azure AD | +| | application or user-assigned managed identity. | +|AZURE_TENANT_ID | The tenant ID of the Azure subscription. | +|AZURE_FEDERATED_TOKEN_FILE | The path of the projected service account token file. | +|-----------------------------------------------------------------------------------| + +With the current implementation, AZURE_CLIENT_ID and AZURE_TENANT_ID are read via AzureClusterIdentity +object and fallback to reading from env variables if not found on AzureClusterIdentity. + +AZURE_FEDERATED_TOKEN_FILE is the path of the projected service account token which is by default +"/var/run/secrets/azure/tokens/azure-identity-token". +The path can be overridden by setting "AZURE_FEDERATED_TOKEN_FILE" env variable. + +*/ + +const ( + // azureFederatedTokenFileEnvKey is the env key for AZURE_FEDERATED_TOKEN_FILE. + azureFederatedTokenFileEnvKey = "AZURE_FEDERATED_TOKEN_FILE" + // azureClientIDEnvKey is the env key for AZURE_CLIENT_ID. + azureClientIDEnvKey = "AZURE_CLIENT_ID" + // azureTenantIDEnvKey is the env key for AZURE_TENANT_ID. + azureTenantIDEnvKey = "AZURE_TENANT_ID" + // azureTokenFilePath is the path of the projected token. + azureTokenFilePath = "/var/run/secrets/azure/tokens/azure-identity-token" // #nosec G101 + // azureFederatedTokenFileRefreshTime is the time interval after which it should be read again. + azureFederatedTokenFileRefreshTime = 5 * time.Minute +) + +type workloadIdentityCredential struct { + assertion string + file string + cred *azidentity.ClientAssertionCredential + lastRead time.Time +} + +// WorkloadIdentityCredentialOptions contains the configurable options for azwi. +type WorkloadIdentityCredentialOptions struct { + azcore.ClientOptions + ClientID string + TenantID string + TokenFilePath string +} + +// NewWorkloadIdentityCredentialOptions returns an empty instance of WorkloadIdentityCredentialOptions. +func NewWorkloadIdentityCredentialOptions() *WorkloadIdentityCredentialOptions { + return &WorkloadIdentityCredentialOptions{} +} + +// WithClientID sets client ID to WorkloadIdentityCredentialOptions. +func (w *WorkloadIdentityCredentialOptions) WithClientID(clientID string) *WorkloadIdentityCredentialOptions { + w.ClientID = strings.TrimSpace(clientID) + return w +} + +// WithTenantID sets tenant ID to WorkloadIdentityCredentialOptions. +func (w *WorkloadIdentityCredentialOptions) WithTenantID(tenantID string) *WorkloadIdentityCredentialOptions { + w.TenantID = strings.TrimSpace(tenantID) + return w +} + +// getProjectedTokenPath return projected token file path from the env variable. +func getProjectedTokenPath() string { + tokenPath := strings.TrimSpace(os.Getenv(azureFederatedTokenFileEnvKey)) + if tokenPath == "" { + return azureTokenFilePath + } + return tokenPath +} + +// WithDefaults sets token file path. It also sets the client tenant ID from injected env in +// case empty values are passed. +func (w *WorkloadIdentityCredentialOptions) WithDefaults() (*WorkloadIdentityCredentialOptions, error) { + w.TokenFilePath = getProjectedTokenPath() + + // Fallback to using client ID from env variable if not set. + if w.ClientID == "" { + w.ClientID = strings.TrimSpace(os.Getenv(azureClientIDEnvKey)) + if w.ClientID == "" { + return nil, errors.New("empty client ID") + } + } + + // Fallback to using tenant ID from env variable. + if w.TenantID == "" { + w.TenantID = strings.TrimSpace(os.Getenv(azureTenantIDEnvKey)) + if w.TenantID == "" { + return nil, errors.New("empty tenant ID") + } + } + return w, nil +} + +// NewWorkloadIdentityCredential returns a workload identity credential. +func NewWorkloadIdentityCredential(options *WorkloadIdentityCredentialOptions) (azcore.TokenCredential, error) { + w := &workloadIdentityCredential{file: options.TokenFilePath} + cred, err := azidentity.NewClientAssertionCredential(options.TenantID, options.ClientID, w.getAssertion, &azidentity.ClientAssertionCredentialOptions{ClientOptions: options.ClientOptions}) + if err != nil { + return nil, err + } + w.cred = cred + return w, nil +} + +// GetToken returns the token for azwi. +func (w *workloadIdentityCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) { + return w.cred.GetToken(ctx, opts) +} + +func (w *workloadIdentityCredential) getAssertion(context.Context) (string, error) { + if now := time.Now(); w.lastRead.Add(azureFederatedTokenFileRefreshTime).Before(now) { + content, err := os.ReadFile(w.file) + if err != nil { + return "", err + } + w.assertion = string(content) + w.lastRead = now + } + return w.assertion, nil +} diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml index 6d0333e5f03..ae3cb57b839 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml @@ -150,6 +150,7 @@ spec: - UserAssignedMSI - ManualServicePrincipal - ServicePrincipalCertificate + - WorkloadIdentity type: string required: - clientID diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 0375abd93a7..ef41eaad2de 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -5,7 +5,9 @@ metadata: namespace: system labels: control-plane: capz-controller-manager + #ToDo: (@sonasingh46): Remove this label as part of aad pod identity deprecation aadpodidbinding: capz-controller-aadpodidentity-selector + spec: selector: matchLabels: @@ -16,6 +18,7 @@ spec: labels: control-plane: capz-controller-manager aadpodidbinding: capz-controller-aadpodidentity-selector + azure.workload.identity/use: "true" annotations: kubectl.kubernetes.io/default-container: manager spec: @@ -28,6 +31,10 @@ spec: image: controller:latest imagePullPolicy: Always name: manager + volumeMounts: + - mountPath: /var/run/secrets/azure/tokens + name: azure-identity-token + readOnly: true ports: - containerPort: 9440 name: healthz @@ -75,3 +82,12 @@ spec: key: node-role.kubernetes.io/master - effect: NoSchedule key: node-role.kubernetes.io/control-plane + volumes: + - name: azure-identity-token + projected: + defaultMode: 420 + sources: + - serviceAccountToken: + audience: api://AzureADTokenExchange + expirationSeconds: 3600 + path: azure-identity-token diff --git a/config/rbac/service_account.yaml b/config/rbac/service_account.yaml index c4180052449..9ab3e039d89 100644 --- a/config/rbac/service_account.yaml +++ b/config/rbac/service_account.yaml @@ -1,5 +1,6 @@ apiVersion: v1 kind: ServiceAccount metadata: + labels: name: manager - namespace: system \ No newline at end of file + namespace: system diff --git a/scripts/kind-with-registry.sh b/scripts/kind-with-registry.sh index 79c6808c762..a4e5933207f 100755 --- a/scripts/kind-with-registry.sh +++ b/scripts/kind-with-registry.sh @@ -21,6 +21,7 @@ set -o pipefail REPO_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. KUBECTL="${REPO_ROOT}/hack/tools/bin/kubectl" KIND="${REPO_ROOT}/hack/tools/bin/kind" +AZWI_ENABLED=${AZWI:-} make --directory="${REPO_ROOT}" "${KUBECTL##*/}" "${KIND##*/}" # Export desired cluster name; default is "capz" @@ -41,6 +42,56 @@ if [ "$(docker inspect -f '{{.State.Running}}' "${reg_name}" 2>/dev/null || true registry:2 fi +# To use workload identity, service account signing key pairs base64 encoded should be exposed via the +# env variables. The function creates the key pair files after reading it from the env variables. +function checkAZWIENVPreReqsAndCreateFiles() { + if [[ -z "${SERVICE_ACCOUNT_SIGNING_PUB}" ]]; then + echo "'SERVICE_ACCOUNT_SIGNING_PUB' is not set." + exit 1 + fi + + if [[ -z "${SERVICE_ACCOUNT_SIGNING_KEY}" ]]; then + echo "'SERVICE_ACCOUNT_SIGNING_KEY' is not set." + exit 1 + fi + mkdir -p "$HOME"/azwi/creds + echo "${SERVICE_ACCOUNT_SIGNING_PUB}" > "$HOME"/azwi/creds/sa.pub + echo "${SERVICE_ACCOUNT_SIGNING_KEY}" > "$HOME"/azwi/creds/sa.key + SERVICE_ACCOUNT_ISSUER="${SERVICE_ACCOUNT_ISSUER:-https://oidcissuercapzci.blob.core.windows.net/oidc-capzci/}" +} + +# This function create a kind cluster for Workload identity which requires key pairs path +# to be mounted on the kind cluster and hence extra mount flags are required. +function createKindForAZWI() { + echo "creating azwi kind" + cat <