fluxcd / helm-operator

Successor: https://github.com/fluxcd/helm-controller — The Flux Helm Operator, once upon a time a solution for declarative Helming.
https://docs.fluxcd.io/projects/helm-operator/
Apache License 2.0
649 stars 262 forks source link

Merging secrets does not work as expected #126

Closed mazzy89 closed 2 years ago

mazzy89 commented 5 years ago

Describe the bug

I have the following YAML in an Helm chart values.yaml

      config:
        global:
          resolve_timeout: 5m

        receivers:
          - name: "opsgenie_team1"
            opsgenie_configs:
              - responders:
                  - name: team1
                    type: team
                message: '{{`{{ template "opsgenie.message" . }}`}}'
                description: '{{`{{ template "opsgenie.description" . }}`}}'
          - name: "opsgenie_team2"
            opsgenie_configs:
              - responders:
                  - name: team2
                    type: team
                message: '{{`{{ template "opsgenie.message" . }}`}}'
                description: '{{`{{ template "opsgenie.description" . }}`}}'

and I fetch secrets in this way:

   valuesFrom:
    - secretKeyRef:
        # Name of the secret, must be in the same namespace as the
        # HelmRelease
        name: alertmanager # mandatory
        # Key in the secret to get thre values from
        key: values.yaml # optional; defaults to values.yaml
        # If set to true successful retrieval of the values file is no
        # longer mandatory
        optional: false # optional; defaults to false

The Secret values.yaml files that is supposed to merged looks like:

values.yaml:
      config:
        receivers:
          - name: "opsgenie_team1"
            opsgenie_configs:
              - api_key: ""
          - name: "opsgenie_team2"
            opsgenie_configs:
              - api_key: ""

but the merging does not work as expected. I believe that it is not possible to merge at that level. am I right?

To Reproduce Steps to reproduce the behaviour:

  1. What's your setup?
  2. Go to '...'
  3. Click on '....'
  4. Scroll down to '....'
  5. See error

Expected behavior A clear and concise description of what you expected to happen.

Logs If applicable, please provide logs. In a standard stand-alone installation, you'd get this by running kubectl logs -n default deploy/flux-helm-operator.

Additional context Add any other context about the problem here, e.g

mazzy89 commented 5 years ago

I have found here the logic behind the merge https://github.com/fluxcd/helm-operator/blob/e4dc79e67be4fd1ae239ea3902e321ddfd4323ac/pkg/release/release.go#L489 and seems that it is the exact same limitation that Helm has in merging arrays.

I'm not sure if this is an expected thing

mazzy89 commented 5 years ago

This test shows in a better way the behviour:

func TestSlicesValues(t *testing.T) {
    falseVal := false

    chartValues, _ := chartutil.ReadValues([]byte(`image:
  tag: 1.1.1
valuesSlices:
  - chart: true
    configs:
      - message: foo`))

    client := fake.NewSimpleClientset(
        &corev1.Secret{
            ObjectMeta: metav1.ObjectMeta{
                Name:      "release-secret",
                Namespace: "flux",
            },
            Data: map[string][]byte{
                "values.yaml": []byte(`valuesSlices:
  - configs:
      - key: secret`),
            },
        },
    )

    valuesFromSource := []helmfluxv1.ValuesFromSource{
        {
            ConfigMapKeyRef: nil,
            SecretKeyRef: &corev1.SecretKeySelector{
                LocalObjectReference: corev1.LocalObjectReference{
                    Name: "release-secret",
                },
                Key:      "values.yaml",
                Optional: &falseVal,
            },
            ExternalSourceRef: nil,
            ChartFileRef:      nil,
        }}

    values, err := Values(client.CoreV1(), "flux", "", valuesFromSource, chartValues)

    assert.NoError(t, err)
    t.Log(values)
}

As results:

    release_test.go:121: map[image:map[tag:1.1.1] valuesSlices:[map[chart:true configs:[map[message:foo]]]]]
kingdonb commented 2 years ago

Sorry if your issue remains unresolved. The Helm Operator is in maintenance mode, we recommend everybody upgrades to Flux v2 and Helm Controller.

A new release of Helm Operator is out this week, 1.4.4.

We will continue to support Helm Operator in maintenance mode for an indefinite period of time, and eventually archive this repository.

Please be aware that Flux v2 has a vibrant and active developer community who are actively working through minor releases and delivering new features on the way to General Availability for Flux v2.

In the mean time, this repo will still be monitored, but support is basically limited to migration issues only. I will have to close many issues today without reading them all in detail because of time constraints. If your issue is very important, you are welcome to reopen it, but due to staleness of all issues at this point a new report is more likely to be in order. Please open another issue if you have unresolved problems that prevent your migration in the appropriate Flux v2 repo.

Helm Operator releases will continue as possible for a limited time, as a courtesy for those who still cannot migrate yet, but these are strongly not recommended for ongoing production use as our strict adherence to semver backward compatibility guarantees limit many dependencies and we can only upgrade them so far without breaking compatibility. So there are likely known CVEs that cannot be resolved.

We recommend upgrading to Flux v2 which is actively maintained ASAP.

I am going to go ahead and close every issue at once today, Thanks for participating in Helm Operator and Flux! 💚 💙