-
Notifications
You must be signed in to change notification settings - Fork 382
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
2. Plumb LogicalCluster type through and generalize lcluster logic #744
Conversation
f680a35
to
ea3dfd2
Compare
Any pointers to the why and how of this deprecation? This is the first I'm hearing of it, and the existing code seems to heavily depend on the existing field. |
ea3dfd2
to
fdae37f
Compare
Would it be possible to break this into smaller pieces to make detailed review a possibility? |
94273bf
to
dfa03d6
Compare
dfa03d6
to
a2d4e97
Compare
} else if decision != authorizer.DecisionAllow { | ||
return kerrors.NewForbidden(tenancyv1beta1.Resource("workspaces"), "", fmt.Errorf("user %q lacks %s permission to workspace %q", user.GetName(), verb, orgClusterName)) | ||
klog.Errorf("user %q lacks (%s) clusterworkspaces/content %q permission for %q in %s: %s", user.GetName(), decisions[decision], verb, orgName, parent, reason) |
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.
general thought: does this already land in audit?
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.
good question. I don't know how it could. We cannot return a detailed error object. Is there some way to give annotations to the audit layer?
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.
yes, i believe so 🤔 let me look at that.
e2ebac3
to
ec8a094
Compare
f57494b
to
5034d72
Compare
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.
There are at least 4 orthogonal concerns represented in this PR:
- logical cluster type
- changes to virtual workspaces
- changes to auth
- new 'team' workspace
Maybe submit these changes separately so they can be reviewed with the rigor they deserve?
d3d6027
to
468b1d5
Compare
f605594
to
a74ceff
Compare
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
This reverts commit 034f5c4.
210db16
to
1163457
Compare
Based on https://github.com/kcp-dev/apimachinery/blob/main/pkg/logicalcluster/logicalcluster.go.
clusterName
s aka logical clusters akalogicalcluster.LogicalCluster
logicalcluster.From(obj) LogicalCluster
(to prepare for deprecation ofmetadata.clusterName
)implemented path semantics on the higher layers (i.e. this opens the door for multiple levels for orgs, but generally greatly clarifies code).Moved to 3. virtual: implement full generalized logical cluster support and prepare for sharding #780.removed duality between v1beta1Moved to 5. kubectl-kcp: get rid of workspace duality and implement logical cluster path semantics #778.Workspaces
in some virtual workspace path and theClusterWorkspaces
in an org. Now you can dokubectl get workspaces
in an orggeneralized the authentication cache inMoved to 3. virtual: implement full generalized logical cluster support and prepare for sharding #780.pkg/virtual/workspaces
to beClusterWorkspaces
-driven on the level of the workspaces the user does requests against, and not depending on the parent. This prepares us for sharding.simplified and complete kubectl plugin to implement the second section in https://docs.google.com/document/d/1XJJCr16bg22QuJnQB2W98A-djRTtrLr2JE2IWZUHzJU/edit#heading=h.nn8danthd1ys.Moved to 5. kubectl-kcp: get rid of workspace duality and implement logical cluster path semantics #778.In general, this PR implements the ideas of https://docs.google.com/document/d/1XJJCr16bg22QuJnQB2W98A-djRTtrLr2JE2IWZUHzJU/edit#heading=h.nn8danthd1ys.
Depends on kcp-dev/kubernetes#53.