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

Allow projected volumes / Allow certain volumes - Support Azure Workload Identity #1078

Open
BenjaminHerbert opened this issue Nov 20, 2024 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@BenjaminHerbert
Copy link

BenjaminHerbert commented Nov 20, 2024

Describe the solution you'd like

I would like to be able to use the operator in AKS with workload identity enabled. This requires that additional volumes can be added on the fly to the pod definition.

I think this is a special case of #586.

Background

I am running an AKS cluster with workload identity enabled. This allows pods to use an annotated service account that can be used to access protected Azure resources.

The pod get s this label: azure.workload.identity/use: "true"

The service-account get's this annotation: azure.workload.identity/client-id: "<USER_ASSIGNED_CLIENT_ID

This requires that the pod gets a new volume, which looks like this:

  - name: azure-identity-token
    projected:
      defaultMode: 420
      sources:
      - serviceAccountToken:
          audience: api://AzureADTokenExchange
          expirationSeconds: 3600
          path: azure-identity-token

Due to some checks, the pod is being restarted as the operator determines that the volumes are different.

This hinders me to use Azure Workload Identity as authentication method e.g. in the azure-vm-agents plugin.

Describe alternatives you've considered

I would like to either

  • have an exception like the hotfix for K8S 1.21, which ignores kube-api-access-<random-suffix> as volume name, or
  • the possibility to configure which volumes should be ignored by a list of patterns which are ignored.

Additional context

Microsoft Entra Workload ID uses Service Account Token Volume Projection (that is, a service account), to enable pods to use a Kubernetes identity. A Kubernetes token is issued and OIDC federation enables Kubernetes applications to access Azure resources securely with Microsoft Entra ID, based on annotated service accounts.

@BenjaminHerbert
Copy link
Author

@brokenpip3 I would like to contribute here.

I think I could implement the "hotfix" approach analog to the kube-api-access hotfix.
Going the configuration route, I'd need some guidance.

@brokenpip3
Copy link
Collaborator

Hi Ben, thanks for offering your help.
The issue is clear to me like all the others that are part of the #586.
The fix is not so complex but it will require to rewrite a bit the checkForPodRecreation here.

The best way that I have in my mind to do it is to add for envs, volumes, annotations, and labels a list of "ignored" resources that will be ignored during the reconciliation. To do so we need:

  • a new go struct that will contains these 4 lists
  • handling the nill/empty case of the new go struct
  • handling the ignore of the elements of that list for each of the category during the pod reconciliation
  • helm chart values and template to handle the list

Another options is to decide the most used one (aks, eks, isto, etc) and put them there hardcoded and each time create a PR to add a new one, but this is way more ugly than the user lists I mention before.

Make sense to you?

@BenjaminHerbert
Copy link
Author

I wonder, if it might be sufficient to ignore projected volumes with "serviceAccountToken" in sources in general?
This should also work for istio.

But if we have to touch it anyhow, maybe it's time to change the hardcoding to something configurable like you describe.

Another idea I had was to use whitelisting for the things we need to ensure and ignore the rest.
But I think this goes against the idea of an operator being owner of the resource. Am I right?

@brokenpip3
Copy link
Collaborator

I wonder, if it might be sufficient to ignore projected volumes with "serviceAccountToken" in sources in general?
This should also work for istio.

we can start with this, but for sure we also need to give to the users an optional list of annotations that needs to be ignored like other operators flow like the zalando pg operator

@BenjaminHerbert
Copy link
Author

I first thought about putting it as a configuration on the operator, but if #706 comes to the conclusion that it's one operator and multiple Jenkins controllers (instances), this would be too limiting.

I'd see IgnoredAnnotations []string as part of JenkinsMaster struct.

Added here would be something like:

  • IgnoredVolumes / IgnoredVolumeNames
  • IgnoredAnnotations
  • IgnoredEnvVars
  • IgnoredLabels

I'd prefer this solution to the hardcoding after having thought about it.

@brokenpip3
Copy link
Collaborator

brokenpip3 commented Nov 26, 2024

Added here would be something like:

* IgnoredVolumes / IgnoredVolumeNames

* IgnoredAnnotations

* IgnoredEnvVars

* IgnoredLabels

I'd prefer this solution to the hardcoding after having thought about it.

yup like I said initially this is will be the most clean and easy to maintain solution for this issue, thanks for speeding time trying to understand the best approach here :)

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

No branches or pull requests

2 participants