OSDOCS 12581 CMA should support bound service account tokens #96335
Conversation
openshift-ci
bot
added
the
size/L
mburke5678
changed the title
|
@maxcao13 Can you PTAL? |
|
Thanks! I believe we should wait for QE to verify this feature before oking this. |
|
@prozehna PTAL |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
mburke5678
removed
the
do-not-merge/work-in-progress
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
mburke5678
added
do-not-merge/work-in-progress
| * If you are using a cluster trigger authentication, specify the `openshift-keda` project. | ||
|
|
||
| . Create a service account and token, if your cluster does not have one: | ||
| . Create a service account, if your cluster does not have one: |
There was a problem hiding this comment.
Nit: I'd probably remove the comma on this line.
| <2> Specifies that this trigger authentication uses a secret for authorization when connecting to the metrics endpoint. | ||
| <3> Specifies the type of authentication to use. | ||
| <4> Specifies the name of the secret to use. | ||
| <4> Specifies the name of the secret to use. See the following example secret with a bearer token. |
There was a problem hiding this comment.
probably all the lines saying 'with a bearer token' should be replaced with 'using a bearer token' ?
mburke5678
force-pushed
the
podauto-cma-2172-updates
branch
from
ad6daf6 to
e0192cf
Compare
mburke5678
changed the title
mburke5678
added
the
merge-review-needed
| name: my-basic-secret | ||
| namespace: default | ||
| data: | ||
| username: "dXNlcm5hbWU=" <1> |
There was a problem hiding this comment.
There must be established best practices by now for example usernames and passwords, my nit here is that "dXNlcm5hbWU=" is not beautiful.
There was a problem hiding this comment.
@prozehna the username and password here are base64 encoded. Those are never pretty. I want to show a base64 example for emphasis. (It's likely username and password in base64.)
| namespace: my-namespace | ||
| data: | ||
| bearerToken: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXV" <1> | ||
| ca-cert.pem: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0... <1> |
There was a problem hiding this comment.
in regard to my other comment, i'm also curious if there are best practices that apply here.
xenolinux
added
merge-review-in-progress
xenolinux
removed
the
merge-review-in-progress
There was a problem hiding this comment.
One more thing, is that we are actually missing a step, and sorry that I caught this late in the review and release, but user's need to create their own rbac in order for keda-operator to be able to request service account tokens from service accounts. Otherwise the feature won't work. (QE verifying this feature was able to catch this).
e.g.
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: keda-operator-token-creator
namespace: <namespace_name> # Replace with the namespace of the service account
rules:
- apiGroups:
- ""
resources:
- serviceaccounts/token
verbs:
- create
resourceNames:
- thanos # Replace with the name of the service account
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: keda-operator-token-creator-binding
namespace: <namespace_name> # Replace with the namespace of the service account
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: keda-operator-token-creator
subjects:
- kind: ServiceAccount
name: keda-operator
namespace: openshift-kedaI've documented the details and sort of why in this part of the upstream docs: https://keda.sh/docs/2.17/authentication-providers/bound-service-account-token/#permissions-for-keda-to-request-service-account-tokens
|
Sorry for the delay. |
mburke5678
force-pushed
the
podauto-cma-2172-updates
branch
from
7e8414a to
264ce93
Compare
|
@mburke5678: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Closed in favor of #99294 |
READY TO MERGE WHEN CUSTOM METRICS AUTOSCALER 2.17.2 RELEASES
https://issues.redhat.com/browse/OSDOCS-12581
Link to docs preview:
Understanding custom metrics autoscaler trigger authentications -- Added three example trigger authentications that use bound service account tokens. Moved example secret to after the associated trigger auth example. Current docs for comparison.
Using trigger authentications -- Updated example trigger auth to use bound service token. Current docs
Configuring the custom metrics autoscaler to use OpenShift Container Platform monitoring -- Updated example trigger auth to use bound service token; removed prereq to have a secret (not needed for new token). Current docs.
QE review: