-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
feat(appset): add support for missingkey=error in ApplicationSets (#13731) #13733
feat(appset): add support for missingkey=error in ApplicationSets (#13731) #13733
Conversation
c3c42a3
to
13a26f4
Compare
815e177
to
3bd23e0
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #13733 +/- ##
=======================================
Coverage 49.25% 49.25%
=======================================
Files 251 251
Lines 43518 43521 +3
=======================================
+ Hits 21435 21438 +3
Misses 19949 19949
Partials 2134 2134
☔ View full report in Codecov by Sentry. |
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.
Could you update the git files generators docs to specifically call out this feature? To me, it has the most intuitive use case: I want to make sure my devs are fully populating their app.yaml
file, and I want the AppSet to fail if they forget to set a field.
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.
Could you update the git files generators docs to specifically call out this feature? To me, it has the most intuitive use case: I want to make sure my devs are fully populating their app.yaml
file, and I want the AppSet to fail if they forget to set a field.
Sure thing - that would be this file, right? And specifically the This feature will only work when using go templates, so do you think it is better to update all the examples to use go templates instead, or just to add an explanation of how to add the extra validation by migrating if you prefer? |
Yep, that's the file!
I do. We'll eventually want to deprecate and then remove fasttemplate support, since that library is no longer maintained. |
Signed-off-by: Radon Rosborough <rrosborough@plaid.com>
3bd23e0
to
f325eeb
Compare
Updated! I think I translated all the syntax correctly but let me know if I made any mistakes. |
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
I tried to rebase, fingers crossed that I didn't break anything. 😬 |
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Thank you! |
…goproj#13731) (argoproj#13733) * Add support for missingkey=error in ApplicationSets Signed-off-by: Radon Rosborough <rrosborough@plaid.com> * options for cluster generator too Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Radon Rosborough <rrosborough@plaid.com> Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
…goproj#13731) (argoproj#13733) * Add support for missingkey=error in ApplicationSets Signed-off-by: Radon Rosborough <rrosborough@plaid.com> * options for cluster generator too Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --------- Signed-off-by: Radon Rosborough <rrosborough@plaid.com> Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Closes #13731.
The implementation is pretty straightforward. I just introduce a new optional parameter
goTemplateOptions
which is a list of options for the template engine. They are passed as is. The new parameter is threaded through most places thatgoTemplate
is.Checklist:
Optional. My organization is added to USERS.md.Please see Contribution FAQs if you have questions about your pull-request.