Skip to content

Include external files#10077

Open
itaispiegel wants to merge 12 commits intohelm:mainfrom
itaispiegel:include-external-files
Open

Include external files#10077
itaispiegel wants to merge 12 commits intohelm:mainfrom
itaispiegel:include-external-files

Conversation

@itaispiegel
Copy link

@itaispiegel itaispiegel commented Aug 28, 2021

What this PR does / why we need it:

This PR adds the flag --include-path which allows you to include external files in the install, upgrade and template commands.

Example

Given the configmap:

---
apiVersion: v1
kind: ConfigMap
metadata:
    name: {{ .Release.Name }}-test
data: {{ (.Files.Glob "/tmp/externals/*").AsConfig | nindent 2 }}

And the directory structure:

├── /tmp/externals
│   ├── external1.txt
│   └── external2.txt

Running helm template test_chart --include-path /tmp/externals will generate the manifest:

---
# Source: test_chart/templates/test.configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
    name: RELEASE-NAME-test
data:
  external1.txt: |
    External file 1
  external2.txt: |
    External file 2

If the --include-path is not passed, the output will be:

Error: template: test_chart/templates/test.configmap.yaml:6:41: executing "test_chart/templates/test.configmap.yaml" at <(.Files.Glob "/tmp/externals/*").AsConfig>: error calling AsConfig: must pass files

Use --debug flag to render out invalid YAML

Special notes for your reviewer:
This PR continues the work that @hunsche started in his PR: #8841.
I tried contacting him to discuss his work, but unfortunately, I couldn't reach him so I implemented this myself after discussing this at the Helm dev call which took place on August 19th.

This is my first contribution to Helm, and my first time working with Go. I'd be glad to get feedback for this regarding code style, best practices, and design.

close #3276
close #8227

Related documentation PR: helm/helm-www#1180

Thanks in advance! :)

@helm-bot helm-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 28, 2021
@itaispiegel itaispiegel force-pushed the include-external-files branch 3 times, most recently from 1fdde8d to a03d6b2 Compare August 29, 2021 05:55
@mattfarina mattfarina added this to the 3.8.0 milestone Aug 30, 2021
@mattfarina
Copy link
Collaborator

There's feedback on #8841 that needs to be taken into account here.

@itaispiegel
Copy link
Author

itaispiegel commented Aug 30, 2021

There's feedback on #8841 that needs to be taken into account here.

@mattfarina Can you please address what feedback?

@mattfarina mattfarina removed this from the 3.8.0 milestone Aug 30, 2021
@itaispiegel
Copy link
Author

@mattfarina Btw, I thought about it more and I think I should revert this commit: d1f9bcb

My initial thought was that I want to be able to access the file in the chart by its path and not by an alias.
On the other hand, this doesn't allow us to treat relative paths, because the user can pass --include-path ../external.txt, but then if he references /external.txt in the chart, it won't recognize this file.
If I revert this commit I can do: --include-path ../external.txt:externals/external.txt and then reference externals/external.txt in the chart.

What do you think about this?

@mattfarina
Copy link
Collaborator

@itaispiegel Using a colon as the separator won't work on Windows. Helm is used on Windows, too.

As for the idea of having a key/value pair, that requires the Helm user (chart user) to know about the internal workings of the chart. For many that is not the case. The consumer of a package (just like someone using apt to install mariadb) doesn't usually know the app that's being installed or how it works. I'm not sure the UX is a great experience for that setup.

Am I missing something?

@itaispiegel
Copy link
Author

@itaispiegel Using a colon as the separator won't work on Windows. Helm is used on Windows, too.

I'm not sure this is an issue. For example, you can see that in Docker they use a colon as a delimiter for the -v flag:
https://docs.docker.com/engine/reference/commandline/run/#mount-volume--v---read-only

As for the idea of having a key/value pair, that requires the Helm user (chart user) to know about the internal workings of the chart. For many that is not the case. The consumer of a package (just like someone using apt to install mariadb) doesn't usually know the app that's being installed or how it works. I'm not sure the UX is a great experience for that setup.

Am I missing something?

How is this different than passing values to a chart during the installation? For example, if I want to install the MariaDB chart, I'd need to go over the README file, understand the internal values, and set them accordingly.
I don't think there's really a difference with external files. If a chart requires them, then its documentation should explain how to pass them correctly.

That's also why I think that changing the behavior to fail if the file isn't included (instead of considering it empty) is a necessary behavior. Because then if you include the wrong file, the chart will fail to install, instead of it being installed in a weird state. I don't think though that it should block this PR.

@hickeyma hickeyma added this to the 3.8.0 milestone Aug 31, 2021
@itaispiegel
Copy link
Author

Hey guys, I'm going on vacation so I won't be able to continue working on this until I come back in October.
I'm very motivated to get the work done, so I'll continue this when I come back.
If anyone wants to contribute to this PR by then, feel free to contact me by email: itai.spiegel@gmail.com :)

@bacongobbler
Copy link
Member

@mattfarina Can you please address what feedback?

I believe @mattfarina is referring to these comments:

#8841 (review)
#8841 (comment)

We discussed them in a developer call, but I don't believe there are any concrete test cases covering this scenario with the --include-file flag.

@bacongobbler
Copy link
Member

bacongobbler commented Sep 9, 2021

Wait. Why was --include-file merged into --include-path? Why are we allowing path globbing? And why do we have to treat globbed filepaths differently than regular file paths?

#3276 (comment) never discussed filepath globbing. Just --include-file.

There are a significant number of changes introduced here compared to the originally agreed upon proposal in #3276 (comment). We'll have to restart the entire review process from square one.

@itaispiegel
Copy link
Author

Hey everyone, just wanted to let you know that I came back from my vacation, but just started some new stuff so I'm quite busy.
I am still motivated to get progress with this and I'll try to get back working on this ASAP! :)

@R-omk
Copy link

R-omk commented Oct 13, 2021

You need not forget about helm get all.

@R-omk
Copy link

R-omk commented Oct 13, 2021

What about named aliases for each individual directory? With this approach, the chart does not have to rely on absolute paths. And when some existing release is updated, it will be possible to individually replace the named directory. It's kind of like a docker mount named volumes only in the opposite direction vol_name:/path/to/dir.
Also, my proposal may be related to #1892 (comment)

@k-jingyang
Copy link

k-jingyang commented Oct 18, 2021

If I revert this commit I can do: --include-path ../external.txt:externals/external.txt and then reference externals/external.txt in the chart.

I like this idea. I think this decouples how the chart was written with how the files can be supplied to the chart.

However, I don't think globbing files will be possible anymore with this approach.

I'm keen to help out, so let me know if there's anything I can help 😄

@anamarija
Copy link

anamarija commented Jan 20, 2022

Hi everyone, I would like to contribute to this feature.
Can I pick up the work and continue the implementation :)

My plan would be to:

  • rebase the current work to the current master
  • extend the loadExternalPaths implementation to support named and default aliases like
--include-path vol_name1@/path/to/dir1 --include-path vol_name2@/path/to/dir2
├── /path/to/dir1
│   ├── external1.txt
│   └── external2.txt
├── /path/to/dir2
│   ├── external1.txt
│   └── external2.txt 
---
apiVersion: v1
kind: ConfigMap
metadata:
    name: {{ .Release.Name }}-test
data: {{ (.Files.Get "vol_name1/external1.txt") | nindent 2 }}
---
apiVersion: v1
kind: ConfigMap
metadata:
    name: {{ .Release.Name }}-test
data: {{ (.Files.Glob "vol_name1/*").AsConfig | nindent 2 }}

example with multiple --include-path without named alias. Helm would define a default alias like 0 and 1.

--include-path /path/to/dir1 --include-path /path/to/dir2
---
apiVersion: v1
kind: ConfigMap
metadata:
    name: {{ .Release.Name }}-test
data: {{ (.Files.Get "0/external1.txt") | nindent 2 }}

Also I thought about not only including files from file directories but also maybe github repostiories. Does it make sense to incorporate such a functionality as part of this PR, to be able to include files from github repositories?

@itaispiegel
Copy link
Author

Be my guest :)
Regarding the option to include files from Github repositories - why would you want that?
Remember you can always clone the repository locally and use a symlink. I'm not sure about this addition, so we should wait and see the maintainers' opinion on this.

@anamarija
Copy link

To give more context :)

From my experience working on different enterprise projects, it is common to have different gh repositories for application and ops.
A common use case in a DevOps automated pipeline could be that the helm chart is part of an ops repository that wants to get the application configuration present in a different application gh repository.
It would make sense to have a capability for helm to automatically get the application configuration and make it available to the chart. Additionally, it would play well with tools like ArgoCD and in an environment where there is a huge amount of services that have different application configurations.

But its just a proposition :)

@ostlerc
Copy link

ostlerc commented Aug 9, 2023

@joejulian that's insightful thank you!

But I think fundamentally helm would be useless without the ability to template. Also most package installs do allow several flags to alter the way it is used. And lastly, linux package managers are fundamentally different than helm install. Once those are installed, users are intended to change user configuration files, but with kubernetes and helm that should be done at the time of install, not at a later step.

I could be wrong, but that's how I see it anyways. My team uses helm extensively for the ability to deploy to different environments the same packages. If we took out the ability to change the configuration per environment, it's just a bunch of yaml files. So I think at the core, this is what helm was meant to solve.

@z4ce
Copy link
Contributor

z4ce commented Aug 9, 2023

I think it's most important to think about the core interface with Helm. Right now it's values.yaml and chart in, and on the other side you get a set of Kubernetes resources. To change the contract to be values.yaml and another set of files, I think would be a mistake. Especially when you start to think about things like Argo and Flux.

I would like to understand the use cases more and figure out how we could still keep the contract of helm as it is, while still accommodating whatever use cases are applicable here.

Why would tooling to help people put files and directories into the values structure not be sufficient?

@zfalen-deloitte
Copy link

zfalen-deloitte commented Nov 9, 2023

Honestly the comparison here to apt or yum etc falls woefully short to me. It's 2023 - we are delivering software in portable containers configured to use the 12 Factor philosophy. The entire point is they run anywhere via injected configuration. We aren't (often) SSH'ing into VMs and running apt-get install someSpecificVersion, modifying a .conf file, and then running sudo systemctl start whateverThing anymore. With Kubernetes and Helm - the point to me and thousands of other users is to pass in configuration, and get a functional software stack out of it using the supplied configuration.

We do this all the time manually with kubectl create configmap --from-file - Helm exists to prevent us needing to do these steps manually.

Not being able to pass in the most common method of configuration (files..... pg_config, myTemplate.pug, nginx.conf) in a coherent way from local / repository filesystems is crazy to me.

helm install someCommonGrafanaStack --set configFilesDir=./but/with/this/configuration is like.... the whole point of the exercise

Have reviewed the various workarounds including...

  • base64 encoding a file into a string and setting it as a value (--set) then decoding it in a template
  • symlinking a directory in the chart to a local one
  • copying the chart directly down and untarring it, scripting a copy of a local dir into the chart while renaming it to a specified convention, then running helm install

... and while functional these are all tremendously silly ways of solving the problem. That people are going to this degree of workaround just to get a conf file into a ConfigMap and then mounted on a container should be a massive signal that this feature is needed.

@yann-soubeyrand
Copy link
Contributor

@zfalen-deloitte if your chart supports passing configuration files via values, I guess the --set-file option is what you need 😉

@gilesknap
Copy link

@yann-soubeyrand I am hoping for this PR to be accepted. My use case is that I'm creating containers for EPICS control systems. I create 1 generic container per type of device I want to control. I provide a YAML file to configure the specific instance of the device. If that were the whole story then --set-file would be adequate.

However, I also wish to support other EPICS users who might not want to use my YAML based approach to generating the required startup files. I therefore allow arbitrary files to be included for configuration by other more traditional methods which require a handful of files to be supplied. I don't know up front what these will be.

I'm currently solving this by making the helm chart on the fly before each deployment and it's not that pretty.

Maybe this sounds like bad design but I'm constrained by the 1980's heritage of the control system that I'm trying to make work in Kubernetes. Most of the benefits of doing that still apply even though I'm stuck with some antique configuration files.

@zfalen-deloitte
Copy link

zfalen-deloitte commented Nov 30, 2023

@yann-soubeyrand I stand by my statement. This level of configuration is a tremendously silly way to solve for a simple need. Let alone supporting an arbitrary number of files.

Arguments exist for all ways of doing this. Current model forces extra complexity and boilerplate where there is no need. A huge variety of use cases exist for a values-only model. Quite a few also exist that benefit from mounting non-YAML files.

Where configuration specificity is high, model A makes sense. I want downstream users to avoid headaches by providing simple YAML config options to drive setup. Where generality is high, or where a long-existing configuration model is present, model B makes more sense - it is faster to develop, more grokkable (in some cases), and lets me ship more flexibly. We're just adding noise to the equation by needing to reinvent the thing in YAML and reverse engineer them back into files 🤷🏼‍♂️

@benjie91
Copy link

So... it's 2024, is there any chance that this can be merged? I am still doing the workaround of copying the configs into the chart folder.

@gilesknap
Copy link

I also am still using the same ugly workaround.

@nooperpudd
Copy link

well, well. 4 years, my child is 4 years old. but I'm still waiting for this feature.

@PauloFerreira25
Copy link

@nooperpudd 4 years?
#10077 (comment)
First request is open in 2017

@kalyanidani
Copy link

Well, it's mid-2024! :(
Eagerly waiting for this feature.

@girdevstack
Copy link

It sure feels like every time I come across a problem with modern open source the problem was either debated until everyone gave up and moved on (from the problem or some from open source at all), or some brave soul was tenacious to work a solution and then it never gets merged because ?????

This managed to be both 😭 😭 😭

I understand maintainers and sponsors and so forth have their visions and perspectives and disagreements. But this sure feels disrespectful to everyone involved - is there some way to find consensus for these type of things or is the solution to just not become attached to a thing? It's so confusing.

Back to tilting at windmills.

@ostlerc
Copy link

ostlerc commented May 6, 2025

I also wish the dialogue from maintainers was more open-minded. But given that I’ve accepted they are not (and likely won’t be), we had to design a solution around that.

What we did was create a new schema and a Helm library function to template out our common use cases. We call it Helmix, our own mixture of Helm. It’s written entirely in text/template (ouch!) The main idea is that all configuration files you want to be easily override-able must now be in YAML format. This isn’t ideal, but it makes it possible to automatically modify configuration without reinventing the wheel each time.

Here’s an example that templates a simple configurable configmap from files on disk:

secrets/foobar.yaml

fooval: {{ include "some.function" . }}
status: foobar

helmix.yaml

foobar:
  configmap:
    folder: secrets
    data:
      foobar.yaml:
        status: overridden-status

We have a lot of defaults defined, but the entire ConfigMap is fully configurable via our schema. The goal is to keep only the most important information visible in the YAML. This is specifically designed for Kubernetes objects.

The project isn’t open source, but the idea should be straightforward for others to re-implement in their own way. If you find this approach useful, let me know, I’d love to hear if others have alternative or better solutions.

@github-actions
Copy link

This pull request has been marked as stale because it has been open for 90 days with no activity. This pull request will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Aug 26, 2025
@PauloFerreira25
Copy link

Vai fechar não bot, pode deixar aberto isso ai.

@github-actions github-actions bot removed the Stale label Sep 3, 2025
@PauloFerreira25
Copy link

Vai ficar aberto até resolver

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

This pull request has been marked as stale because it has been open for 90 days with no activity. This pull request will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Dec 4, 2025
@PauloFerreira25
Copy link

Vai ficar aberto até resolver

@github-actions github-actions bot removed the Stale label Dec 5, 2025
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

This pull request has been marked as stale because it has been open for 90 days with no activity. This pull request will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Mar 5, 2026
@PauloFerreira25
Copy link

Vai ficar aberto até resolver, não vou deixar fechar

@github-actions github-actions bot removed the Stale label Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow helm to add external files to chart deployment