From 9c3e3eeee5d9f119fcd54ac5bcbcdb668e912db8 Mon Sep 17 00:00:00 2001 From: Ashutosh Kumar Date: Sat, 6 May 2023 21:24:04 +0530 Subject: [PATCH] incorporate review comments Signed-off-by: Ashutosh Kumar --- Makefile | 1 - Tiltfile | 2 +- azure/scope/workload_identity.go | 28 ++-- config/default/azwi.yaml | 253 ------------------------------- config/manager/manager.yaml | 2 + config/rbac/service_account.yaml | 1 - test/e2e/config/azwi.yaml | 12 +- 7 files changed, 21 insertions(+), 278 deletions(-) delete mode 100644 config/default/azwi.yaml diff --git a/Makefile b/Makefile index b6ac730c535..a5cc133e431 100644 --- a/Makefile +++ b/Makefile @@ -661,7 +661,6 @@ test-e2e: ## Run "docker-build" and "docker-push" rules then run e2e tests. create-bootstrap-cluster: KIND_CLUSTER_NAME=capz-e2e && ./scripts/kind-with-registry.sh - .PHONY: test-e2e-skip-push test-e2e-skip-push: ## Run "docker-build" rule then run e2e tests. PULL_POLICY=IfNotPresent MANAGER_IMAGE=$(CONTROLLER_IMG)-$(ARCH):$(TAG) \ diff --git a/Tiltfile b/Tiltfile index 046c1cb48df..ed3839c4ef1 100644 --- a/Tiltfile +++ b/Tiltfile @@ -21,7 +21,7 @@ settings = { "kind_cluster_name": "capz", "capi_version": "v1.4.2", "cert_manager_version": "v1.11.1", - "azwi_version": "v0.14.0", + "azwi_version": "v1.0.0", "kubernetes_version": "v1.24.6", "aks_kubernetes_version": "v1.24.6", "flatcar_version": "3374.2.1", diff --git a/azure/scope/workload_identity.go b/azure/scope/workload_identity.go index 4c44d801c5b..fb70b4f34b9 100644 --- a/azure/scope/workload_identity.go +++ b/azure/scope/workload_identity.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Kubernetes Authors. +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. @@ -30,14 +30,14 @@ import ( /* -AZWI (Azure Workload Identity) required deploying AZWI mutating admission webhook +Azure Workload Identity (AZWI) requires deploying AZWI mutating admission webhook for self managed clusters e.g. Kind. The webhook injects the following environment variables to the pod that -uses a service account with a annotation `azure.workload.identity/use=true` +uses a label `azure.workload.identity/use=true` |-----------------------------------------------------------------------------------| |AZURE_AUTHORITY_HOST | The Azure Active Directory (AAD) endpoint. | -|AZURE_CLIENT_ID | The application/client ID of the Azure AD | +|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. | @@ -50,12 +50,12 @@ which is mounted at path `/var/run/secrets/azure/tokens/azure-identity-token` to */ const ( - // AzureFedratedTokenFileENVKey is the env key for AZURE_FEDERATED_TOKEN_FILE. - AzureFedratedTokenFileENVKey = "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" + // AzureFedratedTokenFileEnvKey is the env key for AZURE_FEDERATED_TOKEN_FILE. + AzureFedratedTokenFileEnvKey = "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" ) type workloadIdentityCredential struct { @@ -67,10 +67,10 @@ type workloadIdentityCredential struct { // WorkloadIdentityCredentialOptions contains the configurable options for azwi. type WorkloadIdentityCredentialOptions struct { + azcore.ClientOptions ClientID string TenantID string TokenFilePath string - azcore.ClientOptions } // NewWorkloadIdentityCredentialOptions returns an empty instance of WorkloadIdentityCredentialOptions. @@ -92,7 +92,7 @@ func (w *WorkloadIdentityCredentialOptions) WithTenantID(tenantID string) *Workl // GetProjectedTokenPath return projected token file path from the env variable. func GetProjectedTokenPath() (string, error) { - tokenPath := os.Getenv(AzureFedratedTokenFileENVKey) + tokenPath := os.Getenv(AzureFedratedTokenFileEnvKey) if strings.TrimSpace(tokenPath) == "" { return "", errors.New("projected token path not injected") } @@ -110,7 +110,7 @@ func (w *WorkloadIdentityCredentialOptions) WithDefaults() (*WorkloadIdentityCre // Fallback to using client ID from env variable if not set. if strings.TrimSpace(w.ClientID) == "" { - w.ClientID = os.Getenv(AzureClientIDENVKey) + w.ClientID = os.Getenv(AzureClientIDEnvKey) if strings.TrimSpace(w.ClientID) == "" { return nil, errors.New("empty client ID") } @@ -118,7 +118,7 @@ func (w *WorkloadIdentityCredentialOptions) WithDefaults() (*WorkloadIdentityCre // // Fallback to using tenant ID from env variable. if strings.TrimSpace(w.TenantID) == "" { - w.TenantID = os.Getenv(AzureTenantIDENVKey) + w.TenantID = os.Getenv(AzureTenantIDEnvKey) if strings.TrimSpace(w.TenantID) == "" { return nil, errors.New("empty tenant ID") } diff --git a/config/default/azwi.yaml b/config/default/azwi.yaml deleted file mode 100644 index a74174bbc9b..00000000000 --- a/config/default/azwi.yaml +++ /dev/null @@ -1,253 +0,0 @@ -apiVersion: v1 -kind: ServiceAccount -metadata: - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-admin - namespace: system ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - creationTimestamp: null - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-manager-role - namespace: system -rules: - - apiGroups: - - "" - resources: - - secrets - verbs: - - create - - delete - - get - - list - - patch - - update - - watch ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - creationTimestamp: null - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-manager-role -rules: - - apiGroups: - - "" - resources: - - serviceaccounts - verbs: - - create - - get - - list - - patch - - update - - watch - - apiGroups: - - admissionregistration.k8s.io - resources: - - mutatingwebhookconfigurations - verbs: - - create - - delete - - get - - list - - patch - - update - - watch ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-manager-rolebinding - namespace: system -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: azure-wi-webhook-manager-role -subjects: - - kind: ServiceAccount - name: azure-wi-webhook-admin - namespace: system ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-manager-rolebinding -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: azure-wi-webhook-manager-role -subjects: - - kind: ServiceAccount - name: azure-wi-webhook-admin - namespace: system ---- -apiVersion: v1 -data: - AZURE_ENVIRONMENT: AzurePublicCloud - AZURE_TENANT_ID: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0 -kind: ConfigMap -metadata: - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-config - namespace: system ---- -apiVersion: v1 -kind: Secret -metadata: - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-server-cert - namespace: system ---- -apiVersion: v1 -kind: Service -metadata: - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-webhook-service - namespace: system -spec: - ports: - - port: 443 - targetPort: 9443 - selector: - azure-workload-identity.io/system: "true" ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-controller-manager - namespace: capz-system -spec: - replicas: 2 - selector: - matchLabels: - azure-workload-identity.io/system: "true" - template: - metadata: - labels: - azure-workload-identity.io/system: "true" - spec: - containers: - - args: - - --arc-cluster=false - command: - - /manager - env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - apiVersion: v1 - fieldPath: metadata.namespace - envFrom: - - configMapRef: - name: azure-wi-webhook-config - image: mcr.microsoft.com/oss/azure/workload-identity/webhook:v0.15.0 - imagePullPolicy: IfNotPresent - livenessProbe: - httpGet: - path: /healthz - port: healthz - name: manager - ports: - - containerPort: 9443 - name: webhook-server - protocol: TCP - - containerPort: 8095 - name: metrics - protocol: TCP - - containerPort: 9440 - name: healthz - protocol: TCP - readinessProbe: - httpGet: - path: /readyz - port: healthz - resources: - limits: - cpu: 100m - memory: 30Mi - requests: - cpu: 100m - memory: 20Mi - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL - readOnlyRootFilesystem: true - runAsGroup: 65532 - runAsNonRoot: true - runAsUser: 65532 - seccompProfile: - type: RuntimeDefault - volumeMounts: - - mountPath: /certs - name: cert - readOnly: true - nodeSelector: - kubernetes.io/os: linux - priorityClassName: system-cluster-critical - serviceAccountName: azure-wi-webhook-admin - volumes: - - name: cert - secret: - defaultMode: 420 - secretName: azure-wi-webhook-server-cert ---- -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-controller-manager - namespace: system -spec: - minAvailable: 1 - selector: - matchLabels: - azure-workload-identity.io/system: "true" ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: MutatingWebhookConfiguration -metadata: - creationTimestamp: null - labels: - azure-workload-identity.io/system: "true" - name: azure-wi-webhook-mutating-webhook-configuration -webhooks: - - admissionReviewVersions: - - v1 - - v1beta1 - clientConfig: - service: - name: azure-wi-webhook-webhook-service - namespace: system - path: /mutate-v1-pod - failurePolicy: Ignore - matchPolicy: Equivalent - name: mutation.azure-workload-identity.io - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - pods - sideEffects: None diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 58c9deddaff..66e219a6ad8 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: diff --git a/config/rbac/service_account.yaml b/config/rbac/service_account.yaml index 55b973518af..9ab3e039d89 100644 --- a/config/rbac/service_account.yaml +++ b/config/rbac/service_account.yaml @@ -2,6 +2,5 @@ apiVersion: v1 kind: ServiceAccount metadata: labels: - azure.workload.identity/use: "true" name: manager namespace: system diff --git a/test/e2e/config/azwi.yaml b/test/e2e/config/azwi.yaml index 1dc2d7de1ef..fe2da1ecf7c 100644 --- a/test/e2e/config/azwi.yaml +++ b/test/e2e/config/azwi.yaml @@ -1,3 +1,5 @@ +# This config of azure workload identity is used to deploy in e2e +# The current config is from https://github.com/Azure/azure-workload-identity/releases/tag/v1.0.0 apiVersion: v1 kind: Namespace metadata: @@ -54,22 +56,16 @@ rules: resources: - serviceaccounts verbs: - - create - get - list - - patch - - update - watch - apiGroups: - admissionregistration.k8s.io resources: - mutatingwebhookconfigurations verbs: - - create - - delete - get - list - - patch - update - watch --- @@ -107,7 +103,6 @@ subjects: apiVersion: v1 data: AZURE_ENVIRONMENT: AzurePublicCloud - AZURE_TENANT_ID: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0 kind: ConfigMap metadata: labels: @@ -168,7 +163,7 @@ spec: envFrom: - configMapRef: name: azure-wi-webhook-config - image: mcr.microsoft.com/oss/azure/workload-identity/webhook:v1.0.0-beta.0 + image: mcr.microsoft.com/oss/azure/workload-identity/webhook:v1.0.0 imagePullPolicy: IfNotPresent livenessProbe: failureThreshold: 6 @@ -260,6 +255,7 @@ webhooks: objectSelector: matchLabels: azure.workload.identity/use: "true" + reinvocationPolicy: IfNeeded rules: - apiGroups: - ""