Skip to content

Commit

Permalink
Prevent regular users from creating KIP resource, delete CheCluster
Browse files Browse the repository at this point in the history 
webhook

Signed-off-by: David Kwon <dakwon@redhat.com>
  • Loading branch information
@dkwon17
dkwon17 committed Nov 9, 2023
1 parent 0a2f99a commit 0f17536
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 49 deletions.
4 changes: 2 additions & 2 deletions cmd/webhook/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func main() {
rolebindingValidator := &validatingwebhook.RoleBindingRequestValidator{
Client: cl,
}
checlusterValidator := &validatingwebhook.CheClusterRequestValidator{
k8sImagePullerRequestValidator := &validatingwebhook.K8sImagePullerRequestValidator{
Client: cl,
}
spacebindingrequestValidator := &validatingwebhook.SpaceBindingRequestValidator{
Expand All @@ -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)
Expand Down
14 changes: 9 additions & 5 deletions deploy/webhook/member-operator-webhook.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,22 +204,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
View file
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
20 changes: 7 additions & 13 deletions ...ingwebhook/validate_checluster_request.go → ...ebhook/validate_k8simagepuller_request.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"html"
"io"
"net/http"
"strings"

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

Expand All @@ -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() {
Expand All @@ -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 {
Expand All @@ -51,24 +50,19 @@ 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{
Name: admReview.Request.UserInfo.Username,
}, 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)
Expand Down
56 changes: 28 additions & 28 deletions ...bhook/validate_checluster_request_test.go → ...k/validate_k8simagepuller_request_test.go
View file
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

0 comments on commit 0f17536

Please sign in to comment.