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.

Sign up for GitHub

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

Jump to bottom

Treat null value as any type when generating schema from Helm chart #1677

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

Treat null value as any type when generating schema from Helm chart #1677

wants to merge 1 commit into from
+93 −7

Conversation

mamachanko
Copy link
Contributor

@mamachanko mamachanko commented Jan 22, 2025 •
edited
Loading

What this PR does / why we need it:

This is a proof of concept for treating null values in Helm chart values as any type using a type union of all possible types being nullable. That is because the expected type cannot be inferred from null.

When releasing a Package from a Helm chart that has null values, the result looks like this:

apiVersion: data.packaging.carvel.dev/v1alpha1
kind: Package
metadata:
  creationTimestamp: null
  name: test.mamachanko.com.0.0.0+build.1737555606
spec:
  refName: test.mamachanko.com
  releasedAt: "2025-01-22T14:20:11Z"
  template:
    spec:
      deploy:
      - kapp: {}
      fetch:
      - imgpkgBundle:
          image: index.docker.io/mamachanko/helm-test@sha256:f80aac3740c96da062f922790482c3aebe7c2c57061bdf0b4208b4833e276ac9
      template:
      - helmTemplate:
          path: helm
      - ytt:
          paths:
          - '-'
      - kbld:
          paths:
          - '-'
          - .imgpkg/images.yml
  valuesSchema:
    openAPIv3:
      properties:
        credentials:
          description: helm/values.yaml
          properties:
            password:
              oneOf:
              - default: null
                nullable: true
                type: integer
              - default: null
                nullable: true
                type: number
              - default: null
                nullable: true
                type: boolean
              - default: null
                nullable: true
                type: string
              - default: null
                nullable: true
                type: object
              - default: null
                items: {}
                nullable: true
                type: array
            username:
              oneOf:
              - default: null
                nullable: true
                type: integer
              - default: null
                nullable: true
                type: number
              - default: null
                nullable: true
                type: boolean
              - default: null
                nullable: true
                type: string
              - default: null
                nullable: true
                type: object
              - default: null
                items: {}
                nullable: true
                type: array
          type: object
      type: object
  version: 0.0.0+build.1737555606

as opposed to the previous:

apiVersion: data.packaging.carvel.dev/v1alpha1
kind: Package
metadata:
  creationTimestamp: null
  name: test.mamachanko.com.0.0.0+build.1737555606
spec:
  refName: test.mamachanko.com
  releasedAt: "2025-01-22T14:20:11Z"
  template:
    spec:
      deploy:
      - kapp: {}
      fetch:
      - imgpkgBundle:
          image: index.docker.io/mamachanko/helm-test@sha256:f80aac3740c96da062f922790482c3aebe7c2c57061bdf0b4208b4833e276ac9
      template:
      - helmTemplate:
          path: helm
      - ytt:
          paths:
          - '-'
      - kbld:
          paths:
          - '-'
          - .imgpkg/images.yml
  valuesSchema:
    openAPIv3:
      properties:
        credentials:
          description: helm/values.yaml
          properties:
            password:
              default: "null"
              type: "null"
            username:
              default: "null"
              type: "null"
          type: object
      type: object
  version: 0.0.0+build.1737555606

Which issue(s) this PR fixes:

Fixes #1630

Does this PR introduce a user-facing change?

fix: treat `null` in Helm value as _any_ when generating OpenAPI v3 schema

Additional Notes for your reviewer:

Review Checklist:
  • Follows the developer guidelines
  • [ x Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

Additional documentation e.g., Proposal, usage docs, etc.:

@mamachanko mamachanko mentioned this pull request Jan 22, 2025
Open
nebhale
nebhale approved these changes Jan 22, 2025
View reviewed changes
Copy link

@nebhale nebhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nobody asked me, but I love it!

@mamachanko mamachanko force-pushed the topic/mamachanko/develop/fix-helm-schema branch from 2e22d0d to 668024c Compare January 23, 2025 14:50
@mamachanko mamachanko changed the title Treat null in Helm value as _any_ Treat null value as any type when generating schema from Helm chart Jan 23, 2025
@mamachanko mamachanko marked this pull request as ready for review January 23, 2025 14:52
@mamachanko
Copy link
Contributor Author

whoops. failing e2e tests. i'm on it ...

@mamachanko mamachanko force-pushed the topic/mamachanko/develop/fix-helm-schema branch from 668024c to d64e8fe Compare January 27, 2025 13:49
@mamachanko mamachanko force-pushed the topic/mamachanko/develop/fix-helm-schema branch from d64e8fe to 49bb275 Compare January 28, 2025 12:30
@mamachanko
Copy link
Contributor Author

whoops. failing e2e tests. i'm on it ...

green! ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nebhale nebhale nebhale approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
Carvel
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

kctrl package release creates an invalid OpenAPI spec from Helm Chart
2 participants
@mamachanko @nebhale