Skip to content

Commit

Permalink
[v9.5.x] Alerting: Fix provenance guard checks for Alertmanager confi…
Browse files Browse the repository at this point in the history
…guration to not cause panic when compared nested objects (#69092)

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 e002604)

Co-authored-by: Yuri Tseretyan <yuriy.tseretyan@grafana.com>
  • Loading branch information
grafanabot and yuri-tceretian committed May 25, 2023
1 parent b153b4e commit adbcf7d
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 18 deletions.
22 changes: 8 additions & 14 deletions pkg/services/ngalert/api/api_alertmanager_guards.go
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}
}
Expand Down
25 changes: 21 additions & 4 deletions pkg/services/ngalert/api/api_alertmanager_guards_test.go
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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),
Expand All @@ -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 {
Expand All @@ -320,7 +335,8 @@ func defaultGettableReceiver(t *testing.T, uid string, provenance models.Provena
"url": true,
},
Settings: definitions.RawMessage(`{
"hello": "world"
"hello": "world",
"data": {}
}`),
},
},
Expand All @@ -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" : {}
}`),
},
},
Expand Down

0 comments on commit adbcf7d

Please sign in to comment.