Key duplication in configMaps causing errors on some deployment tools

[nuriel77]

In these two files I found that there is a key duplication in the configMap:

https://github.com/datastax/pulsar-helm-chart/blob/master/helm-chart-sources/pulsar/templates/broker-deployment/broker-configmap.yaml#L222
https://github.com/datastax/pulsar-helm-chart/blob/master/helm-chart-sources/pulsar/templates/zookeeper/zookeeper-configmap.yaml#L40

For example, if I run helm template . and view zookeeper's configMap output:

apiVersion: v1
kind: ConfigMap
metadata:
  name: "pulsar-broker"
  namespace: nuri-test
  labels:
    app: pulsar
    chart: pulsar-2.0.9
    release: RELEASE-NAME
    heritage: Helm
    component: broker
    cluster: pulsar
data:
  zookeeperServers:
    pulsar-zookeeper-ca:2181
  configurationStoreServers:
    pulsar-zookeeper-ca:2181
  clusterName: pulsar
  allowAutoTopicCreationType: "non-partitioned"
  PULSAR_EXTRA_OPTS: -Dpulsar.log.root.level=info
  PULSAR_GC: -XX:+UseG1GC
  PULSAR_LOG_LEVEL: info
  PULSAR_LOG_ROOT_LEVEL: info
  PULSAR_MEM: -Xms2g -Xmx2g -XX:MaxDirectMemorySize=2g -Dio.netty.leakDetectionLevel=disabled
    -Dio.netty.recycler.linkCapacity=1024 -XX:+ExitOnOutOfMemoryError
  backlogQuotaDefaultRetentionPolicy: producer_exception
  brokerDeduplicationEnabled: "false"
  exposeConsumerLevelMetricsInPrometheus: "false"
  exposeTopicLevelMetricsInPrometheus: "true"
  # Workaround for double-quoted values in old values files
  PULSAR_MEM: -Xms2g -Xmx2g -XX:MaxDirectMemorySize=2g -Dio.netty.leakDetectionLevel=disabled -Dio.netty.recycler.linkCapacity=1024 -XX:+ExitOnOutOfMemoryError
  PULSAR_GC: -XX:+UseG1GC 

The last 2 (PULSAR_MEM and PULSAR_GC) appear twice. This isn't a problem for Helm to handle.

However, for deployment automation tools such as kustomize and FluxCD this presents a problem as they depend on kyaml.

This is a known issue: kubernetes-sigs/kustomize#3480
There's even the chance the error will eventually end up in Helm's newer versions.

I haven't noticed this duplication/workaround on apache's pulsar chart.

Would it be possible to remove the duplication or handle it in a way that will not produce double keys in the configMap?
There might be other instances of the duplication in other configMaps, I haven't gone through all of them.

Thank you.

[michaeljmarshall]

@nuriel77 - thank you for this detailed issue. It should definitely be cleaned up. I should be able to fix this and deploy an updated chart early next week.

[michaeljmarshall]

@nuriel77 - #107 resolves this issue. It is technically a breaking change for users that have old values, so we are not going to merge/release this fix until 3.0.0. We plan to release 3.0.0 in the first half of January.

[nuriel77]

@michaeljmarshall Hi! Thanks.

Do I understand correctly that these might be the types of values the workaround was trying to fix: https://github.com/datastax/pulsar-helm-chart/pull/107/files#diff-3be28e7b230396d61d2bc7bd74e5e00070fbfa2c507caa36269a18bfe502df52L37
(as I've notice those would result in getting wrapped by single quotes when using toYaml)

If that is the case, did you consider trying the following approach that might just work for all cases:

---
apiVersion: v1
kind: ConfigMap
metadata:
  name: "cm-pulsar-broker"
  labels:
    app: pulsar
data:
{{- range $key, $val := $.Values.broker.configData }}
  {{ $key }}: {{ $val | replace "\"" "" | trim | quote }}
{{- end }}

Tested with the following values:

broker:
  configData:
    managedLedgerDefaultWriteQuorum: "2"
    zookeeperServers: "cm-pulsar-zookeeper-from-values-file:2181"
    PULSAR_MEM: "\"-Xms312m -Xmx312m -XX:MaxDirectMemorySize=200m -XX:+ExitOnOutOfMemoryError\""
    PULSAR_GC: "-XX:+UseG1GC"
    PULSAR_TEST: some test values

Result:

apiVersion: v1
kind: ConfigMap
metadata:
  name: "cm-pulsar-broker"
  labels:
    app: pulsar
data:
  PULSAR_GC: "-XX:+UseG1GC"
  PULSAR_MEM: "-Xms312m -Xmx312m -XX:MaxDirectMemorySize=200m -XX:+ExitOnOutOfMemoryError"
  PULSAR_TEST: "some test values"
  managedLedgerDefaultWriteQuorum: "2"
  zookeeperServers: "cm-pulsar-zookeeper-from-values-file:2181"

[michaeljmarshall]

Do I understand correctly that these might be the types of values the workaround was trying to fix: https://github.com/datastax/pulsar-helm-chart/pull/107/files#diff-3be28e7b230396d61d2bc7bd74e5e00070fbfa2c507caa36269a18bfe502df52L37
(as I've notice those would result in getting wrapped by single quotes when using toYaml)

@nuriel77 - yes, those are exactly the the values the fix was working around. This looks like a good solution to prevent breaking changes. Are you interested in creating a PR with this fix? If not, I should be able to contribute this fix tomorrow.

[nuriel77]

Are you interested in creating a PR with this fix?

Sure. I will be able to do it tomorrow morning (around European timezone) 👍

[michaeljmarshall]

This was fixed by #110. Thanks for your contribution @nuriel77. I'll run a 2.0.11 release later today that will include this fix.