-
Notifications
You must be signed in to change notification settings - Fork 59
feat: add support for multiple inline templates #519
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
base: main
Are you sure you want to change the base?
feat: add support for multiple inline templates #519
Conversation
|
I see that the previously missing code fragments for Pull Request #488 have now been generated and included. Can these remain, or should I revert them? |
|
@doxsch thanks for the contribution. Can you provide some context around how this would be used in practice? I'm not very familiar with kustomize and I don't see it being executed here so I'm wondering if it is being called in a pipeline or gitops controller like Argo? Thanks |
Why do they need to be reverted? |
In our case, it will be Argo CD that runs kustomize build before applying. However, this would work just as well in a pipeline by using either
I was just wondering if it might make sense to submit this separately, since it’s not really related to the actual feature. |
Ok so this is a way of allowing kustomize to generate a list of templates during composition reconciliation and then the function concatenates the templates at runtime. Out of curiosity - have you considered using the |
6dcd112 to
e17015b
Compare
Kustomize is executed before kubectl apply. This means the composition already contains a fully rendered list of templates; it does not happen during composition reconciliation.
Yes, I looked into that option. However, I think the approach with directly use Kustomize is simpler. |
And they get concatenated into a single template by the function.
Simpler from the kustomize side but at the expense of a slightly more complicated API on the function side. I'm going to ask the other maintainers to weigh in on whether the API change is acceptable. I feel like it makes the function API less clean and I'm not sure that's warranted when there is another solution available. I'm not saying no, just asking for other opinions. @negz @sergenyalcin @turkenf ? |
|
@ezgidemirel @phisco any opinions? |
|
Since the function isn’t meant to be a full templating engine, I like the idea of handling more complex cases with external tools like kustomize and passing the rendered output into the function. This keeps the function simple while still giving users room to scale. I agree that this change adds some API complexity, but I think that can be addressed with clear documentation and guidance around when to use each approach. |
e17015b to
74fd4e9
Compare
7e48dc9 to
cfd6b58
Compare
Signed-off-by: doxsch <28098153+doxsch@users.noreply.github.com>
Signed-off-by: doxsch <28098153+doxsch@users.noreply.github.com>
cfd6b58 to
679d5cc
Compare
Description of your changes
This PR introduces support for multiple inline templates to improve maintainability and scalability when using source: Inline.
Fixes #104
I have: