cdk8s-core icon indicating copy to clipboard operation
cdk8s-core copied to clipboard

Multi-line description on helm chart causes formatting issues after synth

Open BoatPartyJesus opened this issue 2 years ago • 2 comments

Description of the bug:

I have found, when using the OpenEBS helm chart and enabling LVM, that something to do with multiline descriptions and single quotes formatting is causing the synth'd chart to be malformed. I've not been able to pin it down the cause, though.

Reproduction Steps:

cdk.json

language: typescript
app: npx ts-node ./src/main.ts
imports:
  - helm:https://openebs.github.io/charts/[email protected]

chart:

import { Openebs } from "./imports/openebs";
class Storage extends Chart {
  constructor(scope: Construct, name: string) {
    super(scope, name);

    // new Minio(this, "operator");
    // new Tenant(this, "tenant");
    new Openebs(this, "openebs", {
      values: {
        "lvm-localpv": {
          enabled: true
        }
      }
    });
  }
}

pnpm run synth

Notice in synth'd output, the multi-line hasn't been parsed correctly, example:

properties:
  message:
    description: message is a string detailing the encountered erro
    during snapshot creation if specified. NOTE: message may be
    logged, and it should not contain sensitive information.' type: string

Error Log:

Error from server (BadRequest): error when creating "./dist/0001-storage.k8s.yaml": CustomResourceDefinition in version "v1" cannot be handled as a CustomResourceDefinition: strict decoding error: unknown field "spec.versions[0].schema.openAPIV3Schema.properties.apiVersion.of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info", unknown field "spec.versions[0].schema.openAPIV3Schema.properties.kind.object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info"
Error from server (BadRequest): error when creating "./dist/0001-storage.k8s.yaml": CustomResourceDefinition in version "v1" cannot be handled as a CustomResourceDefinition: strict decoding error: unknown field "spec.versions[0].schema.openAPIV3Schema.properties.apiVersion.of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info", unknown field "spec.versions[0].schema.openAPIV3Schema.properties.kind.object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info", unknown field "spec.versions[0].schema.openAPIV3Schema.properties.spec.Required.' properties", unknown field "spec.versions[0].schema.openAPIV3Schema.properties.spec.by a user. More info", unknown field "spec.versions[0].schema.openAPIV3Schema.properties.status.properties.boundVolumeSnapshotContentName.To avoid possible security issues, consumers must verify binding between VolumeSnapshot and VolumeSnapshotContent objects is successful (by validating that both VolumeSnapshot and VolumeSnapshotContent point at each other) before using this object.' type", unknown field "spec.versions[0].schema.openAPIV3Schema.properties.status.properties.error.properties.message.during snapshot creation if specified. NOTE", unknown field "spec.versions[0].schema.openAPIV3Schema.properties.status.properties.error.properties.message.logged, and it should not contain sensitive information.' type"

Environment:

  • Framework Version: 2.151.0
  • OS: Microsoft Windows [Version 10.0.22621.2428] - WSL 2 Debian 12 (Bookworm)

Other Info:

Here is the search against the OpenEBS chart's GitHub highlighting the relavent areas: https://github.com/search?q=repo%3Aopenebs%2Flvm-localpv+sensitive+information.%27&type=code


This is :bug: Bug Report

BoatPartyJesus avatar Oct 22 '23 20:10 BoatPartyJesus

This is an issue with flagger as well. If CRDs are included, the single quotes of description are removed:

          properties:
            apiVersion:
              description: APIVersion defines the versioned schema of this representatio
              of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
              type: string

As you can see, the single quote before "APIVersion" is missing. The closing single quote after "resources" is there. I would expect the whole string to be formatted correctly. With just helm template on the command line, this works as expected.

In the current state, we are not able to use cdk8s to deploy CRDs.

timohirt avatar Feb 16 '24 07:02 timohirt

This is becoming a bigger issue for us, we are having to manually edit before imports which is making us reevaluate the CDK8s as a valid option

BoatPartyJesus avatar Feb 24 '24 18:02 BoatPartyJesus

It looks like the helm chart linked here is not a valid YAML.

After running helm template, the relevant sub-section is as follows:

properties:
  message:
    description: 'message is a string detailing the encountered error
    during snapshot creation if specified. NOTE: message may be
    logged, and it should not contain sensitive information.'
    type: string

This, according to both yamllint and the VSCode YAML (by RedHat) extension, is invalid.

Image Image

Also see https://github.com/openebs/lvm-localpv/issues/269

This corruption exists in the original chart: https://github.com/openebs/lvm-localpv/blob/609b0d5226d58ce733845902c3ce3dcb4694d8cb/deploy/helm/charts/charts/crds/templates/csi-volume-snapshot-content.yaml#L206-L211

I'm not sure why our YAML parser doesn't panic when it encounters this, I suspect it is a bug on their end.

The reason helm preserves the original content is because it probably doesn't try to parse the manually written YAML, it just passes it through. In cdk8s, we do parse the YAML and then serialize it back, which is where the YAML parser bug comes into play.

But regardless, if indeed this is invalid YAML - not much we can do...

@BoatPartyJesus you mention that with helm it works as expected, did you try to deploy the chart as helm produces it? I would think you'll see a kubernetes error at that point.

Given this information, I am removing the p1 classification for this issue, as it doesn't look like a bug for now.

iliapolo avatar Jun 06 '24 10:06 iliapolo

This issue has not received a response in a while and will be closed soon. If you want to keep it open, please leave a comment below @mentioning a maintainer.

github-actions[bot] avatar Jul 06 '24 12:07 github-actions[bot]