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

fix: add configurable automountServiceAccountToken #620

Conversation

stefan-caraiman
Copy link

@stefan-caraiman stefan-caraiman commented Apr 4, 2024

This PR makes automountServiceAccountToken configurable for the deployment definitions.

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • README is updated with new configuration values (if applicable) learn more
  • A PR is opened to update KEDA core (repo) (if applicable, ie. when deployment manifests are modified)

Fixes #

  • configurable automountServiceAccountToken option in pod definitions

@stefan-caraiman stefan-caraiman requested a review from a team as a code owner April 4, 2024 11:36
@stefan-caraiman stefan-caraiman force-pushed the fix/configurable-automountServiceAccountToken branch from c365338 to 8aaed9c Compare April 4, 2024 11:38
@JorTurFer
Copy link
Member

Hello,
What is the goal of this change? I mean, this change suggests that the service account token is something optional, and it isn't. If you don't mount the service account tokens KEDA won't work as all the components have to interact with k8s api server.
Am I missing something?

@stefan-caraiman
Copy link
Author

stefan-caraiman commented Apr 5, 2024

Hello, What is the goal of this change? I mean, this change suggests that the service account token is something optional, and it isn't. If you don't mount the service account tokens KEDA won't work as all the components have to interact with k8s api server. Am I missing something?

Hi there @JorTurFer ,

The change keeps the automountServiceAccountToken defaulting to true just as before.

It makes it configurable though for cases such as with Azure AKS Security policy which scans for any pod/deployments/SA that have it set to true and instead recommended as part of their security benchmarks to disable automounting and injecting the service account tokens as volumes explicitly for each workload that requires it: Kubernetes clusters should disable automounting API credentials

A similar thread on this from cert-manager and hardened values for such setups & rationale

For example in KEDAs cases this would be done as follows:

# Disable auto-mounting and inject it via volumes instead
automountServiceAccountToken: false

volumes:
  keda:
    extraVolumeMounts:
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: serviceaccount-token
      readOnly: true
    extraVolumes:
    - name: serviceaccount-token
      projected:
        defaultMode: 0444
        sources:
        - serviceAccountToken:
            expirationSeconds: 3607
            path: token
        - configMap:
            name: kube-root-ca.crt
            items:
            - key: ca.crt
              path: ca.crt
        - downwardAPI:
            items:
            - path: namespace
              fieldRef:
                apiVersion: v1
                fieldPath: metadata.namespace

# Same extras for the following too
# volumes.webhooks.extraVolumeMounts
# volumes.webhooks.extraVolumes
# volumes.metricsApiServer.extraVolumeMounts
# volumes.metricsApiServer.extraVolumes

This same volume workflow is explained to be done by K8s ServiceAccount admission controller too when setting automountServiceAccountToken: true which still remains the default.

Hope this makes more sense on why it is helpful to make it configurable 😄

@stefan-caraiman stefan-caraiman force-pushed the fix/configurable-automountServiceAccountToken branch 2 times, most recently from 0c8fc72 to 5b2f372 Compare April 6, 2024 05:55
@stefan-caraiman
Copy link
Author

@JorTurFer @tomkerkhove PTAL 👋

@stefan-caraiman stefan-caraiman force-pushed the fix/configurable-automountServiceAccountToken branch from 5b2f372 to 7a1453e Compare April 8, 2024 06:41
@JorTurFer
Copy link
Member

I think that you should expect the rule directly in the Azure Security Center rather than "hacking" the rule. @tomkerkhove ?

@stefan-caraiman
Copy link
Author

stefan-caraiman commented Apr 11, 2024

No harm in setting explicit definitions though, at least imo it seems more of a hack that the internal K8s admission controller does the volume mount of the tokens without the user being aware of it.

Nevertheless will leave it up to you to decide 👍

@JorTurFer
Copy link
Member

I'm not directly against this tbh, but I'd like to see other folks thoughts.
@zroubalik @tomkerkhove ?

@JorTurFer JorTurFer mentioned this pull request Apr 11, 2024
@stefan-caraiman
Copy link
Author

@JorTurFer i will go ahead and close this PR in favour of #625 thanks

@stefan-caraiman stefan-caraiman deleted the fix/configurable-automountServiceAccountToken branch April 15, 2024 12:08
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.

2 participants