From f47690485631b8e5b62ea023e76770ac822042aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=A0tibran=C3=BD?= Date: Fri, 5 Mar 2021 23:54:21 +0100 Subject: [PATCH] Expose deletion of AM config via /multitenant_alertmanager/delete_tenant_config (#3900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: gotjosh Signed-off-by: Peter Štibraný --- CHANGELOG.md | 1 + docs/api/_index.md | 13 +++++++ pkg/alertmanager/alertstore/store.go | 1 + pkg/alertmanager/api.go | 2 ++ pkg/alertmanager/api_test.go | 51 ++++++++++++++++++++++++++++ pkg/api/api.go | 1 + 6 files changed, 69 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eb5d3f041..45df6918d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ * `cortex_bucket_store_chunk_pool_returned_bytes_total` * [ENHANCEMENT] Alertmanager: load alertmanager configurations from object storage concurrently, and only load necessary configurations, speeding configuration synchronization process and executing fewer "GET object" operations to the storage when sharding is enabled. #3898 * [ENHANCEMENT] Blocks storage: Ingester can now stream entire chunks instead of individual samples to the querier. At the moment this feature must be explicitly enabled either by using `-ingester.stream-chunks-when-using-blocks` flag or `ingester_stream_chunks_when_using_blocks` (boolean) field in runtime config file, but these configuration options are temporary and will be removed when feature is stable. #3889 +* [ENHANCEMENT] Alertmanager: New endpoint `/multitenant_alertmanager/delete_tenant_config` to delete configuration for tenant identified by `X-Scope-OrgID` header. This is an internal endpoint, available even if Alertmanager API is not enabled by using `-experimental.alertmanager.enable-api`. #3900 * [BUGFIX] Cortex: Fixed issue where fatal errors and various log messages where not logged. #3778 * [BUGFIX] HA Tracker: don't track as error in the `cortex_kv_request_duration_seconds` metric a CAS operation intentionally aborted. #3745 * [BUGFIX] Querier / ruler: do not log "error removing stale clients" if the ring is empty. #3761 diff --git a/docs/api/_index.md b/docs/api/_index.md index f5346ab9a6..c7e2745fda 100644 --- a/docs/api/_index.md +++ b/docs/api/_index.md @@ -53,6 +53,7 @@ For the sake of clarity, in this document we have grouped API endpoints by servi | [Alertmanager status](#alertmanager-status) | Alertmanager | `GET /multitenant_alertmanager/status` | | [Alertmanager ring status](#alertmanager-ring-status) | Alertmanager | `GET /multitenant_alertmanager/ring` | | [Alertmanager UI](#alertmanager-ui) | Alertmanager | `GET /` | +| [Alertmanager Delete Tenant Configuration](#alertmanager-delete-tenant-configuration) | Alertmanager | `POST /multitenant_alertmanager/delete_tenant_config` | | [Get Alertmanager configuration](#get-alertmanager-configuration) | Alertmanager | `GET /api/v1/alerts` | | [Set Alertmanager configuration](#set-alertmanager-configuration) | Alertmanager | `POST /api/v1/alerts` | | [Delete Alertmanager configuration](#delete-alertmanager-configuration) | Alertmanager | `DELETE /api/v1/alerts` | @@ -683,6 +684,18 @@ Displays the Alertmanager UI. _Requires [authentication](#authentication)._ +### Alertmanager Delete Tenant Configuration + +``` +POST /multitenant_alertmanager/delete_tenant_config +``` + +This endpoint deletes configuration for a tenant identified by `X-Scope-OrgID` header. +It is internal, available even if Alertmanager API is not enabled by using `-experimental.alertmanager.enable-api`. +The endpoint returns a status code of `200` if the user's configuration has been deleted, or it didn't exist in the first place. + +_Requires [authentication](#authentication)._ + ### Get Alertmanager configuration ``` diff --git a/pkg/alertmanager/alertstore/store.go b/pkg/alertmanager/alertstore/store.go index 4006f567ea..e2ddc48e38 100644 --- a/pkg/alertmanager/alertstore/store.go +++ b/pkg/alertmanager/alertstore/store.go @@ -37,6 +37,7 @@ type AlertStore interface { SetAlertConfig(ctx context.Context, cfg alertspb.AlertConfigDesc) error // DeleteAlertConfig deletes the alertmanager configuration for an user. + // If configuration for the user doesn't exist, no error is reported. DeleteAlertConfig(ctx context.Context, user string) error } diff --git a/pkg/alertmanager/api.go b/pkg/alertmanager/api.go index 9cdeba3245..95b2a9d2e7 100644 --- a/pkg/alertmanager/api.go +++ b/pkg/alertmanager/api.go @@ -112,6 +112,8 @@ func (am *MultitenantAlertmanager) SetUserConfig(w http.ResponseWriter, r *http. w.WriteHeader(http.StatusCreated) } +// DeleteUserConfig is exposed via user-visible API (if enabled, uses DELETE method), but also as an internal endpoint using POST method. +// Note that if no config exists for a user, StatusOK is returned. func (am *MultitenantAlertmanager) DeleteUserConfig(w http.ResponseWriter, r *http.Request) { logger := util_log.WithContext(r.Context(), am.logger) userID, err := tenant.TenantID(r.Context()) diff --git a/pkg/alertmanager/api_test.go b/pkg/alertmanager/api_test.go index ab775f28a1..d0625165d5 100644 --- a/pkg/alertmanager/api_test.go +++ b/pkg/alertmanager/api_test.go @@ -2,12 +2,18 @@ package alertmanager import ( "bytes" + "context" "fmt" "io/ioutil" "net/http" "net/http/httptest" "testing" + "github.com/go-kit/kit/log" + "github.com/thanos-io/thanos/pkg/objstore" + + "github.com/cortexproject/cortex/pkg/alertmanager/alertspb" + "github.com/cortexproject/cortex/pkg/alertmanager/alertstore/bucketclient" util_log "github.com/cortexproject/cortex/pkg/util/log" "github.com/stretchr/testify/require" @@ -148,3 +154,48 @@ template_files: }) } } + +func TestMultitenantAlertmanager_DeleteUserConfig(t *testing.T) { + storage := objstore.NewInMemBucket() + alertStore := bucketclient.NewBucketAlertStore(storage, nil, log.NewNopLogger()) + + am := &MultitenantAlertmanager{ + store: alertStore, + logger: util_log.Logger, + } + + require.NoError(t, alertStore.SetAlertConfig(context.Background(), alertspb.AlertConfigDesc{ + User: "test_user", + RawConfig: "config", + })) + + require.Equal(t, 1, len(storage.Objects())) + + req := httptest.NewRequest("POST", "/multitenant_alertmanager/delete_tenant_config", nil) + // Missing user returns error 401. (DeleteUserConfig does this, but in practice, authentication middleware will do it first) + { + rec := httptest.NewRecorder() + am.DeleteUserConfig(rec, req) + require.Equal(t, http.StatusUnauthorized, rec.Code) + require.Equal(t, 1, len(storage.Objects())) + } + + // With user in the context. + ctx := user.InjectOrgID(context.Background(), "test_user") + req = req.WithContext(ctx) + { + rec := httptest.NewRecorder() + am.DeleteUserConfig(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + require.Equal(t, 0, len(storage.Objects())) + } + + // Repeating the request still reports 200 + { + rec := httptest.NewRecorder() + am.DeleteUserConfig(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + require.Equal(t, 0, len(storage.Objects())) + } +} diff --git a/pkg/api/api.go b/pkg/api/api.go index 2b9e6bd8ec..f0172a926c 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -167,6 +167,7 @@ func (a *API) RegisterAlertmanager(am *alertmanager.MultitenantAlertmanager, tar // Ensure this route is registered before the prefixed AM route a.RegisterRoute("/multitenant_alertmanager/status", am.GetStatusHandler(), false, "GET") a.RegisterRoute("/multitenant_alertmanager/ring", http.HandlerFunc(am.RingHandler), false, "GET", "POST") + a.RegisterRoute("/multitenant_alertmanager/delete_tenant_config", http.HandlerFunc(am.DeleteUserConfig), true, "POST") // UI components lead to a large number of routes to support, utilize a path prefix instead a.RegisterRoutesWithPrefix(a.cfg.AlertmanagerHTTPPrefix, am, true)