[v10.0.x] Alerting: Fix provenance guard checks for Alertmanager configuration to not cause panic when compared nested objects (#69094)

Alerting: Fix provenance guard checks for Alertmanager configuration to not cause panic when compared nested objects (#69009)

* fix current settings parsed as new
* replace map comparison with cmp.Diff and log the diff

(cherry picked from commit e00260465b)

Co-authored-by: Yuri Tseretyan <yuriy.tseretyan@grafana.com>
This commit is contained in:
Grot (@grafanabot)
2023-05-25 18:56:01 +03:00
committed by GitHub
co-authored by Yuri Tseretyan
parent e5153d7bf3
commit 085adb57ce
2 changed files with 29 additions and 18 deletions
@@ -9,6 +9,7 @@ import (
amConfig "github.com/prometheus/alertmanager/config" amConfig "github.com/prometheus/alertmanager/config"
"github.com/prometheus/alertmanager/pkg/labels" "github.com/prometheus/alertmanager/pkg/labels"
"github.com/grafana/grafana/pkg/infra/log"
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/util/cmputil" "github.com/grafana/grafana/pkg/util/cmputil"
@@ -21,7 +22,7 @@ func (srv AlertmanagerSrv) provenanceGuard(currentConfig apimodels.GettableUserC
if err := checkTemplates(currentConfig, newConfig); err != nil { if err := checkTemplates(currentConfig, newConfig); err != nil {
return err return err
} }
if err := checkContactPoints(currentConfig.AlertmanagerConfig.Receivers, newConfig.AlertmanagerConfig.Receivers); err != nil { if err := checkContactPoints(srv.log, currentConfig.AlertmanagerConfig.Receivers, newConfig.AlertmanagerConfig.Receivers); err != nil {
return err return err
} }
if err := checkMuteTimes(currentConfig, newConfig); err != nil { if err := checkMuteTimes(currentConfig, newConfig); err != nil {
@@ -67,7 +68,7 @@ func checkTemplates(currentConfig apimodels.GettableUserConfig, newConfig apimod
return nil return nil
} }
func checkContactPoints(currReceivers []*apimodels.GettableApiReceiver, newReceivers []*apimodels.PostableApiReceiver) error { func checkContactPoints(l log.Logger, currReceivers []*apimodels.GettableApiReceiver, newReceivers []*apimodels.PostableApiReceiver) error {
newCPs := make(map[string]*apimodels.PostableGrafanaReceiver) newCPs := make(map[string]*apimodels.PostableGrafanaReceiver)
for _, postedReceiver := range newReceivers { for _, postedReceiver := range newReceivers {
for _, postedContactPoint := range postedReceiver.GrafanaManagedReceivers { for _, postedContactPoint := range postedReceiver.GrafanaManagedReceivers {
@@ -104,21 +105,14 @@ func checkContactPoints(currReceivers []*apimodels.GettableApiReceiver, newRecei
return err return err
} }
newSettings := map[string]interface{}{} newSettings := map[string]interface{}{}
err = json.Unmarshal(contactPoint.Settings, &newSettings) err = json.Unmarshal(postedContactPoint.Settings, &newSettings)
if err != nil { if err != nil {
return err return err
} }
if err != nil { d := cmp.Diff(existingSettings, newSettings)
return err if len(d) > 0 {
} l.Warn("Settings of contact point with provenance status cannot be changed via regular API.", "contactPoint", postedContactPoint.Name, "settingsDiff", d, "error", editErr)
for key, val := range existingSettings { return editErr
if newVal, present := newSettings[key]; present {
if val != newVal {
return editErr
}
} else {
return editErr
}
} }
} }
} }
@@ -9,6 +9,7 @@ import (
"github.com/prometheus/common/model" "github.com/prometheus/common/model"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/log/logtest"
"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
"github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/models"
) )
@@ -277,7 +278,7 @@ func TestCheckContactPoints(t *testing.T) {
}, },
}, },
{ {
name: "editing a provisioned object should fail", name: "editing secure settings of a provisioned object should fail",
shouldErr: true, shouldErr: true,
currentConfig: []*definitions.GettableApiReceiver{ currentConfig: []*definitions.GettableApiReceiver{
defaultGettableReceiver(t, "test-1", models.ProvenanceAPI), defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
@@ -292,10 +293,24 @@ func TestCheckContactPoints(t *testing.T) {
}(), }(),
}, },
}, },
{
name: "editing settings of a provisioned object should fail",
shouldErr: true,
currentConfig: []*definitions.GettableApiReceiver{
defaultGettableReceiver(t, "test-1", models.ProvenanceAPI),
},
newConfig: []*definitions.PostableApiReceiver{
func() *definitions.PostableApiReceiver {
receiver := defaultPostableReceiver(t, "test-1")
receiver.GrafanaManagedReceivers[0].Settings = definitions.RawMessage(`{ "hello": "data", "data": { "test": "test"}}`)
return receiver
}(),
},
},
} }
for _, test := range tests { for _, test := range tests {
t.Run(test.name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
err := checkContactPoints(test.currentConfig, test.newConfig) err := checkContactPoints(&logtest.Fake{}, test.currentConfig, test.newConfig)
if test.shouldErr { if test.shouldErr {
require.Error(t, err) require.Error(t, err)
} else { } else {
@@ -320,7 +335,8 @@ func defaultGettableReceiver(t *testing.T, uid string, provenance models.Provena
"url": true, "url": true,
}, },
Settings: definitions.RawMessage(`{ Settings: definitions.RawMessage(`{
"hello": "world" "hello": "world",
"data": {}
}`), }`),
}, },
}, },
@@ -339,7 +355,8 @@ func defaultPostableReceiver(t *testing.T, uid string) *definitions.PostableApiR
Type: "slack", Type: "slack",
DisableResolveMessage: true, DisableResolveMessage: true,
Settings: definitions.RawMessage(`{ Settings: definitions.RawMessage(`{
"hello": "world" "hello": "world",
"data" : {}
}`), }`),
}, },
}, },