vmware-tanzu / kubeapps

A web-based UI for deploying and managing applications in Kubernetes clusters
Other
4.91k stars 708 forks source link

[carvel] Cannot convert undefined or null to object - calling jsonpatch.compare with null value #6483

Open absoludity opened 1 year ago

absoludity commented 1 year ago

Describe the bug

When deploying a carvel package with a non-default value, the error "Cannot convert undefined or null to object" is displayed on the screen:

Screenshot 2023-07-18 at 5 42 16 pm

Digging down, the reason for this error isn't specific to Carvel per se, but rather, because most (all?) Carvel packages that we use have a default yaml values file that are just comments - so when parsed result in a null. We are then passing this through to jsonpatch.compare(defaults, valuesToCheck) which in turn calls Object.keys(null) causing the error.

It isn't triggered in CI because of an interesting early return in jsonpatch.compare(defaults, valuesToCheck) where if we haven't set any values, the values to check is also null, and the jsonpatch.compare short-circuits returning if they're equal.

So in addition to fixing this (which should be trivial - ensuring we don't call jsonpatch in this situation), we should update CI to change the name of the carvel installation.

To Reproduce Steps to reproduce the behavior:

  1. Install kubeapps with carvel support (and kapp-controller installed)
  2. Add the [carvel test repo](kubectl apply -f https://raw.githubusercontent.com/vmware-tanzu/carvel-kapp-controller/develop/examples/packaging-with-repo/package-repository.yml)
  3. Create a service account with privs in your target namespace
  4. Go to the Kubeapps catalog and select the Carvel test package (version 2)
  5. Set the install name, choose the service account
  6. Update the one editable field
  7. Click deploy

Expected behavior Deploy should start.

Screenshots See above.

Desktop (please complete the following information):

Additional context

Unfortunately the only work-around I can see at the moment is to ensure the default values are not null (ie. just comments), but rather, empty. For example, by adding a {} to the default values, but I don't know that this is possible as I think Carvel may be generating the default values from the data schema intentionally as comments. In which case, the only fix will be for Kubeapps to parse the (commented) default values and use the diff correctly, or disable the diff for carvel apps.

Another (silly, but available) workaround if you're just demoing would be to demo an app where you don't need to change any values (or package the app with the values you need as default).

Thanks to @agapebondservant for pointing out the issue.

absoludity commented 1 year ago

Similarly, if the schema includes type: "object", then when Kubeapps incorrectly passes null as the data rather than {}, a validation error will be returned (obscured by the levels): "An error occurred: the given values do not match the required format. The following errors were found: - : must be object"