Helmfile: `helmfile repos` attempts to re-add already added repo

[bskim45]

Hi, I just updated helmfile from v0.128.0 to v0.129.3 and strange behavior happens when I try to attempt repo-related actions. Am I missing something?

helmfile version: 0.129.3
helm version: 3.3.2

helmfile.yaml

repositories:
- name: stable
  url: https://kubernetes-charts.storage.googleapis.com

...

output:

$ helmfile repos                                                                                                       
Adding repo stable https://kubernetes-charts.storage.googleapis.com
in ./helmfile.yaml: command "/usr/local/bin/helm" exited with non-zero status:

PATH:
  /usr/local/bin/helm

ARGS:
  0: helm (4 bytes)
  1: repo (4 bytes)
  2: add (3 bytes)
  3: stable (6 bytes)
  4: https://kubernetes-charts.storage.googleapis.com (48 bytes)

ERROR:
  exit status 1

EXIT STATUS
  1

STDERR:
  Error: repository name (stable) already exists, please specify a different name

COMBINED OUTPUT:
  Error: repository name (stable) already exists, please specify a different name

[mumoshu]

@bskim45 Recently Helmfile started to rely on helm's behaviour that helm repo adding the existing repo updates it(helm repo up but for the specific repo). Which version of Helm are you using?

[bskim45]

Just found out this is due to the breaking change in recent helm release v3.3.2: https://github.com/helm/helm/releases/tag/v3.3.2

Commands like deps and sync are also affected to this change. Should we have to make some changes according to the recent helm update?

[mumoshu]

@bskim45 Thanks. Good to know. Yeah maybe we'd need to add --force-update for v3.3.2 or greater for our use-case.

I couldn't find any info for changes on deps and sync. Would you mind sharing that?

[bskim45]

Nothing special. What I meant to say is that deps and sync commands are also failing since repo operation also called in those two.

[mumoshu]

@bskim45 Ah, I see! Thanks for clarifying.

[Xtigyro]

+1

[Xtigyro]

@bskim45 Thanks. Good to know. Yeah maybe we'd need to add --force-update for v3.3.2 or greater for our use-case.

I couldn't find any info for changes on deps and sync. Would you mind sharing that?

Confirmed to be working with Helm v3.3.1.

[darkn3rd]

FYI. Helm added a breaking change in v3.3.2 and is no longer idempotent. I have opened an issue under helm with https://github.com/helm/helm/issues/8771

[Xtigyro]

@mumoshu

One breaking change was made: The default behavior of helm repo add was changed: It no longer overwrites repositories by default. The flag --force-update was added to helm repo add to allow a repo to be overwritten. The flag --no-update still exists, but does nothing. This change accords with the Helm policy for allowing minor breaking changes in the name of security.

I don't think Helm will revert that. helmfile needs to adhere to the new security rules and add the same flag for those who prefer to impose the previous behaviour (I guess).

[darkn3rd]

I think the solution for this is to no longer to rely on helm for impotence that is expected by other package managers. One way to avoid checking the helm binary versions, and branching out with different behavior, is to always check if the repo exists, and then if it does, do not try to install it.

Unless there was another reason why helmfile would want to reinstall a repository?

[wi1dcard]

I've opened a PR #1488 that might fix this issue.

[Xtigyro]

It seems Helm are willing to try not to throw error in this case though keep the same behaviour. No timeframe was mentioned. Please check: helm/helm#8771

[mumoshu]

Unless there was another reason why helmfile would want to reinstall a repository?

Helmfile uses helm repo add on existing repo for updating it. helm repo up updates all the repos on the system regardless of which one your helmfile.yaml uses, which adds unnecessary additional run time.

[mumoshu]

Thanks everyone! I'm going to review @wi1dcard's PR ASAP and willing to publish a patch release soon.

[RothAndrew]

@bskim45 Thanks. Good to know. Yeah maybe we'd need to add --force-update for v3.3.2 or greater for our use-case.

I couldn't find any info for changes on deps and sync. Would you mind sharing that?

Could we possibly only add --force-update on the few specific versions that require it? IMO the security implication is valid; if name and url are same helm should return 0 but if name is the same and url is different helm errors.

A new PR is being worked right now to have helm return 0 if name and url are both the same

[ktarplee]

The MR in helm was merged. It is targeted for 3.3.4.

[wi1dcard]

Hi there, I just submitted another PR #1494 to remove --force-update for helm versions other than v3.3.2 and v3.3.3. Please check it out see if there's anything I missed.

[tomassatka]

+1