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
pull/67688/head^2
Yuri Tseretyan 2 years ago committed by GitHub
parent 9a5262d5c8
commit e00260465b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 22
      pkg/services/ngalert/api/api_alertmanager_guards.go
  2. 25
      pkg/services/ngalert/api/api_alertmanager_guards_test.go

@ -9,6 +9,7 @@ import (
amConfig "github.com/prometheus/alertmanager/config"
"github.com/prometheus/alertmanager/pkg/labels"
"github.com/grafana/grafana/pkg/infra/log"
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
"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 {
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
}
if err := checkMuteTimes(currentConfig, newConfig); err != nil {
@ -67,7 +68,7 @@ func checkTemplates(currentConfig apimodels.GettableUserConfig, newConfig apimod
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)
for _, postedReceiver := range newReceivers {
for _, postedContactPoint := range postedReceiver.GrafanaManagedReceivers {
@ -104,21 +105,14 @@ func checkContactPoints(currReceivers []*apimodels.GettableApiReceiver, newRecei
return err
}
newSettings := map[string]interface{}{}
err = json.Unmarshal(contactPoint.Settings, &newSettings)
err = json.Unmarshal(postedContactPoint.Settings, &newSettings)
if err != nil {
return err
}
if err != nil {
return err
}
for key, val := range existingSettings {
if newVal, present := newSettings[key]; present {
if val != newVal {
return editErr
}
} else {
return editErr
}
d := cmp.Diff(existingSettings, newSettings)
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)
return editErr
}
}
}

@ -9,6 +9,7 @@ import (
"github.com/prometheus/common/model"
"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/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,
currentConfig: []*definitions.GettableApiReceiver{
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 {
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 {
require.Error(t, err)
} else {
@ -320,7 +335,8 @@ func defaultGettableReceiver(t *testing.T, uid string, provenance models.Provena
"url": true,
},
Settings: definitions.RawMessage(`{
"hello": "world"
"hello": "world",
"data": {}
}`),
},
},
@ -339,7 +355,8 @@ func defaultPostableReceiver(t *testing.T, uid string) *definitions.PostableApiR
Type: "slack",
DisableResolveMessage: true,
Settings: definitions.RawMessage(`{
"hello": "world"
"hello": "world",
"data" : {}
}`),
},
},

Loading…
Cancel
Save