feat: make SpaceBindingRequest.Spec.MasterUserRecord immutable #470
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,9 +34,9 @@ vendor: | |
|
|
||
| .PHONY: generate-assets | ||
| generate-assets: go-bindata | ||
| @echo "generating webhooks template data..." | ||
| @rm ./pkg/webhook/deploy/templates/template_assets.go 2>/dev/null || true | ||
| @$(GO_BINDATA) -pkg templates -o ./pkg/webhook/deploy/templates/template_assets.go -nocompress -prefix deploy/webhook deploy/webhook | ||
|
There was a problem hiding this comment. wouldn't There was a problem hiding this comment. I've renamed it to |
||
| @echo "generating autoscaler buffer template data..." | ||
| @rm ./pkg/autoscaler/template_assets.go 2>/dev/null || true | ||
| @$(GO_BINDATA) -pkg autoscaler -o ./pkg/autoscaler/template_assets.go -nocompress -prefix deploy/autoscaler deploy/autoscaler | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package test | ||
|
|
||
| var IncorrectRequestObjectJSON = []byte(`{ | ||
| "kind": "AdmissionReview", | ||
| "apiVersion": "admission.k8s.io/v1", | ||
| "request": { | ||
| "uid": "a68769e5-d817-4617-bec5-90efa2bad6f8", | ||
| "name": "busybox1", | ||
| "namespace": "johnsmith-dev", | ||
| "object": { | ||
| "kind": "asbasbf", | ||
| "apiVersion": "v1", | ||
| "metadata": { | ||
| "name": "busybox1", | ||
| "namespace": "johnsmith-dev" | ||
| } | ||
| } | ||
| } | ||
| }`) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package test | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| admissionv1 "k8s.io/api/admission/v1" | ||
| ) | ||
|
|
||
| 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)) | ||
| } | ||
|
|
||
| func VerifyRequestAllowed(t *testing.T, response []byte, UID string) { | ||
| reviewResponse := toReviewResponse(t, response) | ||
| assert.True(t, reviewResponse.Allowed) | ||
| assert.Empty(t, reviewResponse.Result) | ||
| assert.Equal(t, UID, string(reviewResponse.UID)) | ||
| } | ||
|
|
||
| func toReviewResponse(t *testing.T, content []byte) *admissionv1.AdmissionResponse { | ||
| r := admissionv1.AdmissionReview{} | ||
| err := json.Unmarshal(content, &r) | ||
| require.NoError(t, err) | ||
| return r.Response | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,24 +2,23 @@ package validatingwebhook | |
|
|
||
| import ( | ||
| "bytes" | ||
| "io" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
|
|
||
| toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" | ||
| "github.com/codeready-toolchain/member-operator/pkg/webhook/validatingwebhook/test" | ||
|
|
||
| userv1 "github.com/openshift/api/user/v1" | ||
| "github.com/stretchr/testify/assert" | ||
| "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 TestHandleValidateRolebBindingAdmissionRequestBlocked(t *testing.T) { | ||
|
There was a problem hiding this comment. is the extra There was a problem hiding this comment. While trying to fix a typo I've introduced a new one 🤦♂️ . Thanks it should be fixed in 9ddde44 |
||
| v := newRoleBindingRequestValidator(t, "johnsmith", true) | ||
| // given | ||
| ts := httptest.NewServer(http.HandlerFunc(v.HandleValidate)) | ||
|
|
@@ -35,76 +34,76 @@ func TestHandleValidateRolebBndingAdmissionRequestBlocked(t *testing.T) { | |
| require.NoError(t, resp.Body.Close()) | ||
| }() | ||
| assert.NoError(t, err) | ||
| test.VerifyRequestBlocked(t, body, "please create a rolebinding for a specific user or service account to avoid this error", "a68769e5-d817-4617-bec5-90efa2bad6f6") | ||
| } | ||
|
|
||
| func TestValidateRolebBindingAdmissionRequest(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) | ||
| // 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) | ||
| // 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) | ||
| // 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) | ||
| // 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") | ||
| }) | ||
| } | ||
|
|
||
| func TestValidateRolebBindingAdmissionRequestAllowed(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) | ||
|
|
||
| // then | ||
| test.VerifyRequestAllowed(t, response, "a68769e5-d817-4617-bec5-90efa2bad6g7") | ||
| }) | ||
|
|
||
| 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) | ||
| // 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) | ||
| // 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) | ||
| //then | ||
| test.VerifyRequestAllowed(t, response, "a68769e5-d817-4617-bec5-90efa2bad8g8") | ||
| }) | ||
|
|
||
| } | ||
|
|
@@ -118,40 +117,18 @@ func TestValidateRolebBndingAdmissionRequestFailsOnInvalidJson(t *testing.T) { | |
| response := v.validate(rawJSON) | ||
|
|
||
| // then | ||
| test.VerifyRequestBlocked(t, response, "cannot unmarshal string into Go value of type struct", "") | ||
| } | ||
|
|
||
| func TestValidateRolebBndingAdmissionRequestFailsOnInvalidObjectJson(t *testing.T) { | ||
| // given | ||
| v := &RoleBindingRequestValidator{} | ||
|
|
||
| // when | ||
| response := v.validate(test.IncorrectRequestObjectJSON) | ||
|
|
||
| // then | ||
| test.VerifyRequestBlocked(t, response, "unable to unmarshal object or object is not a rolebinding", "a68769e5-d817-4617-bec5-90efa2bad6f8") | ||
| } | ||
|
|
||
| func newRoleBindingRequestValidator(t *testing.T, username string, sandboxUser bool) *RoleBindingRequestValidator { | ||
|
|
@@ -173,23 +150,6 @@ func newRoleBindingRequestValidator(t *testing.T, username string, sandboxUser b | |
| } | ||
| } | ||
|
|
||
| var sandboxUserForAllServiceAccountsJSON = []byte(`{ | ||
| "kind": "AdmissionReview", | ||
| "apiVersion": "admission.k8s.io/v1", | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we also need to cover PATCH, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember adding PATCH initially but was failing, since apparently there is not PATCH operations 🤷♂️ :https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, OK. My bad - I automatically expected that Patch would be one of the operations as well :-)
Thanks for the link and sorry for confusion 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, I expected the same and I'm still a bit confused of why are those the "operations". What is connect for example 😕