diff --git a/test/integration/ownerrefs_test.go b/test/integration/ownerrefs_test.go index d22f907186f7..0c38a70b003b 100644 --- a/test/integration/ownerrefs_test.go +++ b/test/integration/ownerrefs_test.go @@ -34,7 +34,7 @@ func TestOwnerRefRestriction(t *testing.T) { Name: "create-svc", }, Rules: []authorizationapi.PolicyRule{ - authorizationapi.NewRule("create").Groups(kapi.GroupName).Resources("services").RuleOrDie(), + authorizationapi.NewRule("create", "update").Groups(kapi.GroupName).Resources("services").RuleOrDie(), }, }) if err != nil { @@ -63,15 +63,9 @@ func TestOwnerRefRestriction(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - _, err = creatorClient.Core().Services("foo").Create(&kapi.Service{ + actual, err := creatorClient.Core().Services("foo").Create(&kapi.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "my-service", - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: "foo", - Kind: "bar", - Name: "baz", - UID: types.UID("baq"), - }}, }, Spec: kapi.ServiceSpec{ Ports: []kapi.ServicePort{ @@ -79,8 +73,18 @@ func TestOwnerRefRestriction(t *testing.T) { }, }, }) + if err != nil { + t.Fatal(err) + } + actual.OwnerReferences = []metav1.OwnerReference{{ + APIVersion: "foo", + Kind: "bar", + Name: "baz", + UID: types.UID("baq"), + }} + actual, err = creatorClient.Core().Services("foo").Update(actual) if err == nil { - t.Fatalf("missing err") + t.Fatalf("missing error") } if !kapierrors.IsForbidden(err) || !strings.Contains(err.Error(), "cannot set an ownerRef on a resource you can't delete") { t.Fatalf("expecting cannot set an ownerRef on a resource you can't delete, got %v", err) diff --git a/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission.go b/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission.go index ac8af441703e..ca719d5cafb3 100644 --- a/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission.go +++ b/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission.go @@ -95,21 +95,26 @@ func (a *gcPermissionsEnforcement) Validate(attributes admission.Attributes) (er return nil } - deleteAttributes := authorizer.AttributesRecord{ - User: attributes.GetUserInfo(), - Verb: "delete", - Namespace: attributes.GetNamespace(), - APIGroup: attributes.GetResource().Group, - APIVersion: attributes.GetResource().Version, - Resource: attributes.GetResource().Resource, - Subresource: attributes.GetSubresource(), - Name: attributes.GetName(), - ResourceRequest: true, - Path: "", - } - decision, reason, err := a.authorizer.Authorize(deleteAttributes) - if decision != authorizer.DecisionAllow { - return admission.NewForbidden(attributes, fmt.Errorf("cannot set an ownerRef on a resource you can't delete: %v, %v", reason, err)) + // if you are creating a thing, you should always be allowed to set an owner ref since you logically had the power + // to never create it. We still need to check block owner deletion below, because the power to delete does not + // imply the power to prevent deletion on other resources. + if attributes.GetOperation() != admission.Create { + deleteAttributes := authorizer.AttributesRecord{ + User: attributes.GetUserInfo(), + Verb: "delete", + Namespace: attributes.GetNamespace(), + APIGroup: attributes.GetResource().Group, + APIVersion: attributes.GetResource().Version, + Resource: attributes.GetResource().Resource, + Subresource: attributes.GetSubresource(), + Name: attributes.GetName(), + ResourceRequest: true, + Path: "", + } + decision, reason, err := a.authorizer.Authorize(deleteAttributes) + if decision != authorizer.DecisionAllow { + return admission.NewForbidden(attributes, fmt.Errorf("cannot set an ownerRef on a resource you can't delete: %v, %v", reason, err)) + } } // Further check if the user is setting ownerReference.blockOwnerDeletion to @@ -119,7 +124,7 @@ func (a *gcPermissionsEnforcement) Validate(attributes admission.Attributes) (er for _, ref := range newBlockingRefs { records, err := a.ownerRefToDeleteAttributeRecords(ref, attributes) if err != nil { - return admission.NewForbidden(attributes, fmt.Errorf("cannot set blockOwnerDeletion in this case because cannot find RESTMapping for APIVersion %s Kind %s: %v, %v", ref.APIVersion, ref.Kind, reason, err)) + return admission.NewForbidden(attributes, fmt.Errorf("cannot set blockOwnerDeletion in this case because cannot find RESTMapping for APIVersion %s Kind %s: %v", ref.APIVersion, ref.Kind, err)) } // Multiple records are returned if ref.Kind could map to multiple // resources. User needs to have delete permission on all the diff --git a/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission_test.go b/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission_test.go index db805c9b7046..41a9a0525209 100644 --- a/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission_test.go +++ b/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission_test.go @@ -139,7 +139,7 @@ func TestGCAdmission(t *testing.T) { username: "non-deleter", resource: api.SchemeGroupVersion.WithResource("pods"), newObj: &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}}, - checkError: expectCantSetOwnerRefError, + checkError: expectNoError, }, { name: "non-pod-deleter, create, no objectref change", @@ -153,7 +153,7 @@ func TestGCAdmission(t *testing.T) { username: "non-pod-deleter", resource: api.SchemeGroupVersion.WithResource("pods"), newObj: &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}}, - checkError: expectCantSetOwnerRefError, + checkError: expectNoError, }, { name: "non-pod-deleter, create, objectref change, but not a pod",