diff --git a/pkg/registry/authentication/tokenreview/storage.go b/pkg/registry/authentication/tokenreview/storage.go index d14ee1a90f7..8ec84a29853 100644 --- a/pkg/registry/authentication/tokenreview/storage.go +++ b/pkg/registry/authentication/tokenreview/storage.go @@ -68,6 +68,12 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return nil, apierrors.NewBadRequest(fmt.Sprintf("token is required for TokenReview in authentication")) } + if createValidation != nil { + if err := createValidation(obj.DeepCopyObject()); err != nil { + return nil, err + } + } + if r.tokenAuthenticator == nil { return tokenReview, nil } diff --git a/pkg/registry/authorization/localsubjectaccessreview/rest.go b/pkg/registry/authorization/localsubjectaccessreview/rest.go index 2049adb91ce..b2dc7d2c161 100644 --- a/pkg/registry/authorization/localsubjectaccessreview/rest.go +++ b/pkg/registry/authorization/localsubjectaccessreview/rest.go @@ -63,6 +63,12 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return nil, kapierrors.NewBadRequest(fmt.Sprintf("spec.resourceAttributes.namespace must match namespace: %v", namespace)) } + if createValidation != nil { + if err := createValidation(obj.DeepCopyObject()); err != nil { + return nil, err + } + } + authorizationAttributes := authorizationutil.AuthorizationAttributesFrom(localSubjectAccessReview.Spec) decision, reason, evaluationErr := r.authorizer.Authorize(authorizationAttributes) diff --git a/pkg/registry/authorization/selfsubjectaccessreview/rest.go b/pkg/registry/authorization/selfsubjectaccessreview/rest.go index f8e17d706c8..1051b80321e 100644 --- a/pkg/registry/authorization/selfsubjectaccessreview/rest.go +++ b/pkg/registry/authorization/selfsubjectaccessreview/rest.go @@ -60,6 +60,12 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return nil, apierrors.NewBadRequest("no user present on request") } + if createValidation != nil { + if err := createValidation(obj.DeepCopyObject()); err != nil { + return nil, err + } + } + var authorizationAttributes authorizer.AttributesRecord if selfSAR.Spec.ResourceAttributes != nil { authorizationAttributes = authorizationutil.ResourceAttributesFrom(userToCheck, *selfSAR.Spec.ResourceAttributes) diff --git a/pkg/registry/authorization/selfsubjectrulesreview/rest.go b/pkg/registry/authorization/selfsubjectrulesreview/rest.go index a9414904b59..be9cb7c652d 100644 --- a/pkg/registry/authorization/selfsubjectrulesreview/rest.go +++ b/pkg/registry/authorization/selfsubjectrulesreview/rest.go @@ -65,6 +65,13 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation if namespace == "" { return nil, apierrors.NewBadRequest("no namespace on request") } + + if createValidation != nil { + if err := createValidation(obj.DeepCopyObject()); err != nil { + return nil, err + } + } + resourceInfo, nonResourceInfo, incomplete, err := r.ruleResolver.RulesFor(user, namespace) ret := &authorizationapi.SelfSubjectRulesReview{ diff --git a/pkg/registry/authorization/subjectaccessreview/rest.go b/pkg/registry/authorization/subjectaccessreview/rest.go index 91180f6af2a..9a69a2b0454 100644 --- a/pkg/registry/authorization/subjectaccessreview/rest.go +++ b/pkg/registry/authorization/subjectaccessreview/rest.go @@ -55,6 +55,12 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return nil, kapierrors.NewInvalid(authorizationapi.Kind(subjectAccessReview.Kind), "", errs) } + if createValidation != nil { + if err := createValidation(obj.DeepCopyObject()); err != nil { + return nil, err + } + } + authorizationAttributes := authorizationutil.AuthorizationAttributesFrom(subjectAccessReview.Spec) decision, reason, evaluationErr := r.authorizer.Authorize(authorizationAttributes) diff --git a/test/integration/apiserver/admissionwebhook/admission_test.go b/test/integration/apiserver/admissionwebhook/admission_test.go index de3d36b4b99..a1b1d07eaf7 100644 --- a/test/integration/apiserver/admissionwebhook/admission_test.go +++ b/test/integration/apiserver/admissionwebhook/admission_test.go @@ -119,24 +119,27 @@ var ( gvr("admissionregistration.k8s.io", "v1beta1", "mutatingwebhookconfigurations"): true, gvr("admissionregistration.k8s.io", "v1beta1", "validatingwebhookconfigurations"): true, } - // excludedResources lists resources / verb combinations that are not yet tested. this set should trend to zero. - excludedResources = map[schema.GroupVersionResource]sets.String{ - // TODO: verify non-persisted review objects work with webhook admission in place (and determine whether they should be sent to admission) - gvr("authentication.k8s.io", "v1", "tokenreviews"): sets.NewString("*"), - gvr("authentication.k8s.io", "v1beta1", "tokenreviews"): sets.NewString("*"), - gvr("authorization.k8s.io", "v1", "localsubjectaccessreviews"): sets.NewString("*"), - gvr("authorization.k8s.io", "v1", "subjectaccessreviews"): sets.NewString("*"), - gvr("authorization.k8s.io", "v1", "selfsubjectaccessreviews"): sets.NewString("*"), - gvr("authorization.k8s.io", "v1", "selfsubjectrulesreviews"): sets.NewString("*"), - gvr("authorization.k8s.io", "v1beta1", "localsubjectaccessreviews"): sets.NewString("*"), - gvr("authorization.k8s.io", "v1beta1", "subjectaccessreviews"): sets.NewString("*"), - gvr("authorization.k8s.io", "v1beta1", "selfsubjectaccessreviews"): sets.NewString("*"), - gvr("authorization.k8s.io", "v1beta1", "selfsubjectrulesreviews"): sets.NewString("*"), - } parentResources = map[schema.GroupVersionResource]schema.GroupVersionResource{ gvr("extensions", "v1beta1", "replicationcontrollers/scale"): gvr("", "v1", "replicationcontrollers"), } + + // stubDataOverrides holds either non persistent resources' definitions or resources where default stub needs to be overridden. + stubDataOverrides = map[schema.GroupVersionResource]string{ + // Non persistent Reviews resource + gvr("authentication.k8s.io", "v1", "tokenreviews"): `{"metadata": {"name": "tokenreview"}, "spec": {"token": "token", "audience": ["audience1","audience2"]}}`, + gvr("authentication.k8s.io", "v1beta1", "tokenreviews"): `{"metadata": {"name": "tokenreview"}, "spec": {"token": "token", "audience": ["audience1","audience2"]}}`, + gvr("authorization.k8s.io", "v1", "localsubjectaccessreviews"): `{"metadata": {"name": "", "namespace":"` + testNamespace + `"}, "spec": {"uid": "token", "user": "user1","groups": ["group1","group2"],"resourceAttributes": {"name":"name1","namespace":"` + testNamespace + `"}}}`, + gvr("authorization.k8s.io", "v1", "subjectaccessreviews"): `{"metadata": {"name": "", "namespace":""}, "spec": {"user":"user1","resourceAttributes": {"name":"name1", "namespace":"` + testNamespace + `"}}}`, + gvr("authorization.k8s.io", "v1", "selfsubjectaccessreviews"): `{"metadata": {"name": "", "namespace":""}, "spec": {"resourceAttributes": {"name":"name1", "namespace":""}}}`, + gvr("authorization.k8s.io", "v1", "selfsubjectrulesreviews"): `{"metadata": {"name": "", "namespace":"` + testNamespace + `"}, "spec": {"namespace":"` + testNamespace + `"}}`, + gvr("authorization.k8s.io", "v1beta1", "localsubjectaccessreviews"): `{"metadata": {"name": "", "namespace":"` + testNamespace + `"}, "spec": {"uid": "token", "user": "user1","groups": ["group1","group2"],"resourceAttributes": {"name":"name1","namespace":"` + testNamespace + `"}}}`, + gvr("authorization.k8s.io", "v1beta1", "subjectaccessreviews"): `{"metadata": {"name": "", "namespace":""}, "spec": {"user":"user1","resourceAttributes": {"name":"name1", "namespace":"` + testNamespace + `"}}}`, + gvr("authorization.k8s.io", "v1beta1", "selfsubjectaccessreviews"): `{"metadata": {"name": "", "namespace":""}, "spec": {"resourceAttributes": {"name":"name1", "namespace":""}}}`, + gvr("authorization.k8s.io", "v1beta1", "selfsubjectrulesreviews"): `{"metadata": {"name": "", "namespace":"` + testNamespace + `"}, "spec": {"namespace":"` + testNamespace + `"}}`, + + // Other Non persistent resources + } ) type holder struct { @@ -887,7 +890,6 @@ func testSubresourceProxy(c *testContext) { // verify the result c.admissionHolder.verify(c.t) } - } // @@ -979,13 +981,19 @@ func getTestFunc(gvr schema.GroupVersionResource, verb string) testFunc { } func getStubObj(gvr schema.GroupVersionResource, resource metav1.APIResource) (*unstructured.Unstructured, error) { - data, ok := etcd.GetEtcdStorageDataForNamespace(testNamespace)[gvr] - if !ok { + stub := "" + if data, ok := etcd.GetEtcdStorageDataForNamespace(testNamespace)[gvr]; ok { + stub = data.Stub + } + if data, ok := stubDataOverrides[gvr]; ok { + stub = data + } + if len(stub) == 0 { return nil, fmt.Errorf("no stub data for %#v", gvr) } stubObj := &unstructured.Unstructured{Object: map[string]interface{}{}} - if err := json.Unmarshal([]byte(data.Stub), &stubObj.Object); err != nil { + if err := json.Unmarshal([]byte(stub), &stubObj.Object); err != nil { return nil, fmt.Errorf("error unmarshaling stub for %#v: %v", gvr, err) } return stubObj, nil @@ -1021,14 +1029,14 @@ func shouldTestResource(gvr schema.GroupVersionResource, resource metav1.APIReso if !sets.NewString(resource.Verbs...).HasAny("create", "update", "patch", "connect", "delete", "deletecollection") { return false } - return !excludedResources[gvr].Has("*") + return true } func shouldTestResourceVerb(gvr schema.GroupVersionResource, resource metav1.APIResource, verb string) bool { if !sets.NewString(resource.Verbs...).Has(verb) { return false } - return !excludedResources[gvr].Has(verb) + return true } //