[fluentd-elasticsearch] Set OUTPUT_HOSTS though secret

[ArmaanT]

Is this a BUG REPORT or FEATURE REQUEST? (choose one):
FEATURE REQUEST

Version of Helm and Kubernetes:
Helm v3.2.0 and Kubernetes v1.17.5

Which chart in which version:
9.0.0

What happened:
Would it be possible to add the ability to set the OUTPUT_HOSTS environment variable through a secret? it currently is set through elasticsearch.hosts, but we have our values.yaml checked into git and would prefer not to have the hosts visible/stored there.

Let me know if this is a welcome addition and I'll be happy to make a PR for it. I was thinking of removing the elasticsearch.hosts option by default so that way the chart can set OUTPUT_HOSTS only if it exists, but that could be a breaking change.

[rgarcia89]

Why dont you just use a privat repo?

[ArmaanT]

This is for a small, student-run group that open-sources all the work we do. We manage all of our infrastructure using terraform, so this helm chart is a small part of a much larger repo. We try to keep everything we do open-source with an MIT license so that anyone can build upon or modify what we make. A private repo is technically an option, but we would really prefer not to use one.

[monotek]

You could aslo add another if to use it from an existing secret. see:
https://github.com/kiwigrid/helm-charts/blob/master/charts/fluentd-elasticsearch/templates/daemonset.yaml#L67-L71

Add something like:

        valueFrom:
          secretKeyRef:
            name: mysecret
            key: username

Would this work to? So it does not introduce a breaking change.

[ArmaanT]

@monotek I tried that earlier and it seems to work during the first helm install, but whenever I run helm upgrade I get the following error:

Error: UPGRADE FAILED: cannot patch "fluentd-fluentd-elasticsearch" with kind DaemonSet: The order in patch list:
[map[name:OUTPUT_HOSTS valueFrom:map[secretKeyRef:map[key:ELASTIC_HOSTS name:fluentd]]] map[name:OUTPUT_HOSTS value:] map[name:OUTPUT_PATH value:]]
 doesn't match $setElementOrder list:
[map[name:FLUENTD_ARGS] map[name:OUTPUT_HOSTS] map[name:OUTPUT_PATH] map[name:LOGSTASH_PREFIX] map[name:LOGSTASH_FORMAT] map[name:OUTPUT_SCHEME] map[name:OUTPUT_TYPE] map[name:OUTPUT_SSL_VERIFY] map[name:OUTPUT_SSL_VERSION] map[name:OUTPUT_TYPE_NAME] map[name:OUTPUT_BUFFER_CHUNK_LIMIT] map[name:OUTPUT_BUFFER_QUEUE_LIMIT] map[name:OUTPUT_BUFFER_TYPE] map[name:OUTPUT_BUFFER_PATH] map[name:OUTPUT_BUFFER_FLUSH_MODE] map[name:OUTPUT_BUFFER_RETRY_TYPE] map[name:OUTPUT_BUFFER_FLUSH_THREAD_TYPE] map[name:OUTPUT_BUFFER_FLUSH_INTERVAL] map[name:OUTPUT_BUFFER_RETRY_FOREVER] map[name:OUTPUT_BUFFER_RETRY_MAX_INTERVAL] map[name:OUTPUT_BUFFER_OVERFLOW_ACTION] map[name:OUTPUT_LOG_LEVEL] map[name:OUTPUT_INCLUDE_TAG_KEY] map[name:OUTPUT_RECONNECT_ON_ERROR] map[name:OUTPUT_RELOAD_ON_FAILURE] map[name:OUTPUT_RELOAD_CONNECTIONS] map[name:OUTPUT_USER] map[name:OUTPUT_PASSWORD] map[name:OUTPUT_HOSTS] map[name:K8S_NODE_NAME]]

It looks to me like helm doesn't like having OUTPUT_HOSTS defined twice.

Adding an additional if statement definitely works for me. Does something like Values.elasticsearch.load_hosts_from_secret with a default value of false make sense to you?

If so I can make a PR adding that in.