Skip to content

Commit

Permalink
alertmanager: Avoid breaking /api/v1/alerts if legacy prefix is empty (
Browse files Browse the repository at this point in the history
…cortexproject#3905)

If the legacy prefix is an empty string, it's interpreted as a handler for `/*`. Any handlers added afterwards are silently ignored, so requests to `/api/v1/alerts` are routed to the legacy handler, rather than to `am.*UserConfig` as would be expected. Meanwhile any handlers added BEFORE the `/*` legacy handler still work as expected - making this very difficult to debug!

Signed-off-by: Nick Parker <nick@nickbp.com>
  • Loading branch information
nickbp authored and harry671003 committed Mar 11, 2021
1 parent 471b912 commit 8a132fe
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
* [BUGFIX] Prevent panic at start if the http_prefix setting doesn't have a valid value. #3796
* [BUGFIX] Memberlist: fixed panic caused by race condition in `armon/go-metrics` used by memberlist client. #3724
* [BUGFIX] Querier: returning 422 (instead of 500) when query hits `max_chunks_per_query` limit with block storage. #3895
* [BUGFIX] Alertmanager: Ensure that experimental `/api/v1/alerts` endpoints work when `-http.prefix` is empty. #3905

## 1.7.0

Expand Down
16 changes: 9 additions & 7 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,19 +172,21 @@ func (a *API) RegisterAlertmanager(am *alertmanager.MultitenantAlertmanager, tar
a.RegisterRoutesWithPrefix(a.cfg.AlertmanagerHTTPPrefix, am, true)
level.Debug(a.logger).Log("msg", "api: registering alertmanager", "path_prefix", a.cfg.AlertmanagerHTTPPrefix)

// If the target is Alertmanager, enable the legacy behaviour. Otherwise only enable
// the component routed API.
if target {
a.RegisterRoute("/status", am.GetStatusHandler(), false, "GET")
a.RegisterRoutesWithPrefix(a.cfg.LegacyHTTPPrefix, am, true)
}

// MultiTenant Alertmanager Experimental API routes
if apiEnabled {
a.RegisterRoute("/api/v1/alerts", http.HandlerFunc(am.GetUserConfig), true, "GET")
a.RegisterRoute("/api/v1/alerts", http.HandlerFunc(am.SetUserConfig), true, "POST")
a.RegisterRoute("/api/v1/alerts", http.HandlerFunc(am.DeleteUserConfig), true, "DELETE")
}

// If the target is Alertmanager, enable the legacy behaviour. Otherwise only enable
// the component routed API.
if target {
a.RegisterRoute("/status", am.GetStatusHandler(), false, "GET")
// WARNING: If LegacyHTTPPrefix is an empty string, any other paths added after this point will be
// silently ignored by the HTTP service. Therefore, this must be the last route to be configured.
a.RegisterRoutesWithPrefix(a.cfg.LegacyHTTPPrefix, am, true)
}
}

// RegisterAPI registers the standard endpoints associated with a running Cortex.
Expand Down

0 comments on commit 8a132fe

Please sign in to comment.