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

Creating flux package fails with: invalid character '#' looking for beginning of value #4212

Closed
absoludity opened this issue Feb 2, 2022 · 4 comments · Fixed by #4227
Closed
Assignees
Projects

Comments

@absoludity
Copy link
Contributor

Description:

When trying to create an installed package, I see:

invalid-char-hash-looking-for-value

I've tried both apache and ghost and get the same result.

Briefly looked for the cause, checking for a likely recent change and found a change to the yaml parsing library across the codebase landed earlier this week(https://github.com/kubeapps/kubeapps/pull/4179/files). But I tried reverting that and the resulting image still had the same issue.

I haven't bisected further to find the change causing the issue, but have a local kubeapps-apis image from earlier this week, or late last week, which creates the package without issue in the same scenario (ie. from Kubeapps dashboard UI). Not sure where else the issue could be, but happy to dig further and fix if you're too busy @gfichtenholt.

Steps to reproduce the issue:

  1. Deploy Kubeapps with fluxv2 plugin enabled
  2. Browse the catalog
  3. Install apache

Describe the results you received:

Error shown:

An error occurred: Unable to create the installed package for the package "bitnami/apache" using the plugin "fluxv2.packages": invalid character '#' looking for beginning of value

Describe the results you expected:

Apache package installed successfully.

Additional information you deem important (e.g. issue happens only occasionally):

Noticed this earlier this week on main, but still had a branch I was working on which didn't have the issue (ie. seems to have been introduced late last week or earlier this week.

@absoludity
Copy link
Contributor Author

FWIW, I also tried deleting and redeploying kubeapps, in case it was related to any cached data, but still see the error.

@gfichtenholt
Copy link
Contributor

reproduced locally, investigating...

@gfichtenholt
Copy link
Contributor

gfichtenholt commented Feb 3, 2022

unrelated question: why does the UX send values as part of CreateInstalledPackage()? I haven't done any customization, so values are default. In the proto values is optional, so I don't see need to send values as part of CreateInstalledPackage call. Maybe I am missing something?

@gfichtenholt
Copy link
Contributor

gfichtenholt commented Feb 3, 2022

oh, duh, it's trivial, the UX is sending values like this: "values":"# Default values for podinfo.\n\nreplicaCount: 1\n..."
I have recently switched from using "k8s.io/apimachinery/pkg/util/yaml" to Unmarshall the data to "encoding/json". The thing is ""# Default ...." is NOT valid JSON per se, so "encoding/json" blows up. So I made a regression. Need to see if I can revert back to using yaml.Unmarshal

Come to think of it, what UX is sending isn't JSON at all. It is straight up YAML. I guess I was a little bit confused about the semantics of the Values field in the API. The comments in .proto file does not specify the format. When I was first coding it, I guess I assumed it to be serialized JSON based on helm plug-in unit tests. Helm plug-in unit tests certainly assume that serialized JSON will work.

Now, clearly UX is assuming YAML will work as well, so sounds like we need to support both formats.
It would be good to update the .proto file and design doc to state that clearly.

Also, follow-up: when UI does GetAvailablePackageDetail() or GetInstalledPackageDetail(), which format does it expect to see the Values in the response? YAML? JSON? Either? Whatever the answer, I think would be good to also clearly state that in .proto file and the spec.

gfichtenholt added a commit to gfichtenholt/kubeapps that referenced this issue Feb 3, 2022
gfichtenholt added a commit that referenced this issue Feb 3, 2022
…ng for beginning of value #4212  (#4227)

* attempt #2

* fix for  Creating flux package fails with: invalid character '#' looking for beginning of value #4212

* forgot to fix the 'update' use case
Kubeapps automation moved this from Inbox to Done Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Kubeapps
  
Done
3 participants