Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a webhook to prevent regular users from creating a KubernetesImagePuller resource #493

Merged
merged 5 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func main() {
rolebindingValidator := &validatingwebhook.RoleBindingRequestValidator{
Client: cl,
}
checlusterValidator := &validatingwebhook.CheClusterRequestValidator{
k8sImagePullerRequestValidator := &validatingwebhook.K8sImagePullerRequestValidator{
Client: cl,
}
spacebindingrequestValidator := &validatingwebhook.SpaceBindingRequestValidator{
Expand All @@ -113,7 +113,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)
Expand Down
14 changes: 9 additions & 5 deletions deploy/webhook/member-operator-webhook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -218,22 +218,26 @@ 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:
caBundle: ${CA_BUNDLE}
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
Expand Down
3 changes: 2 additions & 1 deletion pkg/webhook/deploy/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"html"
"io"
"net/http"
"strings"

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"

Expand All @@ -16,11 +15,11 @@
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) {

Check warning on line 22 in pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go#L22

Added line #L22 was not covered by tests
var respBody []byte
body, err := io.ReadAll(r.Body)
defer func() {
Expand All @@ -42,7 +41,7 @@
}
}

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 {
Expand All @@ -51,24 +50,19 @@
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{
Name: admReview.Request.UserInfo.Username,
}, requestingUser)

if err != nil {
alexeykazakov marked this conversation as resolved.
Show resolved Hide resolved
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"))

Check warning on line 61 in pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_k8simagepuller_request.go#L60-L61

Added lines #L60 - L61 were not covered by tests
}
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -68,45 +68,45 @@ 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)
require.NoError(t, err)
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",
Expand All @@ -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"
Expand Down
Loading