Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for workload identity #442

Merged
merged 3 commits into from
Dec 28, 2022

Conversation

cgroschupp
Copy link
Contributor

@cgroschupp cgroschupp commented Dec 9, 2022

Implementation details

use the azidentity sdk because it provides built-in support for workload identity and adal will discontinue support on March 31, 2023.

Added a new AuthType environment-azidentity that uses the DefaultAzureCredential helper to get the Azure credentials. This credential helper tries all possible mechisms for authentication, it uses the AZURE_* environment variables.

Tests

Helm values:

controller:
  image:
    repository: cgroschupp/akv2k8s-controller
    tag: workload-identity-support2
    pullPolicy: Always
  logLevel: debug
  podLabels:
    azure.workload.identity/use: "true"
  serviceAccount:
    create: true
    labels:
      azure.workload.identity/use: "true"
    annotations:
      azure.workload.identity/client-id: <replace>
env_injector:
  image:
    repository: cgroschupp/akv2k8s-webhook
    tag: workload-identity-support2
    pullPolicy: Always
  enabled: false
  authService: true
  podLabels:
    azure.workload.identity/use: "true"
  envImage:
    repository: cgroschupp/akv2k8s-vaultenv
    tag: workload-identity-support3
  serviceAccount:
    labels:
      azure.workload.identity/use: "true"
    annotations:
      azure.workload.identity/client-id: <replace>

global:
  keyVaultAuth: environment-azidentity

Addtionals nodes

  • Removed unused code
  • Go modules updated

@cgroschupp cgroschupp closed this Dec 9, 2022
@cgroschupp cgroschupp deleted the workload-identity-support branch December 9, 2022 17:09
@cgroschupp cgroschupp reopened this Dec 9, 2022
@cgroschupp cgroschupp force-pushed the workload-identity-support branch 2 times, most recently from e894157 to 9ae4954 Compare December 9, 2022 17:34
@cgroschupp cgroschupp force-pushed the workload-identity-support branch from 9ae4954 to 0797f06 Compare December 11, 2022 22:04
@cgroschupp cgroschupp marked this pull request as ready for review December 11, 2022 22:45
Copy link
Collaborator

@181192 181192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cgroschupp Thank you so much for you effort, this looks amazing! 😌

@181192
Copy link
Collaborator

181192 commented Dec 28, 2022

After some testing this looks promising, will release a beta version to test further
Getting a strange message on startup, but secrets are retrieved

I1228 20:00:21.016441       1 main.go:173] "Creating event broadcaster"
I1228 20:00:21.016542       1 main.go:179] use `environment-azidentity` as authType
I1228 20:00:21.016610       1 main.go:197] Managed Identity Credential will use IMDS managed identity
I1228 20:00:21.016636       1 main.go:197] NewDefaultAzureCredential failed to initialize some credentials:
        EnvironmentCredential: incomplete environment variable configuration. Only AZURE_TENANT_ID and AZURE_CLIENT_ID are set
I1228 20:00:21.016839       1 controller.go:166] "setting up event handlers"

@181192
Copy link
Collaborator

181192 commented Dec 28, 2022

Released chart version 2.3.1 for label support on service account and images with tag 1.5.0-beta.1.

Upgraded azidentity sdk to 1.3.0-beta.1 in hope for better handling of workload identity, but message on startup is still present

@181192
Copy link
Collaborator

181192 commented Dec 28, 2022

Example of values for anyone looking. Need to get the docs updated

global:
  keyVaultAuth: environment-azidentity
controller:
  image:
    tag: 1.5.0-beta.1
  podLabels:
    azure.workload.identity/use: "true"
  serviceAccount:
    labels:
      azure.workload.identity/use: "true"
    annotations:
      azure.workload.identity/client-id: <managed-client-id>
env_injector:
  image:
    tag: 1.5.0-beta.1
  podLabels:
    azure.workload.identity/use: "true"
  envImage:
    tag: 1.5.0-beta.1
  serviceAccount:
    labels:
      azure.workload.identity/use: "true"
    annotations:
      azure.workload.identity/client-id: <managed-client-id>

@pinkfloydx33
Copy link

Does this let you override the clientId / specify it manually?

As far as AZWI is concerned, azure.workload.identity/client-id is an optional annotation. When present, the mutating webhook adds the associated enviroment variable to the pods. Otherwise it skips it.

The reason for the optionality is because you can have multiple federated credentials/identities associated with a single service account. In those instances you wouldn't specify the annotation, but would rather rely on an application-defined/dependant mechanism for specifying the clientId in lieu of the annotation/environment variable.

The PR that was closed over this, accounted for that. I haven't had a chance to review this PR but wanted to check that this was covered (or acknowledged and intentionally omitted)

@181192
Copy link
Collaborator

181192 commented Dec 29, 2022

@pinkfloydx33 Yes, azure.workload.identity/client-id annotation is optional, in fact the podLabels is unnecessary only service account label azure.workload.identity/use: "true" is required.

If the client-id annotation is not used you will need to provide the AZURE_CLIENT_ID env variable.
The default DefaultAzureCredential from azidentity will handle the authentication method based on AZURE_* envs.

Example with annotations

global:
  keyVaultAuth: environment-azidentity
controller:
  image:
    tag: 1.5.0-beta.1
  serviceAccount:
    labels:
      azure.workload.identity/use: "true"
    annotations:
      azure.workload.identity/client-id: <managed-client-id>
env_injector:
  image:
    tag: 1.5.0-beta.1
  envImage:
    tag: 1.5.0-beta.1
  serviceAccount:
    labels:
      azure.workload.identity/use: "true"
    annotations:
      azure.workload.identity/client-id: <managed-client-id>

Example with env variable set for both controller and env-injector

global:
  keyVaultAuth: environment-azidentity
  env:
    AZURE_CLIENT_ID: <managed-client-id>
controller:
  image:
    tag: 1.5.0-beta.1
  serviceAccount:
    labels:
      azure.workload.identity/use: "true"
env_injector:
  image:
    tag: 1.5.0-beta.1
  envImage:
    tag: 1.5.0-beta.1
  serviceAccount:
    labels:
      azure.workload.identity/use: "true"

If you were to use different managed client ids for both service accounts you will need to set it separate for the controller and env-injector.

Environment variables will take precedence over the webhook annotation

@cgroschupp
Copy link
Contributor Author

@181192 Can you please also build an image for 1.5.0-beta.2 and push it to the docker hub.

@181192
Copy link
Collaborator

181192 commented Feb 12, 2023

Hi, @cgroschupp 1.5.0-beta.2 was only released for azure-keyvault-env, no changes for controller or webhook from 1.5.0-beta.1

But just released 1.5.0-beta.4 for all three components now

@181192
Copy link
Collaborator

181192 commented Feb 12, 2023

"Breaking change" from Azure pod labels are now required additional to service account label for the azure.workload.identity/use: "true" https://learn.microsoft.com/en-us/azure/aks/workload-identity-overview#pod-labels
Azure/azure-workload-identity#751
Was scratching my head, did not understand why it wasn't working on a fresh AKS cluster 😮‍💨

@181192 181192 linked an issue Feb 12, 2023 that may be closed by this pull request
@markrzasa
Copy link

Hi Everyone, is there a projected release for the workload identity integration? The beta controller image seems to work pretty well. If there is anything that needs to be done to get this in a release, I'd be happy to help. Thanks.

@cgroschupp
Copy link
Contributor Author

@markrzasa
Copy link

markrzasa commented Sep 5, 2023

Oh. I'm sorry. That's embarassing :)

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Azure Workload Identity Support
4 participants