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(eck-operator): make automountServiceAccountToken configurable #7690

Conversation

stefan-caraiman
Copy link
Contributor

Keeps the automountServiceAccountToken defaulting to true just as before for ServiceAccount/Statefulset

It makes it configurable though for cases such as with Azure AKS Security policy which scans for any pod/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 behind it.

@botelastic botelastic bot added the triage label Apr 5, 2024
@thbkrkr thbkrkr added :helm-charts >bug Something isn't working labels Apr 5, 2024
@botelastic botelastic bot removed the triage label Apr 5, 2024
Keeps the default behaviour of having automountServiceAccountToken
set to true for ServiceAccount/Statefulset

Signed-off-by: Stefan Caraiman <stefanc.caraiman@gmail.com>
@stefan-caraiman stefan-caraiman force-pushed the fix/configurable-automountServiceAccountToken branch from ada6542 to 05a23bf Compare April 6, 2024 05:55
@thbkrkr thbkrkr added >enhancement Enhancement of existing functionality and removed >bug Something isn't working labels Apr 10, 2024
@stefan-caraiman
Copy link
Contributor Author

@thbkrkr could you please have a look 👀 thanks!

Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

Looks very good overall! I left one question.

Could you please add some unit tests that covers the different cases? Here's a start to get you started:

> cat deploy/eck-operator/templates/tests/statefulset_test.yaml
suite: test operator statefulset
templates:
  - statefulset.yaml
  - configmap.yaml
tests:
  - it: should have automount service account tokens set by default
    asserts:
      - template: statefulset.yaml
        equal:
          path: spec.template.spec.automountServiceAccountToken
          value: true
  - it: should disable automount service account tokens
    set:
      automountServiceAccountToken: false
    asserts:
      - template: statefulset.yaml
        equal:
          path: spec.template.spec.automountServiceAccountToken
          value: false

deploy/eck-operator/values.yaml Outdated Show resolved Hide resolved
deploy/eck-operator/values.yaml Show resolved Hide resolved
Co-authored-by: Thibault Richard <thbkrkr@users.noreply.github.com>
@stefan-caraiman
Copy link
Contributor Author

@thbkrkr added some minimal unit tests, let me know if i should add something else 👍

Test automountServiceAccountToken in the SA/Statefulset.
@stefan-caraiman stefan-caraiman force-pushed the fix/configurable-automountServiceAccountToken branch from 45822f1 to 41ebb57 Compare April 18, 2024 11:07
@thbkrkr
Copy link
Contributor

thbkrkr commented Apr 24, 2024

buildkite test this

@thbkrkr
Copy link
Contributor

thbkrkr commented Apr 24, 2024

Thank you Stephan for this contribution!

@thbkrkr thbkrkr merged commit 1a1507f into elastic:main Apr 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality :helm-charts v2.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants