Skip to content
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

helm_package templates should take a folder #69

Closed
iDiogenes opened this issue Feb 16, 2024 · 2 comments
Closed

helm_package templates should take a folder #69

iDiogenes opened this issue Feb 16, 2024 · 2 comments

Comments

@iDiogenes
Copy link
Contributor

Hello!

Currently, helm_package templates require a list of all Helm template files. This approach can be problematic when you are trying to merge template files from various parts of your repo that do not match the same folder structure. For example, if I need to build a single chart that has template files in infra/charts/templates and oem/vendor/infra/charts/templates, I am currently receiving the following error: packager.go:628: Template path (oem/vendor/infra/charts/templates) does not start with templates root (infra/charts/templates).

To fix this, templates should be able to target a folder, and I see two ways to accomplish this:

Option 1) The least destructive method is to update the installHelmContent function in packager.go to determine if templatesManifest is a folder and respond accordingly.

Option 2) Change the helm_package templates to be a label that takes a path to the templates directory. This approach is a bit cleaner because it requires the user to ensure that everything is co-located properly in a templates folder when submitting.

I am happy to make the necessary changes as I am currently blocked by this issue, so please let me know your thoughts when you have a moment.

Thank you.

iDiogenes added a commit to iDiogenes/rules_helm that referenced this issue Feb 27, 2024
abrisco pushed a commit that referenced this issue Apr 22, 2024
* Add support for templates folder when packaging helm (#69)

* lint

* README
@abrisco
Copy link
Owner

abrisco commented Apr 22, 2024

Sorry for the delay here, is 86b6fd9 the solution for this?

@iDiogenes
Copy link
Contributor Author

@abrisco - Yes it looks like my commit to solve this was merged in, so this can now be closed. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants