Fix duplicated providers listed in application form #3836
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knanao
reviewed
Aug 9, 2022
|
|
||
| func (s *pipedStore) UpdateMetadata(ctx context.Context, id, version, config string, pps []*model.Piped_PlatformProvider, repos []*model.ApplicationGitRepository, se *model.Piped_SecretEncryption, startedAt int64) error { | ||
| return s.update(ctx, id, func(piped *model.Piped) error { | ||
| piped.CloudProviders = nil |
There was a problem hiding this comment.
Nice catch! I have one point I would like to confirm.
Is this safe for the user using only cloudProviders settings in piped?
There was a problem hiding this comment.
The piped execution only refers to its local configuration and itself knows about the CloudProvider (suppose it has only CloudProviders and the piped runs the old version since the new one will merge all CloudProviders -> PlatformProviders), so I think it's okay since they know which "cloudProvider" to use on its side.
Otherwise, though we remove CloudProviders information from the piped model here, it only be used to show on the application adding form selection, so I think it's okay 🤔
knanao
approved these changes
Aug 9, 2022
knanao
pushed a commit
that referenced
this pull request
Aug 9, 2022
Merged
knanao
added a commit
that referenced
this pull request
Aug 9, 2022
Co-authored-by: Khanh Tran <[email protected]>
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
The root cause of this issue is that when the currently running piped reports its metadata to the controlplane, only piped's platformProviders values in controlplane database are updated, but we forget to remove the old cloudProviders value, such that it exists and will be returned later in the web console application adding form.
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: