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

feat: support BoundServiceAccountToken triggerAuth provider #701

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

maxcao13
Copy link

@maxcao13 maxcao13 commented Nov 6, 2024

Updates CRDs to support BoundServiceAccountToken trigger auth provider/source. Also adds a helm value of type array called serviceAccountTokenCreationRoles which allows you to add objects of name and namespace corresponding to service accounts in the cluster that you'd like the keda-operator to be able to request service account tokens from.

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)

Related to kedacore/keda#6272

@maxcao13 maxcao13 requested a review from a team as a code owner November 6, 2024 00:39
@maxcao13 maxcao13 force-pushed the bound-service-account-token branch from 9a06a5d to 923b1d2 Compare November 29, 2024 18:45
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

As default, KEDA doesn't have permission in the RBAC to create these tokens (and not granting it as default is worth IMHO), but maybe we could add an option to include the required permissions if users enable them (requiring explicit activation to include the required extra RBAC)

@maxcao13
Copy link
Author

maxcao13 commented Feb 4, 2025

Added a helm value of serviceAccountTokenCreationRoles which allows you to add objects of name and namespace corresponding to service accounts in the cluster that you'd like the keda-operator to be able to request service account tokens from. This is in: bac167e

I'm sort of new to writing helm charts, so please let me know if a restructure or renaming is needed here.

@maxcao13 maxcao13 force-pushed the bound-service-account-token branch from bac167e to 3fe5b3c Compare February 12, 2025 20:41
@maxcao13 maxcao13 changed the title feat: update CRDs for BoundServiceAccountToken triggerAuth provider feat: support BoundServiceAccountToken triggerAuth provider Feb 12, 2025
@maxcao13 maxcao13 force-pushed the bound-service-account-token branch from 3fe5b3c to dcb6d97 Compare February 25, 2025 20:04
@maxcao13
Copy link
Author

maxcao13 commented Feb 25, 2025

If people want to test this, I created this yaml file and applied it to my kind cluster (it pretty much follows the bound-service-account-e2e test I wrote in the main repo).

If you don't want to do all that, you can just add an item to the serviceAccountTokenCreationRoles with some namespaced name of a valid service account, and then install the chart and just check that the auth works:

# values.yaml
permissions:
  operator:
    restrict:
      serviceAccountTokenCreationRoles:
      - name: mysa
        namespace: default
$ kubectl create sa mysa -n default
serviceaccount/mysa created
$ kubectl auth can-i create serviceaccount/mysa --subresource=token --as system:serviceaccount:keda:keda-operator -n default
yes

If you want to test the bound service account token feature as a whole with helm:

  1. First I installed the helm chart with helm install keda-bsat ./keda/ -n keda --create-namespace
  2. Then I apply this large manifest, which sets up all the logic needed for the test deployment to scale in and out.
apiVersion: v1
kind: Namespace
metadata:
  name: myproject
---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: mysa
  namespace: myproject
---
apiVersion: keda.sh/v1alpha1
kind: TriggerAuthentication
metadata:
  name: bound-trigger
  namespace: myproject
spec:
  boundServiceAccountToken:
    - parameter: token
      serviceAccountName: mysa
---
### metrics api-like scale resource
apiVersion: apps/v1
kind: Deployment
metadata:
  name: bound-metrics-server
  namespace: myproject
  labels:
    app: bound-metrics-server
spec:
  replicas: 1
  selector:
    matchLabels:
      app: bound-metrics-server
  template:
    metadata:
      labels:
        app: bound-metrics-server
    spec:
      containers:
      - name: metrics
        image: ghcr.io/kedacore/tests-bound-service-account-token:latest
        ports:
        - containerPort: 8080
        imagePullPolicy: Always
---
apiVersion: v1
kind: Service
metadata:
  name: bound-metrics-server
  namespace: myproject
spec:
  selector:
    app: bound-metrics-server
  ports:
  - port: 8080
    targetPort: 8080
---
apiVersion: keda.sh/v1alpha1
kind: ScaledObject
metadata:
  name: myboundscaledobject
  namespace: myproject
  labels:
    app: bound-mydeploy
spec:
  scaleTargetRef:
    name: bound-mydeploy
  minReplicaCount: 3
  maxReplicaCount: 20
  cooldownPeriod:  1
  fallback:
    failureThreshold: 3
    replicas: 0
  triggers:
  - type: metrics-api
    metadata:
      targetValue: "5"
      activationTargetValue: "20"
      url: "http://bound-metrics-server.myproject.svc.cluster.local:8080/api/value"
      valueLocation: 'value'
      authMode: "bearer"
    authenticationRef:
      name: bound-trigger
---
### deployment we are scaling
apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: bound-mydeploy
  name: bound-mydeploy
  namespace: myproject
spec:
  selector:
    matchLabels:
      app: bound-mydeploy
  replicas: 0
  template:
    metadata:
      labels:
        app: bound-mydeploy
    spec:
      containers:
      - name: nginx
        image: nginxinc/nginx-unprivileged
---
### Note that these are the rbacs required for the mysa service account to authenticate with the bound-metrics-server container api
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: bound-metrics-server
rules:
- nonResourceURLs:
  - /api/value
  verbs:
  - get
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: bound-metrics-server
subjects:
- kind: ServiceAccount
  name: mysa
  namespace: myproject
roleRef:
  kind: ClusterRole
  name: bound-metrics-server
  apiGroup: rbac.authorization.k8s.io
---
### This is the rbac required for the metrics-deployment to be able to request token reviews and subject access reviews
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: token-review-and-subject-access-review-role
rules:
- apiGroups:
  - "authentication.k8s.io"
  resources:
  - tokenreviews
  verbs:
  - create
- apiGroups:
  - authorization.k8s.io
  resources:
  - subjectaccessreviews
  verbs:
  - create
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: token-review-and-subject-access-review-role-binding
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: token-review-and-subject-access-review-role
subjects:
- kind: ServiceAccount
  name: default
  namespace: myproject
  1. And then I specified in the serviceAccountTokenCreationRoles in values.yaml to be
# values.yaml
permissions:
  operator:
    restrict:
      serviceAccountTokenCreationRoles:
      - name: mysa
        namespace: myproject

and upgraded the chart.

  1. Then finally you can check the deployment is scaled to the minReplicas:
$ oc get deploy -n myproject
NAME                   READY   UP-TO-DATE   AVAILABLE   AGE
bound-metrics-server   1/1     1            1           55m
bound-mydeploy         3/3     3            3           55m

Note that, I also had to change the helm keda images to point to quay.io/macao/keda since the feature is not live yet.

@maxcao13 maxcao13 force-pushed the bound-service-account-token branch from dcb6d97 to e99ad04 Compare February 25, 2025 20:36
The array allows users to supply KEDA with the names and namespaces of service accounts that they would like the keda-operator to request tokens from. These service account tokens are then used in turn for the boundServiceAccountToken trigger source.

Signed-off-by: Max Cao <[email protected]>
@maxcao13 maxcao13 force-pushed the bound-service-account-token branch from e99ad04 to af77d3c Compare February 25, 2025 23:36
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.

4 participants