fix(pkg/helm): Use New to create a complete EnvSettings struct

[samgiles]

This avoids invalid memory addresses from uninitialised fields when initialising plugin environments.

Upgrading Helm brought in this change which relies on config being set to a valid reference. Calling New ensures that it is. :)

Sorry this is pulled into the chart-bump-1.4.0 branch, I was also testing the charts and haven't got time right now to test on master. I can clean up the commit etc tomorrow, just wanted to document this.

Was seeing this error in the 1.4.0-rc1:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xa8 pc=0x16340a2]

goroutine 1 [running]:
k8s.io/cli-runtime/pkg/genericclioptions.(*ConfigFlags).ToRawKubeConfigLoader(0x0, 0x93e02f3e83bd8994, 0x0)
/home/circleci/go/src/github.com/fluxcd/helm-operator/vendor/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go:122 +0x22
helm.sh/helm/v3/pkg/cli.(*EnvSettings).Namespace(0xc0002b0000, 0xc00037d0e0, 0x1d0b1a9)
/home/circleci/go/src/github.com/fluxcd/helm-operator/vendor/helm.sh/helm/v3/pkg/cli/environment.go:177 +0x2f
helm.sh/helm/v3/pkg/cli.(*EnvSettings).EnvVars(0xc0002b0000, 0x160)
/home/circleci/go/src/github.com/fluxcd/helm-operator/vendor/helm.sh/helm/v3/pkg/cli/environment.go:158 +0x5a1
helm.sh/helm/v3/pkg/plugin.SetupPluginEnv(0xc0002b0000, 0xc00003522e, 0x2, 0xc000445ac0, 0x2b)
/home/circleci/go/src/github.com/fluxcd/helm-operator/vendor/helm.sh/helm/v3/pkg/plugin/plugin.go:276 +0x40
helm.sh/helm/v3/pkg/getter.(*pluginGetter).Get(0xc0000ed450, 0xc000445d80, 0x2a, 0xc0003ff1c0, 0x4, 0x4, 0xc000630040, 0x1752278, 0x1ceeba1)
/home/circleci/go/src/github.com/fluxcd/helm-operator/vendor/helm.sh/helm/v3/pkg/getter/plugingetter.go:73 +0x290
helm.sh/helm/v3/pkg/repo.(*ChartRepository).DownloadIndexFile(0xc000095a90, 0xc000095a90, 0x0, 0x0, 0x0)
/home/circleci/go/src/github.com/fluxcd/helm-operator/vendor/helm.sh/helm/v3/pkg/repo/chartrepo.go:127 +0x330
github.com/fluxcd/helm-operator/pkg/helm/v3.(*HelmV3).RepositoryImport(0xc00034b460, 0xc0002bcd53, 0x28, 0x0, 0x0)
/home/circleci/go/src/github.com/fluxcd/helm-operator/pkg/helm/v3/repository.go:111 +0x460
main.main()
/home/circleci/go/src/github.com/fluxcd/helm-operator/cmd/helm-operator/main.go:255 +0x235a

[kingdonb]

Is this an issue with the 1.3.0 release so we should delay publishing it? (I have my finger on the button right now, and just saw your issue. The 1.4.0 release wasn't scheduled for tomorrow, so I'll be happy to include your fix!)

[kingdonb]

I don't see that either branch has upgraded Helm past 3.5.2, the change you linked shows Helm added this at 3.6.2, but I don't fully understand the change that made this necessary, or think any of our test suite is likely to cover Helm Plugins very well, and I did not test this with any helm plugins by hand.

I've already merged 1.3.0 to the 1.3.x branch just as I saw your issue, and was about to kick off the process of releasing it now, but I'd rather delay for a couple of hours rather than publish 1.3.0 quickly followed by 1.3.1 if I can avoid it.

[samgiles]

I think this is an issue with 1.3 too, I tried the 1.3.0-rc1 image just now in my config and get the same error.

[kingdonb]

I've been apprised that since Helm Controller won't support helm plugins, it's not encouraged in Helm Operator and that this PR will enable a lot more than simply fixing the SIGSEGV (things which we don't want to be considered supported, that would complicate your migration to Helm Controller in the future.)

Can you provide some idea of the steps needed to repro the issue, or if it occurs even without the use of Helm Plugins?

[samgiles]

I haven't tested without the Plugin, but this happens everytime with a plugin installed. I think theoretically this code path could be triggered with a "non-plugin" getter, but in this case it doesn't.

My chart values.yaml file I'm testing with is roughly:

image:
  repository: dockerhub.io/fluxcd/helm-operator
  tag: 1.4.0-rc1
  pullPolicy: IfNotPresent

extraEnvs:
  - name: AWS_REGION
    value: eu-west-1

initPlugins:
  enable: true
  plugins:
    - helmVersion: v3
      plugin: https://github.com/hypnoglow/helm-s3.git
      version: 0.10.0

configureRepositories:
  enable: true
  repositories:
    - name: myrepo
      url: s3://my-s3-based-helm-repo


createCRD: false

serviceAccount:
  create: true
  name: "helm-operator"
  annotations:
    eks.amazonaws.com/role-arn: "<ARN ROLE WITH S3 POLICY>"

helm:
  versions: "v3"

You need the plugin to successfully init so the operator can start. Not sure if there's some test plugin that is easy to config.

--
In other news: we're moving to use chartmuseum + v2, but can't just yet because there aren't enough hours in a day 😄, I'm currently running a fork with this fix though and it works well.

[kingdonb]

If you're comfortable with that, then I'm going to go ahead with the release. Glad to hear you have plans to upgrade, and I very much appreciate you bringing this issue to our attention ahead of the release. I will update the CHANGELOG and include in the release notes, so others who are using Helm plugins know this is an issue to be aware of before upgrading is attempted.

[samgiles]

🙏🏼 Yep great! Thanks!

[hiddeco]

Thanks for submitting this patch!

The reason I did not use cli.New() or whatever the equivalent was at the time is, iirc, because it configures (or rather enables) lots of other things I do not want to be set based on arbitrary environmental settings.

I would accept a patch that "mimics" cli.New() but drops the reliance on environmental flags which means we stay in absolute control of runtime configurations.