From a2ce8fefed7fae615b32d3fcf5b78a5e97ea2173 Mon Sep 17 00:00:00 2001 From: Santiago Date: Fri, 19 Apr 2024 13:04:18 +0200 Subject: [PATCH] Alerting: Use a struct when sending a Grafana AM configuration to the remote Alertmanager (#86451) * Alerting: Use a struct when sending a Grafana AM configuration to the remote Alertmanager * remove '-distroless' from mimir image name --- .drone.yml | 14 +++++------ .../blocks/mimir_backend/docker-compose.yaml | 2 +- pkg/services/ngalert/remote/alertmanager.go | 25 +++++++++++-------- .../ngalert/remote/alertmanager_test.go | 18 ++++++++++--- .../client/alertmanager_configuration.go | 12 +++++---- pkg/services/ngalert/remote/client/mimir.go | 3 ++- scripts/drone/utils/images.star | 2 +- 7 files changed, 47 insertions(+), 29 deletions(-) diff --git a/.drone.yml b/.drone.yml index 9f6ba848ad2..b6399fe0c64 100644 --- a/.drone.yml +++ b/.drone.yml @@ -879,7 +879,7 @@ services: - commands: - /bin/mimir -target=backend -alertmanager.grafana-alertmanager-compatibility-enabled environment: {} - image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-remove_id_from_grafana_config-d3826b4f8-WIP + image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-validate_grafana_am_config-1e903e462-WIP name: mimir_backend - environment: {} image: redis:6.2.11-alpine @@ -1327,7 +1327,7 @@ services: - commands: - /bin/mimir -target=backend -alertmanager.grafana-alertmanager-compatibility-enabled environment: {} - image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-remove_id_from_grafana_config-d3826b4f8-WIP + image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-validate_grafana_am_config-1e903e462-WIP name: mimir_backend - environment: {} image: redis:6.2.11-alpine @@ -2329,7 +2329,7 @@ services: - commands: - /bin/mimir -target=backend -alertmanager.grafana-alertmanager-compatibility-enabled environment: {} - image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-remove_id_from_grafana_config-d3826b4f8-WIP + image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-validate_grafana_am_config-1e903e462-WIP name: mimir_backend - environment: {} image: redis:6.2.11-alpine @@ -4127,7 +4127,7 @@ services: - commands: - /bin/mimir -target=backend -alertmanager.grafana-alertmanager-compatibility-enabled environment: {} - image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-remove_id_from_grafana_config-d3826b4f8-WIP + image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-validate_grafana_am_config-1e903e462-WIP name: mimir_backend - environment: {} image: redis:6.2.11-alpine @@ -4646,7 +4646,7 @@ steps: - trivy --exit-code 0 --severity UNKNOWN,LOW,MEDIUM plugins/slack - trivy --exit-code 0 --severity UNKNOWN,LOW,MEDIUM python:3.8 - trivy --exit-code 0 --severity UNKNOWN,LOW,MEDIUM postgres:12.3-alpine - - trivy --exit-code 0 --severity UNKNOWN,LOW,MEDIUM us.gcr.io/kubernetes-dev/mimir:santihernandezc-remove_id_from_grafana_config-d3826b4f8-WIP + - trivy --exit-code 0 --severity UNKNOWN,LOW,MEDIUM us.gcr.io/kubernetes-dev/mimir:santihernandezc-validate_grafana_am_config-1e903e462-WIP - trivy --exit-code 0 --severity UNKNOWN,LOW,MEDIUM mysql:5.7.39 - trivy --exit-code 0 --severity UNKNOWN,LOW,MEDIUM mysql:8.0.32 - trivy --exit-code 0 --severity UNKNOWN,LOW,MEDIUM redis:6.2.11-alpine @@ -4681,7 +4681,7 @@ steps: - trivy --exit-code 1 --severity HIGH,CRITICAL plugins/slack - trivy --exit-code 1 --severity HIGH,CRITICAL python:3.8 - trivy --exit-code 1 --severity HIGH,CRITICAL postgres:12.3-alpine - - trivy --exit-code 1 --severity HIGH,CRITICAL us.gcr.io/kubernetes-dev/mimir:santihernandezc-remove_id_from_grafana_config-d3826b4f8-WIP + - trivy --exit-code 1 --severity HIGH,CRITICAL us.gcr.io/kubernetes-dev/mimir:santihernandezc-validate_grafana_am_config-1e903e462-WIP - trivy --exit-code 1 --severity HIGH,CRITICAL mysql:5.7.39 - trivy --exit-code 1 --severity HIGH,CRITICAL mysql:8.0.32 - trivy --exit-code 1 --severity HIGH,CRITICAL redis:6.2.11-alpine @@ -4925,6 +4925,6 @@ kind: secret name: gcr_credentials --- kind: signature -hmac: 958ed40ca0620498c01fa867b05a1d6c3bcd908067fbb34be691923e95cfc77b +hmac: e67367689de11270bb3139f25a7cabf54e2b980460fd55ad410c590722d122b8 ... diff --git a/devenv/docker/blocks/mimir_backend/docker-compose.yaml b/devenv/docker/blocks/mimir_backend/docker-compose.yaml index 2a9c2dbd98e..76a19755d9f 100644 --- a/devenv/docker/blocks/mimir_backend/docker-compose.yaml +++ b/devenv/docker/blocks/mimir_backend/docker-compose.yaml @@ -1,5 +1,5 @@ mimir_backend: - image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-remove_id_from_grafana_config-d3826b4f8-WIP + image: us.gcr.io/kubernetes-dev/mimir:santihernandezc-validate_grafana_am_config-1e903e462-WIP container_name: mimir_backend command: - -target=backend diff --git a/pkg/services/ngalert/remote/alertmanager.go b/pkg/services/ngalert/remote/alertmanager.go index 66a78e6747b..322e9d4e4ce 100644 --- a/pkg/services/ngalert/remote/alertmanager.go +++ b/pkg/services/ngalert/remote/alertmanager.go @@ -166,7 +166,6 @@ func (am *Alertmanager) ApplyConfig(ctx context.Context, config *models.AlertCon am.log.Error("Unable to upload the state to the remote Alertmanager", "err", err) } am.log.Debug("Completed state upload to remote Alertmanager", "url", am.url) - return nil } @@ -202,20 +201,16 @@ func (am *Alertmanager) CompareAndSendConfiguration(ctx context.Context, config if err != nil { return err } - rawDecrypted, err := json.Marshal(decrypted) - if err != nil { - return err - } // Send the configuration only if we need to. - if !am.shouldSendConfig(ctx, rawDecrypted) { + if !am.shouldSendConfig(ctx, &decrypted) { return nil } am.metrics.ConfigSyncsTotal.Inc() if err := am.mimirClient.CreateGrafanaAlertmanagerConfig( ctx, - string(rawDecrypted), + &decrypted, config.ConfigurationHash, config.CreatedAt, config.Default, @@ -447,15 +442,25 @@ func (am *Alertmanager) getFullState(ctx context.Context) (string, error) { // shouldSendConfig compares the remote Alertmanager configuration with our local one. // It returns true if the configurations are different. -func (am *Alertmanager) shouldSendConfig(ctx context.Context, rawConfig []byte) bool { +func (am *Alertmanager) shouldSendConfig(ctx context.Context, config *apimodels.PostableUserConfig) bool { rc, err := am.mimirClient.GetGrafanaAlertmanagerConfig(ctx) if err != nil { // Log the error and return true so we try to upload our config anyway. - am.log.Error("Unable to get the remote Alertmanager Configuration for comparison", "err", err) + am.log.Error("Unable to get the remote Alertmanager configuration for comparison", "err", err) return true } - return md5.Sum([]byte(rc.GrafanaAlertmanagerConfig)) != md5.Sum(rawConfig) + rawRemote, err := json.Marshal(rc.GrafanaAlertmanagerConfig) + if err != nil { + am.log.Error("Unable to marshal the remote Alertmanager configuration for comparison", "err", err) + return true + } + rawInternal, err := json.Marshal(config) + if err != nil { + am.log.Error("Unable to marshal the internal Alertmanager configuration for comparison", "err", err) + return true + } + return md5.Sum(rawRemote) != md5.Sum(rawInternal) } // shouldSendState compares the remote Alertmanager state with our local one. diff --git a/pkg/services/ngalert/remote/alertmanager_test.go b/pkg/services/ngalert/remote/alertmanager_test.go index d11d8bf903b..947c270ac2d 100644 --- a/pkg/services/ngalert/remote/alertmanager_test.go +++ b/pkg/services/ngalert/remote/alertmanager_test.go @@ -118,7 +118,9 @@ func TestApplyConfig(t *testing.T) { if r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/config") { var c client.UserGrafanaConfig require.NoError(t, json.NewDecoder(r.Body).Decode(&c)) - configSent = c.GrafanaAlertmanagerConfig + amCfg, err := json.Marshal(c.GrafanaAlertmanagerConfig) + require.NoError(t, err) + configSent = string(amCfg) } w.WriteHeader(http.StatusOK) @@ -179,6 +181,8 @@ func TestApplyConfig(t *testing.T) { } func TestCompareAndSendConfiguration(t *testing.T) { + cfgWithSecret, err := notifier.Load([]byte(testGrafanaConfigWithSecret)) + require.NoError(t, err) testValue := []byte("test") testErr := errors.New("test error") decryptFn := func(_ context.Context, payload []byte) ([]byte, error) { @@ -243,7 +247,7 @@ func TestCompareAndSendConfiguration(t *testing.T) { "no error", strings.Replace(testGrafanaConfigWithSecret, `"password":"test"`, fmt.Sprintf("%q:%q", "password", base64.StdEncoding.EncodeToString(testValue)), 1), &client.UserGrafanaConfig{ - GrafanaAlertmanagerConfig: testGrafanaConfigWithSecret, + GrafanaAlertmanagerConfig: cfgWithSecret, }, "", }, @@ -335,7 +339,10 @@ func TestIntegrationRemoteAlertmanagerApplyConfigOnlyUploadsOnce(t *testing.T) { // Next, we need to verify that Mimir received both the configuration and state. config, err := am.mimirClient.GetGrafanaAlertmanagerConfig(ctx) require.NoError(t, err) - require.Equal(t, testGrafanaConfig, config.GrafanaAlertmanagerConfig) + + rawCfg, err := json.Marshal(config.GrafanaAlertmanagerConfig) + require.NoError(t, err) + require.JSONEq(t, testGrafanaConfig, string(rawCfg)) require.Equal(t, fakeConfigHash, config.Hash) require.Equal(t, fakeConfigCreatedAt, config.CreatedAt) require.Equal(t, true, config.Default) @@ -358,7 +365,10 @@ func TestIntegrationRemoteAlertmanagerApplyConfigOnlyUploadsOnce(t *testing.T) { // Next, we need to verify that the config that was uploaded remains the same. config, err := am.mimirClient.GetGrafanaAlertmanagerConfig(ctx) require.NoError(t, err) - require.Equal(t, testGrafanaConfig, config.GrafanaAlertmanagerConfig) + + rawCfg, err := json.Marshal(config.GrafanaAlertmanagerConfig) + require.NoError(t, err) + require.JSONEq(t, testGrafanaConfig, string(rawCfg)) require.Equal(t, fakeConfigHash, config.Hash) require.Equal(t, fakeConfigCreatedAt, config.CreatedAt) require.Equal(t, true, config.Default) diff --git a/pkg/services/ngalert/remote/client/alertmanager_configuration.go b/pkg/services/ngalert/remote/client/alertmanager_configuration.go index 6ddf6dfdaae..e0f19a8efe3 100644 --- a/pkg/services/ngalert/remote/client/alertmanager_configuration.go +++ b/pkg/services/ngalert/remote/client/alertmanager_configuration.go @@ -6,6 +6,8 @@ import ( "encoding/json" "fmt" "net/http" + + apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ) const ( @@ -13,10 +15,10 @@ const ( ) type UserGrafanaConfig struct { - GrafanaAlertmanagerConfig string `json:"configuration"` - Hash string `json:"configuration_hash"` - CreatedAt int64 `json:"created"` - Default bool `json:"default"` + GrafanaAlertmanagerConfig *apimodels.PostableUserConfig `json:"configuration"` + Hash string `json:"configuration_hash"` + CreatedAt int64 `json:"created"` + Default bool `json:"default"` } func (mc *Mimir) GetGrafanaAlertmanagerConfig(ctx context.Context) (*UserGrafanaConfig, error) { @@ -38,7 +40,7 @@ func (mc *Mimir) GetGrafanaAlertmanagerConfig(ctx context.Context) (*UserGrafana return gc, nil } -func (mc *Mimir) CreateGrafanaAlertmanagerConfig(ctx context.Context, cfg, hash string, createdAt int64, isDefault bool) error { +func (mc *Mimir) CreateGrafanaAlertmanagerConfig(ctx context.Context, cfg *apimodels.PostableUserConfig, hash string, createdAt int64, isDefault bool) error { payload, err := json.Marshal(&UserGrafanaConfig{ GrafanaAlertmanagerConfig: cfg, Hash: hash, diff --git a/pkg/services/ngalert/remote/client/mimir.go b/pkg/services/ngalert/remote/client/mimir.go index b74d1df91fe..6db6147ab99 100644 --- a/pkg/services/ngalert/remote/client/mimir.go +++ b/pkg/services/ngalert/remote/client/mimir.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/grafana/grafana/pkg/infra/log" + apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/client" "github.com/grafana/grafana/pkg/services/ngalert/metrics" ) @@ -23,7 +24,7 @@ type MimirClient interface { DeleteGrafanaAlertmanagerState(ctx context.Context) error GetGrafanaAlertmanagerConfig(ctx context.Context) (*UserGrafanaConfig, error) - CreateGrafanaAlertmanagerConfig(ctx context.Context, configuration, hash string, createdAt int64, isDefault bool) error + CreateGrafanaAlertmanagerConfig(ctx context.Context, configuration *apimodels.PostableUserConfig, hash string, createdAt int64, isDefault bool) error DeleteGrafanaAlertmanagerConfig(ctx context.Context) error } diff --git a/scripts/drone/utils/images.star b/scripts/drone/utils/images.star index b8946c746d3..da76d8ce306 100644 --- a/scripts/drone/utils/images.star +++ b/scripts/drone/utils/images.star @@ -21,7 +21,7 @@ images = { "plugins_slack": "plugins/slack", "python": "python:3.8", "postgres_alpine": "postgres:12.3-alpine", - "mimir": "us.gcr.io/kubernetes-dev/mimir:santihernandezc-remove_id_from_grafana_config-d3826b4f8-WIP", + "mimir": "us.gcr.io/kubernetes-dev/mimir:santihernandezc-validate_grafana_am_config-1e903e462-WIP", "mysql5": "mysql:5.7.39", "mysql8": "mysql:8.0.32", "redis_alpine": "redis:6.2.11-alpine",