cdk8s-team / cdk8s

Define Kubernetes native apps and abstractions using object-oriented programming
https://cdk8s.io
Apache License 2.0
4.36k stars 294 forks source link

Multiline configmap data produces output with lines separated by empty lines #172

Closed mziyabo closed 3 years ago

mziyabo commented 4 years ago

Multiline configmap data produces output with lines separated by empty lines

The output of the suggestion #86 is a little off (i.e. not pretty). For example, with the snippet below: You will get a yaml file with empty lines in between each line in the array. To contrast, running console.log(daemonfile) in TypeScript playground(/node for the compiled javascript), the multiline output has no empty lines in between (as required).

In the end the yaml is a little chunkier than it should be. Am I missing something here?

let daemonfile = [
            "# Maximum buffer size in MB (minimum 3). Choose 0 to use 1% of host memory. ",
            "TotalBufferSizeMB: 0",
            "# Maximum number of concurrent calls to AWS X-Ray to upload segment documents.",
            "Concurrency: 8",
            "# Send segments to AWS X-Ray service in a specific region",
            "Region: \"\"",
            "# Change the X-Ray service endpoint to which the daemon sends segment documents.",
            "Endpoint: \"\"",
            "Socket:",
            "  # Change the address and port on which the daemon listens for UDP packets containing segment documents.",
            "  # Make sure we listen on all IP's by default for the k8s setup",
            "  UDPAddress: 0.0.0.0:2000",
            "Logging:",
            "  LogRotation: true",
            "  # Change the log level, from most verbose to least: dev, debug, info, warn, error, prod (default).",
            "  LogLevel: prod",
            "  # Output logs to the specified file path.",
            "  LogPath: \"\"",
            "# Turn on local mode to skip EC2 instance metadata check.",
            "LocalMode: false",
            "# Amazon Resource Name (ARN) of the AWS resource running the daemon.",
            "ResourceARN: \"\"",
            "# Assume an IAM role to upload segments to a different account.",
            "RoleARN: \"\"",
            "# Disable TLS certificate verification.",
            "NoVerifySSL: false",
            "# Upload segments to AWS X-Ray through a proxy.",
            "ProxyAddress: \"\"",
            "# Daemon configuration file format version.",
            "Version: 1"
        ].join("\n");

        new ConfigMap(scope, "xray-config", {
            data: {
                "config.yaml": daemonfile
            }
        });

I also tried with the another pattern, i.e. using a back tick ` e.g. The output wasn't any better:

let daemonfile = `
line1
line2
...
`
new ConfigMap(scope, "xray-config", {
            data: {
                "config.yaml": daemonfile
            }
        });

I could not get away with trying regex to replace new lines as well, e.g.

const regex = new RegExp("\n{2,}",'gi');
let daemonfile = [
            "# Maximum buffer size in MB (minimum 3). Choose 0 to use 1% of host memory. ",
            "TotalBufferSizeMB: 0",
            "# Maximum number of concurrent calls to AWS X-Ray to upload segment documents.",
            "Concurrency: 8",
            "# Send segments to AWS X-Ray service in a specific region",
            "Region: \"\"",
...
]
].join("\n").replace(regex, "");

new ConfigMap(scope, "xray-config", {
            data: {
                "config.yaml": daemonfile
            }
        });
heckler1 commented 3 years ago

I had trouble with this as well, and was able to chase down what I believe to be the root cause, using the following PoC code:

import { App } from "cdk8s";
import { Construct } from "constructs";
import { Chart, ChartProps } from "cdk8s";
import { KubeConfigMap } from "./imports/k8s";

const app = new App();

export class MyChart extends Chart {
  constructor(scope: Construct, id: string, props: ChartProps = {}) {
    super(scope, id, props);
    new KubeConfigMap(this, `test-cm`, {
      metadata: {
        annotations: {
          "annotation/number": "12345",
          "annotation/date": "2020-10-10",
          "annotation/boolean": "on",
          "annotation/string": "normal string",
        },
      },
      data: {
        records: `
- long: document
  multiline: strings
    nested: records
`,
      },
    });
  }
}
new MyChart(app, "testing")
app.synth();

It appears that there was a previous issue, #325, which was due to certain boolean and date types not being parsed correctly. The original incorrect YAML was analogous to:

apiVersion: v1
data: # note the block literal
  records: |

    - long: document
      multiline: strings
        nested: records
kind: ConfigMap
metadata:
  annotations:
    annotation/boolean: on # this is of type boolean
    annotation/date: 2020-10-10 # this is of type date
    annotation/number: "12345" # but the number has been properly stringified
    annotation/string: normal string
  name: testing-test-cm-c8f3662e

The resolution of #325 was #327 which, among other things, included the addition of this line that sets the default string type toQUOTE_DOUBLE.

However, doing this prevents any block literals from being created by the YAML stringifier, so a multiline string ends up escaped many times, as seen in this example:

apiVersion: "v1"
data:
  records: "

    - long: document

    \  multiline: strings

    \    nested: records\n"
kind: "ConfigMap"
metadata:
  annotations:
    annotation/boolean: "on"
    annotation/date: "2020-10-10"
    annotation/number: "12345"
    annotation/string: "normal string"
  name: "testing-test-cm-c8f3662e"

I believe that in the original example, the boolean is not stringified because "on" is not a valid boolean under the default schema for the YAML package, yaml-1.2. The same issue occurs with the date, YYYY-MM-DD is not a valid date format under yaml-1.2. Therefore the YAML stringifier skips stringification of these items. This causes a compatibility issue with other parsers that are using the yaml-1.1 schema.

As we are already loading YAML files as yaml-1.1, I propose we set the schema to yaml-1.1 by default, and revert to the default PLAIN string type, which lets the parser decide when to use quotes or block literals.

That will give us a still correct and much prettier result:

apiVersion: v1
data:
  records: |

    - long: document
      multiline: strings
        nested: records
kind: ConfigMap
metadata:
  annotations:
    annotation/boolean: "on"
    annotation/date: "2020-10-10"
    annotation/number: "12345"
    annotation/string: normal string
  name: testing-test-cm-c8f3662e

If this is an acceptable solution, I'm happy to write the PR. I don't believe there is significant risk of regression here, but I could be wrong, I'm no YAML expert.

github-actions[bot] commented 3 years ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.