Copy assets from appropriate directory for kbn-monaco #178669

eokoneyo · 2024-03-13T18:23:19Z

Summary

Per the configuration in kibana, Monaco by default will attempt to load the specific worker for each language using the public path record defined on window.__kbnPublicPath__ see here but an error occurs whilst attempting to request the script at the url we expect it to be at because it doesn't exist there, as seen here;

Ideally this works but with the current setup for CDN assets it doesn't map 1:1 to how the assets are expected, we should be copying from the directory target_workers to match the same definition for building packages
, similar to what we have here which is what this PR introduces.

The hypothesis above is verifiable by attempting to request the same resource on the same build of the deployment where the error is reported, with the target_workers path prepended, like so https://kibana.estccdn.com/552e8adcca05/bundles/kbn-monaco/target_workers/json.editor.worker.js we get the file.

elasticmachine · 2024-03-13T18:23:22Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

jloleysens

Overall I like this approach and I think it solves the non-origin Worker loading problem comprehensively.

Tested locally with CDN assets built being served from another domain and LGTM! Great work @eokoneyo

jloleysens · 2024-03-14T16:27:03Z

package.json

@@ -1096,6 +1096,7 @@
    "remark-gfm": "1.0.0",
    "remark-parse-no-trim": "^8.0.4",
    "remark-stringify": "^8.0.3",
+    "remote-web-worker": "^0.0.9",


It's a weird quirk that setting worker-src (as we are doing) still does not fix this issue...

I was exploring a fix here but I think patching the global worker object may be the best way to ensure this doesn't trip up other devs. I think we should get @elastic/kibana-security eyes on this too.

FWIW in my local testing these were my CSP response headers:

script-src 'report-sample' 'self' 0719-85-145-78-244.ngrok-free.app; worker-src 'report-sample' 'self' blob: script-src 'report-sample' 'self' 0719-85-145-78-244.ngrok-free.app; worker-src 'report-sample' 'self' blob: 0719-85-145-78-244.ngrok-free.app; style-src 'report-sample' 'self' 'unsafe-inline' 0719-85-145-78-244.ngrok-free.app; connect-src 'self' *.elastic.co 0719-85-145-78-244.ngrok-free.app; font-src 'self' 0719-85-145-78-244.ngrok-free.app; img-src 'self' *.elastic.co data: 0719-85-145-78-244.ngrok-free.app; style-src 'report-sample' 'self' 'unsafe-inline' 0719-85-145-78-244.ngrok-free.app; connect-src 'self' *.elastic.co 0719-85-145-78-244.ngrok-free.app; font-src 'self' 0719-85-145-78-244.ngrok-free.app; img-src 'self' *.elastic.co data: 0719-85-145-78-244.ngrok-free.app

I was serving assets via ngrok to check how it plays over https too. As you can see worker-src includes 0719-85-145-78-244.ngrok-free.app 🤷🏻

And it looks like we still need to fix Console's worker loading

it also caught me by surprise, I'm of the same opinion that we make the patch global so everyone gets it for free moving forward. I'll be pushing a update to this that's close to what you had but mostly builds of this library with an extra conditional to account for how ace editor workers get loaded.

packages/kbn-monaco/src/register_globals.ts

packages/kbn-ui-shared-deps-src/src/polyfills.js

dmlemeshko

kbn-test changes lgtm

kc13greiner · 2024-03-18T14:53:41Z

Heya @eokoneyo ! A couple QQ to help with my understanding:

Ideally this works but with the current setup for CDN assets it doesn't map 1:1 to how the assets are expected

I wasn't sure what this meant; Im not too familiar with the other CDN work going on 😅. Can you explain further?
Is this PR a temporary fix?
- If not, are there any other options than patching every worker to allow this?
- Are the URLs known? Maybe it could be limited to a subset of URLs?
- Could the patch be moved down into kbn-monaco or where the workers are spawned?

eokoneyo · 2024-03-19T11:18:13Z

Hi @kc13greiner!

Heya @eokoneyo ! A couple QQ to help with my understanding:

Ideally this works but with the current setup for CDN assets it doesn't map 1:1 to how the assets are expected

I wasn't sure what this meant; Im not too familiar with the other CDN work going on 😅. Can you explain further?

Sure, kibana packages it's assets so its reference for assets points to the host machine that runs it, but with Serverless that didn't scale well (for example there were cases of missing chunks because a open tab still refers to assets from a previous deployment that doesn't exist again). The CDN work makes it so that the aforementioned example issue doesn't occur.
On how this pertains to this PR, the change makes it such that when retrieving the assets for monaco's workers the same directory structure that gets emitted on build is preserved so that the method provided to monaco that fetches the workers it requires simple works as is.

Is this PR a temporary fix?

If not, are there any other options than patching every worker to allow this?

Are the URLs known? Maybe it could be limited to a subset of URLs?

Could the patch be moved down into kbn-monaco or where the workers are spawned?

As of now this is the most promising approach to circumvent the issue (the issue being that the worker constructor would only execute scripts that obey same-origin policy), and for what it's worth the reason we can't opt for a specific fix that doesn't patch the worker global is because we'd have to patch monaco itself.

kc13greiner · 2024-03-19T15:13:37Z

Sure, kibana packages it's assets so its reference for assets points to the host machine that runs it, but with Serverless that didn't scale well (for example there were cases of missing chunks because a open tab still refers to assets from a previous deployment that doesn't exist again). The CDN work makes it so that the aforementioned example issue doesn't occur. On how this pertains to this PR, the change makes it such that when retrieving the assets for monaco's workers the same directory structure that gets emitted on build is preserved so that the method provided to monaco that fetches the workers it requires simple works as is.

Thank you for the detailed response 🚀

As of now this is the most promising approach to circumvent the issue (the issue being that the worker constructor would only execute scripts that obey same-origin policy), and for what it's worth the reason we can't opt for a specific fix that doesn't patch the worker global is because we'd have to patch monaco itself.

What other approaches were considered? Allowing all workers to circumvent the CSP definitely raises security concerns; is there any other way we could approach this that wouldn't be so global or could limit the impact?

jloleysens · 2024-03-19T15:31:49Z

@kc13greiner in this case we have set worker-src but it seems this directive still does not allow workers to be instantiated with scripts on remote domains. https://developer.mozilla.org/en-US/docs/Web/API/Worker

In this case the workaround is still constrained by ~~connect-src~~ script-src that governsimportScripts. I was hoping to get a sanity check on our use of worker src and whether there are any other issues with this approach. Unfortunately I don't know of another solution than what @eokoneyo has proposed.

kc13greiner · 2024-03-19T23:16:58Z

@eokoneyo @jloleysens ++ I see what you're getting at now - I'm still looking into it and discussing it with the team.

kc13greiner

Overall LGTM - a couple question/nit!

The Workers will still be subject to the CSPs script-src directive since they are using importscripts.

Creating Workers with scripts from remote origins does appear to be a wider issue.

packages/kbn-ui-shared-deps-src/src/polyfills.js

kc13greiner · 2024-03-20T15:52:37Z

packages/kbn-ui-shared-deps-src/src/polyfills.js

+        super(
+          // Check if the URL is remote
+          url.includes('://') && !url.startsWith(window.location.origin) && !url.startsWith('blob:') // to bootstrap the actual script to work around the same origin policy.
+            ? URL.createObjectURL(


Observation/nit: Im not sure how severe is, but I think this may have a small memory leak - seems low impact, just something to keep in mind!

kc13greiner

LGTM!

kibana-ci · 2024-03-21T15:17:44Z

💚 Build Succeeded

Buildkite Build
Commit: 13a08f6
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-178669-13a08f6a0e16
Observability Deployment

Metrics [docs]

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
`module count`	-	5806	+5806
`total size`	-	6.5MB	+6.5MB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`kbnUiSharedDeps-srcJs`	2.9MB	2.9MB	+469.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`@kbn/ui-shared-deps-src`	2	3	+1

Total ESLint disabled count

id	before	after	diff
`@kbn/ui-shared-deps-src`	2	3	+1

History

💔 Build #198705 failed 97cf3c0382b3b2f9002e7625b96f45d8168d9b44
💚 Build #198655 succeeded 7d42074f4f4318b47696f58d57b348bbbe5d4e85
💚 Build #198634 succeeded 9f786df61b4e0dcb1cca333fe09759422a915ad9
💛 Build #198251 was flaky 889dff5a0998b12f2f347969e1a056581072051b
💚 Build #197761 succeeded e423de1e92716623bf29caac4f06a48283308c33

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @eokoneyo

## Summary Fix #179058 Update the `transformRequest` function to create URLs that are callable from inside workers that don't share the same domain. Proposal is to to convert absolute path values to absolute URLs, e.g. `/my/path` => `https://my.origin/my/path`. - [x] Blocked by #178669 ## Test locally with CDN Easiest way to test is via the observability project built on this PR (see buildkite output), otherwise: - Build CDN assets (`node ./scripts/build.js --skip-docker-ubi --skip-docker-ubuntu --skip-docker-fips --skip-os-packages`) - Unpack the CDN assets `target/kibana-8.14.0-SNAPSHOT-cdn-assets.tar.gz` and serve them `npx http-server -p 1772 --cors --gzip --brotli`, remember to change the hash dir name to `XXXXXXXXXXXX` - Serve via different domain eg: ``` ## My test 127.0.0.1 my.cdn.test ``` - Configure Kibana to load assets `server.cdn.url: http://my.cdn.test` --------- Co-authored-by: Eyo Okon Eyo <eyo.eyo@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

eokoneyo added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) ci:project-deploy-observability Create an Observability project labels Mar 13, 2024

eokoneyo self-assigned this Mar 13, 2024

eokoneyo requested a review from a team as a code owner March 13, 2024 18:23

jbudz approved these changes Mar 13, 2024

View reviewed changes

eokoneyo requested a review from a team as a code owner March 14, 2024 09:35

eokoneyo force-pushed the chore/fix-178448 branch from ca2e2bb to 766e921 Compare March 14, 2024 09:44

jloleysens mentioned this pull request Mar 14, 2024

[Serverless-observability-CDN-QA] Failed to construct 'worker' : Script error on clicking on vega maps #178410

Closed

eokoneyo requested a review from a team as a code owner March 14, 2024 11:03

jloleysens mentioned this pull request Mar 14, 2024

[Serverless-observability-CDN-QA] Edit dashboard ->add panel->maps->unable to initialize map #178390

Closed

jloleysens approved these changes Mar 14, 2024

View reviewed changes

jloleysens requested a review from a team March 14, 2024 16:30

jloleysens reviewed Mar 14, 2024

View reviewed changes

packages/kbn-monaco/src/register_globals.ts Outdated Show resolved Hide resolved

kc13greiner self-requested a review March 14, 2024 16:45

eokoneyo force-pushed the chore/fix-178448 branch from f91f9e0 to 3d9442f Compare March 14, 2024 16:50

jbudz self-requested a review March 14, 2024 16:51

eokoneyo force-pushed the chore/fix-178448 branch from 3d9442f to 4e4159e Compare March 14, 2024 16:53

jbudz reviewed Mar 14, 2024

View reviewed changes

packages/kbn-ui-shared-deps-src/src/polyfills.js Outdated Show resolved Hide resolved

jloleysens mentioned this pull request Mar 14, 2024

[HTTP] Workaround for loading worker from non origin #178752

Closed

dmlemeshko approved these changes Mar 18, 2024

View reviewed changes

eokoneyo force-pushed the chore/fix-178448 branch from e423de1 to d3e04ca Compare March 19, 2024 11:20

mattkime mentioned this pull request Mar 20, 2024

[serverless][management] can't save runtime field for imported Data view #178939

Closed

Dosant approved these changes Mar 20, 2024

View reviewed changes

jloleysens added ci:project-deploy-elasticsearch Create an Elasticsearch Serverless project and removed ci:project-deploy-elasticsearch Create an Elasticsearch Serverless project labels Mar 20, 2024

kc13greiner reviewed Mar 20, 2024

View reviewed changes

jsanz mentioned this pull request Mar 20, 2024

[Serverless-observability-CDN-QA] Adding elasticsearch documents on Maps keeps loading layers infinitely #179058

Closed

eokoneyo force-pushed the chore/fix-178448 branch 3 times, most recently from 47a7092 to 7d42074 Compare March 21, 2024 07:06

kc13greiner approved these changes Mar 21, 2024

View reviewed changes

eokoneyo force-pushed the chore/fix-178448 branch from a34a1fd to 97cf3c0 Compare March 21, 2024 12:07

jloleysens mentioned this pull request Mar 21, 2024

[CDN/Maps] Update transformRequest to create absolute URLs #179154

Merged

1 task

eokoneyo and others added 8 commits March 21, 2024 15:03

copy assets from appropriate directory for kbn-monaco

1807a5d

add util for handling cross-origin workers

0473f6a

add mock to support Worker instantiation in jest

809d12b

add polyfill for workers

73dac55

use proper jsdoc markup

28574fb

[CI] Auto-commit changed files from 'node scripts/notice'

8a997a6

address security concerns

a453567

nit: create single reference point for script url

13a08f6

eokoneyo force-pushed the chore/fix-178448 branch from 97cf3c0 to 13a08f6 Compare March 21, 2024 14:04

eokoneyo merged commit bb54184 into elastic:main Mar 21, 2024
17 checks passed

eokoneyo deleted the chore/fix-178448 branch March 21, 2024 15:29

kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Mar 21, 2024

thomheymann mentioned this pull request Mar 26, 2024

[Observability Onboarding] Link all “Add data” buttons in observability to onboarding landing page #179459

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copy assets from appropriate directory for kbn-monaco #178669

Copy assets from appropriate directory for kbn-monaco #178669

eokoneyo commented Mar 13, 2024 •

edited

Loading

elasticmachine commented Mar 13, 2024

jloleysens left a comment

jloleysens Mar 14, 2024

jloleysens Mar 14, 2024

jloleysens Mar 14, 2024

eokoneyo Mar 14, 2024

dmlemeshko left a comment

kc13greiner commented Mar 18, 2024

eokoneyo commented Mar 19, 2024

kc13greiner commented Mar 19, 2024

jloleysens commented Mar 19, 2024 •

edited

Loading

kc13greiner commented Mar 19, 2024

kc13greiner left a comment

kc13greiner Mar 20, 2024

kc13greiner left a comment

kibana-ci commented Mar 21, 2024 •

edited

Loading

ESLint disabled line counts

Total ESLint disabled count

Copy assets from appropriate directory for kbn-monaco #178669

Copy assets from appropriate directory for kbn-monaco #178669

Conversation

eokoneyo commented Mar 13, 2024 • edited Loading

Summary

elasticmachine commented Mar 13, 2024

jloleysens left a comment

Choose a reason for hiding this comment

jloleysens Mar 14, 2024

Choose a reason for hiding this comment

jloleysens Mar 14, 2024

Choose a reason for hiding this comment

jloleysens Mar 14, 2024

Choose a reason for hiding this comment

eokoneyo Mar 14, 2024

Choose a reason for hiding this comment

dmlemeshko left a comment

Choose a reason for hiding this comment

kc13greiner commented Mar 18, 2024

eokoneyo commented Mar 19, 2024

kc13greiner commented Mar 19, 2024

jloleysens commented Mar 19, 2024 • edited Loading

kc13greiner commented Mar 19, 2024

kc13greiner left a comment

Choose a reason for hiding this comment

kc13greiner Mar 20, 2024

Choose a reason for hiding this comment

kc13greiner left a comment

Choose a reason for hiding this comment

kibana-ci commented Mar 21, 2024 • edited Loading

💚 Build Succeeded

Metrics [docs]

Canvas Sharable Runtime

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

eokoneyo commented Mar 13, 2024 •

edited

Loading

jloleysens commented Mar 19, 2024 •

edited

Loading

kibana-ci commented Mar 21, 2024 •

edited

Loading