Inject secrets from provider:"secret" tags
#252
Merged
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
iwahbe
requested review from
thomas11,
blampe and
a team
and removed request for
thomas11
iwahbe
added a commit
that referenced
this pull request
Jul 24, 2024
Following up on #252, this allows `DefaultCheck` to safely and accurately handle secrets application. The new signature for `DefaultCheck` is easier to extend without additional breaking changes. The downside to this design is that this makes `DefaultCheck` special. I think it's worth living with that until #212 is resolved. I'd like to merge shortly after #252 (and before #252 is released) so that user's don't rely on #252 injecting secrets when `CustomCheck` is implemented and `DefaultCheck` is not called.
mjeffryes
reviewed
Jul 25, 2024
Part of #251 Right now, secrets are communicated in the schema and applied via SDKs. This doesn't consistently work as expected for explicit providers.
This was added in go1.22, while we still support go1.21.
iwahbe
force-pushed
the
iwahbe/inject-secrets-from-schemas
branch
from
3acd5ab to
d3c9b59
Compare
mjeffryes
approved these changes
Jul 26, 2024
|
also many thanks for all the explanations/answers to my n00b questions 🙇 |
thomas11
approved these changes
Jul 26, 2024
iwahbe
force-pushed
the
iwahbe/inject-secrets-from-schemas
branch
from
fdef94a to
cd96ba2
Compare
iwahbe
force-pushed
the
iwahbe/inject-secrets-from-schemas
branch
from
cd96ba2 to
4a5e543
Compare
iwahbe
added a commit
that referenced
this pull request
Jul 26, 2024
Following up on #252, this allows `DefaultCheck` to safely and accurately handle secrets application. The new signature for `DefaultCheck` is easier to extend without additional breaking changes. The downside to this design is that this makes `DefaultCheck` special. I think it's worth living with that until #212 is resolved. I'd like to merge shortly after #252 (and before #252 is released) so that user's don't rely on #252 injecting secrets when `CustomCheck` is implemented and `DefaultCheck` is not called.
iwahbe
added a commit
that referenced
this pull request
Jul 26, 2024
Following up on #252, this allows `DefaultCheck` to safely and accurately handle secrets application. The new signature for `DefaultCheck` is easier to extend without additional breaking changes. The downside to this design is that this makes `DefaultCheck` special. I think it's worth living with that until #212 is resolved. I'd like to merge shortly after #252 (and before #252 is released) so that user's don't rely on #252 injecting secrets when `CustomCheck` is implemented and `DefaultCheck` is not called.
iwahbe
added a commit
that referenced
this pull request
Jul 26, 2024
Following up on #252, this allows `DefaultCheck` to safely and accurately handle secrets application. The new signature for `DefaultCheck` is easier to extend without additional breaking changes. The downside to this design is that this makes `DefaultCheck` special. I think it's worth living with that until #212 is resolved. I'd like to merge shortly after #252 (and before #252 is released) so that user's don't rely on #252 injecting secrets when `CustomCheck` is implemented and `DefaultCheck` is not called. Rational for the change: Ideally, most users will not need to implement check. By design, not implementing `CustomCheck` has the same behavior as implementing check with `return infer.DefaultCheck(...)`. If users do implement `CustomCheck`, then we don't want to make any assumptions about what behavior they want. This includes not applying defaults and not injecting secrets.
iwahbe
added a commit
that referenced
this pull request
Jul 27, 2024
Following up on #252, this allows `DefaultCheck` to safely and accurately handle secrets application. The new signature for `DefaultCheck` is easier to extend without additional breaking changes. The downside to this design is that this makes `DefaultCheck` special. I think it's worth living with that until #212 is resolved. I'd like to merge shortly after #252 (and before #252 is released) so that user's don't rely on #252 injecting secrets when `CustomCheck` is implemented and `DefaultCheck` is not called. Rational for the change: Ideally, most users will not need to implement check. By design, not implementing `CustomCheck` has the same behavior as implementing check with `return infer.DefaultCheck(...)`. If users do implement `CustomCheck`, then we don't want to make any assumptions about what behavior they want. This includes not applying defaults and not injecting secrets.
iwahbe
added a commit
that referenced
this pull request
Jul 27, 2024
Following up on #252, this allows `DefaultCheck` to safely and accurately handle secrets application. The new signature for `DefaultCheck` is easier to extend without additional breaking changes. The downside to this design is that this makes `DefaultCheck` special. I think it's worth living with that until #212 is resolved. I'd like to merge shortly after #252 (and before #252 is released) so that user's don't rely on #252 injecting secrets when `CustomCheck` is implemented and `DefaultCheck` is not called. Rational for the change: Ideally, most users will not need to implement check. By design, not implementing `CustomCheck` has the same behavior as implementing check with `return infer.DefaultCheck(...)`. If users do implement `CustomCheck`, then we don't want to make any assumptions about what behavior they want. This includes not applying defaults and not injecting secrets.
|
This PR has been shipped in release v0.21.0. |
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.
This PR add explicit secret injection to
infer. This is necessary for explicit providers in YAML, which does not correctly apply schema based secrets to provider config. This is what the bridge does.To guard against misbehaved or partially implemented SDKs, I have also added explicit secret injection to resources.
Fixes #251