diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index b35f1642..a95e0631 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -93,7 +93,7 @@ func main() { rolebindingValidator := &validatingwebhook.RoleBindingRequestValidator{ Client: cl, } - checlusterValidator := &validatingwebhook.CheClusterRequestValidator{ + k8sImagePullerRequestValidator := &validatingwebhook.K8sImagePullerRequestValidator{ Client: cl, } spacebindingrequestValidator := &validatingwebhook.SpaceBindingRequestValidator{ @@ -104,7 +104,7 @@ 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-checlusters", checlusterValidator.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 ad792cb9..f750de47 100644 --- a/deploy/webhook/member-operator-webhook.yaml +++ b/deploy/webhook/member-operator-webhook.yaml @@ -204,7 +204,11 @@ objects: namespaceSelector: matchLabels: toolchain.dev.openshift.com/provider: codeready-toolchain - - name: users.checlusters.webhook.sandbox + # 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: @@ -212,14 +216,14 @@ objects: service: name: member-operator-webhook namespace: ${NAMESPACE} - path: "/validate-users-checlusters" + path: "/validate-users-kubernetesimagepullers" port: 443 matchPolicy: Equivalent rules: - operations: ["CREATE"] - apiGroups: ["org.eclipse.che"] - apiVersions: ["v2"] - resources: ["checlusters"] + apiGroups: ["che.eclipse.org"] + apiVersions: ["v1alpha1"] + resources: ["kubernetesimagepullers"] scope: "Namespaced" sideEffects: None timeoutSeconds: 5 diff --git a/pkg/webhook/deploy/deployment_test.go b/pkg/webhook/deploy/deployment_test.go index 19e47730..e7a68e87 100644 --- a/pkg/webhook/deploy/deployment_test.go +++ b/pkg/webhook/deploy/deployment_test.go @@ -227,7 +227,8 @@ 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-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) } func serviceAccount(namespace string) string { diff --git a/pkg/webhook/validatingwebhook/validate_checluster_request.go b/pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go similarity index 72% rename from pkg/webhook/validatingwebhook/validate_checluster_request.go rename to pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go index 607bd857..f0f07492 100644 --- a/pkg/webhook/validatingwebhook/validate_checluster_request.go +++ b/pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go @@ -5,7 +5,6 @@ import ( "html" "io" "net/http" - "strings" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" @@ -16,11 +15,11 @@ import ( runtimeClient "sigs.k8s.io/controller-runtime/pkg/client" ) -type CheClusterRequestValidator struct { +type K8sImagePullerRequestValidator struct { Client runtimeClient.Client } -func (v CheClusterRequestValidator) HandleValidate(w http.ResponseWriter, r *http.Request) { +func (v K8sImagePullerRequestValidator) HandleValidate(w http.ResponseWriter, r *http.Request) { var respBody []byte body, err := io.ReadAll(r.Body) defer func() { @@ -42,7 +41,7 @@ func (v CheClusterRequestValidator) HandleValidate(w http.ResponseWriter, r *htt } } -func (v CheClusterRequestValidator) validate(body []byte) []byte { +func (v K8sImagePullerRequestValidator) validate(body []byte) []byte { log.Info("incoming request", "body", string(body)) admReview := admissionv1.AdmissionReview{} if _, _, err := deserializer.Decode(body, nil, &admReview); err != nil { @@ -51,11 +50,6 @@ func (v CheClusterRequestValidator) validate(body []byte) []byte { 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)) } - requestingUsername := admReview.Request.UserInfo.Username - // allow admission request if the user is a system user - if strings.HasPrefix(requestingUsername, "system:") { - return allowAdmissionRequest(admReview) - } //check if the requesting user is a sandbox user requestingUser := &userv1.User{} err := v.Client.Get(context.TODO(), types.NamespacedName{ @@ -63,12 +57,12 @@ func (v CheClusterRequestValidator) validate(body []byte) []byte { }, requestingUser) if err != nil { - log.Error(err, "unable to find the user requesting creation of the CheCluster resource", "username", admReview.Request.UserInfo.Username) - return denyAdmissionRequest(admReview, errors.New("unable to find the user requesting the creation of the CheCluster resource")) + 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 CheCluster", "AdmissionReview", admReview) - return denyAdmissionRequest(admReview, errors.New("this is a Dev Sandbox enforced restriction. you are trying to create a CheCluster resource, which is not allowed")) + 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_checluster_request_test.go b/pkg/webhook/validatingwebhook/validate_k8simagepuller_request_test.go similarity index 58% rename from pkg/webhook/validatingwebhook/validate_checluster_request_test.go rename to pkg/webhook/validatingwebhook/validate_k8simagepuller_request_test.go index b110f21f..09f290e5 100644 --- a/pkg/webhook/validatingwebhook/validate_checluster_request_test.go +++ b/pkg/webhook/validatingwebhook/validate_k8simagepuller_request_test.go @@ -17,37 +17,37 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -func TestHandleValidateCheClusterAdmissionRequest(t *testing.T) { +func TestHandleValidateK8sImagePullerAdmissionRequest(t *testing.T) { // given - v := newCheClusterValidator(t) + v := newK8sImagePullerValidator(t) ts := httptest.NewServer(http.HandlerFunc(v.HandleValidate)) defer ts.Close() - t.Run("sandbox user trying to create a CheCluster resource is denied", func(t *testing.T) { + t.Run("sandbox user trying to create a KubernetesImagePuller resource is denied", func(t *testing.T) { // given - req := newCreateCheClusterAdmissionRequest(t, "johnsmith") + req := newCreateK8sImagePullerAdmissionRequest(t, "johnsmith") // when response := v.validate(req) // then - test.VerifyRequestBlocked(t, response, "this is a Dev Sandbox enforced restriction. you are trying to create a CheCluster resource, which is not allowed", "f0b30997-3ac0-49f2-baf4-6eafd123564c") + 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 CheCluster resource is allowed", func(t *testing.T) { + t.Run("crtadmin user trying to create a KubernetesImagePuller resource is allowed", func(t *testing.T) { // given - req := newCreateCheClusterAdmissionRequest(t, "johnsmith-crtadmin") + req := newCreateK8sImagePullerAdmissionRequest(t, "johnsmith-crtadmin") // when response := v.validate(req) // then - test.VerifyRequestAllowed(t, response, "f0b30997-3ac0-49f2-baf4-6eafd123564c") + test.VerifyRequestAllowed(t, response, "b6ae2ab4-782b-11ee-b962-0242ac120002") }) } -func newCheClusterValidator(t *testing.T) *CheClusterRequestValidator { +func newK8sImagePullerValidator(t *testing.T) *K8sImagePullerRequestValidator { s := scheme.Scheme err := userv1.Install(s) require.NoError(t, err) @@ -68,14 +68,14 @@ func newCheClusterValidator(t *testing.T) *CheClusterRequestValidator { }, } cl := fake.NewClientBuilder().WithScheme(s).WithObjects(johnsmithUser, johnsmithAdmin).Build() - return &CheClusterRequestValidator{ + return &K8sImagePullerRequestValidator{ Client: cl, } } -func newCreateCheClusterAdmissionRequest(t *testing.T, username string) []byte { - tmpl, err := template.New("admission request").Parse(createCheClusterJSONTmpl) +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) @@ -83,30 +83,30 @@ func newCreateCheClusterAdmissionRequest(t *testing.T, username string) []byte { return req.Bytes() } -var createCheClusterJSONTmpl = `{ +var createK8sImagePullerJSONTmpl = `{ "kind": "AdmissionReview", "apiVersion": "admission.k8s.io/v1", "request": { - "uid": "f0b30997-3ac0-49f2-baf4-6eafd123564c", + "uid": "b6ae2ab4-782b-11ee-b962-0242ac120002", "kind": { - "group": "org.eclipse.che", - "version": "v2", - "kind": "CheCluster" + "group": "che.eclipse.org", + "version": "v1alpha1", + "kind": "KubernetesImagePuller" }, "resource": { - "group": "org.eclipse.che", - "version": "v2", - "resource": "checlusters" + "group": "che.eclipse.org", + "version": "v1alpha1", + "resource": "kubernetesimagepullers" }, "requestKind": { - "group": "org.eclipse.che", - "version": "v2", - "kind": "CheCluster" + "group": "che.eclipse.org", + "version": "v1alpha1", + "kind": "KubernetesImagePuller" }, "requestResource": { - "group": "org.eclipse.che", - "version": "v2", - "resource": "checlusters" + "group": "che.eclipse.org", + "version": "v1alpha1", + "resource": "kubernetesimagepullers" }, "name": "test", "namespace": "johnsmith-dev", @@ -118,8 +118,8 @@ var createCheClusterJSONTmpl = `{ ] }, "object": { - "apiVersion": "org.eclipse.che/v2", - "kind": "CheCluster", + "apiVersion": "che.eclipse.org/v1alpha1", + "kind": "KubernetesImagePuller", "metadata": { "name": "test", "namespace": "paul-dev"