Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CDN task assets discovery #177985

Merged
merged 3 commits into from
Mar 6, 2024

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Mar 5, 2024

Summary

Plugin static assets were not being included in the CDN bundle due to the task looking in <plugin_root>/assets. This fix updates the task to look in <plugin_root>/public/assets.

Output structure

tree
❯ tree -dL 3 .
.
└── <hash>
    ├── bundles
    │   ├── core
    │   ├── kbn-monaco
    │   ├── kbn-ui-shared-deps-npm
    │   ├── kbn-ui-shared-deps-src
    │   └── plugin
    ├── plugins
    │   ├── apm
    │   ├── cloudDefend
    │   ├── cloudSecurityPosture
    │   ├── customIntegrations
    │   ├── dashboard
    │   ├── dataViewFieldEditor
    │   ├── discover
    │   ├── enterpriseSearch
    │   ├── fleet
    │   ├── globalSearchBar
    │   ├── home
    │   ├── indexManagement
    │   ├── kibanaOverview
    │   ├── kibanaReact
    │   ├── lens
    │   ├── maps
    │   ├── observability
    │   ├── observabilityAIAssistant
    │   ├── observabilityOnboarding
    │   ├── osquery
    │   ├── remoteClusters
    │   ├── serverlessSearch
    │   └── timelines
    └── ui
        ├── favicons
        └── fonts

35 directories

Test

  1. Build distributable using node scripts/build.js to get CDN assets
  2. Untar ./target/kibana-8.14.0-SNAPSHOT-cdn-assets.tar.gz
  3. Add an entry to /etc/hosts to resolve to 127.0.0.1
  4. cd into the untarred folder and serve the assets, I used npx http-server -p 1772 --cors --gzip --brotli
  5. Add server.cdn.url: "http://my.cdn.test:1772" to your Kibana config
  6. Start Kibana and ES, see assets are loading for the home app from our "CDN"
Screenshot 2024-03-05 at 11 14 45

@jloleysens jloleysens added Team:Operations Team label for Operations Team release_note:skip Skip the PR/issue when compiling release notes v8.14.0 labels Mar 5, 2024
@jloleysens jloleysens self-assigned this Mar 5, 2024
@jloleysens jloleysens requested a review from a team as a code owner March 5, 2024 10:22
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

resolve(pluginRoot, 'assets'),
];
// packages/core/apps/core-apps-server-internal/src/core_app.ts
const assetsDest = resolve(assets, buildSha, 'plugins', manifest.plugin.id, 'assets');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added assets and buildSha as this was copying into <kibana_root>/plugins

@jloleysens jloleysens changed the title Fix CDN assets discovery Fix CDN task assets discovery Mar 5, 2024
resolve(pluginRoot, 'public', 'assets'),
resolve(pluginRoot, 'assets'),
];
// packages/core/apps/core-apps-server-internal/src/core_app.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this comment intending to communicate?

Copy link
Contributor Author

@jloleysens jloleysens Mar 5, 2024

Choose a reason for hiding this comment

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

This is the source of where the paths come from... There is a bit of "weak" reference here because these paths are created by Kibana core server-side code at runtime and we are mirroring that code here.

I just followed the pattern in the rest of the file (eg).

Ideally we could share these values by importing some consts or something, but I guess these paths really don't change often so this has been an "OK" way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to open a follow up issue!

@jloleysens jloleysens requested a review from a team March 5, 2024 10:53
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

cc @jloleysens

@mistic
Copy link
Member

mistic commented Mar 5, 2024

@jloleysens should we fix the location of the assets on those 2 plugins in this PR as well?

/src/plugins/guided_onboarding/assets
x-pack/plugins/serverless/assets

@jloleysens
Copy link
Contributor Author

jloleysens commented Mar 6, 2024

should we fix the location of the assets on those 2 plugins in this PR as well?

@mistic Yeah, I'm not sure these work given that we only expose assets from public/assets so not sure how they're using these. I'll ask the teams but not sure we should block on that.


OK I checked and in both cases assets are either being bundled directly with code (in case of guided_onboarding) or the diagram is from Whimsical and used in README 😄 (in case of serverless)

@jloleysens jloleysens merged commit ca429ab into elastic:main Mar 6, 2024
16 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Mar 6, 2024
@jloleysens jloleysens deleted the fix-cdn-assets-discovery branch March 6, 2024 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants