-
Notifications
You must be signed in to change notification settings - Fork 37
feat(cas-backends): When updating CAS Backends events are sent #2415
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -336,6 +336,7 @@ func (uc *CASBackendUseCase) Update(ctx context.Context, orgID, id, description | |
| } | ||
|
|
||
| var secretName string | ||
| credentialsUpdated := creds != nil | ||
| // We want to rotate credentials | ||
| if creds != nil { | ||
| secretName, err = uc.credsRW.SaveCredentials(ctx, orgID, creds) | ||
|
|
@@ -344,18 +345,46 @@ func (uc *CASBackendUseCase) Update(ctx context.Context, orgID, id, description | |
| } | ||
| } | ||
|
|
||
| after, err := uc.repo.Update(ctx, &CASBackendUpdateOpts{ | ||
| // Update the backend without modifying validation status directly | ||
| // The validation status will be updated through PerformValidation if needed | ||
| // Don't set validation status here - let PerformValidation handle it | ||
| updateOpts := &CASBackendUpdateOpts{ | ||
| ID: uuid, | ||
| CASBackendOpts: &CASBackendOpts{ | ||
| SecretName: secretName, Default: defaultB, Description: description, OrgID: orgUUID, | ||
| ValidationStatus: CASBackendValidationOK, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was a hack. Thanks for taking care. |
||
| ValidationError: ToPtr(""), | ||
| SecretName: secretName, | ||
| Default: defaultB, | ||
| Description: description, | ||
| OrgID: orgUUID, | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| // If we're not updating credentials, preserve the current validation status | ||
| if !credentialsUpdated { | ||
| updateOpts.ValidationStatus = before.ValidationStatus | ||
| updateOpts.ValidationError = before.ValidationError | ||
| } | ||
|
|
||
| after, err := uc.repo.Update(ctx, updateOpts) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // If credentials were updated, perform validation to check if they work | ||
| // This will properly update validation status and send events | ||
| if credentialsUpdated { | ||
| if err := uc.PerformValidation(ctx, id); err != nil { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part was missing, if we rotate the credentials from the Update, we need to perform a validation to update and send the proper status. The |
||
| // Log the validation error but don't fail the update operation | ||
| // The validation status will be updated by PerformValidation | ||
| uc.logger.Warnw("msg", "validation failed after credential update", "ID", id, "error", err) | ||
| } | ||
|
|
||
| // Reload the backend to get the updated validation status | ||
| after, err = uc.repo.FindByIDInOrg(ctx, orgUUID, uuid) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("reloading backend after validation: %w", err) | ||
| } | ||
| } | ||
|
|
||
| // If we just updated the backend from default=true => default=false, we need to set up the fallback as default | ||
| if before.Default && !after.Default { | ||
| if _, err := uc.defaultFallbackBackend(ctx, orgID); err != nil { | ||
|
|
@@ -374,7 +403,7 @@ func (uc *CASBackendUseCase) Update(ctx context.Context, orgID, id, description | |
| Default: after.Default, | ||
| }, | ||
| NewDescription: &description, | ||
| CredentialsChanged: creds != nil, | ||
| CredentialsChanged: credentialsUpdated, | ||
| PreviousDefault: before.Default, | ||
| }, &orgUUID) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed all
omitemptyfrom the bool fields so they are properly serialized in JSON when stored in the database with the proper value.