Operator-sdk: Comma is not allowed in Override Values of watches.yaml in Helm-based Operator

[morvencao]

Bug Report

What did you do?

I created an Helm based operator and try to pass down environment variables from the Operators Deployment all the way to the helm charts templates by adding the overrideValues in watched.yaml:

- group: example.com
  version: v1alpha1
  kind: Nginx
  chart: helm-charts/nginx
  overrideValues:
    watchNamespace: ${WATCH_NAMESPACE}

Then I installed the operator in MULTIPLE_NAMEPACE mode, in that case the env WATCH_NAMESPACE in Operator Deployment will be comma separated string list, eg. ns1,ns2

After the operator is starting running, I found the following error in operator logs and the reconcile errors:

{"level":"error","ts":1603355830.9822102,"logger":"helm.controller","msg":"Failed to get release manager","namespace":"ns1","name":"default","apiVersion":"example.com/v1alpha1","kind":"nginx","error":"failed to parse override values: key \"ns2\" has no value","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error\n\tpkg/mod/github.com/go-logr/[email protected]/zapr.go:128\ngithub.com/operator-framework/operator-sdk/internal/helm/controller.HelmOperatorReconciler.Reconcile\n\tsrc/github.com/operator-framework/operator-sdk/internal/helm/controller/reconcile.go:92\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\tpkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:235\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tpkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:209\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker\n\tpkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:188\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\tpkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:155\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\tpkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:156\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\tpkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.Until\n\tpkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:90"}

What did you expect to see?

Override Values in watched.yaml should support comma separated string.

What did you see instead? Under which circumstances?

Environment

Operator type:

Kubernetes cluster type:

$ operator-sdk version

$ go version (if language is Go)

$ kubectl version

Possible Solution

Additional context

/cc @liqlin2015

[morvencao]

/bug
/helm

[camilamacedo86]

Hi @morvencao,

I am curious if it is not because the ${WATCH_NAMESPACE} has not the values inside of ", such as "ns1,ns2"? Did you try to check it?

[joelanford]

This looks like a bug to me. A Helm strvals line can contain multiple key value pairs, but we expect each strval to be a separate map key in overrideValues.

We convert the map entries into strval lines here. It looks like we need to apply @camilamacedo86's suggestion and change that line to the following:

val := fmt.Sprintf("%s=%q", k, v)

[joelanford]

Actually my above suggestion doesn't seem to work either. We might need to do some research to see if the strvals parser can handle commas in values.

[joelanford]

Ah! Looks like commas and equal signs need to be escaped when they are meant to be literal values.

However, strvals allows nested setting of arrays and values in those arrays, so it is possible to have multiple strvals key/value expressions inside an strval value expression.

It turns out this is pretty complicated and probably something we shouldn't try to interpret because there are lots of possibilities with strvals values.

I think in your case, your best bet would be to make your watchNamespace chart value be an array instead of a string. If you do that, then you should be able to do something like this:

- group: example.com
  version: v1alpha1
  kind: Nginx
  chart: helm-charts/nginx
  overrideValues:
    watchNamespace: '{${WATCH_NAMESPACE}}'

Then, if you need it to be a comma-separated string in templated resources, you could use:

{{ join "," .Values.watchNamespace }}

[joelanford]

Also, out of curiosity, why do your templated resources need to know what namespaces your operator is watching?

It seems like WATCH_NAMESPACE should only be a concern of the operator, not the operands.

[morvencao]

@camilamacedo86 The ${WATCH_NAMESPACE} does have the values inside of the quote, because it is set by the Downward API from annotation:

                - name: WATCH_NAMESPACE
                  valueFrom:
                    fieldRef:
                      fieldPath: metadata.annotations['olm.targetNamespaces']

@joelanford Thanks for the investigation and quick reply, I did try your solution and it is working. Thanks again!!
BTW, the reason why I pass WATCH_NAMESPACE from operator to operand is that our operand is also a controller and it needs to watch its CR, it may sound weird, but we may change the behavior in the future.

[pre]

Workaround of https://github.com/operator-framework/operator-sdk/issues/4084#issuecomment-714533177 also works for Helm overrideValues which contain comma-separated values, such as an env LDAP_BASE_DC whose value is dc=example,dc=com.

Thanks!