Skip to content

Commit

Permalink
fix: forward context in validatingwebhooks (#517)
Browse files Browse the repository at this point in the history
* fix: forward context in validatingwebhooks

Signed-off-by: Francesco Ilario <[email protected]>

* add context in tests

Signed-off-by: Francesco Ilario <[email protected]>

---------

Signed-off-by: Francesco Ilario <[email protected]>
  • Loading branch information
filariow authored Jan 9, 2024
1 parent eac7d4c commit 37be293
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ func (v K8sImagePullerRequestValidator) HandleValidate(w http.ResponseWriter, r
respBody = []byte("unable to read the body of the request")
} else {
// validate the request
respBody = v.validate(body)
respBody = v.validate(r.Context(), body)
w.WriteHeader(http.StatusOK)
}
if _, err := io.WriteString(w, string(respBody)); err != nil {
log.Error(err, "unable to write response")
}
}

func (v K8sImagePullerRequestValidator) validate(body []byte) []byte {
func (v K8sImagePullerRequestValidator) validate(ctx context.Context, body []byte) []byte {
log.Info("incoming request", "body", string(body))
admReview := admissionv1.AdmissionReview{}
if _, _, err := deserializer.Decode(body, nil, &admReview); err != nil {
Expand All @@ -52,7 +52,7 @@ func (v K8sImagePullerRequestValidator) validate(body []byte) []byte {
}
//check if the requesting user is a sandbox user
requestingUser := &userv1.User{}
err := v.Client.Get(context.TODO(), types.NamespacedName{
err := v.Client.Get(ctx, types.NamespacedName{
Name: admReview.Request.UserInfo.Username,
}, requestingUser)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validatingwebhook

import (
"bytes"
"context"
"net/http"
"net/http/httptest"
"testing"
Expand All @@ -28,7 +29,7 @@ func TestHandleValidateK8sImagePullerAdmissionRequest(t *testing.T) {
req := newCreateK8sImagePullerAdmissionRequest(t, "johnsmith")

// when
response := v.validate(req)
response := v.validate(context.TODO(), req)

// then
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")
Expand All @@ -39,7 +40,7 @@ func TestHandleValidateK8sImagePullerAdmissionRequest(t *testing.T) {
req := newCreateK8sImagePullerAdmissionRequest(t, "johnsmith-crtadmin")

// when
response := v.validate(req)
response := v.validate(context.TODO(), req)

// then
test.VerifyRequestAllowed(t, response, "b6ae2ab4-782b-11ee-b962-0242ac120002")
Expand Down
6 changes: 3 additions & 3 deletions pkg/webhook/validatingwebhook/validate_rolebinding_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ func (v RoleBindingRequestValidator) HandleValidate(w http.ResponseWriter, r *ht
respBody = []byte("unable to read the body of the request")
} else {
// validate the request
respBody = v.validate(body)
respBody = v.validate(r.Context(), body)
w.WriteHeader(http.StatusOK)
}
if _, err := io.WriteString(w, string(respBody)); err != nil {
log.Error(err, "unable to write response")
}
}

func (v RoleBindingRequestValidator) validate(body []byte) []byte {
func (v RoleBindingRequestValidator) validate(ctx context.Context, body []byte) []byte {
admReview := admissionv1.AdmissionReview{}
if _, _, err := deserializer.Decode(body, nil, &admReview); err != nil {
//sanitize the body
Expand All @@ -72,7 +72,7 @@ func (v RoleBindingRequestValidator) validate(body []byte) []byte {
for _, sub := range subjects {
if containsSubject(subjectsList, sub) {
requestingUser := &userv1.User{}
err := v.Client.Get(context.TODO(), types.NamespacedName{
err := v.Client.Get(ctx, types.NamespacedName{
Name: admReview.Request.UserInfo.Username,
}, requestingUser)

Expand Down
21 changes: 11 additions & 10 deletions pkg/webhook/validatingwebhook/validate_rolebinding_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validatingwebhook

import (
"bytes"
"context"
"io"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -41,31 +42,31 @@ func TestValidateRoleBindingAdmissionRequest(t *testing.T) {
t.Run("sandbox user trying to create rolebinding for all serviceaccounts is denied", func(t *testing.T) {
v := newRoleBindingRequestValidator(t, "johnsmith", true)
// when
response := v.validate(sandboxUserForAllServiceAccountsJSON)
response := v.validate(context.TODO(), sandboxUserForAllServiceAccountsJSON)
// then
test.VerifyRequestBlocked(t, response, "please create a rolebinding for a specific user or service account to avoid this error", "a68769e5-d817-4617-bec5-90efa2bad6f6")
})

t.Run("sandbox user trying to create rolebinding for all serviceaccounts: is denied", func(t *testing.T) {
v := newRoleBindingRequestValidator(t, "johnsmith", true)
// when
response := v.validate(sandboxUserForAllServiceAccountsJSON2)
response := v.validate(context.TODO(), sandboxUserForAllServiceAccountsJSON2)
// then
test.VerifyRequestBlocked(t, response, "please create a rolebinding for a specific user or service account to avoid this error", "a68769e5-d817-4617-bec5-90efa2bad7g8")
})

t.Run("sandbox user trying to create rolebinding for all authenticated users is denied", func(t *testing.T) {
v := newRoleBindingRequestValidator(t, "johnsmith", true)
// when
response := v.validate(sandboxUserForAllUsersJSON)
response := v.validate(context.TODO(), sandboxUserForAllUsersJSON)
// then
test.VerifyRequestBlocked(t, response, "please create a rolebinding for a specific user or service account to avoid this error", "a68769e5-d817-4617-bec5-90efa2bad8k8")
})

t.Run("sandbox user trying to create rolebinding for all authenticated: is denied", func(t *testing.T) {
v := newRoleBindingRequestValidator(t, "johnsmith", true)
// when
response := v.validate(sandboxUserForAllUsersJSON2)
response := v.validate(context.TODO(), sandboxUserForAllUsersJSON2)
// then
test.VerifyRequestBlocked(t, response, "please create a rolebinding for a specific user or service account to avoid this error", "a68769e5-d817-4617-bec5-90efa2bad9l9")
})
Expand All @@ -76,7 +77,7 @@ func TestValidateRoleBindingAdmissionRequestAllowed(t *testing.T) {
t.Run("SA or kubeadmin trying to create rolebinding is allowed", func(t *testing.T) {
v := newRoleBindingRequestValidator(t, "system:kubeadmin", false)
// when user is kubeadmin
response := v.validate(allowedUserJSON)
response := v.validate(context.TODO(), allowedUserJSON)

// then
test.VerifyRequestAllowed(t, response, "a68769e5-d817-4617-bec5-90efa2bad6g7")
Expand All @@ -85,23 +86,23 @@ func TestValidateRoleBindingAdmissionRequestAllowed(t *testing.T) {
t.Run("non sandbox user trying to create rolebinding is allowed", func(t *testing.T) {
v := newRoleBindingRequestValidator(t, "nonsandbox", false)
// when
response := v.validate(nonSandboxUserJSON)
response := v.validate(context.TODO(), nonSandboxUserJSON)
// then
test.VerifyRequestAllowed(t, response, "a68769e5-d817-4617-bec5-90efa2bad6f7")
})

t.Run("unable to find the requesting user, allow request", func(t *testing.T) {
v := newRoleBindingRequestValidator(t, "random-user", true)
// when
response := v.validate(sandboxUserForAllServiceAccountsJSON)
response := v.validate(context.TODO(), sandboxUserForAllServiceAccountsJSON)
// then
test.VerifyRequestAllowed(t, response, "a68769e5-d817-4617-bec5-90efa2bad6f6")
})

t.Run("sandbox user creating a rolebinding for a specific user is allowed", func(t *testing.T) {
v := newRoleBindingRequestValidator(t, "laracroft", true)
//when
response := v.validate(allowedRbJSON)
response := v.validate(context.TODO(), allowedRbJSON)
//then
test.VerifyRequestAllowed(t, response, "a68769e5-d817-4617-bec5-90efa2bad8g8")
})
Expand All @@ -114,7 +115,7 @@ func TestValidateRolebBndingAdmissionRequestFailsOnInvalidJson(t *testing.T) {
v := &RoleBindingRequestValidator{}

// when
response := v.validate(rawJSON)
response := v.validate(context.TODO(), rawJSON)

// then
test.VerifyRequestBlocked(t, response, "cannot unmarshal string into Go value of type struct", "")
Expand All @@ -125,7 +126,7 @@ func TestValidateRolebBndingAdmissionRequestFailsOnInvalidObjectJson(t *testing.
v := &RoleBindingRequestValidator{}

// when
response := v.validate(test.IncorrectRequestObjectJSON)
response := v.validate(context.TODO(), test.IncorrectRequestObjectJSON)

// then
test.VerifyRequestBlocked(t, response, "unable to unmarshal object or object is not a rolebinding", "a68769e5-d817-4617-bec5-90efa2bad6f8")
Expand Down
6 changes: 3 additions & 3 deletions pkg/webhook/validatingwebhook/validate_spacebindingrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ func (v SpaceBindingRequestValidator) HandleValidate(w http.ResponseWriter, r *h
respBody = []byte("unable to read the body of the request")
} else {
// validate the request
respBody = v.validate(body)
respBody = v.validate(r.Context(), body)
w.WriteHeader(http.StatusOK)
}
if _, err := io.WriteString(w, string(respBody)); err != nil {
log.Error(err, "unable to write response")
}
}

func (v SpaceBindingRequestValidator) validate(body []byte) []byte {
func (v SpaceBindingRequestValidator) validate(ctx context.Context, body []byte) []byte {
admReview := admissionv1.AdmissionReview{}
if _, _, err := deserializer.Decode(body, nil, &admReview); err != nil {
//sanitize the body
Expand All @@ -66,7 +66,7 @@ func (v SpaceBindingRequestValidator) validate(body []byte) []byte {
}
// fetch SBR and check that MUR is unchanged
existingSBR := &toolchainv1alpha1.SpaceBindingRequest{}
if err := v.Client.Get(context.TODO(), types.NamespacedName{
if err := v.Client.Get(ctx, types.NamespacedName{
Name: newSBR.GetName(),
Namespace: newSBR.GetNamespace(),
}, existingSBR); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestHandleValidateSpaceBindingRequest(t *testing.T) {
})

// when
response := v.validate(req)
response := v.validate(context.TODO(), req)

// then
// it should not be allowed to update MUR field
Expand All @@ -65,7 +65,7 @@ func TestHandleValidateSpaceBindingRequest(t *testing.T) {
})

// when
response := v.validate(req)
response := v.validate(context.TODO(), req)

// then
// it should be allowed to update SpaceRole field
Expand All @@ -90,7 +90,7 @@ func TestHandleValidateSpaceBindingRequest(t *testing.T) {
})

// when
response := v.validate(req)
response := v.validate(context.TODO(), req)

// then
// it should not be allowed to create new SBR
Expand All @@ -104,7 +104,7 @@ func TestValidateSpaceBindingRequestFailsOnInvalidJson(t *testing.T) {

t.Run("object is not spacebindingrequest", func(t *testing.T) {
// when
response := v.validate(test.IncorrectRequestObjectJSON)
response := v.validate(context.TODO(), test.IncorrectRequestObjectJSON)

// then
test.VerifyRequestBlocked(t, response, "unable to unmarshal object or object is not a spacebindingrequest", "a68769e5-d817-4617-bec5-90efa2bad6f8")
Expand All @@ -113,7 +113,7 @@ func TestValidateSpaceBindingRequestFailsOnInvalidJson(t *testing.T) {
t.Run("json is invalid", func(t *testing.T) {
// when
rawJSON := []byte(`something wrong !`)
response := v.validate(rawJSON)
response := v.validate(context.TODO(), rawJSON)

// then
test.VerifyRequestBlocked(t, response, "cannot unmarshal string into Go value of type struct", "")
Expand All @@ -139,7 +139,7 @@ func TestValidateSpaceBindingRequestFailsOnGettingSBR(t *testing.T) {
})

// when
response := v.validate(req)
response := v.validate(context.TODO(), req)

// then
test.VerifyRequestBlocked(t, response, "unable to validate the SpaceBindingRequest. SpaceBindingRequest.Name: john-sbr: mock error", "xvadsfasdf")
Expand Down

0 comments on commit 37be293

Please sign in to comment.