markhoerth opened a new pull request, #10847:
URL: https://github.com/apache/gravitino/pull/10847
Follow-up to #10839. #10840 added /health, /health/live, /health/ready
root-level aliases but dropped /health.html during the merge refactor. This
restores /health.html and maps it to the canonical /api/health aggregate
endpoint — legacy GTM onboarding standards commonly hardcode the .html
extension as a well-known probe path.
Adds test coverage for /health.html in TestHealthAliasServlet.
Fixes #10846
### What changes were proposed in this pull request?
Restores `/health.html` as a root-level alias forwarding to `/api/health`.
Three small changes:
1. `HealthAliasServlet.doGet` — map `/health.html` to `/api/health` (the
aggregate endpoint), rather than the default `/api` prefix rule which would
incorrectly target `/api/health.html`.
2. `GravitinoServer` — register the alias servlet at `/health.html` (outside
the `/health/*` glob spec Jetty uses for sub-paths).
3. `TestHealthAliasServlet` — extend the parameterized forward test to cover
`/health.html → /api/health`.
### Why are the changes needed?
#10840 added root-level aliases `/health`, `/health/live`, `/health/ready`
for "enterprise GTM standards that hardcode well-known root paths."
`/health.html` — arguably the most commonly hardcoded GTM probe path (predates
MicroProfile Health, common in enterprise GTM onboarding specs) — was part of
the original PR and was dropped during the merge refactor. Without `.html`,
customers whose GTM standards specifically require `/health.html` cannot use
the aliases.
Smoke test confirms the fix:
curl -s -o /dev/null -w "%{http_code}\n" http://localhost:8090/health
# 200
curl -s -o /dev/null -w "%{http_code}\n" http://localhost:8090/health.html
# 200 (was 404)
curl -s -o /dev/null -w "%{http_code}\n" http://localhost:8090/api/health
# 200
### Does this PR introduce _any_ user-facing change?
Yes — adds one public endpoint (`GET /health.html`). Returns the same
aggregate JSON body as `/health` and `/api/health`. No other endpoint behavior
is changed.
### How was this patch tested?
- `TestHealthAliasServlet` parameterized test extended with `/health.html →
/api/health` case.
- Manual verification against a running Gravitino instance: `/health.html`
returns 200 with the same body as `/api/health` (134-byte aggregate JSON);
existing `/health`, `/health/live`, `/health/ready`, `/api/health*`, and
`/api/version` all unchanged.
### Notes for reviewers
- **Special-case mapping for `.html`**: `HealthAliasServlet` maps
`/health.html` to the aggregate `/api/health`, not `/api/health.html`. `.html`
is a legacy web-server convention used as a sibling of `/health`, not a
sub-path. Keeping the mapping rule explicit in the servlet rather than adding a
separate alias endpoint keeps the servlet single-purpose.
- **Test gap observation**: this regression slipped through #10840 because
`TestHealthAliasServlet` mocks `getRequestDispatcher()`, which validates the
servlet's path-rewriting logic but doesn't exercise Jetty's path-spec matching
in `GravitinoServer.addServlet(...)`. The bug was one level up from what the
unit test covers. Worth considering an integration test that boots the server
and curls each GTM-relevant path — happy to open a separate issue if useful.
- **This change should be cherry-picked to `branch-1.2`** alongside the main
#10840 backport, so RBC and other 1.2 customers get the complete fix in one
coherent change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
