From 0c57eb06dbf48742ddf2278c5135311dafceff34 Mon Sep 17 00:00:00 2001 From: Shane Bryzak Date: Tue, 14 Nov 2023 16:11:15 +1000 Subject: [PATCH] removed migration code (#931) --- .../usersignup/usersignup_controller.go | 48 ---------- .../usersignup/usersignup_controller_test.go | 95 +------------------ 2 files changed, 5 insertions(+), 138 deletions(-) diff --git a/controllers/usersignup/usersignup_controller.go b/controllers/usersignup/usersignup_controller.go index ef3efe919..336fe8c66 100644 --- a/controllers/usersignup/usersignup_controller.go +++ b/controllers/usersignup/usersignup_controller.go @@ -111,20 +111,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. return reconcile.Result{}, nil } - // TODO remove this section (and the referenced function) after migration has completed - // FROM HERE --------- - migrated, err := r.migrateUserSignupClaimsIfNecessary(ctx, userSignup) - if err != nil { - // Error during migration - requeue the request - return reconcile.Result{}, err - } - - if migrated { - // If migration occurred, then queue the UserSignup for reconciliation again - return reconcile.Result{Requeue: true}, nil - } - // TO HERE ^^^^^^^^^^^^ - if userSignup.GetLabels() == nil { userSignup.Labels = make(map[string]string) } @@ -204,40 +190,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. return reconcile.Result{}, r.ensureNewMurIfApproved(ctx, config, userSignup) } -// migrateUserSignupClaimsIfNecessary is a temporary function that will set the UserSignup's IdentityClaims based on the -// existing property values -func (r *Reconciler) migrateUserSignupClaimsIfNecessary(ctx context.Context, userSignup *toolchainv1alpha1.UserSignup) (bool, error) { - - if userSignup.Spec.IdentityClaims.Sub == "" { - userSignup.Spec.IdentityClaims.Sub = userSignup.Spec.Userid - userSignup.Spec.IdentityClaims.FamilyName = userSignup.Spec.FamilyName - userSignup.Spec.IdentityClaims.GivenName = userSignup.Spec.GivenName - userSignup.Spec.IdentityClaims.OriginalSub = userSignup.Spec.OriginalSub - userSignup.Spec.IdentityClaims.PreferredUsername = userSignup.Spec.Username - userSignup.Spec.IdentityClaims.Company = userSignup.Spec.Company - - if val, ok := userSignup.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey]; ok { - userSignup.Spec.IdentityClaims.UserID = val - } - - if val, ok := userSignup.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey]; ok { - userSignup.Spec.IdentityClaims.AccountID = val - } - - if val, ok := userSignup.Annotations[toolchainv1alpha1.UserSignupUserEmailAnnotationKey]; ok { - userSignup.Spec.IdentityClaims.Email = val - } - - if err := r.Client.Update(ctx, userSignup); err != nil { - return false, err - } - - return true, nil - } - - return false, nil -} - // handleDeactivatedUserSignup defines the workflow for deactivated users // // If there is no MasterUserRecord created, yet the UserSignup is marked as Deactivated, set the status, diff --git a/controllers/usersignup/usersignup_controller_test.go b/controllers/usersignup/usersignup_controller_test.go index 9169fe312..17ff4f1ec 100644 --- a/controllers/usersignup/usersignup_controller_test.go +++ b/controllers/usersignup/usersignup_controller_test.go @@ -58,61 +58,6 @@ var event = testsocialevent.NewSocialEvent(test.HostOperatorNs, commonsocialeven testsocialevent.WithUserTier(deactivate80Tier.Name), testsocialevent.WithSpaceTier(base2NSTemplateTier.Name)) -func TestUserSignupMigration(t *testing.T) { - member := NewMemberClusterWithTenantRole(t, "member1", corev1.ConditionTrue) - - userSignup := commonsignup.NewUserSignup( - commonsignup.ApprovedManually()) - - // Clear the identity claims - userSignup.Spec.IdentityClaims = toolchainv1alpha1.IdentityClaimsEmbedded{} - - // Set some other properties - userSignup.Spec.GivenName = "John" - userSignup.Spec.FamilyName = "Smith" - userSignup.Spec.Company = "Acme" - userSignup.Spec.OriginalSub = uuid.Must(uuid.NewV4()).String() - - userSignup.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey] = "123456" - userSignup.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey] = "999988" - - // First reconcile so that we can check that an error is returned if the client update fails - r, req, fakeClient := prepareReconcile(t, userSignup.Name, NewGetMemberClusters(member), userSignup, baseNSTemplateTier) - fakeClient.MockUpdate = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.UpdateOption) error { - return fmt.Errorf("some error") - } - - // Reconcile and confirm we get an error - _, err := r.Reconcile(context.TODO(), req) - require.Error(t, err) - - // Reconcile so that the migration can occur - r, req, _ = prepareReconcile(t, userSignup.Name, NewGetMemberClusters(member), userSignup, baseNSTemplateTier) - res, err := r.Reconcile(context.TODO(), req) - - // then - require.NoError(t, err) - - // Confirm requeue - require.True(t, res.Requeue) - - // Reload the UserSignup - reloaded := &toolchainv1alpha1.UserSignup{} - err = r.Client.Get(context.TODO(), types.NamespacedName{Name: userSignup.Name, Namespace: req.Namespace}, reloaded) - require.NoError(t, err) - - // Confirm the migration - require.Equal(t, userSignup.Spec.Username, reloaded.Spec.IdentityClaims.PreferredUsername) - require.Equal(t, userSignup.Spec.Company, reloaded.Spec.IdentityClaims.Company) - require.Equal(t, userSignup.Spec.FamilyName, reloaded.Spec.IdentityClaims.FamilyName) - require.Equal(t, userSignup.Spec.GivenName, reloaded.Spec.IdentityClaims.GivenName) - require.Equal(t, userSignup.Spec.Userid, reloaded.Spec.IdentityClaims.Sub) - require.Equal(t, userSignup.Spec.OriginalSub, reloaded.Spec.IdentityClaims.OriginalSub) - require.Equal(t, userSignup.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey], reloaded.Spec.IdentityClaims.UserID) - require.Equal(t, userSignup.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey], reloaded.Spec.IdentityClaims.AccountID) - -} - func TestUserSignupCreateMUROk(t *testing.T) { member := NewMemberClusterWithTenantRole(t, "member1", corev1.ConditionTrue) logf.SetLogger(zap.New(zap.UseDevMode(true))) @@ -2489,13 +2434,8 @@ func TestUserSignupFailedToCreateDeactivationNotification(t *testing.T) { } } - // Reconcile as the first reconcile performs a temporary migration. - // FIXME remove after migration complete - _, err := r.Reconcile(context.TODO(), req) - require.NoError(t, err) - // when - _, err = r.Reconcile(context.TODO(), req) + _, err := r.Reconcile(context.TODO(), req) // then require.Error(t, err) @@ -2592,13 +2532,8 @@ func TestUserSignupReactivateAfterDeactivated(t *testing.T) { }), )) - // Reconcile as the first reconcile performs a temporary migration. - // FIXME remove after migration complete - _, err := r.Reconcile(context.TODO(), req) - require.NoError(t, err) - // when - _, err = r.Reconcile(context.TODO(), req) + _, err := r.Reconcile(context.TODO(), req) // then require.NoError(t, err) @@ -2789,11 +2724,6 @@ func TestUserSignupDeactivatedWhenMURAndSpaceAndSpaceBindingExists(t *testing.T) err := r.setSpaceToReady(mur.Name) // given space is ready require.NoError(t, err) - // Reconcile as the first reconcile performs a temporary migration. - // FIXME remove after migration complete - _, err = r.Reconcile(context.TODO(), req) - require.NoError(t, err) - _, err = r.Reconcile(context.TODO(), req) // then @@ -2990,13 +2920,8 @@ func TestUserSignupDeactivatingNotificationCreated(t *testing.T) { }), )) - // Reconcile as the first reconcile performs a temporary migration. - // FIXME remove after migration complete - _, err := r.Reconcile(context.TODO(), req) - require.NoError(t, err) - // when - _, err = r.Reconcile(context.TODO(), req) + _, err := r.Reconcile(context.TODO(), req) // then require.NoError(t, err) @@ -3455,13 +3380,8 @@ func TestUserSignupDeactivatedButMURDeleteFails(t *testing.T) { t.Run("first reconcile", func(t *testing.T) { - // Reconcile again, as the first reconcile performs a temporary migration. - // FIXME remove after migration complete - _, err := r.Reconcile(context.TODO(), req) - require.NoError(t, err) - // when - _, err = r.Reconcile(context.TODO(), req) + _, err := r.Reconcile(context.TODO(), req) require.Error(t, err) // then @@ -3568,13 +3488,8 @@ func TestUserSignupDeactivatedButStatusUpdateFails(t *testing.T) { } } - // Reconcile as the first reconcile performs a temporary migration. - // FIXME remove after migration complete - _, err := r.Reconcile(context.TODO(), req) - require.NoError(t, err) - // when - _, err = r.Reconcile(context.TODO(), req) + _, err := r.Reconcile(context.TODO(), req) require.Error(t, err) // then