feat(ingester): restrict /flush and /shutdown endpoints to POST only

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changelog
 
 ## master / unreleased
+* [CHANGE] Ingester: `/ingester/flush`, `/ingester/shutdown`, `/flush`, and `/shutdown` endpoints now only accept `POST` requests. The index page renders these as HTML form buttons to prevent accidental browser-triggered GET requests. Update any scripts using `wget` or `curl` without `-X POST` to use `curl -X POST` or `wget --post-data=""`. #3243
 * [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). #7160
 * [CHANGE] Cache: Setting `-blocks-storage.bucket-store.metadata-cache.bucket-index-content-ttl` to 0 will disable the bucket-index cache. #7446
 * [CHANGE] HA Tracker: Move `-distributor.ha-tracker.failover-timeout` from a global config to a per-tenant runtime config. The flag name and default value (30s) remain the same. #7481

docs/api/_index.md

@@ -29,8 +29,8 @@ For the sake of clarity, in this document we have grouped API endpoints by servi
 | [OTLP receiver](#otlp-receiver) | Distributor || `POST /api/v1/otlp/v1/metrics` |
 | [Tenants stats](#tenants-stats) | Distributor || `GET /distributor/all_user_stats` |
 | [HA tracker status](#ha-tracker-status) | Distributor || `GET /distributor/ha_tracker` |
-| [Flush blocks](#flush-blocks) | Ingester || `GET,POST /ingester/flush` |
-| [Shutdown](#shutdown) | Ingester || `GET,POST /ingester/shutdown` |
+| [Flush blocks](#flush-blocks) | Ingester || `POST /ingester/flush` |
+| [Shutdown](#shutdown) | Ingester || `POST /ingester/shutdown` |
 | [Ingesters ring status](#ingesters-ring-status) | Ingester || `GET /ingester/ring` |
 | [Ingester tenants stats](#ingester-tenants-stats) | Ingester || `GET /ingester/all_user_stats` |
 | [Ingester mode](#ingester-mode) | Ingester || `GET,POST /ingester/mode` |
@@ -268,10 +268,10 @@ Displays a web page with the current status of the HA tracker, including the ele
 ### Flush blocks
 
 ```
-GET,POST /ingester/flush
+POST /ingester/flush
 
 # Legacy
-GET,POST /flush
+POST /flush
 ```
 
 Triggers a flush of the in-memory time series data to the long-term storage. This endpoint triggers the flush also when `-ingester.flush-on-shutdown-with-wal-enabled` or `-blocks-storage.tsdb.flush-blocks-on-shutdown` are disabled.
@@ -283,10 +283,10 @@ Flush endpoint now also accepts `wait=true` parameter, which makes the call sync
 ### Shutdown
 
 ```
-GET,POST /ingester/shutdown
+POST /ingester/shutdown
 
 # Legacy
-GET,POST /shutdown
+POST /shutdown
 ```
 
 Flushes in-memory time series data from ingester to the long-term storage, and shuts down the ingester service. Notice that the other Cortex services are still running, and the operator (or any automation) is expected to terminate the process with a `SIGINT` / `SIGTERM` signal after the shutdown endpoint returns. In the meantime, `/ready` will not return 200. This endpoint will unregister the ingester from the ring even if `-ingester.unregister-on-shutdown` is disabled.

pkg/api/api.go

@@ -333,16 +333,16 @@ func (a *API) RegisterIngester(i Ingester, pushConfig distributor.Config, overri
 	a.indexPage.AddLink(SectionDangerous, "/ingester/renewTokens", "Renew Ingester Tokens (10%)")
 	a.indexPage.AddLink(SectionDangerous, "/ingester/mode?mode=READONLY", "Set Ingester to READONLY mode")
 	a.indexPage.AddLink(SectionDangerous, "/ingester/mode?mode=ACTIVE", "Set Ingester to ACTIVE mode")
-	a.RegisterRoute("/ingester/flush", http.HandlerFunc(i.FlushHandler), false, "GET", "POST")
-	a.RegisterRoute("/ingester/shutdown", http.HandlerFunc(i.ShutdownHandler), false, "GET", "POST")
+	a.RegisterRoute("/ingester/flush", http.HandlerFunc(i.FlushHandler), false, "POST")
+	a.RegisterRoute("/ingester/shutdown", http.HandlerFunc(i.ShutdownHandler), false, "POST")
 	a.RegisterRoute("/ingester/renewTokens", http.HandlerFunc(i.RenewTokenHandler), false, "GET", "POST")
 	a.RegisterRoute("/ingester/all_user_stats", http.HandlerFunc(i.AllUserStatsHandler), false, "GET")
 	a.RegisterRoute("/ingester/mode", http.HandlerFunc(i.ModeHandler), false, "GET", "POST")
 	a.RegisterRoute("/ingester/push", push.Handler(pushConfig.RemoteWriteV2Enabled, pushConfig.AcceptUnknownRemoteWriteContentType, pushConfig.MaxRecvMsgSize, overrides, a.sourceIPs, i.Push, nil), true, "POST") // For testing and debugging.
 
 	// Legacy Routes
-	a.RegisterRoute("/flush", http.HandlerFunc(i.FlushHandler), false, "GET", "POST")
-	a.RegisterRoute("/shutdown", http.HandlerFunc(i.ShutdownHandler), false, "GET", "POST")
+	a.RegisterRoute("/flush", http.HandlerFunc(i.FlushHandler), false, "POST")
+	a.RegisterRoute("/shutdown", http.HandlerFunc(i.ShutdownHandler), false, "POST")
 	a.RegisterRoute("/push", push.Handler(pushConfig.RemoteWriteV2Enabled, pushConfig.AcceptUnknownRemoteWriteContentType, pushConfig.MaxRecvMsgSize, overrides, a.sourceIPs, i.Push, nil), true, "POST") // For testing and debugging.
 }
 

pkg/api/handlers.go

@@ -93,7 +93,15 @@ var indexPageTemplate = `
 		<p>{{ $s }}</p>
 		<ul>
 			{{ range $path, $desc := $links }}
+				{{ if IsDangerous $s }}
+				<li>
+					<form method="POST" action="{{ AddPathPrefix $path }}" style="display: inline;">
+						<button type="submit">{{ $desc }}</button>
+					</form>
+				</li>
+				{{ else }}
 				<li><a href="{{ AddPathPrefix $path }}">{{ $desc }}</a></li>
+				{{ end }}
 			{{ end }}
 		</ul>
 		{{ end }}
@@ -106,6 +114,9 @@ func indexHandler(httpPathPrefix string, content *IndexPageContent) http.Handler
 		"AddPathPrefix": func(link string) string {
 			return path.Join(httpPathPrefix, link)
 		},
+		"IsDangerous": func(section string) bool {
+			return section == SectionDangerous
+		},
 	})
 	template.Must(templ.Parse(indexPageTemplate))
 

pkg/api/handlers_test.go

@@ -62,6 +62,41 @@ func TestIndexPageContent(t *testing.T) {
 	require.True(t, strings.Contains(resp.Body.String(), "Store Gateway Ring"))
 	require.True(t, strings.Contains(resp.Body.String(), "/shutdown"))
 	require.False(t, strings.Contains(resp.Body.String(), "/compactor/ring"))
+
+	// Non-dangerous links should render as anchors.
+	require.True(t, strings.Contains(resp.Body.String(), `<a href="/ingester/ring">`))
+	// Dangerous links should render as POST form buttons, not anchors.
+	require.True(t, strings.Contains(resp.Body.String(), `action="/shutdown"`))
+	require.True(t, strings.Contains(resp.Body.String(), `method="POST"`))
+	require.False(t, strings.Contains(resp.Body.String(), `<a href="/shutdown">`))
+}
+
+func TestIndexHandlerDangerousLinksUsePostForms(t *testing.T) {
+	c := newIndexPageContent()
+	c.AddLink(SectionDangerous, "/ingester/flush", "Flush")
+	c.AddLink(SectionDangerous, "/ingester/shutdown", "Shutdown")
+
+	for _, tc := range []struct {
+		prefix         string
+		expectedAction string
+	}{
+		{prefix: "", expectedAction: `action="/ingester/flush"`},
+		{prefix: "/test", expectedAction: `action="/test/ingester/flush"`},
+	} {
+		h := indexHandler(tc.prefix, c)
+
+		req := httptest.NewRequest("GET", "/", nil)
+		resp := httptest.NewRecorder()
+
+		h.ServeHTTP(resp, req)
+
+		require.Equal(t, 200, resp.Code)
+		body := resp.Body.String()
+		// Should render as a POST form, not an anchor.
+		require.True(t, strings.Contains(body, tc.expectedAction), "expected form action %q in body", tc.expectedAction)
+		require.True(t, strings.Contains(body, `method="POST"`))
+		require.False(t, strings.Contains(body, `<a href="`))
+	}
 }
 
 type diffConfigMock struct {

tools/migrate-ingester-statefulsets.sh

@@ -39,7 +39,7 @@ while [[ $INSTANCES_TO_DOWNSCALE -gt 0 ]]; do
   # wget (BusyBox version) will fail, but we don't care ... important thing is that it has triggered shutdown.
   # -T causes wget to wait only 5 seconds, otherwise /shutdown takes a long time.
   # Preferably we would wait for /shutdown to return, but unfortunately that doesn't work (even with big timeout), wget complains with weird error.
-  kubectl exec "$POD_TO_SHUTDOWN" --namespace="$NAMESPACE" -- wget -T 5 http://localhost:80/shutdown >/dev/null 2>/dev/null || true
+  kubectl exec "$POD_TO_SHUTDOWN" --namespace="$NAMESPACE" -- wget --post-data="" -T 5 http://localhost:80/shutdown >/dev/null 2>/dev/null || true
 
   # While request to /shutdown completes only after flushing has finished, it unfortunately returns 204 status code,
   # which confuses wget. That is the reason why instead of waiting for /shutdown to complete, this script waits for