Improved UX for updating wif-config. #700
Open
+341
−113
Conversation
This file contains 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
renan-campos
force-pushed
the
OCM-13183
branch
2 times, most recently
from
e2943d7 to
85739cb
Compare
JakobGray
approved these changes
Jan 6, 2025
tzvatot
requested changes
Jan 8, 2025
cmd/ocm/gcp/gcp-client-shim.go
Outdated
| needPolicyUpdate := false | ||
|
|
||
| policy, err := c.gcpClient.GetProjectIamPolicy(ctx, projectName, &cloudresourcemanager.GetIamPolicyRequest{}) | ||
|
|
||
| if err != nil { | ||
| return fmt.Errorf("error fetching policy for project: %v", err) | ||
| return false, fmt.Errorf("error fetching policy for project: %v", err) |
There was a problem hiding this comment.
Not related specifically to this MR: why not using errors.Wrap (as done in other places in this file)?
pkg/utils/jwks.go
Outdated
| jwksStrB string, | ||
| ) bool { | ||
| var jwksA, jwksB struct { | ||
| Keys []struct { |
There was a problem hiding this comment.
What about x5c and x5t?
Is there a a third party comparison tool that we can use? trying not to re-invent the wheel
There was a problem hiding this comment.
updated to use https://pkg.go.dev/github.com/MicahParks/jwkset#JWKSMarshal
renan-campos
force-pushed
the
OCM-13183
branch
from
85739cb to
87d39fd
Compare
…esource updation Ensured all operations modifying GCP cloud resources will log messages to the user. The 'ocm gcp update wif-config' command was unnecessarily updating the oidc data of the workload identity pool, even when there were only formatting differences. By improving the evaluation method for the jwks configuration, the oidc configuration will now only be updated if there is a meaningful difference. Service Account access policies were being during every run of the update command. By checking whether the policies are already in place, updates to the policy will only occur if necassary.
renan-campos
force-pushed
the
OCM-13183
branch
from
87d39fd to
4e0c787
Compare
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.
The included changes provide the following properties to the update command:
Additional logic was needed to check the configuration of jwks and service account access. Prior to this, these resources were being updated every time the command was called.