feat: add monaco-kubernetes for editing IntelliSense #12778

WitoDelnat · 2023-03-09T14:46:49Z

This PR is in collaboration with Monokle from Kubeshop.

It introduces monaco-kubernetes which was demonstrated on the Argo CD/Rollouts Community Meeting of 4th Jan. It adds various improvements to Argo YAML editor including IntelliSense, schema validation, hardening guidelines, etc.

Please let me know if something is missing for this PR to be in accordance with contribution guidelines. I understand that enhancement proposals need to go through the triage process.

The editor is designed with backwards compatibility. You can easily opt-out by disabling useKubernetesEditor or change validation rules (see video below). The UI for such configuration is however out-of-scope for this PR.

Finally, autocomplete and more could also work for the Application CRD editor. However, a modification would be needed. It currently only shows the spec in the editor and not the whole resource. monaco-kubernetes only works on complete resources. This is more a product decision so I decided not to touch it for now.

Autocomplete in service:

Docs and security violation in Deployment:

Toggling the editor between old and new mode for opt-out (This is simply a demo to show that it works. The button has not been committed into the PR itself as it probably belongs elsewhere):

Screen.Recording.2023-03-09.at.14.49.25.mov

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

WitoDelnat · 2023-03-09T15:04:06Z

Builds are failing because: The engine "node" is incompatible with this module. Expected version "^14 || ^16 || >=18". Got "12.18.4". Please advise me on how to continue. We could consider to replace the the module in question in our upstream, but v12 is also past end of life since April 2022.

crenshaw-dev · 2023-03-09T15:23:21Z

@WitoDelnat want to bump the node version in Dockerfile to 14.21.3?

WitoDelnat · 2023-03-09T15:39:40Z

@crenshaw-dev Image build is now succeeding with v14.21.3 ✅

crenshaw-dev · 2023-03-09T15:43:38Z

I'll see if we can go straight to latest LTS: #12779

crenshaw-dev · 2023-03-09T17:45:21Z

Looks like there might be more places to edit the node version. Maybe can update based on my PR? Still trying to get CI to pass there.

WitoDelnat · 2023-03-10T15:21:46Z

There are some difficulties with the Jest tests and TypeScript linter. We use rather modern package.json features to elegantly build differently for Node and Browsers but it's causing more hassle than gain. We're going to change our build a bit to get the builds green here.

Putting this in draft for now and I'll ping you when all checks succeed.

WitoDelnat · 2023-03-15T13:45:47Z

We spent quite some time on trying to get the build work but it remains a hassle. In a nutshell, Jest fails because it cannot properly bundle the application because of discrepancies between ESM and CJS builds. The linter fails because we use modern features that Argo CD does not support.

What we did to try and resolve this:

Use Jest transformers in combination with Babel to avoid this problem. This was the recommendation by the original error. This did not help.
Rework @monokle/validation to no longer use package.imports which we considered some tools might have difficulties with. The problem remained.
Explore vitest instead of Jest. This tool is known to handle ESM better. Our error was resolved but another dependency (monokle-editor) failed. This cannot be fixed on our side.
Give Jest transformers another go, this time to transform our dependency (and its own dependencies) into a CJS bundle. This failed.
Rework monaco-kubernetes to instead also expose a prebuilt CJS bundle. We managed to get the Jests test working with this, exposing a CJS bundle for an ESM package just to get tests running is not optimal but we'll take it at this point. Tests succeed but linter fails.
Reworked @monokle/validation a bit more to no longer rely on private class fields for compatibility with Argo build. Linter still has a problem with AbortController, likely a minor issue. Builds are more worrying as they started to fail now.

crenshaw-dev · 2023-03-15T13:47:58Z

Is there anything we can do on the Argo end to sort this? I'm pretty ignorant in the JS ecosystem, but happy to help if possible.

WitoDelnat · 2023-03-15T13:53:50Z

We're going to give this boy one last go until the end of day. After that we'll have to evaluate on how to progress. I'll give you an update at the end of the day.

Signed-off-by: Delnat Wito <wito.delnat@gmail.com>

WitoDelnat · 2023-03-15T16:49:19Z

Hurray 🎉 We managed to find a subtle side-effect in WebPack and changed it to prevent bundling the NodeJs files (see our PR for details). All checks have passed. Everything appears to work okay during our tests. I tried it on Chrome, Safari and Firefox.

ps: We had to update the TypeScript version as it was quite outdated and we also had to include NodeJs types as a solution to this problem. Finally, when building WebPack will mention a warning in children (i.e. the Kubernetes worker), they mention a flag which you can use to get additional details, but it's safe to ignore this warning. Unfortunately it's not possible to disable it without breaking the build.

Please try it out and let us know how to proceed.

chargio · 2023-03-17T07:55:53Z

Hey, thanks everybody for the help.
Now that all checks have passed, is there anything we need to do to get this PR approved and merged?

crenshaw-dev · 2023-03-17T16:56:33Z

@chargio afaik we just need a review from someone with frontend expertise.

I'd also like to take some time to look at the dependencies we're pulling in. Supply-chain security on the frontend is pretty critical.

crenshaw-dev · 2023-03-17T16:57:16Z

Oh, I also wanted to take a look at the affect on bundle size.

Update: looks like no effect on the entrypoint size. But there's now a web worker I guess?

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets: 
  worker.worker.js (1.48 MiB)
  assets/images/argo.png (279 KiB)
  assets/fonts/fa-solid-900.ttf (388 KiB)
  assets/scripts/redoc.standalone.js (845 KiB)
  main.8d71e7a83adc50424e46.js (5.12 MiB)

WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
Entrypoints:
  main (5.12 MiB)
      main.8d71e7a83adc50424e46.js

crenshaw-dev · 2023-03-17T18:36:38Z

Weird things are happening when I try to build locally.

$ cd ui
$ yarn install
yarn install v1.22.19
[1/4] 🔍  Resolving packages...
warning Resolution field "rxjs@6.6.7" is incompatible with requested version "rxjs@^7.5.5"
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
< warnings removed >
[4/4] 🔨  Building fresh packages...
✨  Done in 3.35s.
$ yarn build
yarn run v1.22.19
$ find ./dist -type f -not -name gitkeep -delete && webpack --config ./src/app/webpack.config.js --mode production --stats-children
CLI for webpack must be installed.
  webpack-cli (https://github.com/webpack/webpack-cli)

We will use "yarn" to install the CLI via "yarn add -D webpack-cli".
Do you want to install 'webpack-cli' (yes/no): yes
Installing 'webpack-cli' (running 'yarn add -D webpack-cli')...
[1/4] 🔍  Resolving packages...
warning Resolution field "rxjs@6.6.7" is incompatible with requested version "rxjs@^7.5.5"
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
< warnings removed >
[4/4] 🔨  Building fresh packages...
success Saved lockfile.
success Saved 6 new dependencies.
info Direct dependencies
└─ webpack-cli@5.0.1
info All dependencies
├─ @webpack-cli/configtest@2.0.1
├─ @webpack-cli/info@2.0.1
├─ @webpack-cli/serve@2.0.1
├─ interpret@3.1.1
├─ rechoir@0.8.0
└─ webpack-cli@5.0.1
[webpack-cli] Failed to load '/Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/src/app/webpack.config.js' config
[webpack-cli] Error: Cannot find module 'monaco-editor-webpack-plugin'
Require stack:
- /Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/src/app/webpack.config.js
- /Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/node_modules/webpack-cli/lib/webpack-cli.js
- /Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/node_modules/webpack-cli/lib/bootstrap.js
- /Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/node_modules/webpack-cli/bin/cli.js
- /Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/node_modules/webpack/bin/webpack.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/src/app/webpack.config.js:3:29)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/src/app/webpack.config.js',
    '/Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/node_modules/webpack-cli/lib/webpack-cli.js',
    '/Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/node_modules/webpack-cli/lib/bootstrap.js',
    '/Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/node_modules/webpack-cli/bin/cli.js',
    '/Users/mcrenshaw/go/src/github.com/argoproj/argo-cd/ui/node_modules/webpack/bin/webpack.js'
  ]
}
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I'm not sure why it complained that webpack-cli was not installed, and then proceeded to upgrade to webpack v5.

WitoDelnat · 2023-03-20T08:42:11Z

Happy Monday Crenshaw.

Let me know if we can assist with the supply-chain security check.

Further you are correct that the entrypoint size remains the same. While the worker is rather large due to the embedded yaml-language-server, the upside is that downloading the worker is only loaded lazily after opening the editor. Even then you can already start editing and once the worker loaded the IntelliSense and validation will appear.

Finally, that's an odd build problem. I'll take a look today to see if I can reproduce. I'll keep you posted.

WitoDelnat · 2023-03-20T16:11:12Z

We're a bit puzzled by your weird build issue. What's clear is that it's a dependency issue, but the yarn install should have properly installed those dependencies. We unsuccessfully tried to reproduce your issue in multiple ways.

Can I suggest you to delete your node_modules and run yarn install after which you build again? Sounds silly but that often solves these problems.

The command I use for production build is this one taken from the Docker file: NODE_ENV='production' NODE_ONLINE_ENV='online' NODE_OPTIONS=--max_old_space_size=8192 yarn build. The yarn build should also work but do note that WebPack's --mode production flag clashes with node env being development in this case.

crenshaw-dev · 2023-03-20T16:18:23Z

@WitoDelnat I'm hitting similar issues on other branches, so I think it's unrelated to this PR. No luck on clearing node_modules... I'll keep poking around.

Thanks for the explanation of the worker! Async and lazy-loaded sounds great.

crenshaw-dev · 2023-03-20T20:10:49Z

Eh, got bit by this: yarnpkg/yarn#2739

export NODE_ENV=, and I'm good to go.

crenshaw-dev · 2023-03-23T21:44:45Z

@WitoDelnat this hasn't slipped my mind, just been busy getting 2.7.0-rc1 ready. Wish we could fit this in 2.7, but I think we should target 2.8. After Monday I'll have more time to push for this.

WitoDelnat · 2023-03-24T13:39:23Z

No worries! I understand that the PR arrived a bit on the late side to be included in v2.7 - I'd rather do it properly than rush it. Let's talk somewhere next week.

Best of luck with the release ;-)

WitoDelnat · 2023-04-11T07:18:00Z

Happy Tuesday @crenshaw-dev, this is a small reminder about this PR. Can we help in any way?

crenshaw-dev · 2023-04-24T13:51:24Z

Hey, @WitoDelnat! I'm pushing for this in 2.8. Don't worry, hasn't slipped off the radar. :-)

Signed-off-by: Remington Breeze <remington@breeze.software>

rbreeze · 2023-04-25T18:54:58Z

@WitoDelnat I'll take a look at this this week

codecov · 2023-04-25T19:10:26Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (467777f) 49.02% compared to head (f05bef3) 49.03%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #12778   +/-   ##
=======================================
  Coverage   49.02%   49.03%           
=======================================
  Files         247      247           
  Lines       42694    42694           
=======================================
+ Hits        20930    20933    +3     
+ Misses      19647    19645    -2     
+ Partials     2117     2116    -1

see 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

rbreeze

LGTM!

WitoDelnat · 2023-05-18T07:30:40Z

This is great news. I'm excited that this will become available in v2.8 🥳 Thanks for the collaboration @crenshaw-dev and @rbreeze!

ps: In my PR description, I mentioned that you can configure YamlEditor to opt-out of the validation or to tweak rules to your preference (e.g. some people like to have request limits while other do not). I unfortunately do not have time to build this settings menu myself, but it might be a neat enhancement to consider.

…j#12778)" This reverts commit f8e016d. Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

#14000) This reverts commit f8e016d. Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: pasha-codefresh <pavel@codefresh.io>

Signed-off-by: Delnat Wito <wito.delnat@gmail.com> Signed-off-by: Remington Breeze <remington@breeze.software> Co-authored-by: Remington Breeze <remington@breeze.software>

…j#12778)" (argoproj#14000) This reverts commit f8e016d. Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: pasha-codefresh <pavel@codefresh.io>

WitoDelnat force-pushed the add-monaco-editor branch from fdf9b39 to 9c41ffe Compare March 9, 2023 14:52

WitoDelnat force-pushed the add-monaco-editor branch 5 times, most recently from 99d3363 to ba5fe6c Compare March 10, 2023 12:52

WitoDelnat marked this pull request as draft March 10, 2023 15:21

WitoDelnat added 3 commits March 15, 2023 16:52

chore: update NodeJs to v18.15.0 (LTS)

dd546a6

Signed-off-by: Delnat Wito <wito.delnat@gmail.com>

feat: add monaco-kubernetes editor for IntelliSense

abdda83

Signed-off-by: Delnat Wito <wito.delnat@gmail.com>

fix: fix lints and testing

c3a2027

Signed-off-by: Delnat Wito <wito.delnat@gmail.com>

WitoDelnat force-pushed the add-monaco-editor branch from 9ad27de to c3a2027 Compare March 15, 2023 15:53

WitoDelnat marked this pull request as ready for review March 15, 2023 16:49

crenshaw-dev requested review from rbreeze and keithchong March 17, 2023 16:55

Merge branch 'master' into add-monaco-editor

f05bef3

Signed-off-by: Remington Breeze <remington@breeze.software>

rbreeze approved these changes May 17, 2023

View reviewed changes

rbreeze merged commit f8e016d into argoproj:master May 17, 2023

alexmt mentioned this pull request May 20, 2023

UI is trying to load JSON from https://plugins.monokle.com #13671

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add monaco-kubernetes for editing IntelliSense #12778

feat: add monaco-kubernetes for editing IntelliSense #12778

WitoDelnat commented Mar 9, 2023 •

edited

Loading

WitoDelnat commented Mar 9, 2023

crenshaw-dev commented Mar 9, 2023

WitoDelnat commented Mar 9, 2023

crenshaw-dev commented Mar 9, 2023

crenshaw-dev commented Mar 9, 2023

WitoDelnat commented Mar 10, 2023

WitoDelnat commented Mar 15, 2023 •

edited

Loading

crenshaw-dev commented Mar 15, 2023

WitoDelnat commented Mar 15, 2023

WitoDelnat commented Mar 15, 2023 •

edited

Loading

chargio commented Mar 17, 2023 •

edited

Loading

crenshaw-dev commented Mar 17, 2023

crenshaw-dev commented Mar 17, 2023 •

edited

Loading

crenshaw-dev commented Mar 17, 2023

WitoDelnat commented Mar 20, 2023

WitoDelnat commented Mar 20, 2023

crenshaw-dev commented Mar 20, 2023

crenshaw-dev commented Mar 20, 2023

crenshaw-dev commented Mar 23, 2023

WitoDelnat commented Mar 24, 2023

WitoDelnat commented Apr 11, 2023

crenshaw-dev commented Apr 24, 2023

rbreeze commented Apr 25, 2023

codecov bot commented Apr 25, 2023

rbreeze left a comment

WitoDelnat commented May 18, 2023

feat: add monaco-kubernetes for editing IntelliSense #12778

feat: add monaco-kubernetes for editing IntelliSense #12778

Conversation

WitoDelnat commented Mar 9, 2023 • edited Loading

WitoDelnat commented Mar 9, 2023

crenshaw-dev commented Mar 9, 2023

WitoDelnat commented Mar 9, 2023

crenshaw-dev commented Mar 9, 2023

crenshaw-dev commented Mar 9, 2023

WitoDelnat commented Mar 10, 2023

WitoDelnat commented Mar 15, 2023 • edited Loading

crenshaw-dev commented Mar 15, 2023

WitoDelnat commented Mar 15, 2023

WitoDelnat commented Mar 15, 2023 • edited Loading

chargio commented Mar 17, 2023 • edited Loading

crenshaw-dev commented Mar 17, 2023

crenshaw-dev commented Mar 17, 2023 • edited Loading

crenshaw-dev commented Mar 17, 2023

WitoDelnat commented Mar 20, 2023

WitoDelnat commented Mar 20, 2023

crenshaw-dev commented Mar 20, 2023

crenshaw-dev commented Mar 20, 2023

crenshaw-dev commented Mar 23, 2023

WitoDelnat commented Mar 24, 2023

WitoDelnat commented Apr 11, 2023

crenshaw-dev commented Apr 24, 2023

rbreeze commented Apr 25, 2023

codecov bot commented Apr 25, 2023

Codecov Report

rbreeze left a comment

Choose a reason for hiding this comment

WitoDelnat commented May 18, 2023

WitoDelnat commented Mar 9, 2023 •

edited

Loading

WitoDelnat commented Mar 15, 2023 •

edited

Loading

WitoDelnat commented Mar 15, 2023 •

edited

Loading

chargio commented Mar 17, 2023 •

edited

Loading

crenshaw-dev commented Mar 17, 2023 •

edited

Loading