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

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

@mattfarina Can you please address what feedback?

@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?

@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 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.

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: [email protected] :)

@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.

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.

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! :)

You need not forget about helm get all.

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)

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 😄

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?

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.

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 :)

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

@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.

@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 🤷🏼‍♂️

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.

I also am still using the same ugly workaround.

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

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

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

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.

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.

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.

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

Vai ficar aberto até resolver

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.

Vai ficar aberto até resolver

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.

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

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.

Fecha não, bota os agents de IA pre resolver...