Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix multiline strings #92

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

filipecaixeta
Copy link

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
Copy link
Author

@cdbartholomew can you review that for me?

@dangermike
Copy link

@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
Copy link
Author

@zzzming

michaeljmarshall pushed a commit to datastax/pulsar-helm-chart that referenced this pull request Mar 15, 2022
Zookeeper has a yaml `zookeeper-metadata.yaml` that contains multiline string fields.
You will see something like this in that file
```yaml
args:
  - >
    bin/pulsar initialize-cluster-metadata \
```

The problem with this is that the main library in Go for parsing yaml contains a [bug](go-yaml/yaml#827) and it is not able to generate the yaml files correctly.


I'm using [Kustomize](https://kustomize.io/) 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

```yaml
args:
  - |
    bin/pulsar initialize-cluster-metadata \
```

kafkaesque-io/pulsar-helm-chart#92
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants