`kctrl package release` creates an invalid OpenAPI spec from Helm Chart

[nebhale]

What steps did you take:

Given the following:

# helm/Chart.yaml
apiVersion: v2
name: test
version: 0.1.0

# helm/values.yaml
credentials:
  username: null
  password: null

# package-build.yml
apiVersion: kctrl.carvel.dev/v1alpha1
kind: PackageBuild
metadata:
  creationTimestamp: null
  name: test.nebhale.com
spec:
  template:
    spec:
      app:
        spec:
          deploy:
          - kapp: {}
          template:
          - helmTemplate:
              path: helm
          - ytt:
              paths:
              - '-'
          - kbld: {}
      export:
      - includePaths:
        - helm

# package-resources.yml
apiVersion: data.packaging.carvel.dev/v1alpha1
kind: Package
metadata:
  creationTimestamp: null
  name: test.nebhale.com.0.0.0
spec:
  refName: test.nebhale.com
  releasedAt: null
  template:
    spec:
      deploy:
      - kapp: {}
      fetch:
      - git: {}
      template:
      - helmTemplate:
          path: helm
      - ytt:
          paths:
          - '-'
      - kbld: {}
  valuesSchema:
    openAPIv3: null
  version: 0.0.0

---
apiVersion: data.packaging.carvel.dev/v1alpha1
kind: PackageMetadata
metadata:
  creationTimestamp: null
  name: test.nebhale.com
spec:
  displayName: test
  longDescription: test.nebhale.com
  shortDescription: test.nebhale.com

---
apiVersion: packaging.carvel.dev/v1alpha1
kind: PackageInstall
metadata:
  annotations:
    kctrl.carvel.dev/local-fetch-0: .
  creationTimestamp: null
  name: test
spec:
  packageRef:
    refName: test.nebhale.com
    versionSelection:
      constraints: 0.0.0
  serviceAccountName: test-sa
status:
  conditions: null
  friendlyDescription: ""
  observedGeneration: 0

run kctrl package release

What happened: The following, invalid OpenAPIV3 was created. Specifically .spec.valuesSchema.openAPIv3.properties.credentials.properties.[password|username].type="null" is invalid as null is not a valid type by spec.

# carvel-artifacts/package.yml
apiVersion: data.packaging.carvel.dev/v1alpha1
kind: Package
metadata:
  creationTimestamp: null
  name: test.nebhale.com.0.0.0+build.1727907345
spec:
  refName: test.nebhale.com
  releasedAt: "2024-10-02T22:15:55Z"
  template:
    spec:
      deploy:
      - kapp: {}
      fetch:
      - imgpkgBundle:
          image: index.docker.io/bhale382/test@sha256:be8d6e5d70f56cf37a5beff589c52d235e852474f0b9dc805d5199166627fd2c
      template:
      - helmTemplate:
          path: helm
      - ytt:
          paths:
          - '-'
      - kbld:
          paths:
          - '-'
          - .imgpkg/images.yml
  valuesSchema:
    openAPIv3:
      properties:
        credentials:
          properties:
            password:
              default: "null"
              type: "null"
            username:
              default: "null"
              type: "null"
          type: object
      type: object
  version: 0.0.0+build.1727907345

What did you expect: I'd have expected a valid type to be put into the schema.

Anything else you would like to add: It appears if the following had been used, the type would have been string.

# helm/values.yaml
credentials:
  username:
  password:

Unfortunately after a run through ytt, the nulls are put into place and that behavior is not preserved. I'd be open to this being the behavior in the place of null as well.

Environment:

• kapp Controller version (execute kubectl get deployment -n kapp-controller kapp-controller -o yaml and the annotation is kbld.k14s.io/images): N/A
• Kubernetes version (use kubectl version): N/A

---

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

[100mik]

I am wondering if this is resolved if PackageBuild does not have a ytt section. Maybe init should not be adding it for helm only scenarios.

[nebhale]

I'm doing a pre-pass with ytt directly for other reasons, so I don't think it's the one in PackageBuild that's the real problem.

[mamachanko]

I am wondering if this is resolved if PackageBuild does not have a ytt section. Maybe init should not be adding it for helm only scenarios.

the issue remains if the ytt section is omitted

❯ git --no-pager d
diff --git a/reproduce/package-build.yml b/reproduce/package-build.yml
index 85188266f..a611767ca 100644
--- a/reproduce/package-build.yml
+++ b/reproduce/package-build.yml
@@ -15,9 +15,6 @@ spec:
           template:
           - helmTemplate:
               path: helm
-          - ytt:
-              paths:
-              - '-'
           - kbld: {}
       export:
       - imgpkgBundle:
diff --git a/reproduce/package-resources.yml b/reproduce/package-resources.yml
index e04a2f141..d2924ce0c 100644
--- a/reproduce/package-resources.yml
+++ b/reproduce/package-resources.yml
@@ -16,9 +16,6 @@ spec:
       template:
       - helmTemplate:
           path: helm
-      - ytt:
-          paths:
-          - '-'
       - kbld: {}
   valuesSchema:
     openAPIv3: null

❯ kctrl package release
...

❯ yq .spec.valuesSchema <carvel-artifacts/packages/test.mamachanko.com/package.yml
openAPIv3:
  properties:
    credentials:
      description: helm/values.yaml
      properties:
        password:
          default: "null"
          type: "null"
        username:
          default: "null"
          type: "null"
      type: object
  type: object

[mamachanko]

I've been looking at possible paths ahead.

When a field's value is null

# a field without an explicit type
anything: null

then, kapp-controller could export its OpenAPI v3 schema type by

• assuming a nullable, lowest-common denominator type, e.g. string

  anything:
    type: string
    nullable: true
    default: null
  

  or,

• omitting it from the values schema, or

• assuming it to be any

  anything:
    oneOf:
    - 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
      nullable: true
    - type: array
      default: null
      nullable: true
      items: {}
  

  There's no easy way to express this other than a union of all types and have them nullable (see: https://swagger.io/docs/specification/v3_0/data-models/data-types/?t#any-type).

  The description would have to be attached to each object in oneOf. That's repetitive, but not obviously wrong. After all I think this should motivate the package author to revisit this field in their Helm chart.

My personal take is that ^ options are ordered worst to best. Will see whether I can draft a PR for the any approach.

Furthermore, authors of Helm chart's can include a JSON schema file values.schema.json (see: https://helm.sh/docs/topics/charts/#schema-files) in their chart. However, whether kctrl should consider this JSON schema is a follow-up imo, because anything might be missing from the JSON schema, the schema might disagree with values.yaml, etc.

@nebhale @100mik

[nebhale]

I don’t think “omitting it from the values schema” is a viable option anywhere in the list. Practically, the chart I was working with when I opened this issue had a handful of these kinds of keys and they were the values that most commonly needed to be set by users of the chart. Leaving them out really feels like the wrong option in that case, especially if you start thinking that the schema is used for automated input UI generation or validation.

[mamachanko]

@nebhale @100mik have a look at https://github.com/carvel-dev/kapp-controller/pull/1677. mind you, it's desperate for clean up, but it's proofing the any approach.

[mamachanko]

@nebhale @100mik have a look at #1677. mind you, it's desperate for clean up, but it's proofing the any approach.

polished. ready for review. 🔬