Add JSON Schema editor view #5530

antgamdia · 2022-10-20T17:48:28Z

Description of the change

The basic forms view has been recently revamped with plain JSON schema support. However, most of the charts (and maybe other packages) do not bring any schema file as part of their bundle.

#3535 brings some ideas on how to solve it, but, this PR is just paving the way with a very simplistic approach: let users edit/add a schema from a text editor.

Currently, there is no way to upload a file externally, but if this happens to be helpful, could be added in a follow-up PR.

Benefits

Users will be able to edit/add json schemas for improving the "basic form" experience (validations, defaults, etc...).

Possible drawbacks

Important: this "edited (or added)" schema is NOT being used on the backend side. Meaning you can't expect a proper validation or any further schema enforcement. Using the schema in the backend as well would imply some extra changes that could be performed in a follow-up PR if really needed.

Applicable issues

fixes Easy way to provide custom values.schema.json files for Helm Charts #3535

Additional information

Note
This PR is part of a series of PRs aimed at closing this milestone. I have split the changes to ease the review process, but as there are many interrelated changes, the tests will be performed in a separate PR (on top of the branch containing all the changes).
PR 1 out of 6

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

netlify · 2022-10-20T17:48:53Z

✅ Deploy Preview for kubeapps-dev ready!

Name	Link
🔨 Latest commit	`a95485b`
🔍 Latest deploy log	https://app.netlify.com/sites/kubeapps-dev/deploys/63591e1e7aaccd00089a97a4
😎 Deploy Preview	https://deploy-preview-5530--kubeapps-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

castelblanque · 2022-10-21T07:16:19Z

Excellent, looks really great.
Just a couple of questions:

Maybe seeing yet another tab named "Schema editor (advanced)" is a bit intimidating for some users. Also there might be admins that do not want Kubeapps users to mess around with the schema. Why not showing it based on a chart feature flag? (e.g. dashboard.advancedMode.enabled or similar).
Why the user editing the schema has to proactively push the "Update schema" button? Can't we do it so that the new schema is automatically applied e.g. when textarea or tab loses focus?

### Description of the change While working on #5436, I noticed some errors during the YAML parsing; well not errors, but unexpected (for me) values being returned by the `genIn` method. In collections, the returned thing is not a js object, but a custom data type from the YAML library. This PR is just to convert collections (YAML sequences and YAML maps) back to plain js objects (even if the function says `toJSON()` 😅 ### Benefits We will be able to use arrays and object fields in the form. ### Possible drawbacks N/A ### Applicable issues - fixes #5519 ### Additional information > **Note** > This PR is part of a series of PRs aimed at closing [this milestone](https://github.com/vmware-tanzu/kubeapps/milestone/27). I have split the changes to ease the review process, but as there are many interrelated changes, the tests will be performed in a separate PR (on top of the branch containing all the changes). > PR 2 out of 6 Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

@jl-beast

### Description of the change As pointed out by @jl-beast at #5512, we could leverage more JSON Schema built-in annotations to enrich the UX. This PR (and the stacked ones) is adding support for `examples`, `readOnly`, `deprecated`, `maxItems`, `multipleOf` amongst others (following http://json-schema.org/draft/2020-12/json-schema-validation.html#name-a-vocabulary-for-structural) ### Benefits Richer UX in the basic form for complex schemas ### Possible drawbacks N/A ### Applicable issues - fixes #5525 ### Additional information > **Note** > This PR is part of a series of PRs aimed at closing [this milestone](https://github.com/vmware-tanzu/kubeapps/milestone/27). I have split the changes to ease the review process, but as there are many interrelated changes, the tests will be performed in a separate PR (on top of the branch containing all the changes). > PR 3 out of 6 Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

@dud225

### Description of the change As pointed out by @dud225, we weren't enforcing the numeric validation consistently in both the slider and the text input. Besides, we were missing some json schema fields like `exclusiveMinimum` and `exclusiveMaximum`. This PR is to fix that. ### Benefits Consistent param validation ### Possible drawbacks N/A ### Applicable issues - fixes #5520 ### Additional information > **Note** > This PR is part of a series of PRs aimed at closing [this milestone](https://github.com/vmware-tanzu/kubeapps/milestone/27). I have split the changes to ease the review process, but as there are many interrelated changes, the tests will be performed in a separate PR (on top of the branch containing all the changes). > PR 4 out of 6 ````json "number-maxmin": { "type": "number", "maximum": 10, "exclusiveMaximum": 9, "minimum": 2, "exclusiveMinimum": 1 }, ````` ![image](https://user-images.githubusercontent.com/11535726/197026623-2c2a5284-f939-4af2-a1a2-9816cf65234d.png) ![image](https://user-images.githubusercontent.com/11535726/197026701-370c21d3-7164-405d-b77b-56dd58472dbb.png) Also, note the HTML5 additional validation (user's browser locale msgs): ![image](https://user-images.githubusercontent.com/11535726/197026840-b5dd2e98-245b-4b55-b52a-1deaf0746866.png) Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

absoludity · 2022-10-24T03:26:14Z

#3535 brings some ideas on how to solve it, but, this PR is just paving the way with a very simplistic approach: let users edit/add a schema from a text editor.

Hi Antonio. What's the use case for this change? The PR seems to address a user scenario something like:

As a Kubeapps user
I want to edit a chart schema each time that I deploy an app
So that I can use a simplified form to deploy each app

I'm not sure, but that doesn't seem too realistic? I would have thought the most useful scenario is

As a Kubeapps operator
I want to edit a chart schema once
So that every time a Kubeapps user installs that chart the updated schema is used for an improved simplified form.

or at least

As a Kubeapps user
I want to edit a chart schema once when deploying an app
So that every future deployment of that app uses my updated schema.

I know you said this is just a simplistic approach, but I don't really see much use in having this UI unless it somehow addresses at least one of those two use-cases (and that could be tricky - I mean, we can easily store a schema in a config map, but there's no concept of a per-user place to store such a config map etc.)

Currently, there is no way to upload a file externally, but if this happens to be helpful, could be added in a follow-up PR.

I'd be keen to only land this PR when that follow-up is designed, tested and ready, otherwise we're cluttering the UX just to demo part of a feature that's not yet beneficial.

Benefits

Users will be able to edit/add json schemas for improving the "basic form" experience (validations, defaults, etc...).

but they'll need to do this every time that they deploy a chart, right? Not sure that's much of a benefit... if I know how to do that every time, it'd be simpler for me to use the json I think? Sorry if I missed something where this was discussed earlier (or code that actually is saving the schema changes)!

### Description of the change Once each of the PRs of the series related to [this milestone](https://github.com/vmware-tanzu/kubeapps/milestone/27) has been merged, this final one is adding/fixing the test cases: adding new ones for the functionality we've added and fixing others. Besides, some minor fixes have been added (which have been discovered with the unit tests :P) ### Benefits Tests will pass now. ### Possible drawbacks N/A ### Applicable issues - related #3535 ### Additional information > **Note** > This PR is part of a series of PRs aimed at closing [this milestone](https://github.com/vmware-tanzu/kubeapps/milestone/27). > PR 6 out of 6 Edit: existing e2e tests are also failing :S, will have a look at them next week. Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

antgamdia · 2022-10-24T16:31:22Z

Thank you both for the feedback here!

Maybe seeing yet another tab named "Schema editor (advanced)" is a bit intimidating for some users. Also there might be admins that do not want Kubeapps users to mess around with the schema. Why not showing it based on a chart feature flag? (e.g. dashboard.advancedMode.enabled or similar).

100% agree. This feature is not useful for everybody, just for those users (or package authors) wanting to play around with the schema... but once we're done with the schema, there's no point in allowing its modification.

Perhaps adding a dashboard.schemaEditor.enabled defaulted to false makes the trick. Will do it in a follow-up PR.

Why the user editing the schema has to proactively push the "Update schema" button? Can't we do it so that the new schema is automatically applied e.g. when textarea or tab loses focus?

Good point; in fact, I did implement it that way. However, the experience is not as good as expected. First of all, rendering the schema is time-expensive, so rendering it on every onChange is not viable.
Let's suppose we do it on every tab focus, as you suggested: that, indeed, works, but while playing around with it, I felt it was kind of misleading: sometimes, when the schema is invalid, it wasn't sure whether or not it was being applied. Also, I wanted to have a look at the former schema before actually loading the new one.

To make the edit/save process as easy as possible, I added both the "update schema" button, a context menu button and a shortcut (crtl/cmd+s) to save it. I'd rather keep it that way instead of auto-saving it, but no strong opinion though.

What's the use case for this change? The PR seems to address a user scenario something like (...)

Totally right, the current use case is not realistic at all. We just went implementing that way (and right now) as it was really useful for us when developing the new basic from view (adding new validations, new fields, etc with kind of a "hot reload").

I know you said this is just a simplistic approach, but I don't really see much use in having this UI unless it somehow addresses at least one of those two use-cases

I also see little benefit in deploying this change for every user, as Rafa also pointed out. Putting this behind a feature flag could be a good tradeoff: we ship the feature for those users or package authors wanting to create/edit a schema but we don't for a default kubeapps installation.

That said, I believe this PR could still be really beneficial in some cases: imagine an organization using charts/packages, whose schema is really basic or even non-existent, from a repo X . Currently, unless they mirror the repo X, there is no way to "override" the schema.
With this PR, operators from this organization can just provide the schema, manually, to the users, so that they can just paste it onto Kubeapps and start filling up the basic form.
This is far from the very ideal situation, but yet solves a potential problem.

To sum up, I'd rahter move forward with this PR, but:

putting the code behind dashboard.schemaEditor.enabled=true flag
considering the work herein performed for the design in Explore, define and design the experience to use multiple values for app deployment in Kubeapps #5560

WDYT?

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

absoludity · 2022-10-25T00:41:45Z

With this PR, operators from this organization can just provide the schema, manually, to the users, so that they can just paste it onto Kubeapps and start filling up the basic form. This is far from the very ideal situation, but yet solves a potential problem.

Yeah - I don't think the above point is realistic, but agree with all the other points (thanks as always for the clarity and thoroughness!) so...

To sum up, I'd rahter move forward with this PR, but:

1. putting the code behind  `dashboard.schemaEditor.enabled=true` flag

2. considering the work herein performed for the design in

+1 . I'll plan to go through the code later today before finishing.

absoludity

I was going to say +1 after you add the feature flag, but I see you've done that separately in #5564 . Great, thanks Antonio.

absoludity · 2022-10-25T20:17:02Z

...tForm/DeploymentFormBody/BasicDeploymentForm/TabularSchemaEditorTable/Params/SliderParam.tsx

+    // twofold validation: using the json schema (with ajv) and the html5 validation
+    setValidated(validateValuesSchema(e.currentTarget.value, param.schema));
+    e.currentTarget?.reportValidity();


Hopefully these can't oppose each other... :P

haha, right. In theory, the html5 validation should be a subset of the schema validation, since we are manually adding stuff like max, min, etc but we are not considering more complex schemas using anyOf, allOf, etc. So the browser validation should be able to provide quick feedback in a native way, whereas the ajv validation will prompt the schema validation error, which could be more difficult to understand for non-experienced users.

### Description of the change As per the discussion in #5530, this PR is adding a `featureFlags.schemaEditor` flag to enable the schema editor for those users who really want it. ### Benefits The schema editor will be available, but only if really needed. ### Possible drawbacks It is a work-in-progress feature until we also save the schema or use in the backend. ### Applicable issues - related #3535 ### Additional information I've fixed some typos here as well. Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> Conflicts: chart/kubeapps/Chart.yaml chart/kubeapps/README.md chart/kubeapps/values.yaml

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Bumps [axios](https://github.com/axios/axios) from 1.3.1 to 1.3.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/axios/axios/releases">axios's releases</a>.</em></p> <blockquote> <h2>Release v1.3.2</h2> <h2>Release notes:</h2> <h3>Bug Fixes</h3> <ul> <li><strong>http:</strong> treat <a href="http://localhost">http://localhost</a> as base URL for relative paths to avoid <code>ERR_INVALID_URL</code> error; (<a href="https://github-redirect.dependabot.com/axios/axios/issues/5528">#5528</a>) (<a href="https://github.com/axios/axios/commit/128d56f4a0fb8f5f2ed6e0dd80bc9225fee9538c">128d56f</a>)</li> <li><strong>http:</strong> use explicit import instead of TextEncoder global; (<a href="https://github-redirect.dependabot.com/axios/axios/issues/5530">#5530</a>) (<a href="https://github.com/axios/axios/commit/6b3c305fc40c56428e0afabedc6f4d29c2830f6f">6b3c305</a>)</li> </ul> <h3>Contributors to this release</h3> <ul> <li> <a href="https://github.com/DigitalBrainJS" title="+2/-1 ([#5530](axios/axios#5530) [#5528](axios/axios#5528) )">Dmitriy Mozgovoy</a></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/axios/axios/blob/v1.x/CHANGELOG.md">axios's changelog</a>.</em></p> <blockquote> <h2><a href="https://github.com/axios/axios/compare/v1.3.1...v1.3.2">1.3.2</a> (2023-02-03)</h2> <h3>Bug Fixes</h3> <ul> <li><strong>http:</strong> treat <a href="http://localhost">http://localhost</a> as base URL for relative paths to avoid <code>ERR_INVALID_URL</code> error; (<a href="https://github-redirect.dependabot.com/axios/axios/issues/5528">#5528</a>) (<a href="https://github.com/axios/axios/commit/128d56f4a0fb8f5f2ed6e0dd80bc9225fee9538c">128d56f</a>)</li> <li><strong>http:</strong> use explicit import instead of TextEncoder global; (<a href="https://github-redirect.dependabot.com/axios/axios/issues/5530">#5530</a>) (<a href="https://github.com/axios/axios/commit/6b3c305fc40c56428e0afabedc6f4d29c2830f6f">6b3c305</a>)</li> </ul> <h3>Contributors to this release</h3> <ul> <li> <a href="https://github.com/DigitalBrainJS" title="+2/-1 ([#5530](axios/axios#5530) [#5528](axios/axios#5528) )">Dmitriy Mozgovoy</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/axios/axios/commit/0b449293fc238f30f39ab9ed0fca86a23c8a6a79"><code>0b44929</code></a> chore(release): v1.3.2 (<a href="https://github-redirect.dependabot.com/axios/axios/issues/5531">#5531</a>)</li> <li><a href="https://github.com/axios/axios/commit/6b3c305fc40c56428e0afabedc6f4d29c2830f6f"><code>6b3c305</code></a> fix(http): use explicit import instead of TextEncoder global; (<a href="https://github-redirect.dependabot.com/axios/axios/issues/5530">#5530</a>)</li> <li><a href="https://github.com/axios/axios/commit/128d56f4a0fb8f5f2ed6e0dd80bc9225fee9538c"><code>128d56f</code></a> fix(http): treat <a href="http://localhost">http://localhost</a> as base URL for relative paths to avoid `ER...</li> <li>See full diff in <a href="https://github.com/axios/axios/compare/v1.3.1...v1.3.2">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=axios&package-manager=npm_and_yarn&previous-version=1.3.1&new-version=1.3.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

antgamdia added 2 commits October 20, 2022 18:56

Add SchemaEditorForm

064b5a4

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

add missing files

00b62e0

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

vmwclabot added the cla-not-required label Oct 20, 2022

antgamdia added 4 commits October 21, 2022 18:31

Merge branch 'main' into 1-3535-customSchema

9fe8529

antgamdia added 4 commits October 24, 2022 09:48

Merge branch 'main' into 1-3535-customSchema

6107fa5

Fix e2e class selectors

895f202

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Fix editor class selectors. Fix minor style margin issue

8a46b81

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Fix selector in test

e3fca14

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

antgamdia mentioned this pull request Oct 25, 2022

SchemaEditor behind a feature flag #5564

Merged

absoludity approved these changes Oct 25, 2022

View reviewed changes

antgamdia added 4 commits October 26, 2022 09:20

Change feature flag from bool to obj

ed3075b

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

Merge branch 'main'

4e7d1a3

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com> Conflicts: chart/kubeapps/Chart.yaml chart/kubeapps/README.md chart/kubeapps/values.yaml

fix typos in the values.yaml file

a95485b

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

antgamdia merged commit cc9eddb into main Oct 26, 2022

antgamdia deleted the 1-3535-customSchema branch October 26, 2022 13:56

This was referenced Oct 26, 2022

Create additional "JSON Schema form" param renderers #5436

Closed

Error decoding arrays in the basic forms #5519

Closed

Validation issues in the basic forms #5520

Closed

Support all Metadata Annotations for Json Schema Milestone #5525

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JSON Schema editor view #5530

Add JSON Schema editor view #5530

antgamdia commented Oct 20, 2022

netlify bot commented Oct 20, 2022 •

edited

Loading

castelblanque commented Oct 21, 2022 •

edited

Loading

absoludity commented Oct 24, 2022

Benefits

antgamdia commented Oct 24, 2022

absoludity commented Oct 25, 2022 •

edited

Loading

absoludity left a comment

absoludity Oct 25, 2022

antgamdia Oct 26, 2022

Add JSON Schema editor view #5530

Add JSON Schema editor view #5530

Conversation

antgamdia commented Oct 20, 2022

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

netlify bot commented Oct 20, 2022 • edited Loading

✅ Deploy Preview for kubeapps-dev ready!

castelblanque commented Oct 21, 2022 • edited Loading

absoludity commented Oct 24, 2022

Benefits

antgamdia commented Oct 24, 2022

absoludity commented Oct 25, 2022 • edited Loading

absoludity left a comment

Choose a reason for hiding this comment

absoludity Oct 25, 2022

Choose a reason for hiding this comment

antgamdia Oct 26, 2022

Choose a reason for hiding this comment

netlify bot commented Oct 20, 2022 •

edited

Loading

castelblanque commented Oct 21, 2022 •

edited

Loading

absoludity commented Oct 25, 2022 •

edited

Loading