diff --git a/charts/catalog/templates/crds.yaml b/charts/catalog/templates/crds.yaml index 1a80a2d87a9..c565ce248ea 100644 --- a/charts/catalog/templates/crds.yaml +++ b/charts/catalog/templates/crds.yaml @@ -55,14 +55,14 @@ spec: - "secretRef" properties: secretRef: - type: string + type: object bearer: type: object required: - "secretRef" properties: secretRef: - type: string + type: object --- @@ -123,14 +123,14 @@ spec: - "secretRef" properties: secretRef: - type: string + type: object bearer: type: object required: - "secretRef" properties: secretRef: - type: string + type: object --- diff --git a/charts/catalog/templates/rbac.yaml b/charts/catalog/templates/rbac.yaml index f0aa749b1ee..5bce33526fc 100644 --- a/charts/catalog/templates/rbac.yaml +++ b/charts/catalog/templates/rbac.yaml @@ -172,6 +172,9 @@ items: - apiGroups: ["servicecatalog.k8s.io"] resources: ["serviceinstances","servicebindings"] verbs: ["get","list","watch"] + - apiGroups: ["authorization.k8s.io"] + resources: ["subjectaccessreviews"] + verbs: ["get","list","create"] {{- if not .Values.namespacedServiceBrokerDisabled }} - apiGroups: ["servicecatalog.k8s.io"] resources: ["serviceclasses"] diff --git a/charts/catalog/templates/webhook-register.yaml b/charts/catalog/templates/webhook-register.yaml index b8fe6044342..6b6480bf8cd 100644 --- a/charts/catalog/templates/webhook-register.yaml +++ b/charts/catalog/templates/webhook-register.yaml @@ -133,6 +133,45 @@ webhooks: apiGroups: ["servicecatalog.k8s.io"] apiVersions: ["v1beta1"] resources: ["serviceinstances"] +- name: validating.clusterservicebrokers.servicecatalog.k8s.io + clientConfig: + caBundle: {{ b64enc $ca.Cert }} + service: + name: {{ template "fullname" . }}-webhook + namespace: "{{ .Release.Namespace }}" + path: "/validating-clusterservicebrokers" + failurePolicy: Fail + rules: + - operations: [ "CREATE", "UPDATE" ] + apiGroups: ["servicecatalog.k8s.io"] + apiVersions: ["v1beta1"] + resources: ["clusterservicebrokers"] +- name: validating.servicebindings.servicecatalog.k8s.io + clientConfig: + caBundle: {{ b64enc $ca.Cert }} + service: + name: {{ template "fullname" . }}-webhook + namespace: "{{ .Release.Namespace }}" + path: "/validating-servicebindings" + failurePolicy: Fail + rules: + - operations: [ "CREATE", "UPDATE" ] + apiGroups: ["servicecatalog.k8s.io"] + apiVersions: ["v1beta1"] + resources: ["servicebindings"] +- name: validating.servicebrokers.servicecatalog.k8s.io + clientConfig: + caBundle: {{ b64enc $ca.Cert }} + service: + name: {{ template "fullname" . }}-webhook + namespace: "{{ .Release.Namespace }}" + path: "/validating-servicebrokers" + failurePolicy: Fail + rules: + - operations: [ "CREATE", "UPDATE" ] + apiGroups: ["servicecatalog.k8s.io"] + apiVersions: ["v1beta1"] + resources: ["servicebrokers"] --- apiVersion: v1 kind: Secret diff --git a/charts/catalog/values.yaml b/charts/catalog/values.yaml index eca6161a673..bfb0c9a6ea3 100644 --- a/charts/catalog/values.yaml +++ b/charts/catalog/values.yaml @@ -1,6 +1,6 @@ # Default values for Service Catalog # service-catalog image to use -image: eu.gcr.io/kyma-project/develop/service-catalog/service-catalog-amd64:crd-0.0.3 +image: eu.gcr.io/kyma-project/develop/service-catalog/service-catalog-amd64:crd-0.0.5 # imagePullPolicy for the service-catalog; valid values are "IfNotPresent", # "Never", and "Always" imagePullPolicy: Always diff --git a/cmd/webhook/server/webhook.go b/cmd/webhook/server/webhook.go index 924db215719..c38839ea6a3 100644 --- a/cmd/webhook/server/webhook.go +++ b/cmd/webhook/server/webhook.go @@ -31,6 +31,9 @@ import ( simutation "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/serviceinstance/mutation" spmutation "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/serviceplan/mutation" + csbrvalidation "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/clusterservicebroker/validation" + sbvalidation "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/servicebinding/validation" + sbrvalidation "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/servicebroker/validation" sivalidation "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/serviceinstance/validation" "github.com/pkg/errors" @@ -81,7 +84,10 @@ func run(opts *WebhookServerOptions, stopCh <-chan struct{}) error { "/mutating-serviceplans": &spmutation.CreateUpdateHandler{}, "/mutating-serviceinstances": simutation.New(), - "/validating-serviceinstances": sivalidation.NewAdmissionHandler(), + "/validating-clusterservicebrokers": csbrvalidation.NewAdmissionHandler(), + "/validating-servicebindings": sbvalidation.NewAdmissionHandler(), + "/validating-servicebrokers": sbrvalidation.NewAdmissionHandler(), + "/validating-serviceinstances": sivalidation.NewAdmissionHandler(), } for path, handler := range webhooks { diff --git a/pkg/apis/servicecatalog/v1beta1/types.go b/pkg/apis/servicecatalog/v1beta1/types.go index 0aec212e78a..ef86a5589c1 100644 --- a/pkg/apis/servicecatalog/v1beta1/types.go +++ b/pkg/apis/servicecatalog/v1beta1/types.go @@ -1496,5 +1496,17 @@ type RemoveKeyTransform struct { func init() { // SchemaBuilder is used to map go structs to GroupVersionKinds. // Solution suggested by the Kubebuilder book: https://book.kubebuilder.io/basics/simple_resource.html - "Scaffolded Boilerplate" section - SchemeBuilderRuntime.Register(&ServiceBinding{}, &ServiceInstance{}, &ClusterServiceClassList{}, &ServiceClassList{}, &ServiceClass{}, &ClusterServiceClass{}, &ClusterServicePlanList{}, &ServicePlanList{}, &ServicePlan{}, &ClusterServicePlan{}) + SchemeBuilderRuntime.Register( + &ServiceBinding{}, + &ServiceInstance{}, + &ClusterServiceClass{}, + &ClusterServiceClassList{}, + &ServiceBroker{}, + &ClusterServiceBroker{}, + &ServiceClass{}, + &ServiceClassList{}, + &ServicePlan{}, + &ServicePlanList{}, + &ClusterServicePlan{}, + &ClusterServicePlanList{}) } diff --git a/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler.go b/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler.go new file mode 100644 index 00000000000..2497bc4db71 --- /dev/null +++ b/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler.go @@ -0,0 +1,144 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "context" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil" + admissionTypes "k8s.io/api/admission/v1beta1" + "net/http" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// Validator is used to implement new validation logic +type Validator interface { + Validate(context.Context, admission.Request, *sc.ClusterServiceBroker, *webhookutil.TracedLogger) *webhookutil.WebhookError +} + +// AdmissionHandler handles ClusterServiceBroker validation +type AdmissionHandler struct { + decoder *admission.Decoder + + CreateValidators []Validator + UpdateValidators []Validator +} + +var _ admission.Handler = &AdmissionHandler{} +var _ admission.DecoderInjector = &AdmissionHandler{} +var _ inject.Client = &AdmissionHandler{} + +// NewAdmissionHandler creates new AdmissionHandler and initializes validators list +func NewAdmissionHandler() *AdmissionHandler { + return &AdmissionHandler{ + CreateValidators: []Validator{&AccessToBroker{}}, + UpdateValidators: []Validator{&AccessToBroker{}}, + } +} + +// Handle handles admission requests. +func (h *AdmissionHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + traced := webhookutil.NewTracedLogger(req.UID) + traced.Infof("Start handling validation operation: %s for %s", req.Operation, req.Kind.Kind) + + csb := &sc.ClusterServiceBroker{} + if err := webhookutil.MatchKinds(csb, req.Kind); err != nil { + traced.Errorf("Error matching kinds: %v", err) + return admission.Errored(http.StatusBadRequest, err) + } + + if err := h.decoder.Decode(req, csb); err != nil { + traced.Errorf("Could not decode request object: %v", err) + return admission.Errored(http.StatusBadRequest, err) + } + + traced.Infof("start validation process for %s: %s/%s", csb.Kind, csb.Namespace, csb.Name) + + var err *webhookutil.WebhookError + + switch req.Operation { + case admissionTypes.Create: + for _, v := range h.CreateValidators { + err = v.Validate(ctx, req, csb, traced) + if err != nil { + break + } + } + case admissionTypes.Update: + for _, v := range h.UpdateValidators { + err = v.Validate(ctx, req, csb, traced) + if err != nil { + break + } + } + default: + traced.Infof("ClusterServiceBroker validation wehbook does not support action %q", req.Operation) + return admission.Allowed("action not taken") + } + + if err != nil { + switch err.Code() { + case http.StatusForbidden: + return admission.Denied(err.Error()) + default: + return admission.Errored(err.Code(), err) + } + } + + traced.Infof("Completed successfully validation operation: %s for %s: %q", req.Operation, req.Kind.Kind, req.Name) + return admission.Allowed("ClusterServiceBroker AdmissionHandler successful") +} + +// InjectDecoder injects the decoder into the handlers +func (h *AdmissionHandler) InjectDecoder(d *admission.Decoder) error { + h.decoder = d + + for _, v := range h.CreateValidators { + _, err := admission.InjectDecoderInto(d, v) + if err != nil { + return err + } + } + for _, v := range h.UpdateValidators { + _, err := admission.InjectDecoderInto(d, v) + if err != nil { + return err + } + } + + return nil +} + +// InjectClient injects the client into the handlers +func (h *AdmissionHandler) InjectClient(c client.Client) error { + for _, v := range h.CreateValidators { + _, err := inject.ClientInto(c, v) + if err != nil { + return err + } + } + for _, v := range h.UpdateValidators { + _, err := inject.ClientInto(c, v) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_access.go b/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_access.go new file mode 100644 index 00000000000..dc7f20524ff --- /dev/null +++ b/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_access.go @@ -0,0 +1,123 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "context" + "fmt" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil" + authenticationapi "k8s.io/api/authentication/v1" + authorizationapi "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + "net/http" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// AccessToBroker handles ClusterServiceBroker validation +type AccessToBroker struct { + decoder *admission.Decoder + client client.Client +} + +var _ admission.DecoderInjector = &AccessToBroker{} +var _ inject.Client = &AccessToBroker{} + +// InjectDecoder injects the decoder +func (h *AccessToBroker) InjectDecoder(d *admission.Decoder) error { + h.decoder = d + return nil +} + +// InjectClient injects the client +func (h *AccessToBroker) InjectClient(c client.Client) error { + h.client = c + return nil +} + +// Validate checks if client has access to cluster service broker if broker requires authentication +// This feature was copied from Service Catalog admission plugin https://github.com/kubernetes-incubator/service-catalog/blob/v0.1.41/plugin/pkg/admission/broker/authsarcheck/admission.go +// If you want to track previous changes please check there. +func (h *AccessToBroker) Validate(ctx context.Context, req admission.Request, csb *sc.ClusterServiceBroker, traced *webhookutil.TracedLogger) *webhookutil.WebhookError { + if csb.Spec.AuthInfo == nil { + traced.Infof("%s %q has no AuthInfo. Operation completed", csb.Kind, csb.Name) + return nil + } + + var secretRef *sc.ObjectReference + if csb.Spec.AuthInfo.Basic != nil { + secretRef = csb.Spec.AuthInfo.Basic.SecretRef + } else if csb.Spec.AuthInfo.Bearer != nil { + secretRef = csb.Spec.AuthInfo.Bearer.SecretRef + } + + if secretRef == nil { + traced.Infof("%s %q has no SecretRef neither in Basic nor Bearer auth. Operation completed", csb.Kind, csb.Name) + return nil + } + + user := req.UserInfo + sar := &authorizationapi.SubjectAccessReview{ + Spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + Namespace: secretRef.Namespace, + Verb: "get", + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + Resource: corev1.ResourceSecrets.String(), + Name: secretRef.Name, + }, + User: user.Username, + Groups: user.Groups, + Extra: convertToSARExtra(user.Extra), + UID: user.UID, + }, + } + + err := h.client.Create(ctx, sar) + if err != nil { + traced.Errorf("Could not create SubjectAccessReview for %s %q: %v", csb.Kind, csb.Name, err) + return webhookutil.NewWebhookError(err.Error(), http.StatusForbidden) + } + + if !sar.Status.Allowed { + msg := fmt.Sprintf( + "broker forbidden access to auth secret (%s): Reason: %s, EvaluationError: %s", + secretRef.Name, + sar.Status.Reason, + sar.Status.EvaluationError) + traced.Info(msg) + return webhookutil.NewWebhookError(msg, http.StatusForbidden) + } + + return nil +} + +func convertToSARExtra(extra map[string]authenticationapi.ExtraValue) map[string]authorizationapi.ExtraValue { + if extra == nil { + return nil + } + + ret := map[string]authorizationapi.ExtraValue{} + for k, v := range extra { + ret[k] = authorizationapi.ExtraValue(v) + } + + return ret +} diff --git a/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_access_test.go b/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_access_test.go new file mode 100644 index 00000000000..dd97ccc9b78 --- /dev/null +++ b/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_access_test.go @@ -0,0 +1,290 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation_test + +import ( + "context" + "errors" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/clusterservicebroker/validation" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + "k8s.io/api/authorization/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "testing" +) + +const ( + AllowedSecretName = "csb-secret-name" + DeniedSecretName = "denied-csb-secret-name" +) + +// Reactors are not implemented in 'sigs.k8s.io/controller-runtime/pkg/client/fake' package +// https://github.com/kubernetes-sigs/controller-runtime/issues/72 +// instead it is used custom client with override Create method +type fakedClient struct { + client.Client +} + +// Create overrides real client Create method for the test +func (m *fakedClient) Create(ctx context.Context, obj runtime.Object, opts ...client.CreateOptionFunc) error { + if _, ok := obj.(*v1.SubjectAccessReview); !ok { + return errors.New("Input object is not SubjectAccessReview type") + } + + if obj.(*v1.SubjectAccessReview).Spec.ResourceAttributes.Name == AllowedSecretName { + obj.(*v1.SubjectAccessReview).Status.Allowed = true + } + + return nil +} + +func TestAdmissionHandlerAccessToBrokerAllowed(t *testing.T) { + // given + err := sc.AddToScheme(scheme.Scheme) + require.NoError(t, err) + + request := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + UID: "3333-cccc", + Name: "test-broker", + Namespace: "test-handler", + Kind: metav1.GroupVersionKind{ + Kind: "ClusterServiceBroker", + Version: "v1beta1", + Group: "servicecatalog.k8s.io", + }, + Object: runtime.RawExtension{}, + }, + } + + decoder, err := admission.NewDecoder(scheme.Scheme) + require.NoError(t, err) + + tests := map[string]struct { + operation admissionv1beta1.Operation + object []byte + }{ + "Request for Create ClusterServiceBroker without AuthInfo should be allowed": { + admissionv1beta1.Create, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ClusterServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local" + } + }`), + }, + "Request for Update ClusterServiceBroker without AuthInfo should be allowed": { + admissionv1beta1.Update, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ClusterServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local" + } + }`), + }, + "Request for Create ClusterServiceBroker with AuthInfo should be allowed": { + admissionv1beta1.Create, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ClusterServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local", + "authInfo": { + "basic": { + "secretRef": { + "namespace": "test-handler", + "name": "` + AllowedSecretName + `" + } + } + } + } + }`), + }, + "Request for Update ClusterServiceBroker with AuthInfo should be allowed": { + admissionv1beta1.Update, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ClusterServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local", + "authInfo": { + "bearer": { + "secretRef": { + "namespace": "test-handler", + "name": "` + AllowedSecretName + `" + } + } + } + } + }`), + }, + } + + for desc, test := range tests { + t.Run(desc, func(t *testing.T) { + // given + handler := validation.AdmissionHandler{} + handler.CreateValidators = []validation.Validator{&validation.AccessToBroker{}} + handler.UpdateValidators = []validation.Validator{&validation.AccessToBroker{}} + + err := handler.InjectDecoder(decoder) + require.NoError(t, err) + err = handler.InjectClient(&fakedClient{}) + require.NoError(t, err) + + request.AdmissionRequest.Operation = test.operation + request.AdmissionRequest.Object.Raw = test.object + + // when + response := handler.Handle(context.Background(), request) + + // then + assert.True(t, response.AdmissionResponse.Allowed) + }) + } +} + +func TestAdmissionHandlerAccessToBrokerDenied(t *testing.T) { + // given + err := sc.AddToScheme(scheme.Scheme) + require.NoError(t, err) + + request := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + UID: "4444-dddd", + Name: "test-broker", + Namespace: "test-handler", + Kind: metav1.GroupVersionKind{ + Kind: "ClusterServiceBroker", + Version: "v1beta1", + Group: "servicecatalog.k8s.io", + }, + Object: runtime.RawExtension{}, + }, + } + + sch := scheme.Scheme + + decoder, err := admission.NewDecoder(sch) + require.NoError(t, err) + + tests := map[string]struct { + operation admissionv1beta1.Operation + object []byte + }{ + "Request for Create ClusterServiceBroker should be denied": { + admissionv1beta1.Create, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ClusterServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local", + "authInfo": { + "bearer": { + "secretRef": { + "namespace": "test-handler", + "name": "` + DeniedSecretName + `" + } + } + } + } + }`), + }, + "Request for Update ClusterServiceBroker should be denied": { + admissionv1beta1.Update, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ClusterServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local", + "authInfo": { + "basic": { + "secretRef": { + "namespace": "test-handler", + "name": "` + DeniedSecretName + `" + } + } + } + } + }`), + }, + } + + for desc, test := range tests { + t.Run(desc, func(t *testing.T) { + // given + handler := validation.AdmissionHandler{} + handler.CreateValidators = []validation.Validator{&validation.AccessToBroker{}} + handler.UpdateValidators = []validation.Validator{&validation.AccessToBroker{}} + + fakeClient := fake.NewFakeClientWithScheme(sch, &sc.ClusterServiceBroker{}) + + err := handler.InjectDecoder(decoder) + require.NoError(t, err) + err = handler.InjectClient(fakeClient) + require.NoError(t, err) + + request.AdmissionRequest.Operation = test.operation + request.AdmissionRequest.Object.Raw = test.object + + // when + response := handler.Handle(context.Background(), request) + + // then + assert.False(t, response.AdmissionResponse.Allowed) + }) + } +} diff --git a/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_test.go b/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_test.go new file mode 100644 index 00000000000..cae720f23f0 --- /dev/null +++ b/pkg/webhook/servicecatalog/clusterservicebroker/validation/handler_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation_test + +import ( + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/clusterservicebroker/validation" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil/tester" +) + +func TestAdmissionHandlerHandleDecoderErrors(t *testing.T) { + tester.DiscardLoggedMsg() + + for _, fn := range []func(t *testing.T, handler tester.TestDecoderHandler, kind string){ + tester.AssertHandlerReturnErrorIfReqObjIsMalformed, + tester.AssertHandlerReturnErrorIfGVKMismatch, + } { + handler := validation.AdmissionHandler{} + fn(t, &handler, "ClusterServiceBroker") + } +} diff --git a/pkg/webhook/servicecatalog/servicebinding/validation/handler.go b/pkg/webhook/servicecatalog/servicebinding/validation/handler.go new file mode 100644 index 00000000000..55b32014632 --- /dev/null +++ b/pkg/webhook/servicecatalog/servicebinding/validation/handler.go @@ -0,0 +1,143 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "context" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil" + admissionTypes "k8s.io/api/admission/v1beta1" + "net/http" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// Validator is used to implement new validation logic +type Validator interface { + Validate(context.Context, admission.Request, *sc.ServiceBinding, *webhookutil.TracedLogger) *webhookutil.WebhookError +} + +// AdmissionHandler handles ServiceBinding validation +type AdmissionHandler struct { + decoder *admission.Decoder + + CreateValidators []Validator + UpdateValidators []Validator +} + +var _ admission.Handler = &AdmissionHandler{} +var _ admission.DecoderInjector = &AdmissionHandler{} +var _ inject.Client = &AdmissionHandler{} + +// NewAdmissionHandler creates new AdmissionHandler and initializes validators list +func NewAdmissionHandler() *AdmissionHandler { + return &AdmissionHandler{ + CreateValidators: []Validator{&ReferenceDeletion{}}, + } +} + +// Handle handles admission requests. +func (h *AdmissionHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + traced := webhookutil.NewTracedLogger(req.UID) + traced.Infof("Start handling validation operation: %s for %s", req.Operation, req.Kind.Kind) + + sb := &sc.ServiceBinding{} + if err := webhookutil.MatchKinds(sb, req.Kind); err != nil { + traced.Errorf("Error matching kinds: %v", err) + return admission.Errored(http.StatusBadRequest, err) + } + + if err := h.decoder.Decode(req, sb); err != nil { + traced.Errorf("Could not decode request object: %v", err) + return admission.Errored(http.StatusBadRequest, err) + } + + traced.Infof("start validation process for %s: %s/%s", sb.Kind, sb.Namespace, sb.Name) + + var err *webhookutil.WebhookError + + switch req.Operation { + case admissionTypes.Create: + for _, v := range h.CreateValidators { + err = v.Validate(ctx, req, sb, traced) + if err != nil { + break + } + } + case admissionTypes.Update: + for _, v := range h.UpdateValidators { + err = v.Validate(ctx, req, sb, traced) + if err != nil { + break + } + } + default: + traced.Infof("ServiceBinding validation wehbook does not support action %q", req.Operation) + return admission.Allowed("action not taken") + } + + if err != nil { + switch err.Code() { + case http.StatusForbidden: + return admission.Denied(err.Error()) + default: + return admission.Errored(err.Code(), err) + } + } + + traced.Infof("Completed successfully validation operation: %s for %s: %q", req.Operation, req.Kind.Kind, req.Name) + return admission.Allowed("ServiceBinding AdmissionHandler successful") +} + +// InjectDecoder injects the decoder into the handlers +func (h *AdmissionHandler) InjectDecoder(d *admission.Decoder) error { + h.decoder = d + + for _, v := range h.CreateValidators { + _, err := admission.InjectDecoderInto(d, v) + if err != nil { + return err + } + } + for _, v := range h.UpdateValidators { + _, err := admission.InjectDecoderInto(d, v) + if err != nil { + return err + } + } + + return nil +} + +// InjectClient injects the client into the handlers +func (h *AdmissionHandler) InjectClient(c client.Client) error { + for _, v := range h.CreateValidators { + _, err := inject.ClientInto(c, v) + if err != nil { + return err + } + } + for _, v := range h.UpdateValidators { + _, err := inject.ClientInto(c, v) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/webhook/servicecatalog/servicebinding/validation/handler_reference.go b/pkg/webhook/servicecatalog/servicebinding/validation/handler_reference.go new file mode 100644 index 00000000000..ad1353cdb4a --- /dev/null +++ b/pkg/webhook/servicecatalog/servicebinding/validation/handler_reference.go @@ -0,0 +1,78 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "context" + "fmt" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil" + "k8s.io/apimachinery/pkg/types" + "net/http" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// ReferenceDeletion handles ServiceBinding validation +type ReferenceDeletion struct { + decoder *admission.Decoder + client client.Client +} + +var _ admission.DecoderInjector = &ReferenceDeletion{} +var _ inject.Client = &ReferenceDeletion{} + +// InjectDecoder injects the decoder +func (h *ReferenceDeletion) InjectDecoder(d *admission.Decoder) error { + h.decoder = d + return nil +} + +// InjectClient injects the client +func (h *ReferenceDeletion) InjectClient(c client.Client) error { + h.client = c + return nil +} + +// Validate checks if instance reference for ServiceBinding is not marked for deletion +// fail ServiceBinding operation if the ServiceInstance is marked for deletion +// This feature was copied from Service Catalog admission plugin https://github.com/kubernetes-incubator/service-catalog/blob/v0.1.41/plugin/pkg/admission/servicebindings/lifecycle/admission.go +// If you want to track previous changes please check there. +func (h *ReferenceDeletion) Validate(ctx context.Context, req admission.Request, sb *sc.ServiceBinding, traced *webhookutil.TracedLogger) *webhookutil.WebhookError { + instanceRef := sb.Spec.InstanceRef + instance := &sc.ServiceInstance{} + + err := h.client.Get(ctx, types.NamespacedName{Namespace: sb.Namespace, Name: instanceRef.Name}, instance) + if err != nil { + traced.Infof("Could not get ServiceInstance by name %q: %v", instanceRef.Name, err) + return nil + } + + if instance.DeletionTimestamp != nil { + warning := fmt.Sprintf( + "ServiceBinding %s/%s references a ServiceInstance that is being deleted: %s/%s", + sb.Namespace, + sb.Name, + sb.Namespace, + instanceRef.Name) + traced.Info(warning) + return webhookutil.NewWebhookError(warning, http.StatusForbidden) + } + + return nil +} diff --git a/pkg/webhook/servicecatalog/servicebinding/validation/handler_reference_test.go b/pkg/webhook/servicecatalog/servicebinding/validation/handler_reference_test.go new file mode 100644 index 00000000000..ed3520655ab --- /dev/null +++ b/pkg/webhook/servicecatalog/servicebinding/validation/handler_reference_test.go @@ -0,0 +1,195 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation_test + +import ( + "context" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/servicebinding/validation" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "testing" + "time" +) + +const ( + UpToDateInstance = "up-to-date-instance" + OutOfDateInstance = "out-of-date-instance" +) + +func TestAdmissionHandlerServiceInstanceReferenceUpToDate(t *testing.T) { + // given + namespace := "test-handler" + err := sc.AddToScheme(scheme.Scheme) + require.NoError(t, err) + + request := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + UID: "1111-aaaa", + Name: "test-binding", + Namespace: namespace, + Kind: metav1.GroupVersionKind{ + Kind: "ServiceBinding", + Version: "v1beta1", + Group: "servicecatalog.k8s.io", + }, + Object: runtime.RawExtension{Raw: []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ServiceBinding", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-binding" + }, + "spec": { + "instanceRef": { + "name": "` + UpToDateInstance + `" + }, + "externalID": "123-abc", + "secretName": "test-binding" + } + }`)}, + }, + } + + sch, err := sc.SchemeBuilderRuntime.Build() + require.NoError(t, err) + + decoder, err := admission.NewDecoder(sch) + require.NoError(t, err) + + tests := map[string]struct { + operation admissionv1beta1.Operation + }{ + "Request for Create ServiceBinding should be allowed": { + admissionv1beta1.Create, + }, + } + + for desc, test := range tests { + t.Run(desc, func(t *testing.T) { + // given + handler := validation.AdmissionHandler{} + handler.CreateValidators = []validation.Validator{&validation.ReferenceDeletion{}} + + fakeClient := fake.NewFakeClientWithScheme(sch, &sc.ServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: UpToDateInstance, + Namespace: namespace, + }, + }) + + err := handler.InjectDecoder(decoder) + require.NoError(t, err) + err = handler.InjectClient(fakeClient) + require.NoError(t, err) + + request.AdmissionRequest.Operation = test.operation + + // when + response := handler.Handle(context.Background(), request) + + // then + assert.True(t, response.AdmissionResponse.Allowed) + }) + } +} + +func TestAdmissionHandlerServiceInstanceReferenceOutOfDate(t *testing.T) { + // given + namespace := "test-handler" + err := sc.AddToScheme(scheme.Scheme) + require.NoError(t, err) + + request := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + UID: "2222-bbbb", + Name: "test-binding", + Namespace: namespace, + Kind: metav1.GroupVersionKind{ + Kind: "ServiceBinding", + Version: "v1beta1", + Group: "servicecatalog.k8s.io", + }, + Object: runtime.RawExtension{Raw: []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ServiceBinding", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-binding" + }, + "spec": { + "instanceRef": { + "name": "` + OutOfDateInstance + `" + }, + "externalID": "123-abc", + "secretName": "test-binding" + } + }`)}, + }, + } + + sch, err := sc.SchemeBuilderRuntime.Build() + require.NoError(t, err) + + decoder, err := admission.NewDecoder(sch) + require.NoError(t, err) + + tests := map[string]struct { + operation admissionv1beta1.Operation + }{ + "Request for Create ServiceBinding should be denied": { + admissionv1beta1.Create, + }, + } + + for desc, test := range tests { + t.Run(desc, func(t *testing.T) { + // given + handler := validation.AdmissionHandler{} + handler.CreateValidators = []validation.Validator{&validation.ReferenceDeletion{}} + + fakeClient := fake.NewFakeClientWithScheme(sch, &sc.ServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: OutOfDateInstance, + Namespace: namespace, + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + }, + }) + + err := handler.InjectDecoder(decoder) + require.NoError(t, err) + err = handler.InjectClient(fakeClient) + require.NoError(t, err) + + request.AdmissionRequest.Operation = test.operation + + // when + response := handler.Handle(context.Background(), request) + + // then + assert.False(t, response.AdmissionResponse.Allowed) + }) + } +} diff --git a/pkg/webhook/servicecatalog/servicebinding/validation/handler_test.go b/pkg/webhook/servicecatalog/servicebinding/validation/handler_test.go new file mode 100644 index 00000000000..12ac08171f4 --- /dev/null +++ b/pkg/webhook/servicecatalog/servicebinding/validation/handler_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation_test + +import ( + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/servicebinding/validation" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil/tester" +) + +func TestAdmissionHandlerHandleDecoderErrors(t *testing.T) { + tester.DiscardLoggedMsg() + + for _, fn := range []func(t *testing.T, handler tester.TestDecoderHandler, kind string){ + tester.AssertHandlerReturnErrorIfReqObjIsMalformed, + tester.AssertHandlerReturnErrorIfGVKMismatch, + } { + handler := validation.AdmissionHandler{} + fn(t, &handler, "ServiceBinding") + } +} diff --git a/pkg/webhook/servicecatalog/servicebroker/validation/handler.go b/pkg/webhook/servicecatalog/servicebroker/validation/handler.go new file mode 100644 index 00000000000..47e4662e7f2 --- /dev/null +++ b/pkg/webhook/servicecatalog/servicebroker/validation/handler.go @@ -0,0 +1,144 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "context" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil" + admissionTypes "k8s.io/api/admission/v1beta1" + "net/http" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// Validator is used to implement new validation logic +type Validator interface { + Validate(context.Context, admission.Request, *sc.ServiceBroker, *webhookutil.TracedLogger) *webhookutil.WebhookError +} + +// AdmissionHandler handles ServiceBroker validation +type AdmissionHandler struct { + decoder *admission.Decoder + + CreateValidators []Validator + UpdateValidators []Validator +} + +var _ admission.Handler = &AdmissionHandler{} +var _ admission.DecoderInjector = &AdmissionHandler{} +var _ inject.Client = &AdmissionHandler{} + +// NewAdmissionHandler creates new AdmissionHandler and initializes validators list +func NewAdmissionHandler() *AdmissionHandler { + return &AdmissionHandler{ + CreateValidators: []Validator{&AccessToBroker{}}, + UpdateValidators: []Validator{&AccessToBroker{}}, + } +} + +// Handle handles admission requests. +func (h *AdmissionHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + traced := webhookutil.NewTracedLogger(req.UID) + traced.Infof("Start handling validation operation: %s for %s", req.Operation, req.Kind.Kind) + + sb := &sc.ServiceBroker{} + if err := webhookutil.MatchKinds(sb, req.Kind); err != nil { + traced.Errorf("Error matching kinds: %v", err) + return admission.Errored(http.StatusBadRequest, err) + } + + if err := h.decoder.Decode(req, sb); err != nil { + traced.Errorf("Could not decode request object: %v", err) + return admission.Errored(http.StatusBadRequest, err) + } + + traced.Infof("start validation process for %s: %s/%s", sb.Kind, sb.Namespace, sb.Name) + + var err *webhookutil.WebhookError + + switch req.Operation { + case admissionTypes.Create: + for _, v := range h.CreateValidators { + err = v.Validate(ctx, req, sb, traced) + if err != nil { + break + } + } + case admissionTypes.Update: + for _, v := range h.UpdateValidators { + err = v.Validate(ctx, req, sb, traced) + if err != nil { + break + } + } + default: + traced.Infof("ServiceBroker validation wehbook does not support action %q", req.Operation) + return admission.Allowed("action not taken") + } + + if err != nil { + switch err.Code() { + case http.StatusForbidden: + return admission.Denied(err.Error()) + default: + return admission.Errored(err.Code(), err) + } + } + + traced.Infof("Completed successfully validation operation: %s for %s: %q", req.Operation, req.Kind.Kind, req.Name) + return admission.Allowed("ServiceBroker AdmissionHandler successful") +} + +// InjectDecoder injects the decoder into the handlers +func (h *AdmissionHandler) InjectDecoder(d *admission.Decoder) error { + h.decoder = d + + for _, v := range h.CreateValidators { + _, err := admission.InjectDecoderInto(d, v) + if err != nil { + return err + } + } + for _, v := range h.UpdateValidators { + _, err := admission.InjectDecoderInto(d, v) + if err != nil { + return err + } + } + + return nil +} + +// InjectClient injects the client into the handlers +func (h *AdmissionHandler) InjectClient(c client.Client) error { + for _, v := range h.CreateValidators { + _, err := inject.ClientInto(c, v) + if err != nil { + return err + } + } + for _, v := range h.UpdateValidators { + _, err := inject.ClientInto(c, v) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/webhook/servicecatalog/servicebroker/validation/handler_access.go b/pkg/webhook/servicecatalog/servicebroker/validation/handler_access.go new file mode 100644 index 00000000000..fea7450c01b --- /dev/null +++ b/pkg/webhook/servicecatalog/servicebroker/validation/handler_access.go @@ -0,0 +1,123 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "context" + "fmt" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil" + authenticationapi "k8s.io/api/authentication/v1" + authorizationapi "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + "net/http" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// AccessToBroker handles ServiceBroker validation +type AccessToBroker struct { + decoder *admission.Decoder + client client.Client +} + +var _ admission.DecoderInjector = &AccessToBroker{} +var _ inject.Client = &AccessToBroker{} + +// InjectDecoder injects the decoder +func (h *AccessToBroker) InjectDecoder(d *admission.Decoder) error { + h.decoder = d + return nil +} + +// InjectClient injects the client +func (h *AccessToBroker) InjectClient(c client.Client) error { + h.client = c + return nil +} + +// Validate checks if client has access to service broker if broker requires authentication +// This feature was copied from Service Catalog admission plugin https://github.com/kubernetes-incubator/service-catalog/blob/v0.1.41/plugin/pkg/admission/broker/authsarcheck/admission.go +// If you want to track previous changes please check there. +func (h *AccessToBroker) Validate(ctx context.Context, req admission.Request, sb *sc.ServiceBroker, traced *webhookutil.TracedLogger) *webhookutil.WebhookError { + if sb.Spec.AuthInfo == nil { + traced.Infof("%s %q has no AuthInfo. Operation completed", sb.Kind, sb.Name) + return nil + } + + var secretRef *sc.LocalObjectReference + if sb.Spec.AuthInfo.Basic != nil { + secretRef = sb.Spec.AuthInfo.Basic.SecretRef + } else if sb.Spec.AuthInfo.Bearer != nil { + secretRef = sb.Spec.AuthInfo.Bearer.SecretRef + } + + if secretRef == nil { + traced.Infof("%s %q has no SecretRef neither in Basic nor Bearer auth. Operation completed", sb.Kind, sb.Name) + return nil + } + + user := req.UserInfo + sar := &authorizationapi.SubjectAccessReview{ + Spec: authorizationapi.SubjectAccessReviewSpec{ + ResourceAttributes: &authorizationapi.ResourceAttributes{ + Namespace: sb.Namespace, + Verb: "get", + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + Resource: corev1.ResourceSecrets.String(), + Name: secretRef.Name, + }, + User: user.Username, + Groups: user.Groups, + Extra: convertToSARExtra(user.Extra), + UID: user.UID, + }, + } + + err := h.client.Create(ctx, sar) + if err != nil { + traced.Errorf("Could not create SubjectAccessReview for %s %q: %v", sb.Kind, sb.Name, err) + return webhookutil.NewWebhookError(err.Error(), http.StatusForbidden) + } + + if !sar.Status.Allowed { + msg := fmt.Sprintf( + "broker forbidden access to auth secret (%s): Reason: %s, EvaluationError: %s", + secretRef.Name, + sar.Status.Reason, + sar.Status.EvaluationError) + traced.Info(msg) + return webhookutil.NewWebhookError(msg, http.StatusForbidden) + } + + return nil +} + +func convertToSARExtra(extra map[string]authenticationapi.ExtraValue) map[string]authorizationapi.ExtraValue { + if extra == nil { + return nil + } + + ret := map[string]authorizationapi.ExtraValue{} + for k, v := range extra { + ret[k] = authorizationapi.ExtraValue(v) + } + + return ret +} diff --git a/pkg/webhook/servicecatalog/servicebroker/validation/handler_access_test.go b/pkg/webhook/servicecatalog/servicebroker/validation/handler_access_test.go new file mode 100644 index 00000000000..46a316d13d6 --- /dev/null +++ b/pkg/webhook/servicecatalog/servicebroker/validation/handler_access_test.go @@ -0,0 +1,290 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation_test + +import ( + "context" + "errors" + sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/servicebroker/validation" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + "k8s.io/api/authorization/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "testing" +) + +const ( + AllowedSecretName = "csb-secret-name" + DeniedSecretName = "denied-csb-secret-name" +) + +// Reactors are not implemented in 'sigs.k8s.io/controller-runtime/pkg/client/fake' package +// https://github.com/kubernetes-sigs/controller-runtime/issues/72 +// instead it is used custom client with override Create method +type fakedClient struct { + client.Client +} + +// Create overrides real client Create method for the test +func (m *fakedClient) Create(ctx context.Context, obj runtime.Object, opts ...client.CreateOptionFunc) error { + if _, ok := obj.(*v1.SubjectAccessReview); !ok { + return errors.New("Input object is not SubjectAccessReview type") + } + + if obj.(*v1.SubjectAccessReview).Spec.ResourceAttributes.Name == AllowedSecretName { + obj.(*v1.SubjectAccessReview).Status.Allowed = true + } + + return nil +} + +func TestAdmissionHandlerAccessToBrokerAllowed(t *testing.T) { + // given + err := sc.AddToScheme(scheme.Scheme) + require.NoError(t, err) + + request := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + UID: "5555-eeee", + Name: "test-broker", + Namespace: "test-handler", + Kind: metav1.GroupVersionKind{ + Kind: "ServiceBroker", + Version: "v1beta1", + Group: "servicecatalog.k8s.io", + }, + Object: runtime.RawExtension{}, + }, + } + + decoder, err := admission.NewDecoder(scheme.Scheme) + require.NoError(t, err) + + tests := map[string]struct { + operation admissionv1beta1.Operation + object []byte + }{ + "Request for Create ServiceBroker without AuthInfo should be allowed": { + admissionv1beta1.Create, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local" + } + }`), + }, + "Request for Update ServiceBroker without AuthInfo should be allowed": { + admissionv1beta1.Update, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local" + } + }`), + }, + "Request for Create ServiceBroker with AuthInfo should be allowed": { + admissionv1beta1.Create, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local", + "authInfo": { + "basic": { + "secretRef": { + "namespace": "test-handler", + "name": "` + AllowedSecretName + `" + } + } + } + } + }`), + }, + "Request for Update ServiceBroker with AuthInfo should be allowed": { + admissionv1beta1.Update, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local", + "authInfo": { + "bearer": { + "secretRef": { + "namespace": "test-handler", + "name": "` + AllowedSecretName + `" + } + } + } + } + }`), + }, + } + + for desc, test := range tests { + t.Run(desc, func(t *testing.T) { + // given + handler := validation.AdmissionHandler{} + handler.CreateValidators = []validation.Validator{&validation.AccessToBroker{}} + handler.UpdateValidators = []validation.Validator{&validation.AccessToBroker{}} + + err := handler.InjectDecoder(decoder) + require.NoError(t, err) + err = handler.InjectClient(&fakedClient{}) + require.NoError(t, err) + + request.AdmissionRequest.Operation = test.operation + request.AdmissionRequest.Object.Raw = test.object + + // when + response := handler.Handle(context.Background(), request) + + // then + assert.True(t, response.AdmissionResponse.Allowed) + }) + } +} + +func TestAdmissionHandlerAccessToBrokerDenied(t *testing.T) { + // given + err := sc.AddToScheme(scheme.Scheme) + require.NoError(t, err) + + request := admission.Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + UID: "6666-ffff", + Name: "test-broker", + Namespace: "test-handler", + Kind: metav1.GroupVersionKind{ + Kind: "ServiceBroker", + Version: "v1beta1", + Group: "servicecatalog.k8s.io", + }, + Object: runtime.RawExtension{}, + }, + } + + sch := scheme.Scheme + + decoder, err := admission.NewDecoder(sch) + require.NoError(t, err) + + tests := map[string]struct { + operation admissionv1beta1.Operation + object []byte + }{ + "Request for Create ServiceBroker should be denied": { + admissionv1beta1.Create, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local", + "authInfo": { + "bearer": { + "secretRef": { + "namespace": "test-handler", + "name": "` + DeniedSecretName + `" + } + } + } + } + }`), + }, + "Request for Update ServiceBroker should be denied": { + admissionv1beta1.Update, + []byte(`{ + "apiVersion": "servicecatalog.k8s.io/v1beta1", + "kind": "ServiceBroker", + "metadata": { + "finalizers": ["kubernetes-incubator/service-catalog"], + "creationTimestamp": null, + "name": "test-broker" + }, + "spec": { + "url": "http://test-broker.local", + "authInfo": { + "basic": { + "secretRef": { + "namespace": "test-handler", + "name": "` + DeniedSecretName + `" + } + } + } + } + }`), + }, + } + + for desc, test := range tests { + t.Run(desc, func(t *testing.T) { + // given + handler := validation.AdmissionHandler{} + handler.CreateValidators = []validation.Validator{&validation.AccessToBroker{}} + handler.UpdateValidators = []validation.Validator{&validation.AccessToBroker{}} + + fakeClient := fake.NewFakeClientWithScheme(sch, &sc.ServiceBroker{}) + + err := handler.InjectDecoder(decoder) + require.NoError(t, err) + err = handler.InjectClient(fakeClient) + require.NoError(t, err) + + request.AdmissionRequest.Operation = test.operation + request.AdmissionRequest.Object.Raw = test.object + + // when + response := handler.Handle(context.Background(), request) + + // then + assert.False(t, response.AdmissionResponse.Allowed) + }) + } +} diff --git a/pkg/webhook/servicecatalog/servicebroker/validation/handler_test.go b/pkg/webhook/servicecatalog/servicebroker/validation/handler_test.go new file mode 100644 index 00000000000..db61060d673 --- /dev/null +++ b/pkg/webhook/servicecatalog/servicebroker/validation/handler_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation_test + +import ( + "testing" + + "github.com/kubernetes-incubator/service-catalog/pkg/webhook/servicecatalog/servicebroker/validation" + "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil/tester" +) + +func TestAdmissionHandlerHandleDecoderErrors(t *testing.T) { + tester.DiscardLoggedMsg() + + for _, fn := range []func(t *testing.T, handler tester.TestDecoderHandler, kind string){ + tester.AssertHandlerReturnErrorIfReqObjIsMalformed, + tester.AssertHandlerReturnErrorIfGVKMismatch, + } { + handler := validation.AdmissionHandler{} + fn(t, &handler, "ServiceBroker") + } +} diff --git a/pkg/webhook/servicecatalog/serviceinstance/mutation/handler.go b/pkg/webhook/servicecatalog/serviceinstance/mutation/handler.go index 93922d8515b..ab4b5dc4350 100644 --- a/pkg/webhook/servicecatalog/serviceinstance/mutation/handler.go +++ b/pkg/webhook/servicecatalog/serviceinstance/mutation/handler.go @@ -27,7 +27,6 @@ import ( admissionTypes "k8s.io/api/admission/v1beta1" utilfeature "k8s.io/apiserver/pkg/util/feature" - "github.com/pkg/errors" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -82,7 +81,7 @@ func (h *CreateUpdateHandler) Handle(ctx context.Context, req admission.Request) case http.StatusForbidden: return admission.Denied(err.Error()) default: - return admission.Errored(err.Code(), errors.New(err.Error())) + return admission.Errored(err.Code(), err) } } diff --git a/pkg/webhook/servicecatalog/serviceinstance/validation/handler.go b/pkg/webhook/servicecatalog/serviceinstance/validation/handler.go index 6da4221f110..d2872177c89 100644 --- a/pkg/webhook/servicecatalog/serviceinstance/validation/handler.go +++ b/pkg/webhook/servicecatalog/serviceinstance/validation/handler.go @@ -23,7 +23,6 @@ import ( sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil" - "github.com/hashicorp/go-multierror" admissionTypes "k8s.io/api/admission/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" @@ -32,13 +31,12 @@ import ( // Validator is used to implement new validation logic type Validator interface { - Validate(context.Context, admission.Request, *sc.ServiceInstance, *webhookutil.TracedLogger) error + Validate(context.Context, admission.Request, *sc.ServiceInstance, *webhookutil.TracedLogger) *webhookutil.WebhookError } // AdmissionHandler handles ServiceInstance validation type AdmissionHandler struct { decoder *admission.Decoder - client client.Client CreateValidators []Validator UpdateValidators []Validator @@ -58,7 +56,7 @@ func NewAdmissionHandler() *AdmissionHandler { // Handle handles admission requests. func (h *AdmissionHandler) Handle(ctx context.Context, req admission.Request) admission.Response { traced := webhookutil.NewTracedLogger(req.UID) - traced.Infof("Start handling AdmissionHandler operation: %s for %s: %q", req.Operation, req.Kind.Kind, req.Name) + traced.Infof("Start handling validation operation: %s for %s", req.Operation, req.Kind.Kind) si := &sc.ServiceInstance{} if err := webhookutil.MatchKinds(si, req.Kind); err != nil { @@ -71,31 +69,40 @@ func (h *AdmissionHandler) Handle(ctx context.Context, req admission.Request) ad return admission.Errored(http.StatusBadRequest, err) } - var errs error + traced.Infof("start validation process for %s: %s/%s", si.Kind, si.Namespace, si.Name) + + var err *webhookutil.WebhookError switch req.Operation { case admissionTypes.Create: for _, v := range h.CreateValidators { - if err := v.Validate(ctx, req, si, traced); err != nil { - errs = multierror.Append(errs, err) + err = v.Validate(ctx, req, si, traced) + if err != nil { + break } } case admissionTypes.Update: for _, v := range h.UpdateValidators { - if err := v.Validate(ctx, req, si, traced); err != nil { - errs = multierror.Append(errs, err) + err = v.Validate(ctx, req, si, traced) + if err != nil { + break } } default: - traced.Infof("ServiceInstance AdmissionHandler wehbook does not support action %q", req.Operation) + traced.Infof("ServiceInstance validation wehbook does not support action %q", req.Operation) return admission.Allowed("action not taken") } - if errs != nil { - return admission.Denied(errs.Error()) + if err != nil { + switch err.Code() { + case http.StatusForbidden: + return admission.Denied(err.Error()) + default: + return admission.Errored(err.Code(), err) + } } - traced.Infof("Completed successfully AdmissionHandler operation: %s for %s: %q", req.Operation, req.Kind.Kind, req.Name) + traced.Infof("Completed successfully validation operation: %s for %s: %q", req.Operation, req.Kind.Kind, req.Name) return admission.Allowed("ServiceInstance AdmissionHandler successful") } @@ -104,10 +111,16 @@ func (h *AdmissionHandler) InjectDecoder(d *admission.Decoder) error { h.decoder = d for _, v := range h.CreateValidators { - admission.InjectDecoderInto(d, v) + _, err := admission.InjectDecoderInto(d, v) + if err != nil { + return err + } } for _, v := range h.UpdateValidators { - admission.InjectDecoderInto(d, v) + _, err := admission.InjectDecoderInto(d, v) + if err != nil { + return err + } } return nil @@ -115,13 +128,17 @@ func (h *AdmissionHandler) InjectDecoder(d *admission.Decoder) error { // InjectClient injects the client into the handlers func (h *AdmissionHandler) InjectClient(c client.Client) error { - h.client = c - for _, v := range h.CreateValidators { - inject.ClientInto(c, v) + _, err := inject.ClientInto(c, v) + if err != nil { + return err + } } for _, v := range h.UpdateValidators { - inject.ClientInto(c, v) + _, err := inject.ClientInto(c, v) + if err != nil { + return err + } } return nil diff --git a/pkg/webhook/servicecatalog/serviceinstance/validation/handler_denyplan.go b/pkg/webhook/servicecatalog/serviceinstance/validation/handler_denyplan.go index b0398db3cf9..f5bbd0f694c 100644 --- a/pkg/webhook/servicecatalog/serviceinstance/validation/handler_denyplan.go +++ b/pkg/webhook/servicecatalog/serviceinstance/validation/handler_denyplan.go @@ -18,13 +18,14 @@ package validation import ( "context" - "errors" "fmt" + "net/http" sc "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" "github.com/kubernetes-incubator/service-catalog/pkg/webhookutil" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) @@ -34,6 +35,9 @@ type DenyPlanChangeIfNotUpdatable struct { client client.Client } +var _ admission.DecoderInjector = &DenyPlanChangeIfNotUpdatable{} +var _ inject.Client = &DenyPlanChangeIfNotUpdatable{} + // InjectDecoder injects the decoder func (h *DenyPlanChangeIfNotUpdatable) InjectDecoder(d *admission.Decoder) error { h.decoder = d @@ -47,7 +51,7 @@ func (h *DenyPlanChangeIfNotUpdatable) InjectClient(c client.Client) error { } // Validate checks if Plan can be changed -func (h *DenyPlanChangeIfNotUpdatable) Validate(ctx context.Context, req admission.Request, si *sc.ServiceInstance, traced *webhookutil.TracedLogger) error { +func (h *DenyPlanChangeIfNotUpdatable) Validate(ctx context.Context, req admission.Request, si *sc.ServiceInstance, traced *webhookutil.TracedLogger) *webhookutil.WebhookError { traced.Info("Starting validation - DenyPlanChangeIfNotUpdatable") if si.Spec.ClusterServiceClassRef == nil { @@ -63,7 +67,7 @@ func (h *DenyPlanChangeIfNotUpdatable) Validate(ctx context.Context, req admissi if err := h.client.Get(ctx, key, csc); err != nil { traced.Infof("Could not locate service class '%v', can not determine if UpdateablePlan.", si.Spec.ClusterServiceClassRef.Name) - return err + return webhookutil.NewWebhookError(err.Error(), http.StatusForbidden) } if csc.Spec.PlanUpdatable { @@ -75,7 +79,7 @@ func (h *DenyPlanChangeIfNotUpdatable) Validate(ctx context.Context, req admissi origInstance := &sc.ServiceInstance{} if err := h.decoder.DecodeRaw(req.OldObject, origInstance); err != nil { traced.Errorf("Could not decode oldObject: %v", err) - return err + return webhookutil.NewWebhookError(err.Error(), http.StatusBadRequest) } externalPlanNameUpdated := si.Spec.ClusterServicePlanExternalName != origInstance.Spec.ClusterServicePlanExternalName @@ -96,7 +100,7 @@ func (h *DenyPlanChangeIfNotUpdatable) Validate(ctx context.Context, req admissi traced.Infof("update Service Instance %v/%v request specified Plan %v while original instance had %v", si.Namespace, si.Name, newPlan, oldPlan) msg := fmt.Sprintf("The Service Class %v does not allow plan changes.", csc.Name) traced.Error(msg) - return errors.New(msg) + return webhookutil.NewWebhookError(msg, http.StatusForbidden) } } diff --git a/pkg/webhook/servicecatalog/serviceinstance/validation/handler_denyplan_test.go b/pkg/webhook/servicecatalog/serviceinstance/validation/handler_denyplan_test.go index 0a32c06e5f2..d0dbbc57061 100644 --- a/pkg/webhook/servicecatalog/serviceinstance/validation/handler_denyplan_test.go +++ b/pkg/webhook/servicecatalog/serviceinstance/validation/handler_denyplan_test.go @@ -122,7 +122,7 @@ func TestAdmissionHandlerDenyPlanChangeIfNotUpdatableSimpleScenarios(t *testing. request.AdmissionRequest.Operation = test.operation // when - response := handler.Handle(context.TODO(), request) + response := handler.Handle(context.Background(), request) // then assert.Equal(t, response.AdmissionResponse.Allowed, test.responseAllowed) @@ -217,7 +217,7 @@ func TestAdmissionHandlerDenyPlanChangeIfNotUpdatablePlanNameChanged(t *testing. request.AdmissionRequest.Operation = admissionv1beta1.Update // when - response := handler.Handle(context.TODO(), request) + response := handler.Handle(context.Background(), request) // then assert.Equal(t, response.AdmissionResponse.Allowed, test.responseAllowed)