fix(ui): Provide prebuilt UI assets in release#5191
fix(ui): Provide prebuilt UI assets in release#5191SoloJacobs merged 1 commit intoprometheus:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCI and Makefile changes add building and packaging of prebuilt frontend assets. The frontend test job generates a tarball and uploads Changes
Sequence DiagramsequenceDiagram
participant CI as CI System
participant ArtifactStore as Artifact Storage
participant Build as Build Job
participant Test as Test Job
participant Release as Release Job
CI->>CI: test_frontend: run frontend build
CI->>CI: test_frontend: make assets-tarball (creates .tar.gz)
CI->>ArtifactStore: upload `ui-dist` (contents of ui/app/dist)
Build->>ArtifactStore: download `ui-dist`
Build->>Build: extract -> ui/app/dist
Build->>Build: continue build (promci / package)
Test->>ArtifactStore: download `ui-dist`
Test->>Test: extract -> ui/app/dist
Test->>Test: run tests
Release->>ArtifactStore: download `ui-dist`
Release->>Release: extract -> ui/app/dist
Release->>Release: build release artifacts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 41-44: The assets-tarball recipe currently creates an uncompressed
".tar" via "tar cf" and misnames the artifact; update the assets-tarball target
to create a compressed gzip archive named ".tar.gz" by using gzip compression
with tar (i.e., replace the tar invocation that uses "tar cf" and outputs ".tar"
with a gzip-producing command such as "tar czf" and change the output filename
to ".tar.gz" so the produced artifact matches the documented
alertmanager-web-ui-<VERSION>.tar.gz; keep the same VERSION expansion ($(file
<VERSION)) and the -C ui/app dist context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3e8d31b2-abb6-46a6-aa96-72d57a0aaeca
📒 Files selected for processing (4)
.github/workflows/ci.yml.github/workflows/release.ymlMakefileui/app/Makefile
In prometheus#5113 I removed the build artifacts from our source repo. This complicated the packaging process for downstream maintainers significantly. We will still not check the artifacts into our repo directly, since this is a burden to the review process and opens us up to attacks such as the https://en.wikipedia.org/wiki/XZ_Utils_backdoor . Instead we provide two new mechanisms to obtain the artifacts: * alertmanager-web-ui-(file <VERSION).tar.gz will provide the `dist` folder needed for calling `make build`. * We will upload artifacts via actions/upload-artifact. These can then be easily retrieved for 90 days using `gh`. Finally, this commit also ensures that `ui/app/dist` is always available during build steps. The reason is that caching `npm` dependencies is not sufficient: `Elm` will download its files again anyway, since it stores them in `~/.elm`. This causes unnecessary network calls. Technical approach: * By moving the validation `.build_stamp` to `dist/`, we don't have to upload that file seperately. `embed` will exclude that file. * Storing artifacts in `.tarballs` is our method for including files in the release. This approach was directly taken from `prometheus/prometheus`. Relates-to: prometheus#5173 Fixes: prometheus#5163 Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
10707a5 to
475b1f2
Compare
|
Note: I plan to port this into 0.32 branch . |
In #5113 I removed the build artifacts from our source repo. This complicated the packaging process for downstream maintainers significantly. We will still not check the artifacts into our repo directly, since this is a burden to the review process and opens us up to attacks such as the https://en.wikipedia.org/wiki/XZ_Utils_backdoor . Instead we provide two new mechanisms to obtain the artifacts: * alertmanager-web-ui-(file <VERSION).tar.gz will provide the `dist` folder needed for calling `make build`. * We will upload artifacts via actions/upload-artifact. These can then be easily retrieved for 90 days using `gh`. Finally, this commit also ensures that `ui/app/dist` is always available during build steps. The reason is that caching `npm` dependencies is not sufficient: `Elm` will download its files again anyway, since it stores them in `~/.elm`. This causes unnecessary network calls. Technical approach: * By moving the validation `.build_stamp` to `dist/`, we don't have to upload that file seperately. `embed` will exclude that file. * Storing artifacts in `.tarballs` is our method for including files in the release. This approach was directly taken from `prometheus/prometheus`. Relates-to: #5173 Fixes: #5163 Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
In #5113 I removed the build artifacts from our source repo. This complicated the packaging process for downstream maintainers significantly.
We will still not check the artifacts into our repo directly, since this is a burden to the review process and opens us up to attacks such as the https://en.wikipedia.org/wiki/XZ_Utils_backdoor .
Instead we provide two new mechanisms to obtain the artifacts:
distfolder needed for callingmake build.gh.Finally, this commit also ensures that
ui/app/distis always available during build steps. The reason is that cachingnpmdependencies is not sufficient:Elmwill download its files again anyway, since it stores them in~/.elm. This causes unnecessary network calls.Technical approach:
.build_stamptodist/, we don't have to upload that file seperately.embedwill exclude that file..tarballsis our method for including files in the release. This approach was directly taken fromprometheus/prometheus.Pull Request Checklist
(Only a manual test on my end).
Summary by CodeRabbit