kafkaesque-io / pulsar-helm-chart

Helm Chart for an Apache Pulsar Cluster
https://helm.kafkaesque.io
Apache License 2.0
31 stars 22 forks source link

fix multiline strings #92

Open filipecaixeta opened 2 years ago

filipecaixeta commented 2 years ago

Zookeeper has a yaml zookeeper-metadata.yaml that contains multiline string fields. You will see something like this in that file

args:
  - >
    bin/pulsar initialize-cluster-metadata \

The problem with this is that the main library in Go for parsing yaml contains a bug and it is not able to generate the yaml files correctly.

I'm using Kustomize to deploy my helm charts (which uses that yaml library) and that is breaking the deployment.

Yaml also has another syntax for multiline string that works just fine

args:
  - |
    bin/pulsar initialize-cluster-metadata \
filipecaixeta commented 2 years ago

@cdbartholomew can you review that for me?

dangermike commented 2 years ago

@filipecaixeta, @cdbartholomew: is there a reason this uses the "folding" block style indicator (>), which should squash newlines when reading the value, but also uses shell line continuation markers (\) at the end of each line? That seems to be saying that the templated value

bin/pulsar initialize-cluster-metadata \
  --cluster {{ template "pulsar.fullname" . }} \
  --zookeeper {{ template "pulsar.fullname" . }}-{{ .Values.zookeeper.component }} \
  --configuration-store {{ template "pulsar.fullname" . }}-{{ .Values.zookeeper.component }} \
  ...

Should be read from the resultant YAML as a single line string:

bin/pulsar initialize-cluster-metadata \  --cluster something \  --zookeeper something-something \  --configuration-store something-something \

It seems like either the proposed change makes sense (change the block from "folding" to "literal") or another change to remove the line continuations makes sense, but what we have now (folding lines with continuations) is a belt-and-suspenders choice we don't need to make.

Or am I reading this wrong? My mental YAML parser doesn't implement the full YAML spec...

filipecaixeta commented 2 years ago

@zzzming