diff --git a/plugin/pkg/admission/gc/gc_admission.go b/plugin/pkg/admission/gc/gc_admission.go index 1bfeaf0a170..89122da5a68 100644 --- a/plugin/pkg/admission/gc/gc_admission.go +++ b/plugin/pkg/admission/gc/gc_admission.go @@ -186,11 +186,9 @@ func (a *gcPermissionsEnforcement) ownerRefToDeleteAttributeRecords(ref metav1.O return ret, err } for _, mapping := range mappings { - ret = append(ret, authorizer.AttributesRecord{ - User: attributes.GetUserInfo(), - Verb: "update", - // ownerReference can only refer to an object in the same namespace, so attributes.GetNamespace() equals to the owner's namespace - Namespace: attributes.GetNamespace(), + ar := authorizer.AttributesRecord{ + User: attributes.GetUserInfo(), + Verb: "update", APIGroup: mapping.Resource.Group, APIVersion: mapping.Resource.Version, Resource: mapping.Resource.Resource, @@ -198,7 +196,12 @@ func (a *gcPermissionsEnforcement) ownerRefToDeleteAttributeRecords(ref metav1.O Name: ref.Name, ResourceRequest: true, Path: "", - }) + } + if mapping.Scope.Name() == meta.RESTScopeNameNamespace { + // if the owner is namespaced, it must be in the same namespace as the dependent is. + ar.Namespace = attributes.GetNamespace() + } + ret = append(ret, ar) } return ret, nil } diff --git a/plugin/pkg/admission/gc/gc_admission_test.go b/plugin/pkg/admission/gc/gc_admission_test.go index 753d4dc3c82..9cf17b4732f 100644 --- a/plugin/pkg/admission/gc/gc_admission_test.go +++ b/plugin/pkg/admission/gc/gc_admission_test.go @@ -68,6 +68,15 @@ func (fakeAuthorizer) Authorize(a authorizer.Attributes) (authorizer.Decision, s return authorizer.DecisionAllow, "", nil } + if username == "non-node-deleter" { + if a.GetVerb() == "delete" && a.GetResource() == "nodes" { + return authorizer.DecisionNoOpinion, "", nil + } + if a.GetVerb() == "update" && a.GetResource() == "nodes" && a.GetSubresource() == "finalizers" { + return authorizer.DecisionNoOpinion, "", nil + } + return authorizer.DecisionAllow, "", nil + } return authorizer.DecisionAllow, "", nil } @@ -347,6 +356,23 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) { Name: "ds1", BlockOwnerDeletion: getFalseVar(), } + blockNode := metav1.OwnerReference{ + APIVersion: "v1", + Kind: "Node", + Name: "node1", + BlockOwnerDeletion: getTrueVar(), + } + notBlockNode := metav1.OwnerReference{ + APIVersion: "v1", + Kind: "Node", + Name: "node", + BlockOwnerDeletion: getFalseVar(), + } + nilBlockNode := metav1.OwnerReference{ + APIVersion: "v1", + Kind: "Node", + Name: "node", + } expectNoError := func(err error) bool { return err == nil @@ -386,7 +412,7 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) { name: "super-user, create, some ownerReferences have blockOwnerDeletion=true", username: "super", resource: api.SchemeGroupVersion.WithResource("pods"), - newObj: podWithOwnerRefs(blockRC1, blockRC2), + newObj: podWithOwnerRefs(blockRC1, blockRC2, blockNode), checkError: expectNoError, }, { @@ -403,6 +429,13 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) { newObj: podWithOwnerRefs(notBlockRC1, nilBlockRC2), checkError: expectNoError, }, + { + name: "non-node-deleter, create, all ownerReferences have blockOwnerDeletion=false", + username: "non-node-deleter", + resource: api.SchemeGroupVersion.WithResource("pods"), + newObj: podWithOwnerRefs(notBlockNode), + checkError: expectNoError, + }, { name: "non-rc-deleter, create, some ownerReferences have blockOwnerDeletion=true", username: "non-rc-deleter", @@ -417,21 +450,28 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) { newObj: podWithOwnerRefs(blockDS1), checkError: expectNoError, }, + { + name: "non-node-deleter, create, some ownerReferences have blockOwnerDeletion=true", + username: "non-node-deleter", + resource: api.SchemeGroupVersion.WithResource("pods"), + newObj: podWithOwnerRefs(blockNode), + checkError: expectCantSetBlockOwnerDeletionError, + }, // cases are for update { name: "super-user, update, no ownerReferences change blockOwnerDeletion", username: "super", resource: api.SchemeGroupVersion.WithResource("pods"), - oldObj: podWithOwnerRefs(nilBlockRC1), - newObj: podWithOwnerRefs(notBlockRC1), + oldObj: podWithOwnerRefs(nilBlockRC1, nilBlockNode), + newObj: podWithOwnerRefs(notBlockRC1, notBlockNode), checkError: expectNoError, }, { name: "super-user, update, some ownerReferences change to blockOwnerDeletion=true", username: "super", resource: api.SchemeGroupVersion.WithResource("pods"), - oldObj: podWithOwnerRefs(notBlockRC1), - newObj: podWithOwnerRefs(blockRC1), + oldObj: podWithOwnerRefs(notBlockRC1, notBlockNode), + newObj: podWithOwnerRefs(blockRC1, blockNode), checkError: expectNoError, }, { @@ -439,7 +479,7 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) { username: "super", resource: api.SchemeGroupVersion.WithResource("pods"), oldObj: podWithOwnerRefs(), - newObj: podWithOwnerRefs(blockRC1), + newObj: podWithOwnerRefs(blockRC1, blockNode), checkError: expectNoError, }, { @@ -466,6 +506,14 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) { newObj: podWithOwnerRefs(blockRC1), checkError: expectCantSetBlockOwnerDeletionError, }, + { + name: "non-node-deleter, update, some ownerReferences change from blockOwnerDeletion=nil to true", + username: "non-node-deleter", + resource: api.SchemeGroupVersion.WithResource("pods"), + oldObj: podWithOwnerRefs(nilBlockNode), + newObj: podWithOwnerRefs(blockNode), + checkError: expectCantSetBlockOwnerDeletionError, + }, { name: "non-rc-deleter, update, some ownerReferences change from blockOwnerDeletion=true to false", username: "non-rc-deleter", @@ -474,6 +522,14 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) { newObj: podWithOwnerRefs(notBlockRC1), checkError: expectNoError, }, + { + name: "non-node-deleter, update, some ownerReferences change from blockOwnerDeletion=true to false", + username: "non-node-deleter", + resource: api.SchemeGroupVersion.WithResource("pods"), + oldObj: podWithOwnerRefs(blockNode), + newObj: podWithOwnerRefs(notBlockNode), + checkError: expectNoError, + }, { name: "non-rc-deleter, update, some ownerReferences change blockOwnerDeletion, but all such references are to daemonset", username: "non-rc-deleter", @@ -506,6 +562,14 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) { newObj: podWithOwnerRefs(blockDS1), checkError: expectNoError, }, + { + name: "non-node-deleter, update, add ownerReferences with blockOwnerDeletion=true", + username: "non-node-deleter", + resource: api.SchemeGroupVersion.WithResource("pods"), + oldObj: podWithOwnerRefs(), + newObj: podWithOwnerRefs(blockNode), + checkError: expectCantSetBlockOwnerDeletionError, + }, } gcAdmit, err := newGCPermissionsEnforcement() if err != nil {