feat(ui): Add static asset caching and pre-compressed serving#5102
Draft
SoloJacobs wants to merge 10 commits intoprometheus:mainfrom
Draft
feat(ui): Add static asset caching and pre-compressed serving#5102SoloJacobs wants to merge 10 commits intoprometheus:mainfrom
SoloJacobs wants to merge 10 commits intoprometheus:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplace Docker-based frontend/tooling with npm/Vite local builds; remove bundled Font Awesome CSS; embed and serve prebuilt app/dist with encoding-aware content negotiation and caching; extract operational/debug endpoints into a new weboperations package; update CI to use root .nvmrc and npm caching; adjust Makefile and main wiring. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client (Browser)
participant Server as Alertmanager (ui/web.go)
participant Assets as Embedded app/dist
participant Decompress as Decompressor
Browser->>Server: GET /assets/app.js\nAccept-Encoding: br, gzip
activate Server
Server->>Assets: Lookup /assets/app.js
Assets-->>Server: file exists
alt Brotli variant exists
Server->>Assets: Read app.js.br
Assets-->>Server: br bytes
Server-->>Browser: 200\nContent-Encoding: br\nVary: Accept-Encoding
else Gzip variant exists
Server->>Assets: Read app.js.gz
Assets-->>Server: gz bytes
Server-->>Browser: 200\nContent-Encoding: gzip\nVary: Accept-Encoding
else Only gz stored but client requests identity
Server->>Assets: Read app.js.gz
Assets-->>Server: gz bytes
Server->>Decompress: decompress gz in-memory
Decompress-->>Server: uncompressed bytes
Server-->>Browser: 200\n[uncompressed bytes]
end
deactivate Server
sequenceDiagram
participant User as Browser
participant Index as index.html
participant MainJS as /src/main.js
participant Elm as Elm.Main
participant Storage as localStorage
User->>Index: Load /
Index->>MainJS: load module
MainJS->>Storage: read firstDayOfWeek, groupExpandAll, defaultCreator
Storage-->>MainJS: return values
MainJS->>Elm: init(flags)
Elm-->>MainJS: subscribe ports
Elm->>MainJS: persistFirstDayOfWeek(value)
MainJS->>Storage: write firstDayOfWeek
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
SoloJacobs
force-pushed
the
example-ui
branch
9 times, most recently
from
feb0289 to
92c42fb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
ui/app/CONTRIBUTING.md (1)
11-21: Clarify command execution directory for new contributors.At Line 14 and Line 20, explicitly stating these commands should run from
ui/appwould reduce ambiguity.📝 Suggested doc tweak
To set up your environment and install all dependencies, run: ```sh +# from ui/app npm ciTo run the tests:
+# from ui/app make test</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@ui/app/CONTRIBUTING.mdaround lines 11 - 21, Update the CONTRIBUTING.md so
the two command examples explicitly state they must be executed from the ui/app
directory: add a short note like "from ui/app" immediately above thenpm ci
example and themake testexample (the sections showing thenpm ciandmake testcommands) so contributors know the correct working directory when running
those commands.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 60-67: The workflow calls npm in the container (step id
cache-paths) before ensuring Node/npm exists; either ensure the container image
includes Node/npm or explicitly install Node/npm inside the job before the
cache-paths step so npm config get cache succeeds; update the job to add an
installation step (or switch the container image) that provides Node/npm, and
keep the actions/cache@v4 step and path (${{ steps.cache-paths.outputs.npm-cache
}}) after that installation so the cache lookup is reliable in the container.In
@Makefile:
- Around line 30-46: The ui Elm generated Data files are only a prerequisite of
the assets target, so build/test/lint that depend on the ui-elm target can run
without src/Data present; update the Makefile so the ui-elm target depends on
$(FRONTEND_DIR)/src/Data (i.e., change the ui-elm rule to "ui-elm:
$(FRONTEND_DIR)/src/Data" or add that prerequisite) so that invoking
build/test/lint will ensure the OpenAPI-generated Data modules exist before
running "cd$(FRONTEND_DIR) && $ (MAKE) build" (you can then remove the redundant
dependency from the assets rule if desired).In
@template/Makefile:
- Around line 4-6: The Makefile target email.tmpl lists a prerequisite
inline-css.js but the recipe calls node ./inline-css (missing the .js
extension); update the recipe for the email.tmpl target to explicitly run the
same file named in the prerequisite (use node ./inline-css.js) or alternatively
change the prerequisite to match the executed script name—ensure the symbols
inline-css.js (prerequisite) and the recipe invocation (node ./inline-css.js)
are consistent.In
@ui/app/Makefile:
- Around line 41-44: The .build_stamp rule currently only depends on
$(ELM_FILES), index.html, and vite.config.mjs, so changes to the JS/CSS entry or
package config won't trigger rebuilds; update the .build_stamp dependency list
to include package.json, src/main.js (or your actual JS entry), and
src/assets/elm-datepicker.css (or other asset files) so that changes to those
files cause npm run build to run and .build_stamp to be updated; modify the
Makefile rule for .build_stamp to add these filenames alongside $(ELM_FILES),
index.html, and vite.config.mjs.- Around line 48-54: The Makefile recipe uses DOCKER_RUN_CURRENT_USER to start a
container named openapi-gen but lacks failure handling, allowing subsequent
docker cp operations to silently copy from an old container; update the recipe
(the block that runs openapi-gen, trap, and docker cp) to abort on error by
enabling set -e (or chain commands with &&) immediately before the docker run,
or check the docker run exit status and only proceed if it succeeded, and ensure
the temporary container is removed on both success and failure (keep the trap
but only run docker cp when the run returned success) so openapi-gen failures
cannot cause stale-copy corruption.In
@ui/app/vite.config.mjs:
- Around line 5-19: The Vite config uses defineConfig(exported object) but lacks
a relative base, causing built asset URLs to be absolute; update the object
passed to defineConfig (the export default defineConfig call) to include base:
"./" at top level so assets are emitted as relative URLs and resolve correctly
when ui.Register mounts the app on a prefixed path; ensure this change is
applied alongside the existing plugins (elm(), compression(...)) in the
vite.config module.In
@ui/web_test.go:
- Around line 152-159: The helper checkCachingHeaders currently treats two
different Cache-Control policies as interchangeable and never asserts Vary, so
update checkCachingHeaders to accept an explicit expectedCacheControl string and
a wantVary bool, assert that res.Header().Get("Cache-Control") equals
expectedCacheControl (not Contains), and when wantVary is true assert
res.Header().Get("Vary") == "Accept-Encoding" (or empty when false); then update
callers (tests that exercise "/", "/favicon.ico", and "/assets/*") to pass
"no-cache, no-store, must-revalidate" for root/favicon, "public,
max-age=31536000, immutable" for assets, and set wantVary appropriately based on
whether the response is compressed.In
@ui/web.go:
- Around line 185-187: The handler for r.Get("/assets/*path") sets a long-lived
Cache-Control header before calling serve, which causes 404/500 responses to be
cached; change the flow so the Cache-Control header is only added for successful
asset responses—either move the header assignment into serve and set it only
when the file is found and served (i.e., when you would write a 200 OK), or have
the handler check existence (using the same http.FileSystem/Open or os.Stat
logic serve uses) and only set Cache-Control after confirming the file exists;
ensure the 404 and error paths (the logic in serve that writes 404/500) do not
set the immutable max-age header.In
@weboperations/weboperations.go:
- Around line 31-38: The POST /-/reload handler can hang; change the local reply
channel to a buffered channel (errc := make(chan error, 1)), remove the defer
close(errc) (handler must not close a channel it doesn't own), and replace the
direct send and receive with context-aware selects that also listen on
req.Context().Done(); specifically, use select { case reloadCh <- errc: ... case
<-req.Context().Done(): return } for queuing the reload and another select {
case err := <-errc: ... case <-req.Context().Done(): return } for waiting for
the reply so both enqueue and reply are cancellation-safe.
Nitpick comments:
In@ui/app/CONTRIBUTING.md:
- Around line 11-21: Update the CONTRIBUTING.md so the two command examples
explicitly state they must be executed from the ui/app directory: add a short
note like "from ui/app" immediately above thenpm ciexample and themake testexample (the sections showing thenpm ciandmake testcommands) so
contributors know the correct working directory when running those commands.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `77ceba66-ab1f-485f-8bf8-870d44f0e778` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5f3188d4ef00ec05a1f75b8f1b0ddbbd7730ead6 and 92c42fbea6a5c61cd019546728e31a98e00169bb. </details> <details> <summary>⛔ Files ignored due to path filters (10)</summary> * `template/package-lock.json` is excluded by `!**/package-lock.json` * `ui/app/lib/bootstrap-4.6.2-dist/css/bootstrap.min.css.map` is excluded by `!**/*.map` * `ui/app/lib/font-awesome-4.7.0/fonts/FontAwesome.otf` is excluded by `!**/*.otf` * `ui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.eot` is excluded by `!**/*.eot` * `ui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.svg` is excluded by `!**/*.svg` * `ui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.ttf` is excluded by `!**/*.ttf` * `ui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woff` is excluded by `!**/*.woff` * `ui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woff2` is excluded by `!**/*.woff2` * `ui/app/package-lock.json` is excluded by `!**/package-lock.json` * `ui/app/public/favicon.ico` is excluded by `!**/*.ico` </details> <details> <summary>📒 Files selected for processing (31)</summary> * `.github/workflows/ci.yml` * `.github/workflows/ui-ci.yml` * `.nvmrc` * `Makefile` * `README.md` * `cmd/alertmanager/main.go` * `template/.gitignore` * `template/Dockerfile` * `template/Makefile` * `template/package.json` * `ui/Dockerfile` * `ui/app/.gitignore` * `ui/app/CONTRIBUTING.md` * `ui/app/Makefile` * `ui/app/index.html` * `ui/app/lib/bootstrap-4.6.2-dist/css/bootstrap.min.css` * `ui/app/lib/font-awesome-4.7.0/css/font-awesome.css` * `ui/app/lib/font-awesome-4.7.0/css/font-awesome.min.css` * `ui/app/package.json` * `ui/app/script.js` * `ui/app/src/Main.elm` * `ui/app/src/Types.elm` * `ui/app/src/Updates.elm` * `ui/app/src/Views.elm` * `ui/app/src/assets/elm-datepicker.css` * `ui/app/src/main.js` * `ui/app/vite.config.mjs` * `ui/web.go` * `ui/web_test.go` * `weboperations/weboperations.go` * `weboperations/weboperations_test.go` </details> <details> <summary>💤 Files with no reviewable changes (7)</summary> * ui/app/src/Main.elm * ui/app/src/Updates.elm * ui/Dockerfile * template/Dockerfile * ui/app/lib/font-awesome-4.7.0/css/font-awesome.min.css * ui/app/src/Types.elm * ui/app/lib/font-awesome-4.7.0/css/font-awesome.css </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ui/app/Makefile (1)
12-12: Add PHONY declarations forallandtesttargets.The static analysis tool correctly flags that
all(Line 12) andtest(Line 24) should be declared as.PHONYtargets since they don't produce files with those names. This prevents unexpected behavior if files namedallortestare ever created.Suggested fix
-.PHONY: build +.PHONY: all build test build: .build_stampAlso applies to: 38-39, 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/Makefile` at line 12, Add a .PHONY declaration for the make targets that do not produce files (at minimum include all and test) so make won’t treat those names as real files; update the Makefile to add a line like “.PHONY: all test” (and include any other non-file-producing targets referenced elsewhere in the file corresponding to the 38-39 and 58-58 occurrences) placed near the top with other meta-targets.ui/web.go (1)
139-139: Type assertionf.(io.ReadSeeker)may panic if the embedded FS changes.While
embed.FSfiles currently implementio.ReadSeeker, this is an unchecked type assertion. A defensive check would prevent a runtime panic if Go's embed implementation changes in the future.Defensive type assertion example
-http.ServeContent(w, req, filePath, time.Time{}, f.(io.ReadSeeker)) +rs, ok := f.(io.ReadSeeker) +if !ok { + http.Error(w, "internal error: file not seekable", http.StatusInternalServerError) + return +} +http.ServeContent(w, req, filePath, time.Time{}, rs)Also applies to: 149-149, 172-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/web.go` at line 139, The direct type assertion f.(io.ReadSeeker) can panic; change to a safe check and fallback: attempt a type assertion (rs, ok := f.(io.ReadSeeker)) and call http.ServeContent with rs if ok, otherwise read the file into memory (io.ReadAll(f)), wrap the bytes with bytes.NewReader(...) (which implements io.ReadSeeker) and pass that to http.ServeContent; ensure f is closed after open/read. Apply the same defensive pattern to the other occurrences referenced (the other http.ServeContent calls).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ui/app/Makefile`:
- Line 12: Add a .PHONY declaration for the make targets that do not produce
files (at minimum include all and test) so make won’t treat those names as real
files; update the Makefile to add a line like “.PHONY: all test” (and include
any other non-file-producing targets referenced elsewhere in the file
corresponding to the 38-39 and 58-58 occurrences) placed near the top with other
meta-targets.
In `@ui/web.go`:
- Line 139: The direct type assertion f.(io.ReadSeeker) can panic; change to a
safe check and fallback: attempt a type assertion (rs, ok := f.(io.ReadSeeker))
and call http.ServeContent with rs if ok, otherwise read the file into memory
(io.ReadAll(f)), wrap the bytes with bytes.NewReader(...) (which implements
io.ReadSeeker) and pass that to http.ServeContent; ensure f is closed after
open/read. Apply the same defensive pattern to the other occurrences referenced
(the other http.ServeContent calls).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ecf2950-c3cf-4bf6-8ed8-3160292886d6
⛔ Files ignored due to path filters (9)
ui/app/lib/bootstrap-4.6.2-dist/css/bootstrap.min.css.mapis excluded by!**/*.mapui/app/lib/font-awesome-4.7.0/fonts/FontAwesome.otfis excluded by!**/*.otfui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.eotis excluded by!**/*.eotui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.svgis excluded by!**/*.svgui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.ttfis excluded by!**/*.ttfui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woffis excluded by!**/*.woffui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woff2is excluded by!**/*.woff2ui/app/package-lock.jsonis excluded by!**/package-lock.jsonui/app/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (25)
.github/workflows/ci.yml.github/workflows/ui-ci.yml.nvmrcMakefileREADME.mdcmd/alertmanager/main.goui/app/.gitignoreui/app/Makefileui/app/index.htmlui/app/lib/bootstrap-4.6.2-dist/css/bootstrap.min.cssui/app/lib/font-awesome-4.7.0/css/font-awesome.cssui/app/lib/font-awesome-4.7.0/css/font-awesome.min.cssui/app/package.jsonui/app/script.jsui/app/src/Main.elmui/app/src/Types.elmui/app/src/Updates.elmui/app/src/Views.elmui/app/src/assets/elm-datepicker.cssui/app/src/main.jsui/app/vite.config.mjsui/web.goui/web_test.goweboperations/weboperations.goweboperations/weboperations_test.go
💤 Files with no reviewable changes (5)
- ui/app/src/Updates.elm
- ui/app/src/Main.elm
- ui/app/src/Types.elm
- ui/app/lib/font-awesome-4.7.0/css/font-awesome.min.css
- ui/app/lib/font-awesome-4.7.0/css/font-awesome.css
✅ Files skipped from review due to trivial changes (3)
- ui/app/.gitignore
- ui/app/package.json
- weboperations/weboperations_test.go
🚧 Files skipped from review as they are similar to previous changes (8)
- .github/workflows/ui-ci.yml
- cmd/alertmanager/main.go
- README.md
- ui/app/vite.config.mjs
- .github/workflows/ci.yml
- ui/app/src/main.js
- ui/web_test.go
- Makefile
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
ui/web_test.go (1)
85-123: AddContent-Typeassertions while you're already walking every embedded file.This suite is the safety net for the new hard-coded
fileTypesmap, but it only checks status, caching, and content encoding. A MIME typo inui/web.gowould still return 200 here and slip through.Also applies to: 174-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/web_test.go` around lines 85 - 123, walkEmbeddedFiles currently builds expected encodings per URL but doesn't assert Content-Type; update it (and the test loop that uses its result) to include and check the expected MIME for each file: while iterating fileTypes (via fileTypes[ext] and fileType.varyEncoding) capture fileType.mime and either return it alongside the encodings from walkEmbeddedFiles or assert it immediately; then in the request assertions (the test block that iterates the returned URLs/encodings around lines 174-215) verify the response header "Content-Type" equals fileType.mime for each url/encoding (use the same fileTypes lookup by extension to derive expected MIME). Ensure references to encGzip, encBrotli, encNone remain unchanged.
🤖 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 39-46: The assets target currently lists the phony ui-elm
prerequisite first which runs the Vite build against checked-in ui/app/src/Data
before $(FRONTEND_DIR)/src/Data is regenerated; change the prerequisite order so
the generated artifact $(FRONTEND_DIR)/src/Data (with its dependency on
api/v2/openapi.yaml) is listed before ui-elm (and keep template/email.tmpl),
ensuring Make updates src/Data first and only then runs the phony ui-elm build;
update the assets rule to depend on $(FRONTEND_DIR)/src/Data before ui-elm and
leave the existing $(FRONTEND_DIR)/src/Data: rule intact.
In `@ui/app/Makefile`:
- Around line 35-36: The Makefile target dev-server currently runs Elm Reactor
which cannot handle bundler-dependent imports in main.js; update the dev-server
recipe to start Vite instead (e.g., run the project's Vite dev command such as
"npx vite" or the equivalent npm/pnpm/yarn script like "npm run dev") so CSS and
npm package imports and the Elm build are served by the bundler; replace the
"npx elm reactor" invocation in the dev-server rule with the Vite start command
and ensure any package manager script name used matches the project's
package.json.
In `@ui/web.go`:
- Around line 64-96: selectEncoding currently conflates "no encoding (identity)
allowed" with "identity explicitly rejected"; add tracking for identity by
introducing an identity variable (initialized to effectUnseen) like
brotli/gzip/wildcard and set it when encountering "identity" (or
"identity;q=..."). At the end, if identity==effectReject or
(wildcard==effectReject && identity==effectUnseen) return a distinct sentinel
(e.g., encNotAcceptable or encRejectIdentity) instead of encNone; update callers
that use selectEncoding (and the encoding type) to handle this new sentinel and
respond with a 406 or similar. Ensure you reference selectEncoding, encoding,
encNone, brotli/gzip/wildcard, and the new identity and encNotAcceptable symbols
in your changes.
- Around line 123-127: The handler currently treats unknown file extensions by
returning http.Error with StatusInternalServerError; change this so that when
the extension lookup (ext := strings.ToLower(path.Ext(filePath)); fileType, ok
:= fileTypes[ext]) fails (ok == false) you call http.NotFound(w, r) and return
instead of sending an internal server error, so unknown/unsupported asset
requests (e.g. /assets/foo.txt) fall through as a 404 like other missing assets;
locate this logic around the ext/fileTypes check in the request handler (the
block using ext, fileTypes and http.Error) and replace the error response with
http.NotFound.
- Around line 30-31: The embed directive "//go:embed app/dist" referenced by the
variable "asset embed.FS" points to a non-existent folder and breaks
compilation; fix it by either committing a placeholder assets directory with the
expected files (so the embed pattern has matches) or change the embed directive
to reference an existing path/pattern that is present in the repository (or gate
the embed behind a build tag and provide a developer-friendly stub) so that the
variable "asset" is only built when matching files exist.
---
Nitpick comments:
In `@ui/web_test.go`:
- Around line 85-123: walkEmbeddedFiles currently builds expected encodings per
URL but doesn't assert Content-Type; update it (and the test loop that uses its
result) to include and check the expected MIME for each file: while iterating
fileTypes (via fileTypes[ext] and fileType.varyEncoding) capture fileType.mime
and either return it alongside the encodings from walkEmbeddedFiles or assert it
immediately; then in the request assertions (the test block that iterates the
returned URLs/encodings around lines 174-215) verify the response header
"Content-Type" equals fileType.mime for each url/encoding (use the same
fileTypes lookup by extension to derive expected MIME). Ensure references to
encGzip, encBrotli, encNone remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e22df37d-5a23-43f7-a16c-5a7367be2bdc
⛔ Files ignored due to path filters (9)
ui/app/lib/bootstrap-4.6.2-dist/css/bootstrap.min.css.mapis excluded by!**/*.mapui/app/lib/font-awesome-4.7.0/fonts/FontAwesome.otfis excluded by!**/*.otfui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.eotis excluded by!**/*.eotui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.svgis excluded by!**/*.svgui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.ttfis excluded by!**/*.ttfui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woffis excluded by!**/*.woffui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woff2is excluded by!**/*.woff2ui/app/package-lock.jsonis excluded by!**/package-lock.jsonui/app/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (19)
MakefileREADME.mdui/app/.gitignoreui/app/Makefileui/app/index.htmlui/app/lib/bootstrap-4.6.2-dist/css/bootstrap.min.cssui/app/lib/font-awesome-4.7.0/css/font-awesome.cssui/app/lib/font-awesome-4.7.0/css/font-awesome.min.cssui/app/package.jsonui/app/script.jsui/app/src/Main.elmui/app/src/Types.elmui/app/src/Updates.elmui/app/src/Views.elmui/app/src/assets/elm-datepicker.cssui/app/src/main.jsui/app/vite.config.mjsui/web.goui/web_test.go
💤 Files with no reviewable changes (5)
- ui/app/src/Main.elm
- ui/app/src/Types.elm
- ui/app/lib/font-awesome-4.7.0/css/font-awesome.min.css
- ui/app/lib/font-awesome-4.7.0/css/font-awesome.css
- ui/app/src/Updates.elm
✅ Files skipped from review due to trivial changes (3)
- ui/app/.gitignore
- ui/app/package.json
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/app/vite.config.mjs
- ui/app/src/main.js
SoloJacobs
added
waiting-for-user-input
There are a few advantages to this setup: * Dependabot can now help with keeping versions recent. * The setup is more idiomatic and simpler. In particular, our build steps are more deterministic due to the inclusion of `package-lock.json`. * We have to get rid of the Docker based approach for the mantime ui, so this change makes things more uniform across the repo. * The command `promu crossbuild` does not support running Docker. With this change, we could delete `email.tmpl` from our repo, and it would not affect our build process in any way. The disadvantage is that this setup requires a local toolchain (stored in `template/node_modules`) to update `email.tmpl`. Why does this PR not delete `email.tmpl`? This file is small, and `go` developers do not need to install any `node` tools to run. ```sh go get github.com/prometheus/alertmanager/template ``` Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
There are a few advantages to this setup: * Dependabot can now help with keeping versions recent. * The setup is more idiomatic and simpler. In particular, our build steps are more deterministic due to the inclusion of `package-lock.json`. * We have to get rid of the Docker based approach for the mantime ui, so this change makes things more uniform across the repo. * The command `promu crossbuild` does not support running Docker. With this change, we could delete `ui/app/script.js` from our repo, and it would not affect our build process in any way. * This change should help users, which are building `podman` with `Alertmanager`. The disadvantage is that this setup requires a local toolchain (stored in `ui/app/node_modules`) to update `ui/app/script.js`. The file `ui/app/script.js` will be removed from the repo in a future commit. This will break the following invocation: ```sh go get github.com/prometheus/alertmanager/ui ``` The dependency `openapitools/openapi-generator-cli` could not be removed, since it depends on `Java`. Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
This (together with moving to `npm ci`) should hopefully get rid of this sporadic failure we have: ``` Errors occurred while compiling your configuration for elm-review. I need your configuration to compile in order to know how to analyze your files. Hopefully the compiler error below will help you figure out how to fix it. make: *** [Makefile:32: test] Error 1 ``` At the very least, this makes as better citizens of the internet. Note: `nvm` picks up its configuration, even if running from a subdirectory. Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
In the future, we want to remove the ui build artefacts from the repository. This will break the usages of ```sh go get github.com/prometheus/alertmanager/ui ``` That is exactly as expected, since the ui can't function correctly without the ui artefacts. This change attempts to make the transition a bit less painful. Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
Previously, bootstrap and font-awesome were manually vendored under
`ui/app/lib/`. They are now fetched via `npm`. Vite bundles these two
and the elm-datepicker at build time into a single file.
URL changes:
- /script.js → /assets/index-[hash].js (content-hashed)
- /lib/bootstrap-*/css/bootstrap.min.css{,.map} → merged into /assets/index-[hash].css
- /lib/elm-datepicker/css/elm-datepicker.css → merged into /assets/index-[hash].css
- /lib/font-awesome-*/css/font-awesome{,.min}.css → merged into /assets/index-[hash].css
- /lib/font-awesome-*/fonts/{woff,woff2,ttf,eot,svg} → /assets/fontawesome-webfont-[hash].{ext}
FontAwesome.otf was removed entirely, this is not a web font.
Files under /assets/ are served with "Cache-Control: public,
max-age=31536000, immutable" (content-hashed filenames make them safe to
cache forever).
In a previous PR (prometheus#5028), `Alertmanager` stopped sending "Last-Modified:
Thu, 01 Jan 1970 00:00:01 GMT". I introduced this behaviour by accident,
but consider this correct behaviour. The findings are documented via a
test.
This change removes various build artifacts, most importantly
`ui/app/script.js`. The following command no longer works in isolation:
```sh
go get github.com/prometheus/alertmanager/ui
```
Users now must build `ui/app/dist` from source.
The MIME types are now hard-coded, removing the dependency on the OS
MIME database. In practice, this would only affect `FontAwesome.otf`,
which we no longer serve anyway.
Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/Makefile`:
- Line 12: The Makefile's phony targets are missing: declare the targets all and
test as .PHONY to prevent accidental file name collisions (add a .PHONY: all
test line); also consider consolidating with the existing .PHONY declaration for
build (reference the targets "all", "test", and "build") so all phony targets
are declared together for maintainability.
In `@ui/app/src/main.js`:
- Around line 10-12: Wrap JSON.parse calls for the persisted flags to prevent
boot-time SyntaxError: replace direct
JSON.parse(localStorage.getItem('firstDayOfWeek')) and
JSON.parse(localStorage.getItem('groupExpandAll')) with a safe parse (either a
small try/catch around each parse or a shared safeParse helper) that returns a
sensible default (e.g., null/false) when parsing fails or value is null; keep
defaultCreator as-is (localStorage.getItem('defaultCreator')) and ensure the
code uses the fallback values when storage is invalid.
In `@ui/web_test.go`:
- Around line 71-77: The test loop is fetching the index URL (`urlPath`) instead
of each asset (`assetPath`), so change the call in the t.Run body to use
assetPath with fetchWithEncoding(router, assetPath, encNone) and ensure the
goroutine captures the correct iteration value (e.g., keep the existing
assetPath := path.Join(...) or rebind inside the loop before t.Run) so each
subtest actually verifies the individual asset.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5df670c-37fc-47fe-bcdd-567e76850ff2
⛔ Files ignored due to path filters (10)
template/package-lock.jsonis excluded by!**/package-lock.jsonui/app/lib/bootstrap-4.6.2-dist/css/bootstrap.min.css.mapis excluded by!**/*.mapui/app/lib/font-awesome-4.7.0/fonts/FontAwesome.otfis excluded by!**/*.otfui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.eotis excluded by!**/*.eotui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.svgis excluded by!**/*.svgui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.ttfis excluded by!**/*.ttfui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woffis excluded by!**/*.woffui/app/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woff2is excluded by!**/*.woff2ui/app/package-lock.jsonis excluded by!**/package-lock.jsonui/app/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (31)
.github/workflows/ci.yml.github/workflows/ui-ci.yml.nvmrcMakefileREADME.mdcmd/alertmanager/main.gotemplate/.gitignoretemplate/Dockerfiletemplate/Makefiletemplate/package.jsonui/Dockerfileui/app/.gitignoreui/app/CONTRIBUTING.mdui/app/Makefileui/app/index.htmlui/app/lib/bootstrap-4.6.2-dist/css/bootstrap.min.cssui/app/lib/font-awesome-4.7.0/css/font-awesome.cssui/app/lib/font-awesome-4.7.0/css/font-awesome.min.cssui/app/package.jsonui/app/script.jsui/app/src/Main.elmui/app/src/Types.elmui/app/src/Updates.elmui/app/src/Views.elmui/app/src/assets/elm-datepicker.cssui/app/src/main.jsui/app/vite.config.mjsui/web.goui/web_test.goweboperations/weboperations.goweboperations/weboperations_test.go
💤 Files with no reviewable changes (7)
- template/Dockerfile
- ui/app/src/Updates.elm
- ui/app/src/Main.elm
- ui/Dockerfile
- ui/app/src/Types.elm
- ui/app/lib/font-awesome-4.7.0/css/font-awesome.css
- ui/app/lib/font-awesome-4.7.0/css/font-awesome.min.css
✅ Files skipped from review due to trivial changes (5)
- template/.gitignore
- ui/app/.gitignore
- ui/app/CONTRIBUTING.md
- template/package.json
- ui/app/package.json
🚧 Files skipped from review as they are similar to previous changes (8)
- cmd/alertmanager/main.go
- .github/workflows/ci.yml
- ui/app/vite.config.mjs
- weboperations/weboperations.go
- ui/app/index.html
- .github/workflows/ui-ci.yml
- README.md
- Makefile
Content-Encoding now varies per request based on Accept-Encoding. Assets are stored only in compressed form (.gz level 9, .br quality 11), which decreases the binary size from ~42.8 MB (v0.31.1) to ~39.7 MB. Clients that accept neither gzip nor brotli receive a response decompressed in-memory from the .gz variant on every request. Adds vite-plugin-compression2 as a build dependency. Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
The GOPATH layout was only necessary before Go modules were introduced in Go 1.11. Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/web.go (1)
187-188: Consider explicit path validation for defense-in-depth clarity.Lines 187–188 construct the file path via
path.Join("assets", route.Param(...)). Whilepath.Joinwill collapse../segments (e.g.,../index.html→index.html), the subsequentappFS.Open()call on line 139/145/151/157 is protected byfs.Sub(asset, "app/dist")on line 118, which prevents directory escape. However, this protection is implicit and fragile; if the code were refactored away fromfs.Sub, the traversal path becomes exploitable.Adding explicit validation at the route handler boundary (checking for
..or usingfs.ValidPath()) would make the safety explicit and maintainable:Optional improvement
r.Get("/assets/*path", func(w http.ResponseWriter, req *http.Request) { + assetPath := route.Param(req.Context(), "path") + if !fs.ValidPath(assetPath) { + http.NotFound(w, req) + return + } - serve(w, req, path.Join("assets", route.Param(req.Context(), "path")), true) + serve(w, req, path.Join("assets", assetPath), true) })A test case for
/assets/../index.htmlwould also prevent regression.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/web.go` around lines 187 - 188, The handler building asset paths uses path.Join("assets", route.Param(req.Context(), "path")) (in the route GET "/assets/*path" and serve function) but relies implicitly on fs.Sub/appFS.Open for safety; add explicit validation at the route boundary by rejecting path parameters containing ".." or using fs.ValidPath() before calling serve, returning a 400/403 for invalid values to ensure defense-in-depth; update the serve invocation to only receive validated, cleanedPath and add a unit/integration test that requests "/assets/../index.html" to assert the server rejects it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/web_test.go`:
- Around line 62-75: fetchIndexAndAssets builds asset URLs with path.Join which
breaks for absolute asset references; replace that logic by parsing the base URL
(url.Parse) from urlPath and resolving each asset reference with
base.ResolveReference(url.Parse(match[1])) so absolute and relative asset
href/src values are handled correctly. Update fetchIndexAndAssets to build
assetPath using url.Parse + ResolveReference, ensure error handling for Parse
failures, and add the necessary import for net/url; keep the existing
fetchWithEncoding calls and test assertions unchanged.
---
Nitpick comments:
In `@ui/web.go`:
- Around line 187-188: The handler building asset paths uses path.Join("assets",
route.Param(req.Context(), "path")) (in the route GET "/assets/*path" and serve
function) but relies implicitly on fs.Sub/appFS.Open for safety; add explicit
validation at the route boundary by rejecting path parameters containing ".." or
using fs.ValidPath() before calling serve, returning a 400/403 for invalid
values to ensure defense-in-depth; update the serve invocation to only receive
validated, cleanedPath and add a unit/integration test that requests
"/assets/../index.html" to assert the server rejects it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3376ef2-c892-486f-a3df-4268a7ab0687
⛔ Files ignored due to path filters (1)
ui/app/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
README.mdui/app/.gitignoreui/app/Makefileui/app/package.jsonui/app/vite.config.mjsui/web.goui/web_test.go
✅ Files skipped from review due to trivial changes (3)
- ui/app/.gitignore
- ui/app/vite.config.mjs
- ui/app/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/app/Makefile
Comment on lines
+62
to
+75
| func fetchIndexAndAssets(t *testing.T, router *route.Router, urlPath string) { | ||
| t.Helper() | ||
| res := fetchWithEncoding(router, urlPath, encNone) | ||
| require.Equal(t, http.StatusOK, res.Code) | ||
|
|
||
| require.Equal(t, http.StatusOK, w.Code) | ||
| require.Contains(t, w.Body.String(), "/debug/pprof/", "pprof page did not load with expected content when using a route prefix") | ||
| re := regexp.MustCompile(`(?:src|href)="([^"]+)"`) | ||
| matches := re.FindAllStringSubmatch(res.Body.String(), -1) | ||
| require.NotEmpty(t, matches, "No assets (src/href) found in index.html. Is the build empty?") | ||
|
|
||
| // Test GET request to pprof heap endpoint | ||
| req = httptest.NewRequest("GET", routePrefix+"/debug/pprof/heap", nil) | ||
| w = httptest.NewRecorder() | ||
| router.ServeHTTP(w, req) | ||
| for _, match := range matches { | ||
| assetPath := path.Join(urlPath, match[1]) | ||
| t.Run(assetPath, func(t *testing.T) { | ||
| res := fetchWithEncoding(router, assetPath, encNone) | ||
| require.Equal(t, http.StatusOK, res.Code) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine ui/web_test.go
find . -name "web_test.go" -path "*/ui/*" 2>/dev/null | head -5Repository: prometheus/alertmanager
Length of output: 83
🏁 Script executed:
# Get the file size to see if we can read it entirely
wc -l ui/web_test.go 2>/dev/null || echo "File not found or error"Repository: prometheus/alertmanager
Length of output: 85
🏁 Script executed:
# Read the specific section mentioned in the review (lines 55-100 for context)
sed -n '55,100p' ui/web_test.go 2>/dev/nullRepository: prometheus/alertmanager
Length of output: 1432
🏁 Script executed:
# Also check the imports at the top of the file
head -20 ui/web_test.goRepository: prometheus/alertmanager
Length of output: 725
🏁 Script executed:
# Check what assets/references are actually in the HTML
# First, let me look at ui/web.go to see how the HTML is generated
head -50 ui/web.goRepository: prometheus/alertmanager
Length of output: 1439
🏁 Script executed:
# Check if there's HTML template content that shows what href/src are generated
grep -n "href\|src=" ui/web.go | head -20Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Let me search for the HTML template more thoroughly
grep -r "assets\|app\.js" ui/ --include="*.go" | grep -E "(href|src|template)" | head -20Repository: prometheus/alertmanager
Length of output: 176
🏁 Script executed:
# Let's understand the structure - check if there's a template file
find ui -type f \( -name "*.html" -o -name "*.tmpl" \)Repository: prometheus/alertmanager
Length of output: 109
🏁 Script executed:
# Check what path.Join actually does with the scenario described
python3 << 'EOF'
import os.path
# Simulate the behavior
urlPath = "/alertmanager/"
assetRef = "/assets/app.js"
# What does path.Join do? In Go, path.Join has different behavior from os.path.join
# Let's test the Go semantics
# path.Join "/alertmanager/" and "/assets/app.js"
# According to Go docs, absolute paths override
# In Go's path.Join:
# - Leading slashes make a path absolute
# - path.Join("/a", "/b") returns "/b" (second absolute path wins)
# - But in this test, urlPath has trailing slash
result = os.path.join("/alertmanager/", "/assets/app.js")
print(f"os.path.join result: {result}")
# Go's path.Join behavior with trailing slash
# path.Join("/alertmanager/", "/assets/app.js") removes trailing slash first
# Then: path.Join("/alertmanager", "/assets/app.js") returns "/assets/app.js"
# Because "/assets/app.js" is absolute, it resets the path
print(f"Expected Go path.Join behavior: /assets/app.js")
print(f"This means the test tries to fetch /assets/app.js, not /alertmanager/assets/app.js")
EOFRepository: prometheus/alertmanager
Length of output: 232
🏁 Script executed:
# Now let's check if the HTML actually contains absolute paths or relative paths
# by looking at any build output or checking how assets are referenced
grep -r "href=\|src=" ui/ --include="*.go" -B2 -A2 | head -40Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Check the HTML files to see what asset references are in them
cat ui/app/index.htmlRepository: prometheus/alertmanager
Length of output: 471
🏁 Script executed:
# Also check the other one
cat ui/mantine-ui/index.htmlRepository: prometheus/alertmanager
Length of output: 483
🏁 Script executed:
# Let me verify Go's path.Join semantics more explicitly with a test
python3 << 'EOF'
# Go's path.Join behavior according to the documentation:
# - It returns the shortest path name equivalent to path by purely lexical processing.
# - It eliminates . and .. elements
# - Leading slashes in subsequent arguments cause the path to be reset
# Let's trace through the exact scenario:
# urlPath = "/alertmanager/"
# match[1] = "/assets/app.js"
# Go path.Join("/alertmanager/", "/assets/app.js")
# Step 1: Clean up both arguments
# Clean("/alertmanager/") -> "/alertmanager"
# "/assets/app.js" starts with "/" so it's treated as absolute
# Result: Since the second argument starts with "/" (is absolute),
# it replaces the entire path, returning "/assets/app.js"
# So the test would try to fetch "/assets/app.js", not "/alertmanager/assets/app.js"
print("Go path.Join semantics:")
print(' path.Join("/alertmanager/", "/assets/app.js") -> "/assets/app.js"')
print("")
print("This means:")
print(" - With "/" route prefix, test fetches /assets/app.js (correct)")
print(" - With /alertmanager/ route prefix, test ALSO fetches /assets/app.js (WRONG!)")
print("")
print("The reviewer's concern might be about a DIFFERENT scenario:")
print(" - If the HTML had relative paths like href=\"assets/app.js\"")
print(" - Then path.Join(\"/alertmanager/\", \"assets/app.js\") -> \"/alertmanager/assets/app.js\"")
print(" - Which would be correct for the prefix case")
print("")
print("The real issue depends on what the HTML actually contains.")
EOFRepository: prometheus/alertmanager
Length of output: 324
🏁 Script executed:
# Let's check if the app/dist directory has built HTML or if it's just source
# Find any index.html files in the embedded assets
find ui/app -name "index.html" -exec head -20 {} \;Repository: prometheus/alertmanager
Length of output: 471
🏁 Script executed:
# Check if there's a build process or vite config that might show asset paths
find . -name "vite.config.*" -o -name "package.json" | xargs ls -la 2>/dev/nullRepository: prometheus/alertmanager
Length of output: 443
🏁 Script executed:
# Let's look at what files are actually in app/dist
ls -la ui/app/dist/ 2>/dev/null || echo "app/dist not found (expected - build artifact)"Repository: prometheus/alertmanager
Length of output: 113
🏁 Script executed:
# Since it's embedded, let's check if there are any built artifacts in git
git ls-files | grep -E "app/dist|app/index" | head -10Repository: prometheus/alertmanager
Length of output: 84
🏁 Script executed:
# Check the vite configurations to understand how the build process handles asset paths
cat ui/app/vite.config.mjsRepository: prometheus/alertmanager
Length of output: 689
🏁 Script executed:
# Also check the other one
cat ui/mantine-ui/vite.config.mjsRepository: prometheus/alertmanager
Length of output: 469
🏁 Script executed:
# Now let's trace through what actually happens with the test
# The key question: what are the BUILT asset references in app/dist?
# Since we can't see app/dist, let's check git to see if it's committed
git log --oneline ui/app/dist/index.html 2>/dev/null | head -5 || echo "Not in git history"Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Check if app/dist is in gitignore
grep -r "app/dist\|dist/" .gitignore 2>/dev/nullRepository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Let's understand the test scenario more carefully by looking at how Register works
grep -n "func Register" ui/web.go -A 20Repository: prometheus/alertmanager
Length of output: 876
🏁 Script executed:
# Check how the router serves these files
grep -n "WithPrefix\|ServeHTTP\|Handler" ui/web.go -B 2 -A 5 | head -50Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Let's trace through exactly what would happen with the current code
# The test extracts "/src/main.js" from the HTML
# Then calls path.Join("/alertmanager/", "/src/main.js")
go run << 'EOF'
package main
import (
"fmt"
"path"
)
func main() {
urlPath := "/alertmanager/"
absPath := "/src/main.js"
result := path.Join(urlPath, absPath)
fmt.Printf("path.Join(%q, %q) = %q\n", urlPath, absPath, result)
// With relative path
relPath := "favicon.ico"
result2 := path.Join(urlPath, relPath)
fmt.Printf("path.Join(%q, %q) = %q\n", urlPath, relPath, result2)
}
EOFRepository: prometheus/alertmanager
Length of output: 89
🏁 Script executed:
# Let's check if the tests are currently passing
git log --oneline -10 -- ui/web_test.goRepository: prometheus/alertmanager
Length of output: 115
🏁 Script executed:
# Check if there's a CI configuration that runs these tests
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "web_test\|go test.*ui" 2>/dev/null | head -3Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Let's look at what the Router.WithPrefix actually does
# Check the route package
find . -path "*/prometheus/common/route*" -o -name "route.go" | head -5Repository: prometheus/alertmanager
Length of output: 86
🏁 Script executed:
# If route is external, let's check if it's in go.mod
grep -A 2 "prometheus/common" go.mod | head -5Repository: prometheus/alertmanager
Length of output: 188
🏁 Script executed:
# Check what actually gets built - look for any CI artifacts or build info
git log --all --oneline --grep="build\|ui\|dist" -- ui/app/dist 2>/dev/null | head -5Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Most importantly, let's verify the actual behavior
# by checking if ui/app/dist is committed to the repository
git ls-tree HEAD ui/app/dist 2>/dev/null | head -10Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# The key insight: with base: "./" in vite, what does it actually produce?
# Let's check if there's a Makefile or build script that shows how this builds
find . -name "Makefile" -o -name "makefile" | xargs grep -l "ui\|app" 2>/dev/nullRepository: prometheus/alertmanager
Length of output: 129
🏁 Script executed:
# Check the Makefile for UI build targets
grep -n "ui\|app/dist" Makefile | head -20Repository: prometheus/alertmanager
Length of output: 534
🏁 Script executed:
# Let's understand: does the test actually pass or fail currently?
# Check if there are any recent PRs or issues about this
git log --all --oneline --decorate | grep -i "test\|ui\|web\|asset" | head -10Repository: prometheus/alertmanager
Length of output: 835
🏁 Script executed:
# Most importantly, let's check if url.Parse approach would work correctly
python3 << 'PYEOF'
from urllib.parse import urljoin, urlparse
# Scenario 1: Current code with absolute paths
urlPath = "/alertmanager/"
assetRef = "/src/main.js"
# path.Join behavior in Go
# path.Join("/alertmanager/", "/src/main.js") -> "/src/main.js" (second absolute path wins)
go_path_join_result = "/src/main.js"
print(f"Current behavior: path.Join('{urlPath}', '{assetRef}') -> '{go_path_join_result}'")
print(f" This would try to fetch: {go_path_join_result}")
print()
# Scenario 2: URL resolution approach (proposed fix)
# Parse base URL
base = urlparse(urlPath)
base_with_scheme = f"http://localhost{urlPath}" # Add scheme for urljoin to work
# URL resolution
resolved = urljoin(base_with_scheme, assetRef)
print(f"URL resolution: urljoin('{base_with_scheme}', '{assetRef}') -> '{resolved}'")
# Extract path only
resolved_path = urlparse(resolved).path
print(f" Extracted path: {resolved_path}")
print()
# Scenario 3: With relative paths (if vite base: "./" works correctly)
assetRef_relative = "src/main.js"
go_path_join_relative = "/alertmanager/src/main.js" # path.Join behavior
print(f"If built as relative: path.Join('{urlPath}', '{assetRef_relative}') -> '{go_path_join_relative}'")
print(f" This would try to fetch: {go_path_join_relative}")
print()
print("ANALYSIS:")
print("- If vite builds with base:./ and converts /src/main.js to src/main.js -> current code works")
print("- If vite keeps absolute paths -> current code is broken, proposal would fix it")
PYEOFRepository: prometheus/alertmanager
Length of output: 660
🏁 Script executed:
# Check if there are any related issues or discussions in git history
git log --all --source --grep="route.*prefix\|prefix.*route" --oneline | head -10Repository: prometheus/alertmanager
Length of output: 598
🏁 Script executed:
# Let's check when the WithPrefix feature was added
git log --all --oneline -S "WithPrefix" -- ui/web_test.go | head -5Repository: prometheus/alertmanager
Length of output: 195
🏁 Script executed:
# Search for any recent discussion about this issue in comments
grep -r "route.prefix\|base.*href\|href.*prefix" . --include="*.go" --include="*.md" 2>/dev/null | head -10Repository: prometheus/alertmanager
Length of output: 1111
🏁 Script executed:
# Let's check the actual UI/Mantine Makefile to see how it builds
cat ui/app/MakefileRepository: prometheus/alertmanager
Length of output: 1801
🏁 Script executed:
# Also check mantine-ui
cat ui/mantine-ui/MakefileRepository: prometheus/alertmanager
Length of output: 121
🏁 Script executed:
# The critical question: does vite with base:./ actually produce relative paths?
# Let's check if there's any documentation or examples in the codebase
grep -r "base.*\./" ui/ --include="*.mjs" --include="*.js"Repository: prometheus/alertmanager
Length of output: 157
🏁 Script executed:
# Let's look at the recent pprof fix mentioned in git history
# as it seems related to route prefix issues
git show 5362823d --statRepository: prometheus/alertmanager
Length of output: 2410
🏁 Script executed:
# Check that specific commit to see what was broken with route prefixes
git show 5362823d | head -100Repository: prometheus/alertmanager
Length of output: 3825
🏁 Script executed:
# Let's trace the actual problem more carefully
# Check the route package being used - what does WithPrefix actually do?
# It's from prometheus/common
# Let's check how it modifies routes
grep -n "WithPrefix\|Prefix" ui/web_test.go -B 2 -A 5Repository: prometheus/alertmanager
Length of output: 418
🏁 Script executed:
# The critical question: with base: "./", does vite output relative or absolute paths?
# Let's check if there are any build artifacts or documentation about this
# Search for any comments or documentation about how assets are served
grep -r "base.*\./" . --include="*.mjs" --include="*.md" --include="*.go" 2>/dev/null | grep -v node_modulesRepository: prometheus/alertmanager
Length of output: 159
🏁 Script executed:
# Let's check what the register function actually does and how routes are matched
sed -n '115,175p' ui/web.goRepository: prometheus/alertmanager
Length of output: 2148
🏁 Script executed:
# Check if tests currently pass - look for test run configurations
find . -name ".github" -type d -exec cat {}'/workflows/*.yml' \; 2>/dev/null | grep -A 20 "web_test\|go test" | head -30Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Most importantly, let's understand exactly what the issue is by checking
# what would happen with the proposed fix
python3 << 'PYEOF'
from urllib.parse import urlparse, urljoin
# Test scenario from the review
urlPath = "/alertmanager/"
# Absolute asset reference (what's in the HTML)
absRef = "/src/main.js"
# Current approach with path.Join
print("=== CURRENT APPROACH ===")
print(f"path.Join('{urlPath}', '{absRef}') in Go -> '/src/main.js'")
print(f"Test would fetch: /src/main.js (WRONG!)")
print()
# Proposed approach with URL resolution
print("=== PROPOSED APPROACH ===")
# Parse base URL
baseURL = urlparse(urlPath)
# Create full URL for urljoin to work
fullBase = f"http://localhost{urlPath}"
resolved = urljoin(fullBase, absRef)
print(f"urljoin('{fullBase}', '{absRef}') -> {resolved}")
print(f"Extracted path: {urlparse(resolved).path}")
print()
# Relative asset reference (if vite uses base: "./" correctly)
relRef = "src/main.js"
print("=== IF VITE OUTPUTS RELATIVE PATHS ===")
print(f"path.Join('{urlPath}', '{relRef}') in Go -> '/alertmanager/src/main.js'")
print(f"Test would fetch: /alertmanager/src/main.js (CORRECT)")
print()
# With proposed approach and relative paths
resolved2 = urljoin(fullBase, relRef)
print(f"urljoin('{fullBase}', '{relRef}') -> {resolved2}")
print(f"Extracted path: {urlparse(resolved2).path}")
PYEOFRepository: prometheus/alertmanager
Length of output: 664
🏁 Script executed:
# The key: check if the route.Router's WithPrefix actually affects how paths are matched
# Let's search for the route package source if available locally, or check how it's used
# Check if there's any special handling for routes with prefixes
grep -n "Get\|Post\|Handler\|ServeHTTP" ui/web.go | head -20Repository: prometheus/alertmanager
Length of output: 350
🏁 Script executed:
# Let's verify if the test TestIndexAssetsAreServedPrefix would actually fail with absolute paths
# by checking how the route prefix interacts with the handler
sed -n '115,145p' ui/web.goRepository: prometheus/alertmanager
Length of output: 1132
🏁 Script executed:
# Check if the tests are currently passing in CI
git log --all --oneline | head -30Repository: prometheus/alertmanager
Length of output: 2239
🏁 Script executed:
# Let's check if there's any indication the test is working or broken
# by looking for test results or issues
grep -r "TestIndexAssetsAreServed" . --include="*.go" --include="*.md" 2>/dev/nullRepository: prometheus/alertmanager
Length of output: 287
🏁 Script executed:
# Let me check what Vite with base: "./" actually produces
# by looking at any documentation or build outputs
# Check if there's a GitHub Actions workflow that builds and tests this
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "ui/app\|frontend" 2>/dev/null | head -3Repository: prometheus/alertmanager
Length of output: 93
🏁 Script executed:
# Let's look at the full context of how handlers are registered
sed -n '179,220p' ui/web.goRepository: prometheus/alertmanager
Length of output: 786
🏁 Script executed:
# Now the critical check: does the route matching work with WithPrefix?
# If register is called with route.New().WithPrefix("/alertmanager"),
# then r.Get("/assets/*path") becomes r.Get("/alertmanager/assets/*path")
# Let me verify the test actually tries to fetch the prefixed path
sed -n '86,92p' ui/web_test.goRepository: prometheus/alertmanager
Length of output: 312
🏁 Script executed:
# Check the CI workflow to see if tests are passing
cat .github/workflows/ci.yml | grep -A 30 "test\|web"Repository: prometheus/alertmanager
Length of output: 2378
🏁 Script executed:
# Let me look for any build output or see if we can understand what Vite actually produces
# by checking if there are any clues in the codebase
# Search for any references to how the built assets should look
grep -r "href=\|src=" ui/app --include="*.mjs" --include="*.tsx" --include="*.ts" 2>/dev/null | head -10Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Check if vite's base option actually rewrites paths or just sets the base
# Let's look for any vite documentation or comments about this
grep -r "base.*relative\|relative.*path" . --include="*.mjs" --include="*.md" 2>/dev/nullRepository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# The crucial question: does Vite with base: "./" actually output relative paths
# in the built HTML, or does it leave absolute paths and rely on <base> tag?
# Check if there's a <base> tag handling anywhere
grep -r "<base\|base.*href" ui/ --include="*.html" --include="*.tsx" --include="*.ts" 2>/dev/nullRepository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Let me verify the concern by checking what route.Router actually does
# by looking at how URLs are matched when WithPrefix is used
# The key: when route.New().WithPrefix("/alertmanager") is used,
# does it match /alertmanager/* or just /* ?
# Let's trace through a test scenario manually
python3 << 'PYEOF'
# The test does:
# 1. router := route.New().WithPrefix("/alertmanager")
# 2. Register(router)
# 3. fetchIndexAndAssets(t, router, "/alertmanager/")
# In Register, it calls:
# r.Get("/", ...) -> becomes /alertmanager/ with prefix
# r.Get("/assets/*path", ...)->becomes /alertmanager/assets/* with prefix
# fetchIndexAndAssets:
# 1. fetches /alertmanager/ -> gets HTML with <script src="/src/main.js">
# 2. extracts /src/main.js
# 3. calls path.Join("/alertmanager/", "/src/main.js") -> /src/main.js
# 4. tries to fetch /src/main.js
# 5. BUT the handler is registered for /alertmanager/assets/*, not /src/*
# So the test SHOULD fail when trying to fetch /src/main.js
# because there's no handler for it!
# UNLESS: the HTML actually has relative paths
# e.g., <script src="src/main.js">
# Then: path.Join("/alertmanager/", "src/main.js") -> /alertmanager/src/main.js
# But there's no handler for /alertmanager/src/* either!
# UNLESS: vite's base: "./" somehow makes the paths work
# Or: the built app/dist has special structure
print("Analysis:")
print("1. Handlers registered: /alertmanager/, /alertmanager/favicon.ico, /alertmanager/assets/*")
print("2. HTML sources have absolute paths: /src/main.js, /src/favicon.svg")
print("3. path.Join('/alertmanager/', '/src/main.js') = /src/main.js")
print("4. Fetching /src/main.js should 404 because handler is at /alertmanager/assets/*")
print()
print("Conclusion: Test should fail UNLESS:")
print(" - HTML is in app/dist with RELATIVE paths (vite rewriting)")
print(" - OR there's special handling we haven't seen")
PYEOFRepository: prometheus/alertmanager
Length of output: 524
🏁 Script executed:
# Let me check the actual built content by looking at what gets embedded
# The key is app/dist - let's see if we can infer its structure
# Check if there's any doc about the build output
grep -r "app/dist\|assets\|src/main" . --include="*.md" --include="*.txt" 2>/dev/null | head -10Repository: prometheus/alertmanager
Length of output: 404
🏁 Script executed:
# Let me check the actual build output by looking at what npm build produces
# Check the package.json to see the build script
cat ui/app/package.jsonRepository: prometheus/alertmanager
Length of output: 492
🏁 Script executed:
# Also check mantine-ui to understand both apps
cat ui/mantine-ui/package.jsonRepository: prometheus/alertmanager
Length of output: 1931
🏁 Script executed:
# The key insight: if the tests are passing in CI, and they do
# route.New().WithPrefix("/alertmanager") + path.Join, then
# the built HTML must have paths that work with this combination
# Let me trace through what MUST be true for tests to pass:
python3 << 'PYEOF'
# For TestIndexAssetsAreServedPrefix to pass:
# 1. Fetch /alertmanager/ -> must get HTML with asset refs
# 2. Extract refs like "xyz"
# 3. path.Join("/alertmanager/", "xyz") -> some path
# 4. Fetch that path -> must return 200
# Handlers registered with WithPrefix("/alertmanager"):
# - /alertmanager/
# - /alertmanager/favicon.ico
# - /alertmanager/assets/*
# What must be in the HTML for the test to pass?
# - favicon.ico -> path.Join("/alertmanager/", "favicon.ico") = "/alertmanager/favicon.ico" ✓
# - /assets/something -> path.Join("/alertmanager/", "/assets/something") = "/assets/something" ✗
# - assets/something -> path.Join("/alertmanager/", "assets/something") = "/alertmanager/assets/something" ✓
print("For test to pass, HTML must contain RELATIVE paths like:")
print(" - favicon.ico")
print(" - assets/something")
print(" NOT absolute paths like:")
print(" - /favicon.ico (would become /favicon.ico, no handler)")
print(" - /assets/something (would become /assets/something, no handler)")
print()
print("Vite's base: './' likely converts absolute paths to relative during build")
print("This makes the test pass, which is correct!")
print()
print("The reviewer's concern might be:")
print(" - If vite configuration ever breaks and produces absolute paths")
print(" - The test might still pass (with wrong assets)")
print(" - OR if there are edge cases")
PYEOFRepository: prometheus/alertmanager
Length of output: 611
🏁 Script executed:
# Let's check if there are any recent issues or PRs about asset handling
git log --all --oneline --grep="asset\|path\|prefix" -- ui/web_test.go | head -10Repository: prometheus/alertmanager
Length of output: 208
🏁 Script executed:
# Check when web_test.go was created
git log --all --follow --oneline -- ui/web_test.go | tail -5Repository: prometheus/alertmanager
Length of output: 257
🏁 Script executed:
# Let's look at the commit that added these tests
git show c53fff68 --statRepository: prometheus/alertmanager
Length of output: 436
🏁 Script executed:
# Check the actual content of that commit
git show c53fff68Repository: prometheus/alertmanager
Length of output: 1606
🏁 Script executed:
# Now let's verify if the current test is actually working correctly
# by understanding what Vite with base: "./" actually does
# Create a small test to understand Vite's behavior
python3 << 'PYEOF'
# According to Vite documentation, when base is set to "./":
# - It's a relative base URL
# - Script/link references in HTML are rewritten to be relative to the base
# - So /src/main.js becomes just src/main.js in the built HTML
# - This makes the assets work correctly with route prefixes
# Verification scenario:
# 1. HTML source has: <script src="/src/main.js">
# 2. Vite builds with base: "./"
# 3. Built HTML should have: <script src="src/main.js"> (relative)
# 4. Test fetches /alertmanager/ -> gets HTML
# 5. Test extracts "src/main.js"
# 6. Test calls path.Join("/alertmanager/", "src/main.js") -> "/alertmanager/src/main.js"
# 7. Test fetches /alertmanager/src/main.js -> ???
# BUT handlers are registered for /alertmanager/assets/*, not /alertmanager/src/*
# So this still doesn't work!
# Unless... the built app/dist structure is different from the source
# OR the HTML is being generated differently
print("The mystery: what's actually in app/dist?")
print("Source HTML has /src/main.js")
print("But handlers are for /assets/*")
print("Vite must be doing something to map /src/ to /assets/")
PYEOFRepository: prometheus/alertmanager
Length of output: 222
🏁 Script executed:
# Let me check if there's any transformation from /src to /assets
grep -r "src.*assets\|assets.*src" ui/app --include="*.mjs" --include="*.tsx" --include="*.ts" 2>/dev/null | head -10Repository: prometheus/alertmanager
Length of output: 49
🏁 Script executed:
# Check if the /assets route actually works for serving app code
# by looking at how the register function maps paths
sed -n '187,195p' ui/web.goRepository: prometheus/alertmanager
Length of output: 385
🏁 Script executed:
# So the handler is: /assets/*path maps to path.Join("assets", route.Param(...))
# This means /alertmanager/assets/foo -> serves assets/foo from appFS
# But the HTML has /src/main.js, not /assets/main.js
# Unless Vite rewrites it differently...
# Let me check if there's index.html in app/dist after build
# Or if the files are served differently
git show c53fff68 | head -200Repository: prometheus/alertmanager
Length of output: 1606
🏁 Script executed:
# Wait, the test TestWebRoutes from c53fff68 calls Register with 3 parameters
# But the current Register function only takes 1 parameter!
# Let me check the actual current Register signature
grep -n "^func Register" ui/web.goRepository: prometheus/alertmanager
Length of output: 103
🏁 Script executed:
# Let me see the full Register function definition
sed -n '115,125p' ui/web.goRepository: prometheus/alertmanager
Length of output: 493
🏁 Script executed:
# It seems the file might have changed. Let me check the actual current state
# by looking at lines around the Register function
sed -n '110,220p' ui/web.goRepository: prometheus/alertmanager
Length of output: 2955
🏁 Script executed:
# There's a discrepancy. The test from c53fff68 calls Register with 3 args
# but I saw only 1 arg. Let me check the current file again carefully
head -150 ui/web.go | tail -50Repository: prometheus/alertmanager
Length of output: 1640
🏁 Script executed:
# Let me get a complete picture of the current ui/web.go around Register
wc -l ui/web.goRepository: prometheus/alertmanager
Length of output: 80
🏁 Script executed:
# The file is 200+ lines. Let me get the full Register function
grep -n "^func Register" ui/web.go -A 50Repository: prometheus/alertmanager
Length of output: 2029
🏁 Script executed:
# Let me check if mantine-ui has a vite.config
cat ui/mantine-ui/vite.config.mjs 2>/dev/null || echo "File not found"Repository: prometheus/alertmanager
Length of output: 469
🏁 Script executed:
# Check the current HEAD to see what UI is being used
git log --oneline -1Repository: prometheus/alertmanager
Length of output: 115
🏁 Script executed:
# The key question: does the current test actually pass?
# Let me check if there are any recent test failures or PRs about this
git log --all --oneline -- ui/web_test.go | head -15Repository: prometheus/alertmanager
Length of output: 257
🏁 Script executed:
# Let me verify the actual scenario by understanding app/dist structure
# Since we can't see the built artifacts, let me check if there's any documentation
grep -r "app/dist\|assets" . --include="*.md" | grep -i "structure\|path\|route" | head -10Repository: prometheus/alertmanager
Length of output: 137
🏁 Script executed:
# Most critical: let me check if the test is actually testing what the review claims
# by reading the full fetchIndexAndAssets function as currently implemented
sed -n '62,92p' ui/web_test.goRepository: prometheus/alertmanager
Length of output: 1063
🏁 Script executed:
# Check if mantine-ui or elm-ui app is the one being built
grep -r "app/dist" . --include="*.go" | head -5Repository: prometheus/alertmanager
Length of output: 333
🏁 Script executed:
# Check if there's any build step that moves/renames files from src to assets
grep -r "src.*assets\|mv.*src\|cp.*src" . --include="Makefile" --include="*.sh" 2>/dev/nullRepository: prometheus/alertmanager
Length of output: 237
🏁 Script executed:
# Most importantly: verify what's actually in the app/dist after building
# by checking if the test would actually pass with absolute paths
python3 << 'PYEOF'
# The critical insight:
# If HTML has: <script src="/src/main.js">
# And vite.config has base: "./", vite REWRITES it to: <script src="src/main.js">
# Then path.Join("/alertmanager/", "src/main.js") = "/alertmanager/src/main.js"
# But handlers serve /assets/*, not /src/*
# So either:
# 1. Built files are in dist/assets/, not dist/src/
# 2. Or there's a /src/* handler we haven't seen
# Let me check if the register function has more routes
print("Need to verify: are there handlers for /src/* routes?")
print("Or are all asset files in assets/ subdirectory of app/dist?")
PYEOFRepository: prometheus/alertmanager
Length of output: 180
🏁 Script executed:
# Check if there are other route handlers beyond what we've seen
grep "r.Get\|r.Post\|r.Delete\|r.Put" ui/web.goRepository: prometheus/alertmanager
Length of output: 334
🏁 Script executed:
# Final verification: check what the current test actually extracts from the HTML
# by checking if there are any absolute paths that would break
# Let me look at what vite outputs for mantine-ui which doesn't have base: "./"
python3 << 'PYEOF'
# mantine-ui has NO base option, which means it defaults to "/"
# This means it will keep absolute paths like /src/main.tsx
# And the test would try to fetch: path.Join("/alertmanager/", "/src/main.tsx") = "/src/main.tsx"
# But there's no handler for /src/*, so it would 404
# elm-ui (ui/app) has base: "./" which means:
# 1. It rewrites /src/main.js to relative paths
# 2. Vite might also move files to different locations
# The question: which app is actually being tested?
print("Critical: which UI framework is actually embedded in app/dist?")
print("- elm-ui with base: './' (relative paths, would work)")
print("- mantine-ui without base (absolute paths, would fail)")
PYEOFRepository: prometheus/alertmanager
Length of output: 238
🏁 Script executed:
# Check which app is the primary one
grep -r "mantine-ui\|elm.*ui\|app/dist" . --include="Makefile" --include="*.go" | grep -v node_modules | grep -v ".git"Repository: prometheus/alertmanager
Length of output: 371
🏁 Script executed:
# The Makefile showed FRONTEND_DIR = $(BIN_DIR)/ui/app
# So elm-ui (ui/app) is the one being used
# Which has base: "./" that makes relative paths
# But let me verify the source HTML actually has those absolute paths
grep -o 'src="[^"]*"\|href="[^"]*"' ui/app/index.htmlRepository: prometheus/alertmanager
Length of output: 104
🏁 Script executed:
# Perfect - now I have all the information needed
# The review comment's concern is valid, but the current code works due to vite config
# Let me prepare the rewritten review
echo "Verification complete"Repository: prometheus/alertmanager
Length of output: 88
The test currently works due to vite's base: "./" configuration, but relies on fragile assumptions about build output.
fetchIndexAndAssets uses path.Join to construct URLs, which resets the path when the asset reference is absolute (e.g., path.Join("/alertmanager/", "/src/main.js") becomes /src/main.js). The test passes because vite's base: "./" setting in ui/app/vite.config.mjs converts absolute paths to relative ones during the build, making path.Join behave correctly by coincidence rather than by design.
If the vite configuration changes, the build process shifts, or absolute paths accidentally slip through, the test could silently pass while assets fail in production. URL resolution should use proper URL semantics (via url.Parse and ResolveReference) rather than path manipulation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/web_test.go` around lines 62 - 75, fetchIndexAndAssets builds asset URLs
with path.Join which breaks for absolute asset references; replace that logic by
parsing the base URL (url.Parse) from urlPath and resolving each asset reference
with base.ResolveReference(url.Parse(match[1])) so absolute and relative asset
href/src values are handled correctly. Update fetchIndexAndAssets to build
assetPath using url.Parse + ResolveReference, ensure error handling for Parse
failures, and add the necessary import for net/url; keep the existing
fetchWithEncoding calls and test assertions unchanged.
This makes it so `make` will no longer skip targets, if files such as `ui/app/all` exist. More importantly, it will stop this from comming up in AI code reviews. Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
Previously, the Makefile used `:=` for the TEMPOPENAPI variable, causing `mktemp` to run at parse time on every `make` invocation. The temp dir was only cleaned up inside the `src/Data` recipe. In the GitHub workflow, this left a root-owned `700` directory on the host, which blocks `hashFiles` in post-job cleanups. Because of this, it was previously not possible to invoke any `ui/app/Makefile` command during the `test` stage. Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR uses
viteto build the Elm ui. This has a number of consequences, which are detailed in the commit messages.This is an overview over the files provided by Alertmanager
v0.31.1via theui, which might be handy for debugging:/favicon.icoimage/vnd.microsoft.icon/text/html; charset=utf-8/script.jstext/javascript; charset=utf-8/lib/bootstrap-4.6.2-dist/css/bootstrap.min.csstext/css; charset=utf-8/lib/bootstrap-4.6.2-dist/css/bootstrap.min.css.maptext/plain; charset=utf-8/lib/elm-datepicker/css/elm-datepicker.csstext/css; charset=utf-8/lib/font-awesome-4.7.0/css/font-awesome.csstext/css; charset=utf-8/lib/font-awesome-4.7.0/css/font-awesome.min.csstext/css; charset=utf-8/lib/font-awesome-4.7.0/fonts/FontAwesome.otfapplication/vnd.oasis.opendocument.formula-template/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.eotapplication/vnd.ms-fontobject/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.svgimage/svg+xml/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.ttffont/ttf/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.wofffont/woff/lib/font-awesome-4.7.0/fonts/fontawesome-webfont.woff2font/woff2Pull Request Checklist
(Compression is tested and we are also testing the
Cache-Controlheaders. However, we don't have a fulle end-to-end test of which requests a browser would actually send.)(I checked the binary sizes, and
vitewill show the web artifacts.)ui. This can be done via a work space:Which user-facing changes does this PR introduce?
Summary by CodeRabbit
New Features
Chores