Skip to content

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

Merged
jloleysens merged 44 commits intoelastic:mainfrom
jloleysens:http/remove-replace-buildnr-with-sha
Feb 7, 2024
Merged

[Http] Replace buildNr with buildSha in static asset paths#175898
jloleysens merged 44 commits intoelastic:mainfrom
jloleysens:http/remove-replace-buildnr-with-sha

Conversation

@jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Jan 30, 2024

Summary

Follow up of first CDN PR. 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.

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 now scoping more static assets to a SHA and so every new Kibana release will require clients to, for example, download fonts again.

For maintainers

@jloleysens jloleysens added Feature:http Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// release_note:skip Skip the PR/issue when compiling release notes v8.13.0 labels Jan 30, 2024
@jloleysens
Copy link
Contributor Author

/ci

@jloleysens jloleysens force-pushed the http/remove-replace-buildnr-with-sha branch from b97a046 to 2562311 Compare January 31, 2024 12:00
@jloleysens
Copy link
Contributor Author

/ci

@jloleysens
Copy link
Contributor Author

/ci

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x-pack/plugins/screenshotting lgtm

@azasypkin azasypkin self-requested a review February 5, 2024 14:24
Copy link
Contributor

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! Tested locally with the custom base path and without in two different SSR cases we have in Security plugin - everything works as expected.

@kibana-ci
Copy link

💚 Build Succeeded

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/config 47 48 +1
@kbn/core-http-server 180 181 +1
@kbn/core-http-server-internal 52 72 +20
total +22
Unknown metric groups

API count

id before after diff
@kbn/config 76 77 +1
@kbn/core-http-server 457 459 +2
@kbn/core-http-server-internal 58 84 +26
total +29

History

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

cc @jloleysens

@jloleysens jloleysens added v8.14.0 and removed v8.13.0 labels Feb 6, 2024
@jloleysens jloleysens merged commit e90c209 into elastic:main Feb 7, 2024
@jloleysens jloleysens deleted the http/remove-replace-buildnr-with-sha branch February 7, 2024 08:54
@jloleysens jloleysens removed the v8.13.0 label Feb 7, 2024
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.14 The branch "8.14" does not exist

Manual backport

To create the backport manually run:

node scripts/backport --pr 175898

Questions ?

Please refer to the Backport tool documentation

jloleysens added a commit that referenced this pull request Feb 7, 2024
* main: (224 commits)
  [Http] Replace `buildNr` with `buildSha` in static asset paths (#175898)
  [Ops] Fix GCS bucket access for future buildkite agents (#174756)
  [api-docs] 2024-02-07 Daily api_docs build (#176362)
  skip flaky suite (#176002)
  skip failing es promotion suite (#176359)
  [Cloud Security] [Grouping] Add URL Params support to the grouping components (#175749)
  chore(NA): update versions after v8.12.2 bump (#176309)
  chore(NA): update versions after v7.17.19 bump (#176313)
  skip failing test suite (#176352)
  [SLO] Enable burn rate alert by default during creation via UI (#176317)
  [Fleet] Add the uptime capability to observability projects (#176285)
  [Security Solution][Endpoint] Fix Manifest Manger so that it works with large (>10k) (#174411)
  [ResponseOps] Alert creation delay based on user definition (#175851)
  [data views] Default field formatters based on field meta values (#174973)
  [Cloud Security]Detection Rules counter on Rules Flyout (#176041)
  [Security Solution] Data Quality Dashboard persistence (#175673)
  [Ent Search] Connector client copy cleanup (#176290)
  [ML] Anomaly Detection: Adds actions menu to anomaly markers in Single Metric Viewer chart. (#175556)
  [ML] Anomaly Detection: Fix `values-dots` colors (#176303)
  [Fleet] Logstash Output - being compliant to RFC-952 (#176298)
  ...
fkanout pushed a commit to fkanout/kibana that referenced this pull request Feb 7, 2024
…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>
jbudz added a commit that referenced this pull request Feb 9, 2024
#175898 updates routes to cache
bust using the build checksum instead of build number. This updates the
directory structure of the CDN assets archive accordingly.

```
kibana-8.13.0-SNAPSHOT-cdn-assets
└── e96cc06
    ├── bundles
    │   ├── core
    │   ├── kbn-monaco
    │   ├── kbn-ui-shared-deps-npm
    │   ├── kbn-ui-shared-deps-src
    │   └── plugin
    └── ui
        ├── favicons
        ├── fonts
        ├── legacy_dark_theme.css
        ├── legacy_dark_theme.min.css
        ├── legacy_light_theme.css
        └── legacy_light_theme.min.css

11 directories, 4 files
```
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 11, 2024
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 175898 locally

1 similar comment
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 175898 locally

@jbudz jbudz added the backport:skip This PR does not require backporting label Feb 14, 2024
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 14, 2024
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…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>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…#176186)

elastic#175898 updates routes to cache
bust using the build checksum instead of build number. This updates the
directory structure of the CDN assets archive accordingly.

```
kibana-8.13.0-SNAPSHOT-cdn-assets
└── e96cc06
    ├── bundles
    │   ├── core
    │   ├── kbn-monaco
    │   ├── kbn-ui-shared-deps-npm
    │   ├── kbn-ui-shared-deps-src
    │   └── plugin
    └── ui
        ├── favicons
        ├── fonts
        ├── legacy_dark_theme.css
        ├── legacy_dark_theme.min.css
        ├── legacy_light_theme.css
        └── legacy_light_theme.min.css

11 directories, 4 files
```
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…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>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…#176186)

elastic#175898 updates routes to cache
bust using the build checksum instead of build number. This updates the
directory structure of the CDN assets archive accordingly.

```
kibana-8.13.0-SNAPSHOT-cdn-assets
└── e96cc06
    ├── bundles
    │   ├── core
    │   ├── kbn-monaco
    │   ├── kbn-ui-shared-deps-npm
    │   ├── kbn-ui-shared-deps-src
    │   └── plugin
    └── ui
        ├── favicons
        ├── fonts
        ├── legacy_dark_theme.css
        ├── legacy_dark_theme.min.css
        ├── legacy_light_theme.css
        └── legacy_light_theme.min.css

11 directories, 4 files
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:http release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.14.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants