From 26ea0afcb59fab7fd5674f70c5c2b785a53a4bd6 Mon Sep 17 00:00:00 2001 From: Kevin Mingtarja Date: Sun, 24 Mar 2024 04:44:33 +0800 Subject: [PATCH] Ingester: allow only POST method on `/ingester/shutdown` Signed-off-by: Kevin Mingtarja --- CHANGELOG.md | 1 + cmd/mimir/config-descriptor.json | 11 +++++++++++ cmd/mimir/help-all.txt.tmpl | 2 ++ .../mimir/configure/configuration-parameters/index.md | 6 ++++++ pkg/api/api.go | 9 ++++++++- 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7539640db68..d33421e37d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -179,6 +179,7 @@ * [BUGFIX] Fix metadata API using wrong JSON field names. #7475 * [BUGFIX] Ruler: fix native histogram recording rule result corruption. #7552 * [BUGFIX] Querier: fix HTTP status code translations for remote read requests. Previously, remote-read had conflicting behaviours: when returning samples all internal errors were translated to HTTP 400; when returning chunks all internal errors were translated to HTTP 500. #7487 +* [CHANGE] Ingester: allow only POST method on `/ingester/shutdown`, as previously it was too easy to accidentally trigger through GET requests. At the same time, add an option to keep the existing behavior by introducing an `-ingester.allow-get-request-for-shutdown` flag. This flag will be removed in Mimir 2.14. #7707 ### Mixin diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json index 7f43420589d..ab7caabe6e6 100644 --- a/cmd/mimir/config-descriptor.json +++ b/cmd/mimir/config-descriptor.json @@ -96,6 +96,17 @@ "fieldType": "boolean", "fieldCategory": "deprecated" }, + { + "kind": "field", + "name": "allow_get_request_for_ingester_shutdown", + "required": false, + "desc": "If true, allow GET requests to the /ingester/shutdown endpoint to trigger an ingester shutdown. This is a potentially dangerous operation and should only be enabled consciously.", + "fieldValue": null, + "fieldDefaultValue": false, + "fieldFlag": "ingester.allow-get-request-for-shutdown", + "fieldType": "boolean", + "fieldCategory": "deprecated" + }, { "kind": "field", "name": "alertmanager_http_prefix", diff --git a/cmd/mimir/help-all.txt.tmpl b/cmd/mimir/help-all.txt.tmpl index 7e0a9676c73..8c2a985a367 100644 --- a/cmd/mimir/help-all.txt.tmpl +++ b/cmd/mimir/help-all.txt.tmpl @@ -1267,6 +1267,8 @@ Usage of ./cmd/mimir/mimir: After what time a series is considered to be inactive. (default 10m0s) -ingester.active-series-metrics-update-period duration How often to update active series metrics. (default 1m0s) + -ingester.allow-get-request-for-shutdown + If true, allow GET requests to the /ingester/shutdown endpoint to trigger an ingester shutdown. This is a potentially dangerous operation and should only be enabled consciously. (default false) -ingester.client.backoff-max-period duration Maximum delay when backing off. (default 10s) -ingester.client.backoff-min-period duration diff --git a/docs/sources/mimir/configure/configuration-parameters/index.md b/docs/sources/mimir/configure/configuration-parameters/index.md index 1a986fbf4c7..aaefd02fc11 100644 --- a/docs/sources/mimir/configure/configuration-parameters/index.md +++ b/docs/sources/mimir/configure/configuration-parameters/index.md @@ -146,6 +146,12 @@ api: # CLI flag: -distributor.enable-otlp-metadata-storage [enable_otel_metadata_translation: | default = true] + # (deprecated) If true, allow GET requests to the /ingester/shutdown endpoint + # to trigger an ingester shutdown. This is a potentially dangerous operation + # and should only be enabled consciously. + # CLI flag: -ingester.allow-get-request-for-shutdown + [allow_get_request_for_ingester_shutdown: | default = false] + # (advanced) HTTP URL path under which the Alertmanager ui and api will be # served. # CLI flag: -http.alertmanager-http-prefix diff --git a/pkg/api/api.go b/pkg/api/api.go index 393e1eab321..ffca73d6e31 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -53,6 +53,9 @@ type Config struct { // TODO: Remove option in Mimir 2.14. EnableOtelMetadataStorage bool `yaml:"enable_otel_metadata_translation" category:"deprecated"` + // TODO: Remove option in Mimir 2.14. + AllowGETRequestForIngesterShutdown bool `yaml:"allow_get_request_for_ingester_shutdown" category:"deprecated"` + AlertmanagerHTTPPrefix string `yaml:"alertmanager_http_prefix" category:"advanced"` PrometheusHTTPPrefix string `yaml:"prometheus_http_prefix" category:"advanced"` @@ -71,6 +74,7 @@ type Config struct { func (cfg *Config) RegisterFlags(f *flag.FlagSet) { f.BoolVar(&cfg.SkipLabelNameValidationHeader, "api.skip-label-name-validation-header-enabled", false, "Allows to skip label name validation via X-Mimir-SkipLabelNameValidation header on the http write path. Use with caution as it breaks PromQL. Allowing this for external clients allows any client to send invalid label names. After enabling it, requests with a specific HTTP header set to true will not have label names validated.") f.BoolVar(&cfg.EnableOtelMetadataStorage, "distributor.enable-otlp-metadata-storage", true, "If true, store metadata when ingesting metrics via OTLP. This makes metric descriptions and types available for metrics ingested via OTLP.") + f.BoolVar(&cfg.AllowGETRequestForIngesterShutdown, "ingester.allow-get-request-for-shutdown", false, "If true, allow GET requests to the /ingester/shutdown endpoint to trigger an ingester shutdown. This is a potentially dangerous operation and should only be enabled consciously.") cfg.RegisterFlagsWithPrefix("", f) } @@ -296,7 +300,10 @@ func (a *API) RegisterIngester(i Ingester) { a.RegisterRoute("/ingester/flush", http.HandlerFunc(i.FlushHandler), false, true, "GET", "POST") a.RegisterRoute("/ingester/prepare-shutdown", http.HandlerFunc(i.PrepareShutdownHandler), false, true, "GET", "POST", "DELETE") a.RegisterRoute("/ingester/prepare-partition-downscale", http.HandlerFunc(i.PreparePartitionDownscaleHandler), false, true, "GET", "POST", "DELETE") - a.RegisterRoute("/ingester/shutdown", http.HandlerFunc(i.ShutdownHandler), false, true, "GET", "POST") + a.RegisterRoute("/ingester/shutdown", http.HandlerFunc(i.ShutdownHandler), false, true, "POST") + if a.cfg.AllowGETRequestForIngesterShutdown { + a.RegisterDeprecatedRoute("/ingester/shutdown", http.HandlerFunc(i.ShutdownHandler), false, true, "GET") + } a.RegisterRoute("/ingester/tsdb_metrics", http.HandlerFunc(i.UserRegistryHandler), true, true, "GET") a.indexPage.AddLinks(defaultWeight, "Ingester", []IndexPageLink{