-
Notifications
You must be signed in to change notification settings - Fork 138
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
[EV-5126] Rename and update tigera-crds clusterrole #3458
Conversation
eedf0af
to
e9f7321
Compare
pkg/render/apiserver.go
Outdated
@@ -1906,3 +1914,25 @@ func (c *apiServerComponent) multiTenantManagedClusterAccessClusterRoles() []cli | |||
|
|||
return objects | |||
} | |||
|
|||
func (c *apiServerComponent) getDeprecatedResources() []client.Object { | |||
renamedRscList := []client.Object{} |
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.
nit: it could be replaced with empty slice declaration:
var renamedRscList []client.Object
pkg/render/apiserver.go
Outdated
@@ -279,6 +279,9 @@ func (c *apiServerComponent) Objects() ([]client.Object, []client.Object) { | |||
// Explicitly delete any global OSS objects. | |||
// Namespaced objects will be handled by namespace deletion. | |||
objsToDelete = append(objsToDelete, globalCalicoObjects...) | |||
|
|||
// Explicitly delete any renamed/deprecated objects. | |||
objsToDelete = append(objsToDelete, c.getDeprecatedResources()...) |
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 think this can be outside (under) the if-else statement. This always needs to get deleted, regardless of whether the cluster is enterprise right now.
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 moved it outside, but for opensource we dont have the "tigera-crds" clusterrole and the apiserver binding. Doesn't it matter or wouldn't it fail if they dont exist?
@@ -1258,26 +1261,31 @@ func (c *apiServerComponent) tigeraCustomResourcesClusterRole() *rbacv1.ClusterR | |||
"patch", | |||
}, | |||
}, | |||
{ |
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.
Please if you could add a comment as to why we need this permission, it will help us track over time why we added permissions and also make it easier to remove unused permissions.
7437633
to
7b9e125
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
…o it Changes: 1. Rename "tigera-crds" clusterrole to "tigera-apiserver" 2. Add "authorizationreviews" rbac to "tigera-apiserver" clusterrole; queryserver needs this rbac to return authorized list of items in the resultset. 3. Rename "tigera-apiserver-access-tigera-crds" clusterrolebinding to "tiger-apiserver"
7b9e125
to
5724208
Compare
Changes:
Description
For PR author
make gen-files
make gen-versions
For PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bug
if this is a bugfix.kind/enhancement
if this is a a new feature.enterprise
if this PR applies to Calico Enterprise only.