Skip to content

Commit

Permalink
feat: make SpaceBindingRequest.Spec.MasterUserRecord immutable (#792)
Browse files Browse the repository at this point in the history
* add test for sbr MUR update
  • Loading branch information
mfrancisc authored Sep 11, 2023
1 parent 3f23df6 commit 0edcbef
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 10 deletions.
17 changes: 10 additions & 7 deletions test/e2e/parallel/spacebindingrequest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"sort"
"testing"
"time"

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/gofrs/uuid"
Expand Down Expand Up @@ -194,10 +195,10 @@ func TestUpdateSpaceBindingRequest(t *testing.T) {
require.NoError(t, err)
})

t.Run("update space binding request MasterUserRecord", func(t *testing.T) {
t.Run("update space binding request MasterUserRecord is denied", func(t *testing.T) {
// when
space, spaceBindingRequest, _ := NewSpaceBindingRequest(t, awaitilities, memberAwait, hostAwait, "admin")
// let's create another MUR that will have access to the space
// let's create another MUR that will be used for the update request
username := uuid.Must(uuid.NewV4()).String()
_, newmur := NewSignupRequest(awaitilities).
Username(username).
Expand All @@ -207,24 +208,26 @@ func TestUpdateSpaceBindingRequest(t *testing.T) {
RequireConditions(ConditionSet(Default(), ApprovedByAdmin())...).
NoSpace().
WaitForMUR().Execute(t).Resources()
// and we update the MUR in the SBR
_, err := memberAwait.UpdateSpaceBindingRequest(t, types.NamespacedName{Namespace: spaceBindingRequest.Namespace, Name: spaceBindingRequest.Name},
// and we try to update the MUR in the SBR
// with lower timeout since it will fail as expected
_, err := memberAwait.WithRetryOptions(TimeoutOption(time.Second*2)).UpdateSpaceBindingRequest(t, types.NamespacedName{Namespace: spaceBindingRequest.Namespace, Name: spaceBindingRequest.Name},
func(s *toolchainv1alpha1.SpaceBindingRequest) {
s.Spec.MasterUserRecord = newmur.GetName() // set to the new MUR
},
)
require.NoError(t, err)
require.Error(t, err) // an error from the validating webhook is expected when trying to update the MUR field
require.EqualError(t, err, "admission webhook \"users.spacebindingrequests.webhook.sandbox\" denied the request: SpaceBindingRequest.MasterUserRecord field cannot be changed. Consider deleting and creating a new SpaceBindingRequest resource")

//then
// wait for both SpaceBindingRequest and SpaceBinding to have same MUR
spaceBindingRequest, err = memberAwait.WaitForSpaceBindingRequest(t, types.NamespacedName{Namespace: spaceBindingRequest.GetNamespace(), Name: spaceBindingRequest.GetName()},
UntilSpaceBindingRequestHasConditions(spacebindingrequesttestcommon.Ready()),
UntilSpaceBindingRequestHasSpecSpaceRole(spaceBindingRequest.Spec.SpaceRole),
UntilSpaceBindingRequestHasSpecMUR(newmur.GetName()), // new MUR
UntilSpaceBindingRequestHasSpecMUR(spaceBindingRequest.Spec.MasterUserRecord), // MUR should be the same
)
require.NoError(t, err)
_, err = hostAwait.WaitForSpaceBinding(t, spaceBindingRequest.Spec.MasterUserRecord, space.Name,
UntilSpaceBindingHasMurName(newmur.GetName()), // has new MUR
UntilSpaceBindingHasMurName(spaceBindingRequest.Spec.MasterUserRecord), // MUR should be the same
UntilSpaceBindingHasSpaceName(space.Name),
UntilSpaceBindingHasSpaceRole(spaceBindingRequest.Spec.SpaceRole),
)
Expand Down
30 changes: 27 additions & 3 deletions testsupport/wait/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@ func (a *MemberAwaitility) UpdateSpaceBindingRequest(t *testing.T, spaceBindingR
modifySpaceBindingRequest(freshSpaceBindingRequest)
if err := a.Client.Update(context.TODO(), freshSpaceBindingRequest); err != nil {
t.Logf("error updating SpaceBindingRequest '%s' in namespace '%s': %s. Will retry again...", spaceBindingRequestNamespacedName.Name, spaceBindingRequestNamespacedName.Name, err.Error())
return false, nil
return false, err
}
sr = freshSpaceBindingRequest
return true, nil
Expand Down Expand Up @@ -2150,7 +2150,7 @@ func (a *MemberAwaitility) WaitForMemberWebhooks(t *testing.T, image string) {
a.waitForWebhookDeployment(t, image)
ca := a.verifySecret(t)
a.verifyUserPodWebhookConfig(t, ca)
a.verifyUsersRolebindingsWebhookConfig(t, ca)
a.verifyValidatingWebhookConfig(t, ca)
}

func (a *MemberAwaitility) waitForUsersPodPriorityClass(t *testing.T) {
Expand Down Expand Up @@ -2267,7 +2267,7 @@ func (a *MemberAwaitility) verifyUserPodWebhookConfig(t *testing.T, ca []byte) {
assert.Equal(t, admv1.NamespacedScope, *rule.Scope)
}

func (a *MemberAwaitility) verifyUsersRolebindingsWebhookConfig(t *testing.T, ca []byte) {
func (a *MemberAwaitility) verifyValidatingWebhookConfig(t *testing.T, ca []byte) {
t.Logf("checking ValidatingWebhookConfiguration '%s'", "member-operator-validating-webhook")
actualValWbhConf := &admv1.ValidatingWebhookConfiguration{}
a.waitForResource(t, "", "member-operator-validating-webhook", actualValWbhConf)
Expand Down Expand Up @@ -2318,6 +2318,30 @@ func (a *MemberAwaitility) verifyUsersRolebindingsWebhookConfig(t *testing.T, ca
assert.Equal(t, []string{"checlusters"}, checlusterRule.Resources)
assert.Equal(t, admv1.NamespacedScope, *checlusterRule.Scope)

// TODO: uncomment once: https://github.com/codeready-toolchain/member-operator/pull/470
// is merged.
//spacebindingrequestWebhook := actualValWbhConf.Webhooks[2]
//assert.Equal(t, "users.spacebindingrequests.webhook.sandbox", spacebindingrequestWebhook.Name)
//assert.Equal(t, []string{"v1"}, spacebindingrequestWebhook.AdmissionReviewVersions)
//assert.Equal(t, admv1.SideEffectClassNone, *spacebindingrequestWebhook.SideEffects)
//assert.Equal(t, int32(5), *spacebindingrequestWebhook.TimeoutSeconds)
//assert.Equal(t, admv1.Ignore, *spacebindingrequestWebhook.FailurePolicy)
//assert.Equal(t, admv1.Equivalent, *spacebindingrequestWebhook.MatchPolicy)
//assert.Equal(t, codereadyToolchainProviderLabel, spacebindingrequestWebhook.NamespaceSelector.MatchLabels)
//assert.Equal(t, ca, spacebindingrequestWebhook.ClientConfig.CABundle)
//assert.Equal(t, "member-operator-webhook", spacebindingrequestWebhook.ClientConfig.Service.Name)
//assert.Equal(t, a.Namespace, spacebindingrequestWebhook.ClientConfig.Service.Namespace)
//assert.Equal(t, "/validate-spacebindingrequests", *spacebindingrequestWebhook.ClientConfig.Service.Path)
//assert.Equal(t, int32(443), *spacebindingrequestWebhook.ClientConfig.Service.Port)
//require.Len(t, spacebindingrequestWebhook.Rules, 1)
//
//spacebindingrequestRule := spacebindingrequestWebhook.Rules[0]
//assert.Equal(t, []admv1.OperationType{admv1.Create, admv1.Update}, spacebindingrequestRule.Operations)
//assert.Equal(t, []string{"toolchain.dev.openshift.com"}, spacebindingrequestRule.APIGroups)
//assert.Equal(t, []string{"v1alpha1"}, spacebindingrequestRule.APIVersions)
//assert.Equal(t, []string{"spacebindingrequests"}, spacebindingrequestRule.Resources)
//assert.Equal(t, admv1.NamespacedScope, *spacebindingrequestRule.Scope)

}

func (a *MemberAwaitility) WaitForAutoscalingBufferApp(t *testing.T) {
Expand Down

0 comments on commit 0edcbef

Please sign in to comment.