From 96d35eb4dd3ea44424e1bf985a55df0258c2eca4 Mon Sep 17 00:00:00 2001 From: David Kwon Date: Thu, 18 Jan 2024 10:42:18 -0500 Subject: [PATCH 1/2] Remove kubernetesimagepuller webhook Signed-off-by: David Kwon --- cmd/webhook/main.go | 4 - deploy/webhook/member-operator-webhook.yaml | 28 ---- pkg/webhook/deploy/deployment_test.go | 3 +- .../validate_k8simagepuller_request.go | 69 --------- .../validate_k8simagepuller_request_test.go | 138 ------------------ 5 files changed, 1 insertion(+), 241 deletions(-) delete mode 100644 pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go delete mode 100644 pkg/webhook/validatingwebhook/validate_k8simagepuller_request_test.go diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index 225a7058..28be7a6f 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -102,9 +102,6 @@ func main() { rolebindingValidator := &validatingwebhook.RoleBindingRequestValidator{ Client: cl, } - k8sImagePullerRequestValidator := &validatingwebhook.K8sImagePullerRequestValidator{ - Client: cl, - } spacebindingrequestValidator := &validatingwebhook.SpaceBindingRequestValidator{ Client: cl, } @@ -113,7 +110,6 @@ func main() { mux.HandleFunc("/mutate-users-pods", mutatingwebhook.HandleMutateUserPods) mux.HandleFunc("/mutate-virtual-machines", mutatingwebhook.HandleMutateVirtualMachines) mux.HandleFunc("/validate-users-rolebindings", rolebindingValidator.HandleValidate) - mux.HandleFunc("/validate-users-kubernetesimagepullers", k8sImagePullerRequestValidator.HandleValidate) mux.HandleFunc("/validate-spacebindingrequests", spacebindingrequestValidator.HandleValidate) webhookServer := &http.Server{ //nolint:gosec //TODO: configure ReadHeaderTimeout (gosec G112) diff --git a/deploy/webhook/member-operator-webhook.yaml b/deploy/webhook/member-operator-webhook.yaml index 47880204..cee45e37 100644 --- a/deploy/webhook/member-operator-webhook.yaml +++ b/deploy/webhook/member-operator-webhook.yaml @@ -218,34 +218,6 @@ objects: namespaceSelector: matchLabels: toolchain.dev.openshift.com/provider: codeready-toolchain - # The users.kubernetesimagepullers.webhook.sandbox validation webhook ensures that KubernetesImagePuller CRs cannot be created by a sandbox user. - # This webhook is needed to prevent user-created KubernetesImagePuller CRs from interfering with the devworkspace-controller-manager-* pod, as high memory - # usage was previously observed. - # The webhook code is available at member-operator/pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go - - name: users.kubernetesimagepullers.webhook.sandbox - admissionReviewVersions: - - v1 - clientConfig: - caBundle: ${CA_BUNDLE} - service: - name: member-operator-webhook - namespace: ${NAMESPACE} - path: "/validate-users-kubernetesimagepullers" - port: 443 - matchPolicy: Equivalent - rules: - - operations: ["CREATE"] - apiGroups: ["che.eclipse.org"] - apiVersions: ["v1alpha1"] - resources: ["kubernetesimagepullers"] - scope: "Namespaced" - sideEffects: None - timeoutSeconds: 5 - reinvocationPolicy: Never - failurePolicy: Fail - namespaceSelector: - matchLabels: - toolchain.dev.openshift.com/provider: codeready-toolchain # The users.spacebindingrequests.webhook.sandbox webhook validates SpaceBindingRequest CRs, # Specifically it makes sure that once a SBR resource is created, the SpaceBindingRequest.Spec.MasterUserRecord field is not changed by the user. # The reason for making SpaceBindingRequest.Spec.MasterUserRecord field immutable is that as of now the SpaceBinding resource name is composed as follows: -checksum(-), diff --git a/pkg/webhook/deploy/deployment_test.go b/pkg/webhook/deploy/deployment_test.go index ebeb59aa..c5756e0f 100644 --- a/pkg/webhook/deploy/deployment_test.go +++ b/pkg/webhook/deploy/deployment_test.go @@ -227,8 +227,7 @@ func mutatingWebhookConfig(namespace, caBundle string) string { } func validatingWebhookConfig(namespace, caBundle string) string { - return fmt.Sprintf(`{ - "apiVersion": "admissionregistration.k8s.io/v1","kind": "ValidatingWebhookConfiguration","metadata": {"labels": {"app": "member-operator-webhook","toolchain.dev.openshift.com/provider": "codeready-toolchain"},"name": "member-operator-validating-webhook"},"webhooks": [{"admissionReviewVersions": ["v1"],"clientConfig": {"caBundle": "%[1]s","service": {"name": "member-operator-webhook","namespace": "%[2]s","path": "/validate-users-rolebindings","port": 443}},"failurePolicy": "Ignore","matchPolicy": "Equivalent","name": "users.rolebindings.webhook.sandbox","namespaceSelector": {"matchLabels": {"toolchain.dev.openshift.com/provider": "codeready-toolchain"}},"reinvocationPolicy": "Never","rules": [{"apiGroups": ["rbac.authorization.k8s.io","authorization.openshift.io"],"apiVersions": ["v1"],"operations": ["CREATE","UPDATE"],"resources": ["rolebindings"],"scope": "Namespaced"}],"sideEffects": "None","timeoutSeconds": 5},{"admissionReviewVersions": ["v1"],"clientConfig": {"caBundle": "%[1]s","service": {"name": "member-operator-webhook","namespace": "%[2]s","path": "/validate-users-kubernetesimagepullers","port": 443}},"failurePolicy": "Fail","matchPolicy": "Equivalent","name": "users.kubernetesimagepullers.webhook.sandbox","namespaceSelector": {"matchLabels": {"toolchain.dev.openshift.com/provider": "codeready-toolchain"}},"reinvocationPolicy": "Never","rules": [{"apiGroups": ["che.eclipse.org"],"apiVersions": ["v1alpha1"],"operations": ["CREATE"],"resources": ["kubernetesimagepullers"],"scope": "Namespaced"}],"sideEffects": "None","timeoutSeconds": 5},{"admissionReviewVersions": ["v1"],"clientConfig": {"caBundle": "%[1]s","service": {"name": "member-operator-webhook","namespace": "%[2]s","path": "/validate-spacebindingrequests","port": 443}},"failurePolicy": "Fail","matchPolicy": "Equivalent","name": "users.spacebindingrequests.webhook.sandbox","namespaceSelector": {"matchLabels": {"toolchain.dev.openshift.com/provider": "codeready-toolchain"}},"reinvocationPolicy": "Never","rules": [{"apiGroups": ["toolchain.dev.openshift.com"],"apiVersions": ["v1alpha1"],"operations": ["CREATE","UPDATE"],"resources": ["spacebindingrequests"],"scope": "Namespaced"}],"sideEffects": "None","timeoutSeconds": 5}]}`, caBundle, namespace) + return fmt.Sprintf(`{"apiVersion":"admissionregistration.k8s.io/v1","kind":"ValidatingWebhookConfiguration","metadata":{"labels":{"app":"member-operator-webhook","toolchain.dev.openshift.com/provider":"codeready-toolchain"},"name":"member-operator-validating-webhook"},"webhooks":[{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-users-rolebindings","port":443}},"failurePolicy":"Ignore","matchPolicy":"Equivalent","name":"users.rolebindings.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["rbac.authorization.k8s.io","authorization.openshift.io"],"apiVersions":["v1"],"operations":["CREATE","UPDATE"],"resources":["rolebindings"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5},{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-users-checlusters","port":443}},"failurePolicy":"Fail","matchPolicy":"Equivalent","name":"users.checlusters.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["org.eclipse.che"],"apiVersions":["v2"],"operations":["CREATE"],"resources":["checlusters"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5},{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-spacebindingrequests","port":443}},"failurePolicy":"Fail","matchPolicy":"Equivalent","name":"users.spacebindingrequests.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["toolchain.dev.openshift.com"],"apiVersions":["v1alpha1"],"operations":["CREATE", "UPDATE"],"resources":["spacebindingrequests"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5}]}`, caBundle, namespace) } func serviceAccount(namespace string) string { diff --git a/pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go b/pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go deleted file mode 100644 index 3ad1b23d..00000000 --- a/pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go +++ /dev/null @@ -1,69 +0,0 @@ -package validatingwebhook - -import ( - "context" - "html" - "io" - "net/http" - - toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" - - userv1 "github.com/openshift/api/user/v1" - "github.com/pkg/errors" - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/apimachinery/pkg/types" - runtimeClient "sigs.k8s.io/controller-runtime/pkg/client" -) - -type K8sImagePullerRequestValidator struct { - Client runtimeClient.Client -} - -func (v K8sImagePullerRequestValidator) HandleValidate(w http.ResponseWriter, r *http.Request) { - var respBody []byte - body, err := io.ReadAll(r.Body) - defer func() { - if err := r.Body.Close(); err != nil { - log.Error(err, "unable to close the body") - } - }() - if err != nil { - log.Error(err, "unable to read the body of the request") - w.WriteHeader(http.StatusInternalServerError) - respBody = []byte("unable to read the body of the request") - } else { - // validate the request - respBody = v.validate(r.Context(), body) - w.WriteHeader(http.StatusOK) - } - if _, err := io.WriteString(w, string(respBody)); err != nil { - log.Error(err, "unable to write response") - } -} - -func (v K8sImagePullerRequestValidator) validate(ctx context.Context, body []byte) []byte { - log.Info("incoming request", "body", string(body)) - admReview := admissionv1.AdmissionReview{} - if _, _, err := deserializer.Decode(body, nil, &admReview); err != nil { - // sanitize the body - escapedBody := html.EscapeString(string(body)) - log.Error(err, "unable to deserialize the admission review object", "body", escapedBody) - return denyAdmissionRequest(admReview, errors.Wrapf(err, "unable to deserialize the admission review object - body: %v", escapedBody)) - } - //check if the requesting user is a sandbox user - requestingUser := &userv1.User{} - err := v.Client.Get(ctx, types.NamespacedName{ - Name: admReview.Request.UserInfo.Username, - }, requestingUser) - - if err != nil { - log.Error(err, "unable to find the user requesting creation of the KubernetesImagePuller resource", "username", admReview.Request.UserInfo.Username) - return denyAdmissionRequest(admReview, errors.New("unable to find the user requesting the creation of the KubernetesImagePuller resource")) - } - if requestingUser.GetLabels()[toolchainv1alpha1.ProviderLabelKey] == toolchainv1alpha1.ProviderLabelValue { - log.Info("sandbox user is trying to create a KubernetesImagePuller", "AdmissionReview", admReview) - return denyAdmissionRequest(admReview, errors.New("this is a Dev Sandbox enforced restriction. you are trying to create a KubernetesImagePuller resource, which is not allowed")) - } - // at this point, it is clear the user isn't a sandbox user, allow request - return allowAdmissionRequest(admReview) -} diff --git a/pkg/webhook/validatingwebhook/validate_k8simagepuller_request_test.go b/pkg/webhook/validatingwebhook/validate_k8simagepuller_request_test.go deleted file mode 100644 index 518ede22..00000000 --- a/pkg/webhook/validatingwebhook/validate_k8simagepuller_request_test.go +++ /dev/null @@ -1,138 +0,0 @@ -package validatingwebhook - -import ( - "bytes" - "context" - "net/http" - "net/http/httptest" - "testing" - "text/template" - - toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" - "github.com/codeready-toolchain/member-operator/pkg/webhook/validatingwebhook/test" - - userv1 "github.com/openshift/api/user/v1" - "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func TestHandleValidateK8sImagePullerAdmissionRequest(t *testing.T) { - // given - v := newK8sImagePullerValidator(t) - ts := httptest.NewServer(http.HandlerFunc(v.HandleValidate)) - defer ts.Close() - - t.Run("sandbox user trying to create a KubernetesImagePuller resource is denied", func(t *testing.T) { - // given - req := newCreateK8sImagePullerAdmissionRequest(t, "johnsmith") - - // when - response := v.validate(context.TODO(), req) - - // then - test.VerifyRequestBlocked(t, response, "this is a Dev Sandbox enforced restriction. you are trying to create a KubernetesImagePuller resource, which is not allowed", "b6ae2ab4-782b-11ee-b962-0242ac120002") - }) - - t.Run("crtadmin user trying to create a KubernetesImagePuller resource is allowed", func(t *testing.T) { - // given - req := newCreateK8sImagePullerAdmissionRequest(t, "johnsmith-crtadmin") - - // when - response := v.validate(context.TODO(), req) - - // then - test.VerifyRequestAllowed(t, response, "b6ae2ab4-782b-11ee-b962-0242ac120002") - }) - -} - -func newK8sImagePullerValidator(t *testing.T) *K8sImagePullerRequestValidator { - s := scheme.Scheme - err := userv1.Install(s) - require.NoError(t, err) - johnsmithUser := &userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: "johnsmith", - Labels: map[string]string{ - toolchainv1alpha1.ProviderLabelKey: toolchainv1alpha1.ProviderLabelValue, - }, - }, - } - johnsmithAdmin := &userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: "johnsmith-crtadmin", - Labels: map[string]string{ - "provider": "sandbox-sre", - }, - }, - } - cl := fake.NewClientBuilder().WithScheme(s).WithObjects(johnsmithUser, johnsmithAdmin).Build() - return &K8sImagePullerRequestValidator{ - Client: cl, - } - -} - -func newCreateK8sImagePullerAdmissionRequest(t *testing.T, username string) []byte { - tmpl, err := template.New("admission request").Parse(createK8sImagePullerJSONTmpl) - require.NoError(t, err) - req := &bytes.Buffer{} - err = tmpl.Execute(req, username) - require.NoError(t, err) - return req.Bytes() -} - -var createK8sImagePullerJSONTmpl = `{ - "kind": "AdmissionReview", - "apiVersion": "admission.k8s.io/v1", - "request": { - "uid": "b6ae2ab4-782b-11ee-b962-0242ac120002", - "kind": { - "group": "che.eclipse.org", - "version": "v1alpha1", - "kind": "KubernetesImagePuller" - }, - "resource": { - "group": "che.eclipse.org", - "version": "v1alpha1", - "resource": "kubernetesimagepullers" - }, - "requestKind": { - "group": "che.eclipse.org", - "version": "v1alpha1", - "kind": "KubernetesImagePuller" - }, - "requestResource": { - "group": "che.eclipse.org", - "version": "v1alpha1", - "resource": "kubernetesimagepullers" - }, - "name": "test", - "namespace": "johnsmith-dev", - "operation": "CREATE", - "userInfo": { - "username": "{{ . }}", - "groups": [ - "system:authenticated" - ] - }, - "object": { - "apiVersion": "che.eclipse.org/v1alpha1", - "kind": "KubernetesImagePuller", - "metadata": { - "name": "test", - "namespace": "paul-dev" - } - }, - "oldObject": null, - "dryRun": false, - "options": { - "kind": "CreateOptions", - "apiVersion": "meta.k8s.io/v1", - "fieldManager": "kubectl-client-side-apply", - "fieldValidation": "Ignore" - } - } -}` From dac8983fcfac0dfffceeb0e3a883970b4819633c Mon Sep 17 00:00:00 2001 From: David Kwon Date: Thu, 18 Jan 2024 14:01:53 -0500 Subject: [PATCH 2/2] Update test Signed-off-by: David Kwon --- pkg/webhook/deploy/deployment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/webhook/deploy/deployment_test.go b/pkg/webhook/deploy/deployment_test.go index c5756e0f..7fedf38a 100644 --- a/pkg/webhook/deploy/deployment_test.go +++ b/pkg/webhook/deploy/deployment_test.go @@ -227,7 +227,7 @@ func mutatingWebhookConfig(namespace, caBundle string) string { } func validatingWebhookConfig(namespace, caBundle string) string { - return fmt.Sprintf(`{"apiVersion":"admissionregistration.k8s.io/v1","kind":"ValidatingWebhookConfiguration","metadata":{"labels":{"app":"member-operator-webhook","toolchain.dev.openshift.com/provider":"codeready-toolchain"},"name":"member-operator-validating-webhook"},"webhooks":[{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-users-rolebindings","port":443}},"failurePolicy":"Ignore","matchPolicy":"Equivalent","name":"users.rolebindings.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["rbac.authorization.k8s.io","authorization.openshift.io"],"apiVersions":["v1"],"operations":["CREATE","UPDATE"],"resources":["rolebindings"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5},{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-users-checlusters","port":443}},"failurePolicy":"Fail","matchPolicy":"Equivalent","name":"users.checlusters.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["org.eclipse.che"],"apiVersions":["v2"],"operations":["CREATE"],"resources":["checlusters"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5},{"admissionReviewVersions":["v1"],"clientConfig":{"caBundle":"%[1]s","service":{"name":"member-operator-webhook","namespace":"%[2]s","path":"/validate-spacebindingrequests","port":443}},"failurePolicy":"Fail","matchPolicy":"Equivalent","name":"users.spacebindingrequests.webhook.sandbox","namespaceSelector":{"matchLabels":{"toolchain.dev.openshift.com/provider":"codeready-toolchain"}},"reinvocationPolicy":"Never","rules":[{"apiGroups":["toolchain.dev.openshift.com"],"apiVersions":["v1alpha1"],"operations":["CREATE", "UPDATE"],"resources":["spacebindingrequests"],"scope":"Namespaced"}],"sideEffects":"None","timeoutSeconds":5}]}`, caBundle, namespace) + return fmt.Sprintf(`{"apiVersion": "admissionregistration.k8s.io/v1","kind": "ValidatingWebhookConfiguration","metadata": {"labels": {"app": "member-operator-webhook","toolchain.dev.openshift.com/provider": "codeready-toolchain"},"name": "member-operator-validating-webhook"},"webhooks": [{"admissionReviewVersions": ["v1"],"clientConfig": {"caBundle": "%[1]s","service": {"name": "member-operator-webhook","namespace": "%[2]s","path": "/validate-users-rolebindings","port": 443}},"failurePolicy": "Ignore","matchPolicy": "Equivalent","name": "users.rolebindings.webhook.sandbox","namespaceSelector": {"matchLabels": {"toolchain.dev.openshift.com/provider": "codeready-toolchain"}},"reinvocationPolicy": "Never","rules": [{"apiGroups": ["rbac.authorization.k8s.io","authorization.openshift.io"],"apiVersions": ["v1"],"operations": ["CREATE","UPDATE"],"resources": ["rolebindings"],"scope": "Namespaced"}],"sideEffects": "None","timeoutSeconds": 5},{"admissionReviewVersions": ["v1"],"clientConfig": {"caBundle": "%[1]s","service": {"name": "member-operator-webhook","namespace": "%[2]s","path": "/validate-spacebindingrequests","port": 443}},"failurePolicy": "Fail","matchPolicy": "Equivalent","name": "users.spacebindingrequests.webhook.sandbox","namespaceSelector": {"matchLabels": {"toolchain.dev.openshift.com/provider": "codeready-toolchain"}},"reinvocationPolicy": "Never","rules": [{"apiGroups": ["toolchain.dev.openshift.com"],"apiVersions": ["v1alpha1"],"operations": ["CREATE","UPDATE"],"resources": ["spacebindingrequests"],"scope": "Namespaced"}],"sideEffects": "None","timeoutSeconds": 5}]}`, caBundle, namespace) } func serviceAccount(namespace string) string {