Wrong Helm values reported on Update form (#3621)

[castelblanque]

Signed-off-by: Rafa Castelblanque rcastelblanq@vmware.com

Description of the change

This PR tackles issue #3621 with one of the possible approaches.
Problem is caused by the way Kubeapps serializes/deserializes the modified values.
Just by having a question mark ? in a value without quotes makes the process to fail.

• Values are initially correctly stored by Kubeapps and Helm
• At the moment of retrieval for update is where the problem occurs here.
• yaml module removes double quotes when performing doc.toString()

Possible workarounds

1. Setting up yaml module to double quote any modified node in yaml with YAML.scalarOptions.str.defaultType = Type.QUOTE_DOUBLE; . However, this is a global setting and double quotes both key and value (see this PR tests). I don't think this is a nice and clean solution 😄 , as every everyt modified node shows double quoted.
2. Trying with js-yaml module instead of yaml. -> It doesn't work due to Yaml comments being erased
3. Trying to upgrade yaml module to major version "2.x" could make it. This major release contains differentiated settings for double quoting keys or values. But after trying the upgrade, dashboard breaks. This could be the most effective solution, as we could double quote only values and not keys. It requires a major upgrade to the module that could bring unforeseen effects in other parts of the app.

Workaround 1 is included in this PR.

Benefits

This PR is just for discussion by now. No real benefit so far.

Possible drawbacks

To be discussed.

Applicable issues

• fixes UI: Helm values not correctly reported #3621

Additional information

[absoludity]

Remember to set it to ready for review once you're ready (I'll assume if a PR is draft that you don't yet want people to review it, unless explicitly asked).

[castelblanque]

The implemented approach in this PR is not the best one, but the quickest.
Let's have it merged if you agree after I fix some failing tests, and I'll create another task to upgrade major version of YAML library and improve this.
With that version in place, we can get rid of double quotes on modified Yaml field names.

[absoludity]

The implemented approach in this PR is not the better one, but the quickest. Let's have it merged if you agree after I fix some failing tests, and I'll create another task to upgrade major version of YAML library and improve this. With that version in place, we can get rid of double quotes on modified Yaml field names.

Yep, happy to take a look... all I meant with my comment above was that you've got the PR in a Draft state, which usually means you don't want people to look at it yet.

Note that the dashboard tests are failing with a related error, so it's not clear to me yet whether that means the change here isn't working or is but tests haven't been updated (they seem to be for tests that you've explicitly updated).

[absoludity]

Once the tests are passing, +1 . At first I thought that the failing tests were related to the test changes you made here, but I see that the test changes here are just to ensure the result for comparison can contain double-quotes... great. So the failing tests are just other examples where values come back double-quoted (and I assume just need updating to match).