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 <[email protected]>
  • Loading branch information
dkwon17 committed Nov 9, 2023
1 parent 0a2f99a commit 3095ff5
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 54 deletions.
4 changes: 2 additions & 2 deletions cmd/webhook/main.go
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
32 changes: 16 additions & 16 deletions deploy/webhook/member-operator-webhook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -204,22 +204,27 @@ objects:
namespaceSelector:
matchLabels:
toolchain.dev.openshift.com/provider: codeready-toolchain
- name: users.checlusters.webhook.sandbox
# 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: <Space.Name>-checksum(<Space.Name>-<MasterUserRecord.Name>),
# thus changing it will trigger an updated of the SpaceBinding content but the name will still be based on the old MUR name.
# The webhook code is available at member-operator/pkg/webhook/validatingwebhook/validate_spacebindingrequest.go
- name: users.spacebindingrequests.webhook.sandbox
admissionReviewVersions:
- v1
clientConfig:
caBundle: ${CA_BUNDLE}
service:
name: member-operator-webhook
namespace: ${NAMESPACE}
path: "/validate-users-checlusters"
path: "/validate-spacebindingrequests"
port: 443
matchPolicy: Equivalent
rules:
- operations: ["CREATE"]
apiGroups: ["org.eclipse.che"]
apiVersions: ["v2"]
resources: ["checlusters"]
- operations: ["CREATE", "UPDATE"]
apiGroups: ["toolchain.dev.openshift.com"]
apiVersions: ["v1alpha1"]
resources: ["spacebindingrequests"]
scope: "Namespaced"
sideEffects: None
timeoutSeconds: 5
Expand All @@ -228,27 +233,22 @@ objects:
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: <Space.Name>-checksum(<Space.Name>-<MasterUserRecord.Name>),
# thus changing it will trigger an updated of the SpaceBinding content but the name will still be based on the old MUR name.
# The webhook code is available at member-operator/pkg/webhook/validatingwebhook/validate_spacebindingrequest.go
- name: users.spacebindingrequests.webhook.sandbox
- name: users.kubernetesimagepullers.webhook.sandbox
admissionReviewVersions:
- v1
clientConfig:
caBundle: ${CA_BUNDLE}
service:
name: member-operator-webhook
namespace: ${NAMESPACE}
path: "/validate-spacebindingrequests"
path: "/validate-users-kubernetesimagepullers"
port: 443
matchPolicy: Equivalent
rules:
- operations: ["CREATE", "UPDATE"]
apiGroups: ["toolchain.dev.openshift.com"]
- operations: ["CREATE"]
apiGroups: ["che.eclipse.org"]
apiVersions: ["v1alpha1"]
resources: ["spacebindingrequests"]
resources: ["kubernetesimagepullers"]
scope: "Namespaced"
sideEffects: None
timeoutSeconds: 5
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/deploy/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},{"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}]}`, caBundle, namespace)
}

func serviceAccount(namespace string) string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,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 +42,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 @@ -63,12 +63,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)
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

0 comments on commit 3095ff5

Please sign in to comment.