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

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

Closed
BoatPartyJesus opened this issue Oct 22, 2023 · 4 comments
Labels
closed-for-staleness Issue/PR was closed due to staleness needs-triage Priority and effort undetermined yet response-requested Awaiting response from author

Comments

@BoatPartyJesus
Copy link

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 Report

@BoatPartyJesus BoatPartyJesus added bug Something isn't working needs-triage Priority and effort undetermined yet labels Oct 22, 2023
@timohirt
Copy link

timohirt commented Feb 16, 2024

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.

@BoatPartyJesus
Copy link
Author

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

@sumupitchayan sumupitchayan added effort/medium 1 week tops priority/p1 Should be on near term plans and removed needs-triage Priority and effort undetermined yet labels Apr 11, 2024
@iliapolo
Copy link
Member

iliapolo commented Jun 6, 2024

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 openebs/lvm-localpv#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 iliapolo added response-requested Awaiting response from author needs-triage Priority and effort undetermined yet and removed bug Something isn't working effort/medium 1 week tops priority/p1 Should be on near term plans labels Jun 6, 2024
Copy link

github-actions bot commented Jul 6, 2024

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 github-actions bot added the closing-soon Issue/PR will be closing soon if no response is provided label Jul 6, 2024
@github-actions github-actions bot added closed-for-staleness Issue/PR was closed due to staleness and removed closing-soon Issue/PR will be closing soon if no response is provided labels Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-for-staleness Issue/PR was closed due to staleness needs-triage Priority and effort undetermined yet response-requested Awaiting response from author
Projects
None yet
Development

No branches or pull requests

4 participants