[HTTP] Add support for configuring a CDN (part I) #169408

jloleysens · 2023-10-19T14:42:01Z

Summary

This PR prepares the Kibana core code for configuring and using a CDN to serve the vast majority of assets.

Goals

(1) Configure a CDN URL: server.cdn.url
(2) Reach a new domain: updating our CSP to allow this
(3) Serve JS bundles, CSS, fonts via CDN

How to test

Make a release/distribution build of Kibana (node ./scripts/build.js --release --skip-os-packages), will output to ./build/kibana.
Edit /etc/hosts
1. I added 127.0.0.1 my.cdn.test
Edit config/kibana.dev.yml adding server.cdn.url: "http://my.cdn.test:1772"
cd into build/kibana once the release build is done
Download and run this (hacky) script there. It should (🤞🏻 ) create a folder structure mirroring our server paths: https://gist.github.com/jloleysens/86cae7ea3d7512e6a6e92cebbf1566f4
Start a file server in build/kibana. For example: npx http-server -p 1772 --cors --gzip --brotli -- this will be the "CDN" server
Start Kibana yarn start and ES yarn es snapshot
Open Kibana, checking the Network tab of your browser for anything that fails to load: 404s, CORs or CSP is almost definitely a bug in this code.

Future work

Include the translations file to the CDN (~~should not be much effort~~)
Update the build process to create a folder structure that mirrors what we serve from Kibana today, this will replace step 5 in the testing process
- This includes deploying new assets to the CDN as part of our promotion pipeline

Screenshots

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Changing the existing behaviour	med	high	based on the existing test coverage I expect us to not change the current non-CDN asset serving
Use of undocumented feature	Very low	Low	While this could brick Kibana, it would be a simple configuration change

For maintainers

This was checked for breaking API changes and was labeled appropriately

…hings for clarity

elasticmachine · 2023-10-19T14:42:04Z

Pinging @elastic/kibana-core (Team:Core)

rudolf

This looks like a great foundation to start iterating on. Something we should explore with operations is how to write a smoke test for this as a bug here would be high impact. Would be great if once we've published assets we could spin up Kibana and make sure it can render from the CDN.

rudolf · 2023-10-19T15:02:44Z

packages/core/http/core-http-server-internal/src/cdn.ts

+      default_src: [hostname],
+      font_src: [hostname],
+      img_src: [hostname],
+      script_src: [hostname],
+      style_src: [hostname],
+      worker_src: [hostname],
+      connect_src: [hostname],
+      frame_src: [hostname],


@elastic/kibana-security IIRC you've always reviewed changes to our CSP so would be good to get a +1 from you

pgayvallet

Just a few remarks and question, but overall LGTM

pgayvallet · 2023-10-19T15:00:27Z

packages/core/http/core-http-server-internal/src/csp/config.ts

+export type CspAdditionalConfig = Pick<
+  Partial<CspConfigType>,
+  | 'connect_src'
+  | 'default_src'
+  | 'font_src'
+  | 'frame_src'
+  | 'img_src'
+  | 'script_src'
+  | 'style_src'
+  | 'worker_src'
+>;


NIT: I would just have gone with Partial<CspConfigType>, but what you did is probably better 😄

Yeah, I had that initially, but there is other funky stuff in there and I wanted scope the overrides to just the policy directives

pgayvallet · 2023-10-19T15:01:32Z

packages/core/http/core-http-server-internal/src/cdn.ts

+  public static from(input: Input = {}) {
+    return new CdnConfig(input);
+  }


NIT: FMI, what's the upside of the private constructor + static creator? (or maybe that's just how we're already doing with other http sub configs?)

Good q, I slightly refactored this, bc I realised I was using Input in both the constructor and in this state creator. The constructor know uses it's own, separated args as they are IMO easier to use in the constructor.

But yeah, not massive benefit, as you point out.

packages/core/http/core-http-server-internal/src/static_assets.ts

pgayvallet · 2023-10-19T15:09:15Z

packages/core/rendering/core-rendering-server-internal/src/rendering_service.tsx

+          // TODO: Make this load as part of static assets!
          translationsUrl: `${basePath}/translations/${i18n.getLocale()}.json`,


++ (but good luck 😄)

packages/core/http/core-http-server-internal/src/static_assets.ts

legrego · 2023-10-19T15:14:18Z

packages/core/http/core-http-server-internal/src/cdn.ts

+      default_src: [hostname],
+      font_src: [hostname],
+      img_src: [hostname],
+      script_src: [hostname],
+      style_src: [hostname],
+      worker_src: [hostname],
+      connect_src: [hostname],
+      frame_src: [hostname],


Have you verified that Kibana requires all of these declarations for the CDN to function? A few initial thoughts:

We don't supply a default_src today, and I don't think the CDN should be the first to do so, unless there's a compelling reason.

Will Kibana host frames whose src points to the CDN? If not, then we likely don't need frame_src.

This CDN will (I assume) host static assets. It seems unlikely that connect_src would be required.

Will the CDN be hosting scripts used by web workers? If not, then worker_src is unlikely to be required. I don't know where we're using working within Kibana today, so this is a genuine question.

All valid points :) there is some amount of unknown, since I don't know what all will be served from the CDN, but:

Sounds good to me!

I don't know, but we can remove for now

I believe we do have script based loading of certain assets

We will be hosting Monaco editor's worker scripts on the CDN, yes

jbudz · 2023-10-19T17:18:23Z

Do we think falling back to local assets if the CDN is down is worthwhile?

jloleysens · 2023-10-20T13:04:27Z

Do we think falling back to local assets if the CDN is down is worthwhile?

Sounds worthwhile to me, probably as an enhancement to this first iteration. There are two failure scenarios I can think of:

The CDN is down/unavailable
The CDN does not have assets deployed (404s), perhaps our deployment failed and we notice to late

To address both of these are you imagining something like this:

https://stackoverflow.com/a/2607534

Perhaps webpack already has something for this?

jloleysens · 2023-10-20T13:04:39Z

@jbudz

kibana-ci · 2023-10-20T13:59:48Z

💚 Build Succeeded

Buildkite Build
Commit: 2d03945

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/core-http-server-internal`	52	53	+1
`@kbn/core-http-server-mocks`	42	43	+1
total			+2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/core-http-server-internal`	6	7	+1

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-http-server-internal`	58	59	+1
`@kbn/core-http-server-mocks`	43	44	+1
total			+2

History

💔 Build #169722 failed 35c6a49
💔 Build #169689 failed 8b68ba3
💛 Build #169560 was flaky a9fe5fa
💔 Build #169532 failed 02a2781
💔 Build #169502 failed d9f2c2a

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

jbudz · 2023-10-26T13:51:24Z

packages/core/rendering/core-rendering-server-internal/src/render_utils.ts

-          `${regularBundlePath}/kbn-ui-shared-deps-src/${UiSharedDepsSrc.cssDistFilename}`,
-          `${basePath}/ui/legacy_light_theme.min.css`,
+          `${bundlesHref}/kbn-ui-shared-deps-src/${UiSharedDepsSrc.cssDistFilename}`,
+          `${baseHref}/ui/legacy_light_theme.min.css`,


Have we considered putting this route behind the build number? Behind a CDN I'm not sure how would push updates through to these files.

Closes #169427 Adds a new build step `createCdnAssets` that will create an archive `kibana-<version>-cdn-assets.tar.gz` with static assets organized using the request structure of the kibana client. - By default CDN assets are created - Adding the flag `node scripts/build --skip-cdn-assets` will skip creation - `ci:build-cdn-assets` can be used to create and upload the archive for testing Testing: see #169408. Builds are available in the artifacts tab on the `Build Distribution` step. 1) Extract builds 2) ``` python3 -m http.server -b localhost -d kibana-8.12.0-SNAPSHOT-cdn-assets 8000 ``` 3) ``` echo 'server.cdn.url: http://localhost:8000' >> kibana-8.12.0-SNAPSHOT/config/kibana.yml ``` 4) ``` kibana-8.12.0-SNAPSHOT/bin/kibana ```

## Summary Follow up of [first CDN PR](#169408). Primary focus is replacing our build nr with SHA that allows cache busting and maintains anti-collision properties. ## How to test Start Kibana as usual navigating around the app with the network tab open in your browser of choice. Keep an eye out for any asset loading errors. It's tricky to test every possible asset since there are many permutations, but generally navigating around Kibana should work exactly as it did before regarding loading bundles and assets. ## Notes * did a high-level audit of usages of `buildNum` in `packages`, `src` and `x-pack` adding comments where appropriate. * In non-distributable builds (like dev) static asset paths will be prefixed with `XXXXXXXXXXXX` instead of Node's `Number.MAX_SAFE_INTEGER` * Added some validation to ensure the CDN url is of the expected form: nothing trailing the pathname ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### Risk Matrix | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | We break some first or third party dependencies on existing asset routes | Med | High | Attempting to mitgate by serving static assets from both old and new paths where paths have updated to include the build SHA. Additioanlly: it is very bad practice to rely on the values of the static paths, but someone might be | | Cache-busting is more aggressive | High | Low | Unlikely to be a big problem, but we are not scoping more static assets to a SHA and so every new Kibana release will require clients to, for example, download fonts again. | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

…ic#175898) ## Summary Follow up of [first CDN PR](elastic#169408). Primary focus is replacing our build nr with SHA that allows cache busting and maintains anti-collision properties. ## How to test Start Kibana as usual navigating around the app with the network tab open in your browser of choice. Keep an eye out for any asset loading errors. It's tricky to test every possible asset since there are many permutations, but generally navigating around Kibana should work exactly as it did before regarding loading bundles and assets. ## Notes * did a high-level audit of usages of `buildNum` in `packages`, `src` and `x-pack` adding comments where appropriate. * In non-distributable builds (like dev) static asset paths will be prefixed with `XXXXXXXXXXXX` instead of Node's `Number.MAX_SAFE_INTEGER` * Added some validation to ensure the CDN url is of the expected form: nothing trailing the pathname ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### Risk Matrix | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | We break some first or third party dependencies on existing asset routes | Med | High | Attempting to mitgate by serving static assets from both old and new paths where paths have updated to include the build SHA. Additioanlly: it is very bad practice to rely on the values of the static paths, but someone might be | | Cache-busting is more aggressive | High | Low | Unlikely to be a big problem, but we are not scoping more static assets to a SHA and so every new Kibana release will require clients to, for example, download fonts again. | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

Adds support for setting `server.cdn.url` in our docker containers. Implemented at #169408.

Adds support for setting `server.cdn.url` in our docker containers. Implemented at elastic#169408. (cherry picked from commit 023f5d6)

# Backport This will backport the following commits from `main` to `8.13`: - [[bin/kibana-docker] Add server.cdn.url (#177491)](#177491)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jon <jon@elastic.co>

…ic#175898) ## Summary Follow up of [first CDN PR](elastic#169408). Primary focus is replacing our build nr with SHA that allows cache busting and maintains anti-collision properties. ## How to test Start Kibana as usual navigating around the app with the network tab open in your browser of choice. Keep an eye out for any asset loading errors. It's tricky to test every possible asset since there are many permutations, but generally navigating around Kibana should work exactly as it did before regarding loading bundles and assets. ## Notes * did a high-level audit of usages of `buildNum` in `packages`, `src` and `x-pack` adding comments where appropriate. * In non-distributable builds (like dev) static asset paths will be prefixed with `XXXXXXXXXXXX` instead of Node's `Number.MAX_SAFE_INTEGER` * Added some validation to ensure the CDN url is of the expected form: nothing trailing the pathname ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### Risk Matrix | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | We break some first or third party dependencies on existing asset routes | Med | High | Attempting to mitgate by serving static assets from both old and new paths where paths have updated to include the build SHA. Additioanlly: it is very bad practice to rely on the values of the static paths, but someone might be | | Cache-busting is more aggressive | High | Low | Unlikely to be a big problem, but we are not scoping more static assets to a SHA and so every new Kibana release will require clients to, for example, download fonts again. | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

Adds support for setting `server.cdn.url` in our docker containers. Implemented at elastic#169408.

jloleysens added 11 commits October 18, 2023 11:34

checkpoint 0: added new concept of "StaticAssets" and renamed a few t…

79d7aea

…hings for clarity

add new files

cf9ef1e

added config to allow use of cdns w.r.t. csp

78cd78c

more tests!

e8969c3

remove # private

8a3f036

minor tweaks

f64cf84

server bootstrap JS from kibana server

a5a6e9c

added cdn test file

bc68067

I guess we won't make translations static yet

d209f6a

added a comment

45c43e7

update tests

d9f2c2a

jloleysens added Feature:http Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes v8.12.0 labels Oct 19, 2023

jloleysens requested a review from a team as a code owner October 19, 2023 14:42

rudolf approved these changes Oct 19, 2023

View reviewed changes

pgayvallet approved these changes Oct 19, 2023

View reviewed changes

jloleysens commented Oct 19, 2023

View reviewed changes

packages/core/http/core-http-server-internal/src/static_assets.ts Show resolved Hide resolved

legrego reviewed Oct 19, 2023

View reviewed changes

jloleysens added 3 commits October 19, 2023 17:21

added static assets test and minor refactor

02a2781

update CSP based on feedback and added a test

b9787a0

fix http config

deb0031

lukeelmers mentioned this pull request Oct 19, 2023

Support for serving static assets over a CDN #72880

Closed

jloleysens and others added 4 commits October 19, 2023 17:41

Merge branch 'main' into introduce-cdn-as-config-and-use-it

c16b0b2

rename var

5cbb75a

slight update to tests

8654895

update test snapshot

a9fe5fa

mistic mentioned this pull request Oct 19, 2023

Deploy Kibana static assets into a CDN #169427

Closed

jloleysens and others added 3 commits October 19, 2023 23:13

remove scoped static assets path

8b68ba3

Merge branch 'main' into introduce-cdn-as-config-and-use-it

35c6a49

update jest test

2d03945

jloleysens merged commit 8727c68 into elastic:main Oct 21, 2023
30 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Oct 21, 2023

jloleysens deleted the introduce-cdn-as-config-and-use-it branch October 21, 2023 13:40

benakansara pushed a commit to benakansara/kibana that referenced this pull request Oct 22, 2023

[HTTP] Add support for configuring a CDN (part I) (elastic#169408)

1931286

benakansara pushed a commit to benakansara/kibana that referenced this pull request Oct 22, 2023

[HTTP] Add support for configuring a CDN (part I) (elastic#169408)

3e17d50

jbudz reviewed Oct 26, 2023

View reviewed changes

jloleysens mentioned this pull request Nov 2, 2023

Epic: Smooth UI upgrades #170421

Closed

jbudz mentioned this pull request Nov 3, 2023

[build] Create CDN assets #169707

Merged

jloleysens mentioned this pull request Jan 30, 2024

[Http] Replace buildNr with buildSha in static asset paths #175898

Merged

2 tasks

jbudz mentioned this pull request Feb 21, 2024

[bin/kibana-docker] Add server.cdn.url #177491

Merged

jbudz added a commit that referenced this pull request Feb 22, 2024

[bin/kibana-docker] Add server.cdn.url (#177491)

023f5d6

Adds support for setting `server.cdn.url` in our docker containers. Implemented at #169408.

fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024

[bin/kibana-docker] Add server.cdn.url (elastic#177491)

dba7bf7

Adds support for setting `server.cdn.url` in our docker containers. Implemented at elastic#169408.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HTTP] Add support for configuring a CDN (part I) #169408

[HTTP] Add support for configuring a CDN (part I) #169408

jloleysens commented Oct 19, 2023 •

edited

Loading

elasticmachine commented Oct 19, 2023

rudolf left a comment

rudolf Oct 19, 2023

pgayvallet left a comment

pgayvallet Oct 19, 2023

jloleysens Oct 19, 2023 •

edited

Loading

pgayvallet Oct 19, 2023

jloleysens Oct 19, 2023

pgayvallet Oct 19, 2023

legrego Oct 19, 2023

jloleysens Oct 19, 2023

jbudz commented Oct 19, 2023

jloleysens commented Oct 20, 2023 •

edited

Loading

jloleysens commented Oct 20, 2023

kibana-ci commented Oct 20, 2023

API count

jbudz Oct 26, 2023

		// TODO: Make this load as part of static assets!
		translationsUrl: `${basePath}/translations/${i18n.getLocale()}.json`,

[HTTP] Add support for configuring a CDN (part I) #169408

[HTTP] Add support for configuring a CDN (part I) #169408

Conversation

jloleysens commented Oct 19, 2023 • edited Loading

Summary

Goals

How to test

Future work

Screenshots

Checklist

Risk Matrix

For maintainers

elasticmachine commented Oct 19, 2023

rudolf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jloleysens Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbudz commented Oct 19, 2023

jloleysens commented Oct 20, 2023 • edited Loading

jloleysens commented Oct 20, 2023

kibana-ci commented Oct 20, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Public APIs missing exports

API count

History

Choose a reason for hiding this comment

jloleysens commented Oct 19, 2023 •

edited

Loading

jloleysens Oct 19, 2023 •

edited

Loading

jloleysens commented Oct 20, 2023 •

edited

Loading