helm: Remove Unnecessary RBAC Permissions for Agent #19053
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nathanjsweet
added
area/daemon
nathanjsweet
force-pushed
the
pr/nathanjsweet/update-problematic-rbac-permissions-that-are-unnecessary
branch
from
0f49b97
to
e72d112
Compare
sayboras
approved these changes
Mar 7, 2022
|
/test |
aanm
approved these changes
Mar 7, 2022
In October 2020, we made changes[1] to the cilium-agent's ClusterRole to be more permissive. We did this, because Openshift enables[2] the OwnerReferencesPermissionEnforcement[3] admission controller. This admissions controller prevents changes to the metadata.ownerReferences of any object unless the entity (the cilium-agent in this case) has permission to delete the object as well. Furthermore, the controller allows protects metadata.ownerReferences[x].blockOwnerDeletion of a resource unless the entity (again, the cilium-agent) has "update" access to the finalizer of the object having its deletion blocked. The original PR mistakenly assumed we set ownerReferences on pods and expanded cilium-agent's permissions beyond what was necessary. Cilium-agent only sets ownerReferences on a CiliumEndpoint and the blockOwnerDeletion field propagates up to the "owning" pod of the endpoint. Cilium-agent only needs to be able to delete CiliumEndpoints (which it has always been able to) and "update" pod/finalizers (to set the blockOwnerDeletion field on CiliumEndpoints). All other changes contained in #13369 were unnecessary. 1 #13369 2 https://docs.openshift.com/container-platform/4.6/architecture/admission-plug-ins.html 3 https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
nathanjsweet
force-pushed
the
pr/nathanjsweet/update-problematic-rbac-permissions-that-are-unnecessary
branch
from
e72d112
to
ab30b7d
Compare
|
The tests were previously green, merging. |
aanm
deleted the
pr/nathanjsweet/update-problematic-rbac-permissions-that-are-unnecessary
branch
nathanjsweet
added a commit
that referenced
this pull request
Mar 7, 2022
The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
ldelossa
pushed a commit
that referenced
this pull request
Mar 17, 2022
The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
qmonnet
added
backport-done/1.11
qmonnet
pushed a commit
that referenced
this pull request
Mar 29, 2022
[ upstream commit 75f597b ] The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet
pushed a commit
that referenced
this pull request
Mar 29, 2022
[ upstream commit 75f597b ] The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet
pushed a commit
that referenced
this pull request
Mar 30, 2022
[ upstream commit 75f597b ] The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet
pushed a commit
that referenced
this pull request
Mar 30, 2022
[ upstream commit 75f597b ] The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet
pushed a commit
that referenced
this pull request
Mar 30, 2022
[ upstream commit 75f597b ] The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet
pushed a commit
that referenced
this pull request
Apr 4, 2022
[ upstream commit 75f597b ] The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
pchaigno
pushed a commit
that referenced
this pull request
Apr 4, 2022
[ upstream commit 75f597b ] The Clustermesh-APIServer creates a CiliumEndPoint and sets a node as its ownerReference, also setting blockOwnerDeletion to "true". If the OwnerReferencesPermissionEnforcement admission controller is enabled (such as in environments like Openshift) then the Clustermesh-APIServer will fail to create the CiliumEndPoint as it has insufficient privileges to set blockOwnerDeletion of a node. It needs to be able to "update" "nodes/finalizers" in order to do this. See #19053 for more details and references. Signed-off-by: Nate Sweet <nathanjsweet@pm.me> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
This was referenced Apr 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In October 2020, we made changes to the cilium-agent's
ClusterRole to be more permissive. We did this, because
Openshift enables the OwnerReferencesPermissionEnforcement
admission controller. This admissions controller prevents
changes to the metadata.ownerReferences of any object
unless the entity (the cilium-agent in this case) has
permission to delete the object as well. Furthermore,
the controller allows protects metadata.ownerReferences[x].blockOwnerDeletion
of a resource unless the entity (again, the cilium-agent) has
"update" access to the finalizer of the object having its
deletion blocked. The original PR mistakenly assumed we
set ownerReferences on pods and expanded cilium-agent's
permissions beyond what was necessary. Cilium-agent
only sets ownerReferences on a CiliumEndpoint and the
blockOwnerDeletion field propagates up to the "owning"
pod of the endpoint. Cilium-agent only needs to be able
to delete CiliumEndpoints (which it has always been able to)
and "update" pod/finalizers (to set the blockOwnerDeletion field
on CiliumEndpoints). All other changes contained in #13369
were unnecessary.
Signed-off-by: Nate Sweet nathanjsweet@pm.me