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.

Sign up for GitHub

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
Merged

Fix CDN task assets discovery #177985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f0ff0c
added flag to help
jloleysens Mar 5, 2024
f93f9b0
read from multiple sources and copy plugin static assets into archive
jloleysens Mar 5, 2024
7743827
only one assets dir
jloleysens Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/dev/build/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ if (showHelp) {
--docker-cross-compile {dim Produce arm64 and amd64 Docker images}
--docker-contexts {dim Only build the Docker build contexts}
--skip-canvas-shareable-runtime {dim Don't build the Canvas shareable runtime}
--skip-cdn-assets {dim Don't build CDN assets}
delanni marked this conversation as resolved.
Show resolved Hide resolved
--skip-docker-ubi {dim Don't build the docker ubi image}
--skip-docker-ubuntu {dim Don't build the docker ubuntu image}
--skip-docker-fips {dim Don't build the docker fips image}
Expand Down
8 changes: 3 additions & 5 deletions src/dev/build/tasks/create_cdn_assets_task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,14 @@ export const CreateCdnAssets: Task = {
const manifest = Jsonc.parse(readFileSync(path, 'utf8')) as any;
if (manifest?.plugin?.id) {
const pluginRoot = resolve(dirname(path));

// 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 •
edited
Loading

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!

const assetsSource = resolve(pluginRoot, 'public', 'assets');
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

try {
// packages/core/plugins/core-plugins-server-internal/src/plugins_service.ts
const assetsSource = resolve(pluginRoot, 'assets');
const assetsDest = resolve('plugins', manifest.plugin.id, 'assets');
await access(assetsSource);
await mkdirp(assetsDest);
await copyAll(assetsSource, assetsDest);
} catch (e) {
// assets are optional
if (!(e.code === 'ENOENT' && e.syscall === 'access')) throw e;
}

Expand Down