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 setting automountServiceAccountToken #2298

Merged

Conversation

Aransh
Copy link
Contributor

@Aransh Aransh commented Oct 29, 2024

Purpose of this PR

By default, kubernetes enables "automountServiceAccountToken" for all pods.
This poses a security risk, as pods might get kube-api permissions unintentionally.
More specifically, this fails security compliance tests:
https://learn.microsoft.com/en-us/azure/governance/policy/samples/built-in-policies
https://www.azadvertizer.net/azpolicyadvertizer/kubernetes_block-automount-token.html

Solution - Disable "automountServiceAccountToken", create projected volume for the token, and mount it on relevant containers

Fixes #1189

Proposed changes:

  • Allow setting this field for all spark-operator workloads and serviceAccounts
  • No further changes needed, as projected volume can already be added using exiting "volumes" and "volumeMounts" values

Change Category

  • Bugfix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that could affect existing functionality)
  • Documentation update

Rationale

Checklist

  • I have conducted a self-review of my own code.
  • I have updated documentation accordingly.
  • I have added tests that prove my changes are effective or that my feature works.
  • Existing unit tests pass locally with my changes.

Additional Notes

While default behavior remains unchanged, and anyone disabling "automountServiceAccountToken" should know the consequences (as this is a standard k8s feature, not a spark-operator one), I am adding an example values file for deploying the operator with it disabled:

controller:
  serviceAccount:
    automountServiceAccountToken: false
  volumes:
    - name: kube-api-access
      projected:
        defaultMode: 420
        sources:
        - serviceAccountToken:
            expirationSeconds: 3607
            path: token
        - configMap:
            items:
            - key: ca.crt
              path: ca.crt
            name: kube-root-ca.crt
        - downwardAPI:
            items:
            - fieldRef:
                apiVersion: v1
                fieldPath: metadata.namespace
              path: namespace
  volumeMounts:
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access
      readOnly: true

spark:
  serviceAccount:
    automountServiceAccountToken: false

webhook:
  serviceAccount:
    automountServiceAccountToken: false
  volumes:
    - name: kube-api-access
      projected:
        defaultMode: 420
        sources:
        - serviceAccountToken:
            expirationSeconds: 3607
            path: token
        - configMap:
            items:
            - key: ca.crt
              path: ca.crt
            name: kube-root-ca.crt
        - downwardAPI:
            items:
            - fieldRef:
                apiVersion: v1
                fieldPath: metadata.namespace
              path: namespace
  volumeMounts:
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access
      readOnly: true

…counts

Signed-off-by: Aran Shavit <Aranshavit@gmail.com>
Signed-off-by: Aran Shavit <Aranshavit@gmail.com>
Copy link
Contributor

@ImpSy ImpSy left a comment

Choose a reason for hiding this comment

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

/lgtm

@Aransh
Copy link
Contributor Author

Aransh commented Nov 3, 2024

/assign @ChenYi015

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ChenYi015, ImpSy, malki-akamai

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ChenYi015
Copy link
Contributor

/lgtm

@google-oss-prow google-oss-prow bot merged commit 515d805 into kubeflow:master Nov 4, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

automountServiceAccountToken
4 participants