-
Notifications
You must be signed in to change notification settings - Fork 69
feat(cli): display warning when --role-arn is used with gc command #893
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #893 +/- ##
==========================================
+ Coverage 83.23% 83.49% +0.25%
==========================================
Files 71 71
Lines 10396 10401 +5
Branches 1306 1315 +9
==========================================
+ Hits 8653 8684 +31
+ Misses 1705 1679 -26
Partials 38 38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Why not implement |
In the command documentation, roleArn is defined for when we invoke Cloudformation. gc seems to have some direct calls to s3/Ecr. I think there is scope for implementing this functionality but it might need a bigger discussion on the design implications. I can investigate this more and make an update later for this if needed, this should work as a primary fix meanwhile. Thanks for the feedback |
kaizencc
left a comment
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.
what about a test?
ef70730 to
f22b829
Compare
fca5c80 to
d6d2708
Compare
…ance after each test run
The gc command currently ignores the --role-arn parameter and always uses the user's default credentials, regardless of whether a role ARN is specified. This can potentially lead to silent failures, permission issues, as well as a bad user experience due to confusion.
This pr adds a check to throw a warning when the --role-arn parameter is used in the gc command. This helps improve user experience and avoid mismanaged expectations.
A consideration to throw an error was made. However, this could be a breaking change for existing ci/cd pipeline users. Therefore, this idea was discarded
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license