Repository: carvel-dev/kapp-controller (go)

Issue:
`kctrl package release` creates an invalid OpenAPI spec from Helm Chart
**What steps did you take:**

Given the following:

```yaml
# helm/Chart.yaml
apiVersion: v2
name: test
version: 0.1.0
```

```yaml
# helm/values.yaml
credentials:
  username: null
  password: null
```

```yaml
# 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
```

```yaml
# 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](https://spec.openapis.org/oas/v3.0.3#data-types).

```yaml
# 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`.

```yaml
# helm/values.yaml
credentials:
  username:
  password:
```

Unfortunately after a run through `ytt`, the `null`s 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.


Pull request:
Treat `null` value as any type when generating schema from Helm chart
<!--  Thanks for sending a pull request!  Here are some tips for you:

If this is your first time, please read our contributor guidelines: https://github.com/carvel-dev/kapp-controller/blob/develop/CONTRIBUTING.md and developer guide https://github.com/carvel-dev/kapp-controller/blob/develop/docs/dev.md
-->

#### What this PR does / why we need it:

This change treats `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:

```yaml
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:

```yaml
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:
<!--
If no issue exists for this change, please create an issue and link it here.
-->
Fixes #1630 

#### Does this PR introduce a user-facing change?
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. 

-->
```release-note
fix: treat `null` in Helm value as _any_ when generating OpenAPI v3 schema
```

#### Additional Notes for your reviewer:

##### Review Checklist:

- [x] Follows the [developer guidelines](https://carvel.dev/shared/docs/latest/development_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.:

```docs

```


Patch:
diff --git a/cli/pkg/kctrl/cmd/package/release/schemagenerator/helm_openapi_schema_gen.go b/cli/pkg/kctrl/cmd/package/release/schemagenerator/helm_openapi_schema_gen.go
index 373af5f9..4a89f6c8 100644
--- a/cli/pkg/kctrl/cmd/package/release/schemagenerator/helm_openapi_schema_gen.go
+++ b/cli/pkg/kctrl/cmd/package/release/schemagenerator/helm_openapi_schema_gen.go
@@ -26,6 +26,7 @@ const (
 	itemsKey       = "items"
 	propertiesKey  = "properties"
 	defaultKey     = "default"
+	nullableKey    = "nullable"
 )
 
 var keyOrder = map[string]int{
@@ -35,6 +36,7 @@ var keyOrder = map[string]int{
 	itemsKey:       4,
 	propertiesKey:  5,
 	defaultKey:     6,
+	nullableKey:    7,
 }
 
 const (
@@ -211,6 +213,11 @@ func (h HelmValuesSchemaGen) calculateProperties(key *yaml3.Node, value *yaml3.N
 		if err != nil {
 			return nil, err
 		}
+		if value.Tag == nullTag {
+			// We cannot infer a key's type from a null value and must assume "any".
+			apiKeys = append(apiKeys, newAnyType())
+			break
+		}
 		apiKeys = append(apiKeys, &MapItem{Key: typeKey, Value: h.openAPIType(value.Tag, value.Value)})
 		apiKeys = append(apiKeys, &MapItem{Key: defaultKey, Value: defaultVal})
 		if value.Tag == floatTag {
@@ -266,7 +273,6 @@ func (h HelmValuesSchemaGen) openAPIType(tag, value string) string {
 		}
 	}
 	return "string"
-
 }
 
 func (h HelmValuesSchemaGen) getDefaultValue(tag, value string) (interface{}, error) {
@@ -281,3 +287,29 @@ func (h HelmValuesSchemaGen) getDefaultValue(tag, value string) (interface{}, er
 		return value, nil
 	}
 }
+
+func newAnyType() *MapItem {
+	nullable := func(t string) map[string]interface{} {
+		n := map[string]interface{}{
+			typeKey:     t,
+			defaultKey:  nil,
+			nullableKey: true,
+		}
+		if t == "array" {
+			n["items"] = map[string]string{}
+		}
+		return n
+	}
+	return &MapItem{
+		Key: "oneOf",
+		Value: []map[string]interface{}{
+			nullable("integer"),
+			nullable("number"),
+			nullable("boolean"),
+			nullable("string"),
+			nullable("object"),
+			nullable("array"),
+		},
+	}
+}
+


Test patch:
diff --git a/cli/pkg/kctrl/cmd/package/release/schemagenerator/helm_openapi_schema_gen_test.go b/cli/pkg/kctrl/cmd/package/release/schemagenerator/helm_openapi_schema_gen_test.go
index fb0d8446..58c7c467 100644
--- a/cli/pkg/kctrl/cmd/package/release/schemagenerator/helm_openapi_schema_gen_test.go
+++ b/cli/pkg/kctrl/cmd/package/release/schemagenerator/helm_openapi_schema_gen_test.go
@@ -69,7 +69,8 @@ arrKeyEmpty: []
       type: string
     type: array
 type: object
-`},
+`,
+		},
 		{
 			name: "object with different values",
 			input: `
@@ -123,7 +124,8 @@ objExample: {}
     description: Object example
     type: object
 type: object
-`},
+`,
+		},
 		{
 			name: "nested complex object",
 			input: `
@@ -176,7 +178,8 @@ containers:
         type: string
     type: object
 type: object
-`},
+`,
+		},
 		{
 			name: "Alias Node",
 			input: `
@@ -223,7 +226,40 @@ aliasEx:
       type: object
     type: array
 type: object
-`},
+`,
+		},
+		{
+			name: "unknown type",
+			input: `
+# a field without a type
+anything: null
+`,
+			want: `properties:
+  anything:
+    description: a field without a type
+    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
+`,
+		},
 	}
 
 	for _, test := range tests {
diff --git a/cli/test/e2e/package_authoring_e2e_test.go b/cli/test/e2e/package_authoring_e2e_test.go
index 99986c9f..92684dd5 100644
--- a/cli/test/e2e/package_authoring_e2e_test.go
+++ b/cli/test/e2e/package_authoring_e2e_test.go
@@ -336,8 +336,26 @@ spec:
               default: quay.io/mongodb
               type: string
             imagePullSecrets:
-              default: ""
-              type: string
+              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
             initAppDb:
               default: quay.io/mongodb
               type: string