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.

Sign up for GitHub

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

apply generic update #1078

Merged
merged 3 commits into from
Dec 6, 2024
Merged

apply generic update #1078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e5f5ced
apply generic update
rsoaresd Dec 5, 2024
54f7c78
Merge branch 'master' into apply_generic_update
rsoaresd Dec 5, 2024
27a9a80
requested changes
rsoaresd Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/e2e/no_user_identity_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestCreationOfUserAndIdentityIsSkipped(t *testing.T) {
t.Run("user and identity stay there when user is deactivated", func(t *testing.T) {
// when
userSignup, err := wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}).
Update(signup.Name,
Update(signup.Name, hostAwait.Namespace,
Copy link
Collaborator

Choose a reason for hiding this comment

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

seeing all these changes, I'm wondering if we really need to provide the namespace explicitly? couldn't it be read from the provided "awaitility" instance?
Just to be clear, it's fine to keep it as it is for now and address it later if we figure out that it would simplify the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were some update functions like UpdateServiceAccount, UpdateSpaceRequest, and UpdateConfigMap that had the namespace as input, that's why I changed a little the generic update function

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, I see 👍 Thanks for clarification

func(us *toolchainv1alpha1.UserSignup) {
states.SetDeactivated(us, true)
})
Expand Down
13 changes: 7 additions & 6 deletions test/e2e/parallel/e2e_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func TestE2EFlow(t *testing.T) {

// when
_, err = wait.For(t, memberAwait.Awaitility, &userv1.User{}).
Update(user.Name, func(u *userv1.User) {
Update(user.Name, memberAwait.Namespace, func(u *userv1.User) {
u.Identities = []string{}
})

Expand All @@ -180,7 +180,7 @@ func TestE2EFlow(t *testing.T) {

// when
_, err = wait.For(t, memberAwait.Awaitility, &userv1.Identity{}).
Update(identity.Name, func(i *userv1.Identity) {
Update(identity.Name, memberAwait.Namespace, func(i *userv1.Identity) {
i.User = corev1.ObjectReference{Name: "", UID: ""}
})

Expand Down Expand Up @@ -325,9 +325,10 @@ func TestE2EFlow(t *testing.T) {

t.Run("remove finalizer", func(t *testing.T) {
// when removing the finalizer from the CM
_, err = memberAwait.UpdateConfigMap(t, cm.Namespace, cmName, func(cm *corev1.ConfigMap) {
cm.Finalizers = nil
})
_, err = wait.For(t, memberAwait.Awaitility, &corev1.ConfigMap{}).
Update(cmName, cm.Namespace, func(cm *corev1.ConfigMap) {
cm.Finalizers = nil
})
require.NoError(t, err)

// then check remaining resources are deleted
Expand Down Expand Up @@ -488,7 +489,7 @@ func TestE2EFlow(t *testing.T) {

// Now deactivate the UserSignup
userSignup, err := wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}).
Update(originalSubJohnSignup.Name,
Update(originalSubJohnSignup.Name, hostAwait.Namespace,
func(us *toolchainv1alpha1.UserSignup) {
states.SetDeactivated(us, true)
})
Expand Down
24 changes: 14 additions & 10 deletions test/e2e/parallel/nstemplatetier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func TestResetDeactivatingStateWhenPromotingUser(t *testing.T) {
Execute(t)
// Set the deactivating state on the UserSignup
updatedUserSignup, err := wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}).
Update(user.UserSignup.Name,
Update(user.UserSignup.Name, hostAwait.Namespace,
func(us *toolchainv1alpha1.UserSignup) {
states.SetDeactivating(us, true)
})
Expand Down Expand Up @@ -344,9 +344,11 @@ func TestFeatureToggles(t *testing.T) {
// when

// Now let's disable the feature for the Space by removing the feature annotation
_, err := hostAwait.UpdateSpace(t, user.Space.Name, func(s *toolchainv1alpha1.Space) {
delete(s.Annotations, toolchainv1alpha1.FeatureToggleNameAnnotationKey)
})
_, err := wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.Space{}).
Update(user.Space.Name, hostAwait.Namespace, func(s *toolchainv1alpha1.Space) {
delete(s.Annotations, toolchainv1alpha1.FeatureToggleNameAnnotationKey)
})

require.NoError(t, err)

// then
Expand All @@ -359,12 +361,14 @@ func TestFeatureToggles(t *testing.T) {
// when

// Now let's re-enable the feature for the Space by restoring the feature annotation
_, err := hostAwait.UpdateSpace(t, user.Space.Name, func(s *toolchainv1alpha1.Space) {
if s.Annotations == nil {
s.Annotations = make(map[string]string)
}
s.Annotations[toolchainv1alpha1.FeatureToggleNameAnnotationKey] = "test-feature"
})
_, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.Space{}).
Update(user.Space.Name, hostAwait.Namespace, func(s *toolchainv1alpha1.Space) {
if s.Annotations == nil {
s.Annotations = make(map[string]string)
}
s.Annotations[toolchainv1alpha1.FeatureToggleNameAnnotationKey] = "test-feature"
})

require.NoError(t, err)

// then
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/parallel/proxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
commonproxy "github.com/codeready-toolchain/toolchain-common/pkg/proxy"
"github.com/codeready-toolchain/toolchain-common/pkg/test"
testspace "github.com/codeready-toolchain/toolchain-common/pkg/test/space"
spacebindingrequesttestcommon "github.com/codeready-toolchain/toolchain-common/pkg/test/spacebindingrequest"
. "github.com/codeready-toolchain/toolchain-e2e/testsupport"
Expand Down Expand Up @@ -1084,9 +1083,10 @@ func TestSpaceLister(t *testing.T) {
require.NoError(t, err)

// when
_, err = memberAwait.UpdateSpaceBindingRequest(t, test.NamespacedName(failingSBR.Namespace, failingSBR.Name), func(sbr *toolchainv1alpha1.SpaceBindingRequest) {
sbr.Spec.SpaceRole = "admin"
})
_, err = wait.For(t, memberAwait.Awaitility, &toolchainv1alpha1.SpaceBindingRequest{}).
Update(failingSBR.Name, failingSBR.Namespace, func(sbr *toolchainv1alpha1.SpaceBindingRequest) {
sbr.Spec.SpaceRole = "admin"
})

// then
require.NoError(t, err)
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/parallel/registration_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ func TestSignupOK(t *testing.T) {
assert.Equal(t, "error creating UserSignup resource", mp["details"])

userSignup, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}).
Update(userSignup.Name,
Update(userSignup.Name, hostAwait.Namespace,
func(instance *toolchainv1alpha1.UserSignup) {
// Approve usersignup.
states.SetApprovedManually(instance, true)
Expand Down Expand Up @@ -446,7 +446,7 @@ func TestSignupOK(t *testing.T) {

// Deactivate the usersignup
userSignup, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}).
Update(userSignup.Name,
Update(userSignup.Name, hostAwait.Namespace,
func(us *toolchainv1alpha1.UserSignup) {
states.SetDeactivated(us, true)
})
Expand Down Expand Up @@ -612,7 +612,7 @@ func TestPhoneVerification(t *testing.T) {
require.False(t, mpStatus["verificationRequired"].(bool))

userSignup, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}).
Update(userSignup.Name,
Update(userSignup.Name, hostAwait.Namespace,
func(instance *toolchainv1alpha1.UserSignup) {
// Now approve the usersignup.
states.SetApprovedManually(instance, true)
Expand Down Expand Up @@ -670,7 +670,7 @@ func TestPhoneVerification(t *testing.T) {
require.NoError(t, err)

userSignup, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}).
Update(userSignup.Name,
Update(userSignup.Name, hostAwait.Namespace,
func(instance *toolchainv1alpha1.UserSignup) {
// Now mark the original UserSignup as deactivated
states.SetDeactivated(instance, true)
Expand Down Expand Up @@ -725,7 +725,7 @@ func TestActivationCodeVerification(t *testing.T) {
require.NoError(t, err)
// explicitly approve the usersignup (see above, config for parallel test has automatic approval disabled)
userSignup, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.UserSignup{}).
Update(userSignup.Name,
Update(userSignup.Name, hostAwait.Namespace,
func(us *toolchainv1alpha1.UserSignup) {
states.SetApprovedManually(us, true)
})
Expand Down Expand Up @@ -799,7 +799,7 @@ func TestActivationCodeVerification(t *testing.T) {
})) // need to reload event
require.NoError(t, err)
event, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.SocialEvent{}).
UpdateStatus(event.Name,
UpdateStatus(event.Name, hostAwait.Namespace,
func(ev *toolchainv1alpha1.SocialEvent) {
ev.Status.ActivationCount = event.Spec.MaxAttendees // activation count identical to `MaxAttendees`
})
Expand Down
67 changes: 35 additions & 32 deletions test/e2e/parallel/retarget_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
. "github.com/codeready-toolchain/toolchain-e2e/testsupport"
. "github.com/codeready-toolchain/toolchain-e2e/testsupport/space"
"github.com/codeready-toolchain/toolchain-e2e/testsupport/spacebinding"
. "github.com/codeready-toolchain/toolchain-e2e/testsupport/wait"
"github.com/codeready-toolchain/toolchain-e2e/testsupport/wait"

"github.com/stretchr/testify/require"
)
Expand All @@ -24,13 +24,14 @@ func TestRetargetUserByChangingSpaceTargetClusterWhenSpaceIsNotShared(t *testing
ManuallyApprove().
TargetCluster(member1Await).
EnsureMUR().
RequireConditions(ConditionSet(Default(), ApprovedByAdmin())...).
RequireConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin())...).
Execute(t)

// when
space, err := hostAwait.UpdateSpace(t, user.Space.Name, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
space, err := wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.Space{}).
Update(user.Space.Name, hostAwait.Namespace, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
require.NoError(t, err)

// then
Expand All @@ -41,7 +42,7 @@ func TestRetargetUserByChangingSpaceTargetClusterWhenSpaceIsNotShared(t *testing
// and provisioned on member-2
_, err = member2Await.WaitForNSTmplSet(t, space.Name)
require.NoError(t, err)
VerifyResourcesProvisionedForSpace(t, awaitilities, space.Name, UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
VerifyResourcesProvisionedForSpace(t, awaitilities, space.Name, wait.UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
VerifyUserRelatedResources(t, awaitilities, user.UserSignup, user.MUR.Spec.TierName, ExpectUserAccountIn(member2Await))
}

Expand All @@ -57,23 +58,23 @@ func TestRetargetUserByChangingSpaceTargetClusterWhenSpaceIsShared(t *testing.T)
ManuallyApprove().
TargetCluster(member1Await).
WaitForMUR().
RequireConditions(ConditionSet(Default(), ApprovedByAdmin())...).
RequireConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin())...).
Execute(t)
murToShareWith1 := userToShareWith1.MUR

userToShareWith2 := NewSignupRequest(awaitilities).
ManuallyApprove().
TargetCluster(member2Await).
WaitForMUR().
RequireConditions(ConditionSet(Default(), ApprovedByAdmin())...).
RequireConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin())...).
Execute(t)
murToShareWith2 := userToShareWith2.MUR

user := NewSignupRequest(awaitilities).
ManuallyApprove().
TargetCluster(member1Await).
EnsureMUR().
RequireConditions(ConditionSet(Default(), ApprovedByAdmin())...).
RequireConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin())...).
Execute(t)
ownerMur := user.MUR
spaceToMove := user.Space
Expand All @@ -84,14 +85,15 @@ func TestRetargetUserByChangingSpaceTargetClusterWhenSpaceIsShared(t *testing.T)
tier, err := hostAwait.WaitForNSTemplateTier(t, spaceToMove.Spec.TierName)
require.NoError(t, err)
_, err = member1Await.WaitForNSTmplSet(t, spaceToMove.Name,
UntilNSTemplateSetHasSpaceRoles(
SpaceRole(tier.Spec.SpaceRoles["admin"].TemplateRef, ownerMur.Name, murToShareWith1.Name, murToShareWith2.Name)))
wait.UntilNSTemplateSetHasSpaceRoles(
wait.SpaceRole(tier.Spec.SpaceRoles["admin"].TemplateRef, ownerMur.Name, murToShareWith1.Name, murToShareWith2.Name)))
require.NoError(t, err)

// when
spaceToMove, err = hostAwait.UpdateSpace(t, spaceToMove.Name, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
spaceToMove, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.Space{}).
Update(spaceToMove.Name, hostAwait.Namespace, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
require.NoError(t, err)

// then
Expand All @@ -101,18 +103,18 @@ func TestRetargetUserByChangingSpaceTargetClusterWhenSpaceIsShared(t *testing.T)

// and provisioned with the set of usernames for admin role as before on member-2
_, err = member2Await.WaitForNSTmplSet(t, spaceToMove.Name,
UntilNSTemplateSetHasSpaceRoles(
SpaceRole(tier.Spec.SpaceRoles["admin"].TemplateRef, ownerMur.Name, murToShareWith1.Name, murToShareWith2.Name)))
wait.UntilNSTemplateSetHasSpaceRoles(
wait.SpaceRole(tier.Spec.SpaceRoles["admin"].TemplateRef, ownerMur.Name, murToShareWith1.Name, murToShareWith2.Name)))
require.NoError(t, err)
VerifyResourcesProvisionedForSpace(t, awaitilities, spaceToMove.Name, UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
VerifyResourcesProvisionedForSpace(t, awaitilities, spaceToMove.Name, wait.UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
VerifyUserRelatedResources(t, awaitilities, user.UserSignup, ownerMur.Spec.TierName, ExpectUserAccountIn(member2Await))

// the move doesn't have any effect on the other signups and MURs
VerifyUserRelatedResources(t, awaitilities, userToShareWith1.UserSignup, murToShareWith1.Spec.TierName, ExpectUserAccountIn(member1Await))
VerifyResourcesProvisionedForSpace(t, awaitilities, murToShareWith1.Name, UntilSpaceHasStatusTargetCluster(member1Await.ClusterName))
VerifyResourcesProvisionedForSpace(t, awaitilities, murToShareWith1.Name, wait.UntilSpaceHasStatusTargetCluster(member1Await.ClusterName))

VerifyUserRelatedResources(t, awaitilities, userToShareWith2.UserSignup, murToShareWith2.Spec.TierName, ExpectUserAccountIn(member2Await))
VerifyResourcesProvisionedForSpace(t, awaitilities, murToShareWith2.Name, UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
VerifyResourcesProvisionedForSpace(t, awaitilities, murToShareWith2.Name, wait.UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
}

// NOTE:
Expand All @@ -131,23 +133,23 @@ func TestRetargetUserWithSBRByChangingSpaceTargetClusterWhenSpaceIsShared(t *tes
ManuallyApprove().
TargetCluster(member1Await).
WaitForMUR().
RequireConditions(ConditionSet(Default(), ApprovedByAdmin())...).
RequireConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin())...).
Execute(t)
murToShareWith1 := userToShareWith1.MUR

userToShareWith2 := NewSignupRequest(awaitilities).
ManuallyApprove().
TargetCluster(member2Await).
WaitForMUR().
RequireConditions(ConditionSet(Default(), ApprovedByAdmin())...).
RequireConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin())...).
Execute(t)
murToShareWith2 := userToShareWith2.MUR

user := NewSignupRequest(awaitilities).
ManuallyApprove().
TargetCluster(member1Await).
EnsureMUR().
RequireConditions(ConditionSet(Default(), ApprovedByAdmin())...).
RequireConditions(wait.ConditionSet(wait.Default(), wait.ApprovedByAdmin())...).
Execute(t)
ownerMur := user.MUR
spaceToMove := user.Space
Expand All @@ -164,14 +166,15 @@ func TestRetargetUserWithSBRByChangingSpaceTargetClusterWhenSpaceIsShared(t *tes
tier, err := hostAwait.WaitForNSTemplateTier(t, spaceToMove.Spec.TierName)
require.NoError(t, err)
_, err = member1Await.WaitForNSTmplSet(t, spaceToMove.Name,
UntilNSTemplateSetHasSpaceRoles(
SpaceRole(tier.Spec.SpaceRoles["admin"].TemplateRef, ownerMur.Name, murToShareWith1.Name, murToShareWith2.Name)))
wait.UntilNSTemplateSetHasSpaceRoles(
wait.SpaceRole(tier.Spec.SpaceRoles["admin"].TemplateRef, ownerMur.Name, murToShareWith1.Name, murToShareWith2.Name)))
require.NoError(t, err)

// when
spaceToMove, err = hostAwait.UpdateSpace(t, spaceToMove.Name, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
spaceToMove, err = wait.For(t, hostAwait.Awaitility, &toolchainv1alpha1.Space{}).
Update(spaceToMove.Name, hostAwait.Namespace, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
require.NoError(t, err)

// then
Expand All @@ -181,18 +184,18 @@ func TestRetargetUserWithSBRByChangingSpaceTargetClusterWhenSpaceIsShared(t *tes

// the users added via SBR will lose access to the Space, since SBRs where persisted on the namespace on member1 which was deleting while retargeting
_, err = member2Await.WaitForNSTmplSet(t, spaceToMove.Name,
UntilNSTemplateSetHasSpaceRoles(
SpaceRole(tier.Spec.SpaceRoles["admin"].TemplateRef, ownerMur.Name)))
wait.UntilNSTemplateSetHasSpaceRoles(
wait.SpaceRole(tier.Spec.SpaceRoles["admin"].TemplateRef, ownerMur.Name)))
require.NoError(t, err)
VerifyResourcesProvisionedForSpace(t, awaitilities, spaceToMove.Name, UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
VerifyResourcesProvisionedForSpace(t, awaitilities, spaceToMove.Name, wait.UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
VerifyUserRelatedResources(t, awaitilities, user.UserSignup, ownerMur.Spec.TierName, ExpectUserAccountIn(member2Await))

// the move doesn't have any effect on the other signups and MURs
VerifyUserRelatedResources(t, awaitilities, userToShareWith1.UserSignup, murToShareWith1.Spec.TierName, ExpectUserAccountIn(member1Await))
VerifyResourcesProvisionedForSpace(t, awaitilities, murToShareWith1.Name, UntilSpaceHasStatusTargetCluster(member1Await.ClusterName))
VerifyResourcesProvisionedForSpace(t, awaitilities, murToShareWith1.Name, wait.UntilSpaceHasStatusTargetCluster(member1Await.ClusterName))

VerifyUserRelatedResources(t, awaitilities, userToShareWith2.UserSignup, murToShareWith2.Spec.TierName, ExpectUserAccountIn(member2Await))
VerifyResourcesProvisionedForSpace(t, awaitilities, murToShareWith2.Name, UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))
VerifyResourcesProvisionedForSpace(t, awaitilities, murToShareWith2.Name, wait.UntilSpaceHasStatusTargetCluster(member2Await.ClusterName))

// no SBRs are present
sbrsFound, err := awaitilities.Member2().ListSpaceBindingRequests(GetDefaultNamespace(spaceToMove.Status.ProvisionedNamespaces))
Expand Down
Loading
Loading