feat: add authentication to /health and /ready - #27575
davidby-influx wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in authorization gate for /health and /ready that preserves existing probe semantics (status codes unchanged) while redacting diagnostic detail unless the caller has operator permissions. It also wires this through --health-auth-enabled (and --hardening-enabled implication) and adds unit/integration tests plus updated operator documentation.
Changes:
- Gate
/healthand/readydetail behind operator permissions, including bounded concurrent credential resolution and “startup window” behavior. - Refactor authentication middleware to expose a log-free
Authorizemethod via a newCredentialResolverinterface. - Add CLI wiring for
--health-auth-enabled(with hardening implication + opt-out), plus comprehensive tests and docs updates.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| http/check_handler.go | Adds auth-gated detail levels for /health and /ready, with bounded credential resolution and redacted response shapes. |
| http/check_handler_race_test.go | Adds a race-focused test for concurrent auth wiring while serving probe traffic. |
| http/check_handler_jsoncompat_test.go | Updates JSON compatibility tests to reuse shared decoding helpers. |
| http/check_handler_auth_test.go | Adds a detailed permission/behavior matrix and wire-format pins for redacted vs full responses. |
| http/authentication_test.go | Extends authentication handler tests to pin Authorize contract and user-lookup failure behavior. |
| http/authentication_middleware.go | Introduces CredentialResolver + Authorize, refactors ServeHTTP to use it, and adds inactive-user classification. |
| HEALTH_READY.md | Documents the new auth behavior, startup window, wedged-store guard, and configuration semantics. |
| cmd/influxd/launcher/launcher.go | Wires health auth into startup, installs token-only resolver early, adds resolver replacement once sessions are available, and splits SQL migration timing. |
| cmd/influxd/launcher/health_ready_test.go | Updates launcher endpoint tests to support authenticated/anonymous requests and assert envelope fields. |
| cmd/influxd/launcher/health_ready_auth_test.go | Adds end-to-end tests for health-auth enabled, hardening implication, and opt-out behavior. |
| cmd/influxd/launcher/cmd.go | Adds --health-auth-enabled, tracks explicit user setting, and applies hardening implications deterministically. |
| cmd/influxd/launcher/cmd_test.go | Adds tests covering implication/opt-out resolution across CLI args, env, and config file. |
| authorizer/authorize.go | Adds AllowedAll helper for context-free permission checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Flags are parsed by the time PreRunE fires, so this is the earliest | ||
| // point at which an explicit --health-auth-enabled=false on the command | ||
| // line is distinguishable from the flag sitting at its default. | ||
| c.PreRunE = func(c *cobra.Command, _ []string) error { | ||
| o.HealthAuthEnabledSet = o.HealthAuthEnabledSet || c.Flags().Changed(healthAuthEnabledFlag) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
All the logic for being able to set --healt-auth-enabled=false (of which this is a small piece) is way to complicated and fragile for my taste.
Part of the complexity is because there are two hardening options (1) flux/pkger IP validation, and 2) HSTS) that do not have a fine-grained parameter to enable them. Contrast this with disabling template file:// URLs. which does have a parameter that can turn it and only it on. If flux/pkger IP validation and HSTS had discrete command line parameters, there may not be a need to have --health-auth-enabled=false do anything special. If you look at the commit that added --hardening-enabled (2c930fd), the intent was to have fine-grained options in addition the umbrella.
Another simpler option would be to support --health-auth-enabled=(auto|required|disabled). auto would follow the umbrella --hardening-enabled option. required and disabled explicitly override it without regard to --hardening-enabled.
There was a problem hiding this comment.
I like the trivalent flag. Just pushed a fix with it.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
--health-auth-mode takes auto, required, or disabled. auto follows --hardening-enabled; required and disabled override it without regard to hardening. auto is the unset state, so the command line, the environment and the config file need no special handling, and healthAuthRequired resolves the two options as a pure read wherever the answer is needed. The sidecar and the mutate-before-the-config-handler ordering go away. /api/v2/config and print-config report the mode as configured; auto beside hardening-enabled is the enforced answer.
dfd40dc to
d498397
Compare
Add optional authentication to
/healthand/readychecks. Authenticated callers get error information; unauthenticated callers merely get status with details.