-
Notifications
You must be signed in to change notification settings - Fork 160
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
doc: Correction of rbac api group for AppVersionRevision #1154
Conversation
patriknw
commented
May 19, 2023
- pods is in core "" api group
- replicasets is in "apps" api group
* pods is in core "" api group * replicasets is in "apps" api group
resources: ["pods"] # access to get replicaset | ||
verbs: ["get", "list"] | ||
- apiGroups: ["apps"] | ||
resources: ["replicasets"] # access to get revision annotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it works to define it as ["apps", ""]
but it's rather confusing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed, better now
@@ -296,7 +296,7 @@ PUTs must contain resourceVersions. Response: | |||
Unmarshal(response.entity) | |||
.to[String] | |||
.map(body => | |||
throw new PodCostException( | |||
throw new UnauthorizedException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't matter what exception is used here, but since handleUnauthorized
is also used for the revision
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
resources: ["pods"] # access to get replicaset | ||
verbs: ["get", "list"] | ||
- apiGroups: ["apps"] | ||
resources: ["replicasets"] # access to get revision annotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed, better now