fix: 🐛 Replace simple strings.Contains match with regex
#320
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
daniel-anova
changed the title
strings.Contains match with regex
Use a more refined regex to match valid injectable secret names. Argo Workflow passes a enviromnet variable with the workflow manifest in JSON, this was being picked up by string.contains match causing it to fail. The regex match is a lot stricter only trying to replace enviroment variables that only have a injectable secret name. refactor: ♻️ Compile the regex only once Regex was being compiled every for loop instead of only once refactor: ♻️ Use raw strings for regex fix: 🐛 Correct condition refactor: ♻️ Use for each instead of checking the len and getting 0 element fix: 🐛 Correct regex to make ? optional fix: 🐛 Correct return for akvsName
daniel-anova
force-pushed
the
fix/secret-inject-match
branch
from
cf2454d to
445cec2
Compare
181192
approved these changes
Apr 18, 2022
|
Thank you very much @daniel-anova! |
|
@daniel-anova can you please check this issue - #415 |
The mentioned fix in #415 does resolve the issue with certificates, however, I also realized my initial regex was "naive" and doesn't match RFC 1123 DNS names. A more correct regex would be: Copying the regex used by |
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.
Use a more refined regex to match valid injectable secret names.
Argo Workflows passes an environment variable with the workflow manifest in JSON, this was being picked up by
strings.Containsmatch causing it to fail.The regex match is a lot stricter only trying to replace environment variables that only have a injectable secret name.
Resolves #281