Helmfile should not pass --args parameter to Helm Secrets plugin

[smeierhofer]

My Helmfile defines a release that specifies a secret. when I run "helmfile.exe sync --args '--dry-run --debug' so that it passes those options to Helm. But it looks like those options are passed to Helm when running the Helm Secrets plugin. The result is that the Helm Secrets plugin fails because it doesn't recognize the --dry-run option.

Can the helmfile.exe command only pass the --args options to the helm install/upgrade command? Also, it would be useful if the Helmfile "sync" command allows me to pass --dry-run and --debug directly rather than needing to use the --args option.

[Nuru]

We have similar issue with helmfile diff in version 0.80.1. We run

$ helmfile diff --args=“--context 5 --no-color --suppress-secrets”
...
Error: unknown flag: --context

The issue appears to stem from helmfile running

exec: helm repo add repo-name https://repo-host/repo/ --context 5 --no-color --suppress-secrets: 

and --context is not a flag helm repo add understands.

So similarly, we would ask that --args only be passed to helm diff

This used to work under helmfile version 0.73.1

[mumoshu]

Hey!

Generally speaking, I'm going to deprecate --args.

That's because a helmfile command usually results in running two or more helm commands and it's fundamentally hard to decide which args should be passed to which helm command.

@smeierhofer For your use-case, can we just add helmfile sync --dry-run --debug that passes --dry-run --debug` (and possibly makes some other side-effective operations of helmfile to noop, and set --log-level=debug)?

@Nuru helmfile diff has --suppress-secrets so you can safely move it out of --args. For other two, how about just enhancing helmfile diff to accept those flags?

[osterman]

Yes, makes sense to me that we would enhance helmfile diff to handle those flags one way or another.

[Nuru]

It would be great if you could somehow automate your builds so that for every helmfile command you could have it accept any flags that the helm commands it runs would accept, and then pass them to only the helm commands that accept them. I think short of that, you may have a maintenance nightmare trying to keep up with all the flags as helm evolves.

Still, if you are up for it, I want to be able to use --no-color everywhere, so that when I'm running scripts and logging output to a file I don't get escape codes cluttering up everything. --context 5 only makes sense for diff, so just adding it there is fine.

[mumoshu]

It would be great if you could somehow automate your builds so that for every helmfile command you could have it accept any flags that the helm commands it runs would accept, and then pass them to only the helm commands that accept them

Definitely. I have no concrete solution at the moment tho. Maybe both helmfile and helm's code-bases must be refactored so that it becomes possible :)

Still, if you are up for it, I want to be able to use --no-color everywhere

I believe the only helmfile commands with colored output are helmfile apply and helmfile diff. So are you basically requesting to add --no-color to those two?

[Nuru]

Just because an option has no impact does not mean you should not allow it. :-)

I am asking you to implement/accept --no-color --suppress-secrets for all commands. This makes it easier to automate the usage of hemlfile because the scripts can just add those arguments globally. For the commands that do not deal with secrets or color output, those flags can just be ignored.

[mumoshu]

@Nuru I'm still inclined not to make --suppress-secrets global as it is technically impossible to mechanically filter out all the things considered "secret" while everyone would expect it to do so.

But if you think that it's ok for the global flag to work like passing --suppress-secrets to all the helm commands that supports it, I'm ok to make it global.

If I could add, I'd name the global flag --suppress-secrets-from-diff so that it is clear that it only works for helm diff that called from various helm commands(diff and apply right now).

[Nuru]

@mumoshu If a command cannot actually fulfill the expectation of --suppress-secrets then I agree the command should not accept that option. I doubt there is a similar problem with --no-color.

Please, definitely do NOT call the option --suppress-secrets-from-diff. If you only want to accept --suppress-secrets from apply and diff because that is the only place you can ensure the suppression of secrets, then I'm OK with it not being a global option.

[mumoshu]

@Nuru Got it. Thanks for clarifying.

[sstarcher]

@mumoshu we are possibly seeing a similar issue with --args='--set blah=blah' is this another case where you think helmfile sync/apply should have a --set argument and to move away from --args --set

[mumoshu]

@sstarcher Yeah that probably make sense.

Out of curiosity - are you trying to set chart values for all the releases for all the involved helmfile.yaml files, right?

[sstarcher]

We are doing it as part of automation. We have some old legacy services that need certain hooks enabled/disabled. So during some of our upgrades/stop/starts etc we will fun a apply with --set for blah.enabled=false or blah.boostrap=true

We also just had an issue with diff and --args='--allow-unreleased' due to it calling helmfile repo update.

[mumoshu]

We are doing it as part of automation. We have some old legacy services that need certain hooks enabled/disabled. So during some of our upgrades/stop/starts etc we will fun a apply with --set for blah.enabled=false or blah.boostrap=true

That makes sense! Thanks for clarifying.

We also just had an issue with diff and --args='--allow-unreleased' due to it calling helmfile repo update.

AFAICS, helmfile specifies --allow-unreleased by default. So you won't need that?

[Nuru]

@mumoshu said:

AFAICS, helmfile specifies --allow-unreleased by default. So you won't need that?

I just ran

helmfile diff --suppress-secrets --skip-deps --args="--allow-unreleased --context 5 --no-color"

with helmfile v0.82.0 and it failed

  Error: "oidc-role" has no deployed releases
  Error: plugin "diff" exited with error

Did you stop passing --args as you suggested you might?

[mumoshu]

Did you stop passing --args as you suggested you might?

Nope. Are you seeing it's passed to repo up as well? Perhaps we should make --args passed to helm diff only for helmfile diff as before?

[Nuru]

Did you stop passing --args as you suggested you might?

Nope. Are you seeing it's passed to repo up as well? Perhaps we should make --args passed to helm diff only for helmfile diff as before?

The issue I was referring to was that it seemed --args="--allow-unreleased" did NOT pass the --allow-unreleased arg to helm diff. That is, the command

helmfile diff --suppress-secrets --skip-deps --args="--allow-unreleased --context 5 --no-color"

failed because the release was being installed for the first time so there was no deployed release to diff. I don't know when that started, but I think that is new behavior.

[mumoshu]

@Nuru Umm, could it be due to an old version of helmfile perhaps you're using? Looking into https://github.com/roboll/helmfile/blob/fb2041555e98a77aca45793a1113373864e14b58/pkg/helmexec/exec.go#L227, it seems like we do add --allow-unreleased and there's no way to remove it.