-
Notifications
You must be signed in to change notification settings - Fork 274
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
fix: validate secrets before action resolution #6822
Conversation
Validate missing secrets when instantiating a `ResolveActionTask` , rather than when adding configs. This would ensure we only get the error when missing secrets would actually cause a failure for the given command, while still failing relatively early even if the action happens to use another action’s outputs.
…r if action references missing secrets when user is not logged in
…erences missing secrets The missing secrets check was moved to `ResolveActionTask` constructor.
…erences missing secrets The missing secrets check was moved to `ResolveActionTask` constructor.
…rences missing secrets
…ferences missing secrets
…nfig" tests suite
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.
Great work! 👍 I had a preliminary look and already sharing some things that I noticed
* Not logged in * Has no secrets
The error message depends on the login status to be less confusing.
…r if module references missing secrets
Some secrets might be missing and not resolvable now, because the missing secrets check was moved to `ResolveActionTask` constructor.
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.
This is amazing! Thank you for looking into this.
I have additional suggestions regarding the error messages, those are totally fine as follow-up or in this PR as well, as you wish!
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.
Super Awesome!
What this PR does / why we need it:
Validate missing secrets when instantiating a
ResolveActionTask
, rather than when adding configs.This would ensure we only get the error when missing secrets would actually cause a failure for the given command, while still failing relatively early even if the action happens to use another action’s outputs.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: