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.

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

Regenerated from API - made UserSignup.Spec.IdentityClaims required #973

Merged
merged 28 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7ef1414
regenerated api, temporarily replace API dependency, fixed compile er…
sbryzak Dec 29, 2023
d3ca327
fixed test
sbryzak Dec 29, 2023
0250bf7
dependencies
sbryzak Jan 2, 2024
878cfa6
regenerated
sbryzak Jan 2, 2024
8ddfa39
dependencies
sbryzak Jan 6, 2024
6d340b7
regenerated
sbryzak Jan 8, 2024
70cbe5b
Merge remote-tracking branch 'origin/master' into SANDBOX-504
sbryzak Jan 8, 2024
37e33ee
Merge remote-tracking branch 'origin/master' into SANDBOX-504
sbryzak Jan 11, 2024
c649f68
replaced toolchain-common
sbryzak Jan 11, 2024
2fee81d
Merge remote-tracking branch 'origin' into SANDBOX-504-host
sbryzak Jan 22, 2024
3cef112
fixed tests, removed migration code
sbryzak Jan 23, 2024
c38a4bd
update toolchain-common dependency
sbryzak Jan 27, 2024
64d9388
Merge remote-tracking branch 'origin/master' into SANDBOX-504-host
sbryzak Jan 27, 2024
7456cb1
dependencies
sbryzak Jan 31, 2024
e155414
Merge branch 'master' into SANDBOX-504-host
mfrancisc Feb 2, 2024
0236eda
updated dependences
sbryzak Feb 4, 2024
13cecd4
regenerated api
sbryzak Feb 4, 2024
79b07ad
Merge branch 'SANDBOX-504-host' of https://github.com/sbryzak/host-op…
sbryzak Feb 4, 2024
333c130
Merge remote-tracking branch 'origin/master' into SANDBOX-504-host
sbryzak Feb 6, 2024
96660f8
updated to latest api and toolchain-common
sbryzak Feb 7, 2024
69b2bea
Merge remote-tracking branch 'origin/master' into SANDBOX-504-host
sbryzak Feb 7, 2024
dcd49dc
regenerated from API - made UserSignup.Spec.IdentityClaims required
sbryzak Feb 26, 2024
f7d1f93
temp replace toolchain-common
sbryzak Feb 26, 2024
694d23c
updated dependencies
sbryzak Feb 27, 2024
cfec392
regenerated
sbryzak Feb 27, 2024
d9eeebf
Merge remote-tracking branch 'origin/master' into SANDBOX-544
sbryzak Feb 27, 2024
68caf07
merged
sbryzak Feb 27, 2024
625ca36
remove replaced dependencies, update to latest versions
sbryzak Feb 27, 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
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,6 @@ spec:
be able to login (but the underlying UserAccounts still exists)
"false" is assumed by default
type: boolean
originalSub:
description: OriginalSub is an optional property temporarily introduced
for the purpose of migrating the users to a new IdP provider client,
and contains the user's "original-sub" claim
type: string
propagatedClaims:
description: PropagatedClaims contains a selection of claim values
from the SSO Identity Provider which are intended to be "propagated"
Expand Down Expand Up @@ -118,10 +113,6 @@ spec:
x-kubernetes-list-map-keys:
- targetCluster
x-kubernetes-list-type: map
userID:
description: UserID is the user ID from RHD Identity Provider token
(“sub” claim)
type: string
type: object
status:
description: MasterUserRecordStatus defines the observed state of MasterUserRecord
Expand Down
31 changes: 5 additions & 26 deletions config/crd/bases/toolchain.dev.openshift.com_usersignups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ spec:
scope: Namespaced
versions:
- additionalPrinterColumns:
- jsonPath: .spec.username
- jsonPath: .spec.identityClaims.preferredUsername
name: Username
type: string
- jsonPath: .spec.givenName
- jsonPath: .spec.identityClaims.givenName
name: First Name
priority: 1
type: string
- jsonPath: .spec.familyName
- jsonPath: .spec.identityClaims.familyName
name: Last Name
priority: 1
type: string
- jsonPath: .spec.company
- jsonPath: .spec.identityClaims.company
name: Company
priority: 1
type: string
Expand Down Expand Up @@ -56,7 +56,7 @@ spec:
- jsonPath: .status.compliantUsername
name: CompliantUsername
type: string
- jsonPath: .metadata.annotations.toolchain\.dev\.openshift\.com/user-email
- jsonPath: .spec.identityClaims.email
name: Email
type: string
name: v1alpha1
Expand All @@ -79,15 +79,6 @@ spec:
spec:
description: UserSignupSpec defines the desired state of UserSignup
properties:
company:
description: The user's company name, obtained from the identity provider.
type: string
familyName:
description: The user's last name, obtained from the identity provider.
type: string
givenName:
description: The user's first name, obtained from the identity provider.
type: string
identityClaims:
description: IdentityClaims contains as-is claim values extracted
from the user's access token
Expand Down Expand Up @@ -129,11 +120,6 @@ spec:
- preferredUsername
- sub
type: object
originalSub:
description: OriginalSub is an optional property temporarily introduced
for the purpose of migrating the users to a new IdP provider client,
and contains the user's "original-sub" claim
type: string
states:
description: States contains a number of values that reflect the desired
state of the UserSignup.
Expand All @@ -145,13 +131,6 @@ spec:
description: The cluster in which the user is provisioned in If not
set then the target cluster will be picked automatically
type: string
userid:
description: The user's user ID, obtained from the identity provider
from the 'sub' (subject) claim
type: string
username:
description: The user's username, obtained from the identity provider.
type: string
type: object
status:
description: UserSignupStatus defines the observed state of UserSignup
Expand Down
9 changes: 1 addition & 8 deletions controllers/masteruserrecord/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,6 @@ func setupSynchronizerItems() (toolchainv1alpha1.MasterUserRecord, toolchainv1al
Labels: map[string]string{
toolchainv1alpha1.TierLabelKey: "base1ns",
},
Annotations: map[string]string{
toolchainv1alpha1.UserEmailAnnotationKey: "[email protected]",
},
},
Spec: toolchainv1alpha1.UserAccountSpec{
PropagatedClaims: toolchainv1alpha1.PropagatedClaims{
Expand All @@ -113,11 +110,7 @@ func setupSynchronizerItems() (toolchainv1alpha1.MasterUserRecord, toolchainv1al
},
}
record := toolchainv1alpha1.MasterUserRecord{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
toolchainv1alpha1.UserEmailAnnotationKey: "[email protected]",
},
},
ObjectMeta: metav1.ObjectMeta{},
Spec: toolchainv1alpha1.MasterUserRecordSpec{
PropagatedClaims: toolchainv1alpha1.PropagatedClaims{
UserID: "foo",
Expand Down
51 changes: 2 additions & 49 deletions controllers/usersignup/usersignup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -705,7 +657,8 @@ func (r *Reconciler) provisionMasterUserRecord(

// track the MUR creation as an account activation event in Segment
if r.SegmentClient != nil {
r.SegmentClient.TrackAccountActivation(compliantUsername, userSignup.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey], userSignup.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey])
r.SegmentClient.TrackAccountActivation(compliantUsername, userSignup.Spec.IdentityClaims.UserID,
userSignup.Spec.IdentityClaims.AccountID)
} else {
logger.Info("segment client not configured to track account activations")
}
Expand Down
54 changes: 0 additions & 54 deletions controllers/usersignup/usersignup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,60 +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)))
Expand Down
2 changes: 1 addition & 1 deletion deploy/registration-service/registration-service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ objects:
successThreshold: 1
timeoutSeconds: 1
readinessProbe:
failureThreshold: 30
sbryzak marked this conversation as resolved.
Show resolved Hide resolved
failureThreshold: 1
httpGet:
path: /api/v1/health
port: 8080
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
module github.com/codeready-toolchain/host-operator

require (
github.com/codeready-toolchain/api v0.0.0-20240116164228-8d18c9262420
github.com/codeready-toolchain/toolchain-common v0.0.0-20240130202956-94befa4c786c
github.com/codeready-toolchain/api v0.0.0-20240207000013-661b63025269
github.com/codeready-toolchain/toolchain-common v0.0.0-20240207000544-9cd055b3a18c
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/ghodss/yaml v1.0.0
github.com/go-bindata/go-bindata v3.1.2+incompatible
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ github.com/cncf/xds/go v0.0.0-20210312221358-fbca930ec8ed/go.mod h1:eXthEFrGJvWH
github.com/cockroachdb/datadriven v0.0.0-20200714090401-bf6692d28da5/go.mod h1:h6jFvWxBdQXxjopDMZyH2UVceIRfR84bdzbkoKrsWNo=
github.com/cockroachdb/errors v1.2.4/go.mod h1:rQD95gz6FARkaKkQXUksEje/d9a6wBJoCr5oaCLELYA=
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI=
github.com/codeready-toolchain/api v0.0.0-20240116164228-8d18c9262420 h1:rxbURSqAgiMCsyKDRwsHMjw7bIktK0IUU/OXpxwu0Zk=
github.com/codeready-toolchain/api v0.0.0-20240116164228-8d18c9262420/go.mod h1:FO7kgXH1x1LqkF327D5a36u0WIrwjVCbeijPkzgwaZc=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240130202956-94befa4c786c h1:83NTlxL6LkpKyu39QxyheGV1tWRXSPOmFyu9fC0qUAA=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240130202956-94befa4c786c/go.mod h1:f+h8e03UjCldtgI1NsZm/yXq2b/uZv2jsz5p8OK9Z+c=
github.com/codeready-toolchain/api v0.0.0-20240207000013-661b63025269 h1:YS5Q6YsTYq9Fo8qA6NQOTWAcVg86VEwulT1UfNWknIQ=
github.com/codeready-toolchain/api v0.0.0-20240207000013-661b63025269/go.mod h1:FO7kgXH1x1LqkF327D5a36u0WIrwjVCbeijPkzgwaZc=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240207000544-9cd055b3a18c h1:WKjD9qRSLqQsi4XduuXjsaF8qFH4yj157qWdA5wOOtg=
github.com/codeready-toolchain/toolchain-common v0.0.0-20240207000544-9cd055b3a18c/go.mod h1:+COaw79DVTLSb2unqVwcBtYOg6sh7MbMHgXU1/ht2I8=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
Expand Down
2 changes: 1 addition & 1 deletion test/segment/assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

func AssertMessageQueuedForProvisionedMur(t *testing.T, cl *segment.Client, us *toolchainv1alpha1.UserSignup, murName string) {
assertMessageQueued(t, cl, murName, us.Annotations[toolchainv1alpha1.SSOUserIDAnnotationKey], us.Annotations[toolchainv1alpha1.SSOAccountIDAnnotationKey], "account activated")
assertMessageQueued(t, cl, murName, us.Spec.IdentityClaims.UserID, us.Spec.IdentityClaims.AccountID, "account activated")
}

func assertMessageQueued(t *testing.T, cl *segment.Client, username, userID, accountID string, event string) {
Expand Down
Loading