-
Notifications
You must be signed in to change notification settings - Fork 391
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
authorization: clarify/fix service accounts and workspace initialization #1261
Conversation
Skipping CI for Draft Pull Request. |
/test all |
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.
We're about to rip out how the initializer bits work, maybe not worth the time to mess around with this?
|
||
By default, workspaces are only accessible to a user if they are in phse `Ready`. Workspaces that are initializing | ||
can be access only by users that are granted `initialize` verb on the `clusterworkspaces/content` resource in the | ||
parent workspace. |
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.
I don't think this is necessarily what we want ...
- some RBAC (where???) gives you global access to the initializingworkspaces virtual workspace, where you can see all workspaces that need your initializer
- as a first pass, actions through that workspace impersonate the user who created the workspace
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.
as discussed OOB: handling service accounts from parent workspaces won't be handled in this PR.
|
||
Kubernetes service accounts is granted access to the workspaces they are defined in and that are ready. | ||
|
||
E.g. a service account "default" in `root:org:ws:ws` is granted access to `root:org:ws:ws`, and through the |
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.
Why double-ws
?
b0b3c5c
to
9d88afb
Compare
80d7f75
to
518d7e1
Compare
### Initializing Workspaces | ||
|
||
By default, workspaces are only accessible to a user if they are in `Ready` phase. Workspaces that are initializing | ||
can be access only by users that are granted `admin` verb on the `clusterworkspaces/content` resource in the |
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.
can be access only by users that are granted `admin` verb on the `clusterworkspaces/content` resource in the | |
can be accessed only by users that are granted `admin` verb on the `clusterworkspaces/content` resource in the |
cluster, err := genericapirequest.ValidClusterFrom(ctx) | ||
if err != nil { | ||
return authorizer.DecisionNoOpinion, fmt.Sprintf("%q workspace access not permitted", cluster.Name), err | ||
} | ||
if cluster == nil || cluster.Name.Empty() { | ||
// empty or non-root based workspaces have no meaning in the context of authorizing workspace content. | ||
if cluster == nil || cluster.Name.Empty() || !cluster.Name.HasPrefix(tenancyv1alpha1.RootCluster) { | ||
return authorizer.DecisionNoOpinion, fmt.Sprintf("%q workspace access not permitted", cluster.Name), nil |
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.
follow-up: workspaceAccessNotPermittedReason everywhere.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: s-urbaniak, sttts 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 |
This PR adds:
follow-up for #996