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

Add VM validation webhook #607

Merged
merged 14 commits into from
Nov 26, 2024
4 changes: 4 additions & 0 deletions cmd/webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,17 @@ func main() {
sspRequestValidator := &validatingwebhook.SSPRequestValidator{
Client: cl,
}
vmRequestValidator := &validatingwebhook.VMRequestValidator{
Client: cl,
}
mux := http.NewServeMux()

mux.HandleFunc("/mutate-users-pods", mutatingwebhook.HandleMutateUserPods)
mux.HandleFunc("/mutate-virtual-machines", mutatingwebhook.HandleMutateVirtualMachines)
mux.HandleFunc("/validate-users-rolebindings", rolebindingValidator.HandleValidate)
mux.HandleFunc("/validate-spacebindingrequests", spacebindingrequestValidator.HandleValidate)
mux.HandleFunc("/validate-ssprequests", sspRequestValidator.HandleValidate) // SSP is a CNV specific resource
mux.HandleFunc("/validate-vmrequests", vmRequestValidator.HandleValidate)

webhookServer := &http.Server{ //nolint:gosec //TODO: configure ReadHeaderTimeout (gosec G112)
Addr: ":8443",
Expand Down
24 changes: 24 additions & 0 deletions deploy/webhook/member-operator-webhook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,30 @@ objects:
namespaceSelector:
matchLabels:
toolchain.dev.openshift.com/provider: codeready-toolchain
- name: users.virtualmachines.validating.webhook.sandbox
admissionReviewVersions:
- v1
clientConfig:
caBundle: ${CA_BUNDLE}
service:
name: member-operator-webhook
namespace: ${NAMESPACE}
path: "/validate-vmrequests"
port: 443
matchPolicy: Equivalent
rules:
- operations: ["CREATE", "UPDATE"]
apiGroups: ["kubevirt.io"]
apiVersions: ["*"]
resources: ["virtualmachines"]
scope: "Namespaced"
sideEffects: None
timeoutSeconds: 5
reinvocationPolicy: Never
failurePolicy: Fail
namespaceSelector:
matchLabels:
toolchain.dev.openshift.com/provider: codeready-toolchain
parameters:
- name: NAMESPACE
value: 'toolchain-member-operator'
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-%[2]s"},"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-ssprequests","port": 443}},"failurePolicy": "Fail","matchPolicy": "Equivalent","name": "users.virtualmachines.ssp.webhook.sandbox","namespaceSelector": {"matchLabels": {"toolchain.dev.openshift.com/provider": "codeready-toolchain"}},"reinvocationPolicy": "Never","rules": [{"apiGroups": ["ssp.kubevirt.io"],"apiVersions": ["*"],"operations": ["CREATE","UPDATE"],"resources": ["ssps"],"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-%[2]s"},"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-ssprequests","port": 443}},"failurePolicy": "Fail","matchPolicy": "Equivalent","name": "users.virtualmachines.ssp.webhook.sandbox","namespaceSelector": {"matchLabels": {"toolchain.dev.openshift.com/provider": "codeready-toolchain"}},"reinvocationPolicy": "Never","rules": [{"apiGroups": ["ssp.kubevirt.io"],"apiVersions": ["*"],"operations": ["CREATE","UPDATE"],"resources": ["ssps"],"scope": "Namespaced"}],"sideEffects": "None","timeoutSeconds": 5},{"admissionReviewVersions": ["v1"],"clientConfig": {"caBundle": "%[1]s","service": {"name": "member-operator-webhook","namespace": "%[2]s","path": "/validate-vmrequests","port": 443}},"failurePolicy": "Fail","matchPolicy": "Equivalent","name": "users.virtualmachines.validating.webhook.sandbox","namespaceSelector": {"matchLabels": {"toolchain.dev.openshift.com/provider": "codeready-toolchain"}},"reinvocationPolicy": "Never","rules": [{"apiGroups": ["kubevirt.io"],"apiVersions": ["*"],"operations": ["CREATE","UPDATE"],"resources": ["virtualmachines"],"scope": "Namespaced"}],"sideEffects": "None","timeoutSeconds": 5}]}`, caBundle, namespace)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not related to this PR - but TBH I find this a bit difficult to maintain , maybe in the future we could remove/simplify this check/test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree 💯

}

func serviceAccount(namespace string) string {
Expand Down
4 changes: 2 additions & 2 deletions pkg/webhook/validatingwebhook/test/verify_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
func VerifyRequestBlocked(t *testing.T, response []byte, msg string, UID string) {
reviewResponse := toReviewResponse(t, response)
assert.False(t, reviewResponse.Allowed)
assert.NotEmpty(t, reviewResponse.Result)
assert.Contains(t, reviewResponse.Result.Message, msg)
assert.Equal(t, UID, string(reviewResponse.UID))
require.NotEmpty(t, reviewResponse.Result)
assert.Contains(t, reviewResponse.Result.Message, msg)
}

func VerifyRequestAllowed(t *testing.T, response []byte, UID string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestValidateSSPAdmissionRequest(t *testing.T) {
t.Run("sandbox user trying to update a SSP resource is denied", func(t *testing.T) {
// given
v := newSSPRequestValidator(t, "johnsmith", true)
req := newCreateSSPAdmissionRequest(t, SSPAdmReviewTmplParams{"CREATE", "johnsmith"})
req := newCreateSSPAdmissionRequest(t, SSPAdmReviewTmplParams{"UPDATE", "johnsmith"})

// when
response := v.validate(context.TODO(), req)
Expand Down
76 changes: 76 additions & 0 deletions pkg/webhook/validatingwebhook/validate_vm_request.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package validatingwebhook

import (
"html"
"io"
"net/http"

"github.com/pkg/errors"
admissionv1 "k8s.io/api/admission/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
runtimeClient "sigs.k8s.io/controller-runtime/pkg/client"
)

type VMRequestValidator struct {
Client runtimeClient.Client
}

func (v VMRequestValidator) 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")
}

Check warning on line 24 in pkg/webhook/validatingwebhook/validate_vm_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_vm_request.go#L23-L24

Added lines #L23 - L24 were not covered by tests
}()
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")

Check warning on line 29 in pkg/webhook/validatingwebhook/validate_vm_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_vm_request.go#L27-L29

Added lines #L27 - L29 were not covered by tests
} else {
// validate the request
respBody = v.validate(body)
w.WriteHeader(http.StatusOK)
}
if _, err := io.WriteString(w, string(respBody)); err != nil {
log.Error(err, "unable to write response")
}

Check warning on line 37 in pkg/webhook/validatingwebhook/validate_vm_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_vm_request.go#L36-L37

Added lines #L36 - L37 were not covered by tests
}

func (v VMRequestValidator) validate(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 warning on line 48 in pkg/webhook/validatingwebhook/validate_vm_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_vm_request.go#L44-L48

Added lines #L44 - L48 were not covered by tests

unstructuredRequestObj := &unstructured.Unstructured{}
if err := unstructuredRequestObj.UnmarshalJSON(admReview.Request.Object.Raw); err != nil {
log.Error(err, "unable to check runStrategy in VirtualMachine", "VirtualMachine", unstructuredRequestObj)
return denyAdmissionRequest(admReview, errors.New("failed to validate VirtualMachine request"))
}

Check warning on line 54 in pkg/webhook/validatingwebhook/validate_vm_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_vm_request.go#L52-L54

Added lines #L52 - L54 were not covered by tests

hasRunStrategy, err := hasRunningStrategy(unstructuredRequestObj)
if err != nil {
log.Error(err, "failed to unmarshal VirtualMachine json object", "AdmissionReview", admReview)
return denyAdmissionRequest(admReview, errors.New("failed to validate VirtualMachine request"))
}

Check warning on line 60 in pkg/webhook/validatingwebhook/validate_vm_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_vm_request.go#L58-L60

Added lines #L58 - L60 were not covered by tests
if hasRunStrategy {
log.Info("sandbox user is trying to create a VM with RunStrategy configured", "AdmissionReview", admReview) // not allowed because it interferes with the Dev Sandbox Idler
return denyAdmissionRequest(admReview, errors.New("this is a Dev Sandbox enforced restriction. Configuring RunStrategy is not allowed"))
}
// at this point, it is clear the user isn't a sandbox user, allow request
return allowAdmissionRequest(admReview)
}

func hasRunningStrategy(unstructuredObj *unstructured.Unstructured) (bool, error) {
_, runStrategyFound, err := unstructured.NestedString(unstructuredObj.Object, "spec", "runStrategy")
if err != nil {
return runStrategyFound, err
}

Check warning on line 73 in pkg/webhook/validatingwebhook/validate_vm_request.go

View check run for this annotation

Codecov / codecov/patch

pkg/webhook/validatingwebhook/validate_vm_request.go#L72-L73

Added lines #L72 - L73 were not covered by tests

return runStrategyFound, nil
}
228 changes: 228 additions & 0 deletions pkg/webhook/validatingwebhook/validate_vm_request_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
package validatingwebhook

import (
"bytes"
"io"
"net/http"
"net/http/httptest"
"testing"
"text/template"

"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 TestHandleValidateVMAdmissionRequestBlocked(t *testing.T) {
v := newVMRequestValidator(t, "johnsmith")
// given
ts := httptest.NewServer(http.HandlerFunc(v.HandleValidate))
defer ts.Close()

// when
resp, err := http.Post(ts.URL, "application/json", bytes.NewBuffer(newCreateVMAdmissionRequest(t, VMAdmReviewTmplParams{"CREATE", "johnsmith"}, createVMWithRunStrategyJSONTmpl)))

// then
require.NoError(t, err)
body, err := io.ReadAll(resp.Body)
defer func() {
require.NoError(t, resp.Body.Close())
}()
require.NoError(t, err)
test.VerifyRequestBlocked(t, body, "this is a Dev Sandbox enforced restriction. Configuring RunStrategy is not allowed", "b6ae2ab4-782b-11ee-b962-0242ac120002")
}

func TestValidateVMAdmissionRequest(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not something to address now - but should we have all those tests going through the handler ? I mean merging this one with the TestHandleValidateVMAdmissionRequestBlocked into something like TestHandleValidateVMAdmissionRequest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I should have done it that way. Done 71f6795

t.Run("sandbox user trying to create a VM resource with RunStrategy is denied", func(t *testing.T) {
// given
v := newVMRequestValidator(t, "johnsmith")
req := newCreateVMAdmissionRequest(t, VMAdmReviewTmplParams{"CREATE", "johnsmith"}, createVMWithRunStrategyJSONTmpl)

// when
response := v.validate(req)

// then
test.VerifyRequestBlocked(t, response, "this is a Dev Sandbox enforced restriction. Configuring RunStrategy is not allowed", "b6ae2ab4-782b-11ee-b962-0242ac120002")
})

t.Run("sandbox user trying to update a VM resource with RunStrategy is denied", func(t *testing.T) {
// given
v := newVMRequestValidator(t, "johnsmith")
req := newCreateVMAdmissionRequest(t, VMAdmReviewTmplParams{"UPDATE", "johnsmith"}, createVMWithRunStrategyJSONTmpl)

// when
response := v.validate(req)

// then
test.VerifyRequestBlocked(t, response, "this is a Dev Sandbox enforced restriction. Configuring RunStrategy is not allowed", "b6ae2ab4-782b-11ee-b962-0242ac120002")
})

t.Run("sandbox user trying to create a VM resource without RunStrategy is allowed", func(t *testing.T) {
// given
v := newVMRequestValidator(t, "johnsmith")
req := newCreateVMAdmissionRequest(t, VMAdmReviewTmplParams{"CREATE", "johnsmith"}, createVMWithoutRunStrategyJSONTmpl)

// when
response := v.validate(req)

// then
test.VerifyRequestAllowed(t, response, "b6ae2ab4-782b-11ee-b962-0242ac120002")
})

t.Run("sandbox user trying to update a VM resource without RunStrategy is allowed", func(t *testing.T) {
// given
v := newVMRequestValidator(t, "johnsmith")
req := newCreateVMAdmissionRequest(t, VMAdmReviewTmplParams{"UPDATE", "johnsmith"}, createVMWithoutRunStrategyJSONTmpl)

// when
response := v.validate(req)

// then
test.VerifyRequestAllowed(t, response, "b6ae2ab4-782b-11ee-b962-0242ac120002")
})

}

func newVMRequestValidator(t *testing.T, username string) *VMRequestValidator {

Check failure on line 90 in pkg/webhook/validatingwebhook/validate_vm_request_test.go

View workflow job for this annotation

GitHub Actions / GolangCI Lint

`newVMRequestValidator` - `username` always receives `"johnsmith"` (unparam)
s := scheme.Scheme
err := userv1.Install(s)
require.NoError(t, err)
testUser := &userv1.User{
ObjectMeta: metav1.ObjectMeta{
Name: username,
},
}

cl := fake.NewClientBuilder().WithScheme(s).WithObjects(testUser).Build()
return &VMRequestValidator{
Client: cl,
}

}

func newCreateVMAdmissionRequest(t *testing.T, params VMAdmReviewTmplParams, tmplJSON string) []byte {
tmpl, err := template.New("admission request").Parse(tmplJSON)
require.NoError(t, err)
req := &bytes.Buffer{}
err = tmpl.Execute(req, params)
require.NoError(t, err)
return req.Bytes()
}

type VMAdmReviewTmplParams struct {
ReqType string
Username string
}

var createVMWithRunStrategyJSONTmpl = `{
"kind": "AdmissionReview",
"apiVersion": "admission.k8s.io/v1",
"request": {
"uid": "b6ae2ab4-782b-11ee-b962-0242ac120002",
"kind": {
"group": "kubevirt.io",
"version": "v1",
"kind": "VirtualMachine"
},
"resource": {
"group": "kubevirt.io",
"version": "v1",
"resource": "virtualmachines"
},
"requestKind": {
"group": "kubevirt.io",
"version": "v1",
"kind": "VirtualMachine"
},
"requestResource": {
"group": "kubevirt.io",
"version": "v1",
"resource": "virtualmachines"
},
"name": "test",
"namespace": "{{.Username}}-dev",
"operation": "{{.ReqType}}",
"userInfo": {
"username": "{{.Username}}",
"groups": [
"system:authenticated"
]
},
"object": {
"apiVersion": "kubevirt.io",
"kind": "VirtualMachine",
"metadata": {
"name": "{{.Username}}",
"namespace": "{{.Username}}-dev"
},
"spec": {
"runStrategy": "Always"
}
},
"oldObject": null,
"dryRun": false,
"options": {
"kind": "CreateOptions",
"apiVersion": "meta.k8s.io/v1",
"fieldManager": "kubectl-client-side-apply",
"fieldValidation": "Ignore"
}
}
}`

var createVMWithoutRunStrategyJSONTmpl = `{
"kind": "AdmissionReview",
"apiVersion": "admission.k8s.io/v1",
"request": {
"uid": "b6ae2ab4-782b-11ee-b962-0242ac120002",
"kind": {
"group": "kubevirt.io",
"version": "v1",
"kind": "VirtualMachine"
},
"resource": {
"group": "kubevirt.io",
"version": "v1",
"resource": "virtualmachines"
},
"requestKind": {
"group": "kubevirt.io",
"version": "v1",
"kind": "VirtualMachine"
},
"requestResource": {
"group": "kubevirt.io",
"version": "v1",
"resource": "virtualmachines"
},
"name": "test",
"namespace": "{{.Username}}-dev",
"operation": "{{.ReqType}}",
"userInfo": {
"username": "{{.Username}}",
"groups": [
"system:authenticated"
]
},
"object": {
"apiVersion": "kubevirt.io",
"kind": "VirtualMachine",
"metadata": {
"name": "{{.Username}}",
"namespace": "{{.Username}}-dev"
}
},
"oldObject": null,
"dryRun": false,
"options": {
"kind": "CreateOptions",
"apiVersion": "meta.k8s.io/v1",
"fieldManager": "kubectl-client-side-apply",
"fieldValidation": "Ignore"
}
}
}`
Loading