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 2 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
21 changes: 12 additions & 9 deletions test/e2e/parallel/retarget_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ func TestRetargetUserByChangingSpaceTargetClusterWhenSpaceIsNotShared(t *testing
Execute(t)

// when
space, err := hostAwait.UpdateSpace(t, user.Space.Name, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
space, err := 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 Down Expand Up @@ -89,9 +90,10 @@ func TestRetargetUserByChangingSpaceTargetClusterWhenSpaceIsShared(t *testing.T)
require.NoError(t, err)

// when
spaceToMove, err = hostAwait.UpdateSpace(t, spaceToMove.Name, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
spaceToMove, err = 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 Down Expand Up @@ -169,9 +171,10 @@ func TestRetargetUserWithSBRByChangingSpaceTargetClusterWhenSpaceIsShared(t *tes
require.NoError(t, err)

// when
spaceToMove, err = hostAwait.UpdateSpace(t, spaceToMove.Name, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = member2Await.ClusterName
})
spaceToMove, err = 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 Down
46 changes: 24 additions & 22 deletions test/e2e/parallel/serviceaccount_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,24 @@ func TestDoNotOverrideServiceAccount(t *testing.T) {
dummyPullSecretRefName := fmt.Sprintf("dummy-pull-secret-%d", i)

// update the ServiceAccount values so we can verify that some parts will stay and some will be reverted to the needed values
_, err := member.UpdateServiceAccount(t, nsName, "namespace-manager", func(sa *corev1.ServiceAccount) {
// when we add an annotation to the SA resource then it should stay there
sa.Annotations = map[string]string{
"should": "stay",
}
// add secret and ImagePullSecret refs and expect that they will stay there.
// the actual secrets don't exist, but that's fine - we need to check only if the refs stay in the SA resource
sa.Secrets = append(sa.Secrets, corev1.ObjectReference{
Name: dummySecretRefName,
})
sa.ImagePullSecrets = append(sa.ImagePullSecrets, corev1.LocalObjectReference{
Name: dummyPullSecretRefName,
_, err := wait.For(t, member.Awaitility, &corev1.ServiceAccount{}).
Update("namespace-manager", nsName, func(sa *corev1.ServiceAccount) {
// when we add an annotation to the SA resource then it should stay there
sa.Annotations = map[string]string{
"should": "stay",
}
// add secret and ImagePullSecret refs and expect that they will stay there.
// the actual secrets don't exist, but that's fine - we need to check only if the refs stay in the SA resource
sa.Secrets = append(sa.Secrets, corev1.ObjectReference{
Name: dummySecretRefName,
})
sa.ImagePullSecrets = append(sa.ImagePullSecrets, corev1.LocalObjectReference{
Name: dummyPullSecretRefName,
})

// also delete the space label key as we expect that this change should be reverted by the next reconcile loop
delete(sa.Labels, v1alpha1.SpaceLabelKey)
})

// also delete the space label key as we expect that this change should be reverted by the next reconcile loop
delete(sa.Labels, v1alpha1.SpaceLabelKey)
})
require.NoError(t, err)

// add the generated secret refs to the expected ones
Expand All @@ -77,12 +78,13 @@ func TestDoNotOverrideServiceAccount(t *testing.T) {
})

// Update the namespace annotations & labels to trigger the reconciliation
_, err = member.UpdateNamespace(t, nsName, func(ns *corev1.Namespace) {
// drop the last-applied-space-roles annotation, so we are sure that the content of the roles are re-applied
delete(ns.Annotations, v1alpha1.LastAppliedSpaceRolesAnnotationKey)
// change the tier name, so we are sure that the content of the tier template is re-applied
ns.Labels[v1alpha1.TierLabelKey] = "appstudio"
})
_, err = wait.For(t, member.Awaitility, &corev1.Namespace{}).
Update(nsName, member.Namespace, func(ns *corev1.Namespace) {
// drop the last-applied-space-roles annotation, so we are sure that the content of the roles are re-applied
delete(ns.Annotations, v1alpha1.LastAppliedSpaceRolesAnnotationKey)
// change the tier name, so we are sure that the content of the tier template is re-applied
ns.Labels[v1alpha1.TierLabelKey] = "appstudio"
})
require.NoError(t, err)

// then
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/parallel/socialevent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestCreateSocialEvent(t *testing.T) {
t.Run("update with valid tier name", func(t *testing.T) {
// when
event, err = For(t, hostAwait.Awaitility, &toolchainv1alpha1.SocialEvent{}).
Update(event.Name,
Update(event.Name, hostAwait.Namespace,
func(ev *toolchainv1alpha1.SocialEvent) {
ev.Spec.UserTier = "deactivate30"
})
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestCreateSocialEvent(t *testing.T) {
t.Run("update with valid tier name", func(t *testing.T) {
// when
event, err = For(t, hostAwait.Awaitility, &toolchainv1alpha1.SocialEvent{}).
Update(event.Name,
Update(event.Name, hostAwait.Namespace,
func(ev *toolchainv1alpha1.SocialEvent) {
ev.Spec.SpaceTier = "base"
})
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/parallel/space_cleanup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestSpaceAndSpaceBindingCleanup(t *testing.T) {
// when
// deactivate the UserSignup so that the MUR will be deleted
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 All @@ -76,7 +76,7 @@ func TestSpaceAndSpaceBindingCleanup(t *testing.T) {
// when
// we deactivate the UserSignup so that the MUR will be deleted
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
41 changes: 23 additions & 18 deletions test/e2e/parallel/space_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@ func TestCreateSpace(t *testing.T) {

t.Run("unable to delete space that was already provisioned", func(t *testing.T) {
// given
s, err := hostAwait.UpdateSpace(t, space.Name, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = "unknown"
})
s, err := For(t, hostAwait.Awaitility, &toolchainv1alpha1.Space{}).
Copy link
Contributor

Choose a reason for hiding this comment

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

wait.For() would look better then just For() . But it's cosmetic :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed! Addressed, thanks!

Update(space.Name, hostAwait.Namespace, func(ss *toolchainv1alpha1.Space) {
ss.Spec.TargetCluster = "unknown"
})
require.NoError(t, err)

// when
Expand All @@ -96,9 +97,10 @@ func TestCreateSpace(t *testing.T) {

t.Run("update target cluster to unblock deletion", func(t *testing.T) {
// when
s, err = hostAwait.UpdateSpace(t, s.Name, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = memberAwait.ClusterName
})
s, err = For(t, hostAwait.Awaitility, &toolchainv1alpha1.Space{}).
Update(s.Name, hostAwait.Namespace, func(s *toolchainv1alpha1.Space) {
s.Spec.TargetCluster = memberAwait.ClusterName
})
require.NoError(t, err)

// then
Expand Down Expand Up @@ -204,10 +206,11 @@ func TestSpaceRoles(t *testing.T) {

t.Run("set owner user as maintainer instead", func(t *testing.T) {
// when
ownerBinding, err = hostAwait.UpdateSpaceBinding(t, ownerBinding.Name, func(sb *toolchainv1alpha1.SpaceBinding) {
// given an appstudio space with `owner` user as an admin of it
sb.Spec.SpaceRole = "maintainer"
})
ownerBinding, err = For(t, hostAwait.Awaitility, &toolchainv1alpha1.SpaceBinding{}).
Update(ownerBinding.Name, hostAwait.Namespace, func(sb *toolchainv1alpha1.SpaceBinding) {
// given an appstudio space with `owner` user as an admin of it
sb.Spec.SpaceRole = "maintainer"
})
require.NoError(t, err)

// then
Expand All @@ -223,10 +226,11 @@ func TestSpaceRoles(t *testing.T) {

t.Run("set owner user as contributor instead", func(t *testing.T) {
// when
ownerBinding, err = hostAwait.UpdateSpaceBinding(t, ownerBinding.Name, func(sb *toolchainv1alpha1.SpaceBinding) {
// given an appstudio space with `owner` user as an admin of it
sb.Spec.SpaceRole = "contributor"
})
ownerBinding, err = For(t, hostAwait.Awaitility, &toolchainv1alpha1.SpaceBinding{}).
Update(ownerBinding.Name, hostAwait.Namespace, func(sb *toolchainv1alpha1.SpaceBinding) {
// given an appstudio space with `owner` user as an admin of it
sb.Spec.SpaceRole = "contributor"
})

// then
require.NoError(t, err)
Expand Down Expand Up @@ -337,10 +341,11 @@ func TestSubSpaces(t *testing.T) {
t.Run("we update role in parentSpaceBinding and expect change to be reflected in subSpaces", func(t *testing.T) {
// when
// we update the parentSpace bindings
parentSpaceBindings, err = hostAwait.UpdateSpaceBinding(t, parentSpaceBindings.Name, func(sb *toolchainv1alpha1.SpaceBinding) {
// the parentSpace role was "downgraded" to maintainer
sb.Spec.SpaceRole = "maintainer"
})
parentSpaceBindings, err = For(t, hostAwait.Awaitility, &toolchainv1alpha1.SpaceBinding{}).
Update(parentSpaceBindings.Name, hostAwait.Namespace, func(sb *toolchainv1alpha1.SpaceBinding) {
// the parentSpace role was "downgraded" to maintainer
sb.Spec.SpaceRole = "maintainer"
})

// then
// downgrade of the usernames is done in parentSpace
Expand Down
Loading
Loading