-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
docs(proposal): support for post renderers #16749
base: master
Are you sure you want to change the base?
Conversation
1f34c40
to
0c697e9
Compare
Signed-off-by: surajnarwade <[email protected]>
0c697e9
to
4afacfc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #16749 +/- ##
==========================================
- Coverage 49.73% 49.48% -0.25%
==========================================
Files 274 271 -3
Lines 48948 47728 -1220
==========================================
- Hits 24343 23620 -723
+ Misses 22230 21773 -457
+ Partials 2375 2335 -40 ☔ View full report in Codecov by Sentry. |
Signed-off-by: surajnarwade <[email protected]>
... | ||
helm: | ||
... | ||
postRenderers: |
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.
While the proposal naming comes from Helm's post-renderer feature, this proposal isn't expecting to use it directly, and indeed has a custom implementation for kustomize.
To be more broadly useful could this be under source.postRenderers
instead, applying to any sort of application source? (e.g. you might want to post-render some external repo container kustomize, or some custom plugin)
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.
Agreed. I think source.kustomizePatches
makes sense.
Whether or not we nest it in a postRenderers
list is another question.
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.
make sense, I have moved the postRenderers under the source
field now
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.
thanks @mikebryant :) @crenshaw-dev I've moved the struct and also linked the POC.
Signed-off-by: surajnarwade <[email protected]>
I have the POC on my fork: surajnarwade/argo-cd@5bb7cd9...a399863 for the same. |
Heyo, any ETA on when you may get the feature in? I need to use a private helm chart and customize it which is s a LOT more painful than I expected and my options are either to hack it with Kustomize (i.e. this issue) and a Plugin, else if this feature is very soon going to be released I would just use this :) |
@crenshaw-dev Is there anything more we need to do here, or can this proposal be accepted? |
@crenshaw-dev Is there anything more we need in this proposal, can you review again please, TIA 🙏 |
I feel like this could be done via a CMP not? |
I agree, this is what CMPs are for. |
Can you send what argocd generate into a CMP? If it's straightforward maybe this should be an option in the argocd helm chart seeing with the demand for it. It does not seems has straightforward to me between file management, patches to generate and more. |
Checklist: