Skip to content

Commit

Permalink
CLOUDP-241826: Fix Alert Config translation panic (#2838)
Browse files Browse the repository at this point in the history 
Signed-off-by: jose.vazquez <[email protected]>
Co-authored-by: Sergiusz Urbaniak <[email protected]>
Co-authored-by: Gustavo Bazan <[email protected]>
  • Loading branch information
@josvazg @s-urbaniak @gssbzn
3 people authored Apr 11, 2024
1 parent 0ab26fe commit 6f46574
Show file tree
Hide file tree
Showing 4 changed files with 194 additions and 47 deletions.
6 changes: 3 additions & 3 deletions internal/cli/alerts/settings/settings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ func (opts *ConfigOpts) newMetricThreshold() *atlasv2.ServerlessMetricThreshold

func (opts *ConfigOpts) newMatcher() map[string]interface{} {
result := make(map[string]interface{})
result["FieldName"] = strings.ToUpper(opts.matcherFieldName)
result["Operator"] = strings.ToUpper(opts.matcherOperator)
result["Value"] = strings.ToUpper(opts.matcherValue)
result["fieldName"] = strings.ToUpper(opts.matcherFieldName)
result["operator"] = strings.ToUpper(opts.matcherOperator)
result["value"] = strings.ToUpper(opts.matcherValue)
return result
}

Expand Down
112 changes: 77 additions & 35 deletions internal/kubernetes/operator/project/project.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
package project

import (
"errors"
"fmt"
"strings"

"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/kubernetes/operator/features"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/kubernetes/operator/resources"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/kubernetes/operator/secrets"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/log"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/pointer"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/store"
akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1"
Expand Down Expand Up @@ -683,41 +685,6 @@ func buildAlertConfigurations(acProvider store.AlertConfigurationLister, project
return nil, nil, err
}

convertMatchers := func(atlasMatcher []map[string]interface{}) []akov2.Matcher {
var res []akov2.Matcher
for _, m := range atlasMatcher {
res = append(res, akov2.Matcher{
FieldName: (m["FieldName"]).(string),
Operator: (m["Operator"]).(string),
Value: (m["Value"]).(string),
})
}
return res
}

convertMetricThreshold := func(atlasMT *atlasv2.ServerlessMetricThreshold) *akov2.MetricThreshold {
if atlasMT == nil {
return &akov2.MetricThreshold{}
}
return &akov2.MetricThreshold{
MetricName: atlasMT.MetricName,
Operator: store.StringOrEmpty(atlasMT.Operator),
Threshold: fmt.Sprintf("%f", pointer.GetOrDefault(atlasMT.Threshold, 0.0)),
Units: store.StringOrEmpty(atlasMT.Units),
Mode: store.StringOrEmpty(atlasMT.Mode),
}
}

convertThreshold := func(atlasT *atlasv2.GreaterThanRawThreshold) *akov2.Threshold {
if atlasT == nil {
return &akov2.Threshold{}
}
return &akov2.Threshold{
Operator: store.StringOrEmpty(atlasT.Operator),
Units: store.StringOrEmpty(atlasT.Units),
Threshold: fmt.Sprintf("%d", pointer.GetOrDefault(atlasT.Threshold, 0)),
}
}
convertNotifications := func(atlasNotifications []atlasv2.AlertsNotificationRootForGroup) ([]akov2.Notification, []*corev1.Secret) {
var (
akoNotifications []akov2.Notification
Expand Down Expand Up @@ -835,6 +802,81 @@ func buildAlertConfigurations(acProvider store.AlertConfigurationLister, project
return results, secretResults, nil
}

func convertMatchers(atlasMatcher []map[string]interface{}) []akov2.Matcher {
res := make([]akov2.Matcher, 0, len(atlasMatcher))
for _, m := range atlasMatcher {
matcher, err := toMatcher(m)
if err != nil {
_, _ = log.Warningf("Skipping matcher %v, conversion failed: %v\n", m, err.Error())
continue
}
res = append(res, matcher)
}
return res
}

func toMatcher(m map[string]interface{}) (akov2.Matcher, error) {
var matcher akov2.Matcher
if len(m) == 0 {
return matcher, errors.New("empty map cannot be converted to Matcher")
}
fieldName, err := keyAsString(m, "fieldName")
if err != nil {
return matcher, err
}
operator, err := keyAsString(m, "operator")
if err != nil {
return matcher, err
}
value, err := keyAsString(m, "value")
if err != nil {
return matcher, err
}
matcher.FieldName = fieldName
matcher.Operator = operator
matcher.Value = value
return matcher, nil
}

func keyAsString(m map[string]interface{}, key string) (string, error) {
v, ok := m[key]
if !ok {
return "", fmt.Errorf("no key %q in %v", key, m)
}
if v == nil {
return "", fmt.Errorf("%q is unset in %v", key, m)
}
s, ok := (v).(string)
if !ok {
return "", fmt.Errorf("%q is not a string in %v", key, m)
}
return s, nil
}

func convertMetricThreshold(atlasMT *atlasv2.ServerlessMetricThreshold) *akov2.MetricThreshold {
if atlasMT == nil {
return &akov2.MetricThreshold{}
}
return &akov2.MetricThreshold{
MetricName: atlasMT.MetricName,
Operator: store.StringOrEmpty(atlasMT.Operator),
Threshold: fmt.Sprintf("%f", atlasMT.GetThreshold()),
Units: atlasMT.GetUnits(),
Mode: atlasMT.GetMode(),
}
}

func convertThreshold(atlasT *atlasv2.GreaterThanRawThreshold) *akov2.Threshold {
if atlasT == nil {
return &akov2.Threshold{}
}
return &akov2.Threshold{
Operator: store.StringOrEmpty(atlasT.Operator),
Units: store.StringOrEmpty(atlasT.Units),
Threshold: fmt.Sprintf("%d", atlasT.GetThreshold()),
}
}

func generateName(base string) string {
return k8snames.SimpleNameGenerator.GenerateName(base)
}
Expand Down
102 changes: 96 additions & 6 deletions internal/kubernetes/operator/project/project_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package project

import (
"fmt"
"log"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -166,9 +167,9 @@ func TestBuildAtlasProject(t *testing.T) {
EventTypeName: pointer.Get("TestEventTypeName"),
Matchers: &[]map[string]interface{}{
{
"FieldName": "TestFieldName",
"Operator": "TestOperator",
"Value": "TestValue",
"fieldName": "TestFieldName",
"operator": "TestOperator",
"value": "TestValue",
},
},
MetricThreshold: &atlasv2.ServerlessMetricThreshold{
Expand Down Expand Up @@ -327,9 +328,9 @@ func TestBuildAtlasProject(t *testing.T) {
}
expectedMatchers := []akov2.Matcher{
{
FieldName: (alertConfigs[0].GetMatchers()[0]["FieldName"]).(string),
Operator: (alertConfigs[0].GetMatchers()[0]["Operator"]).(string),
Value: (alertConfigs[0].GetMatchers()[0]["Value"]).(string),
FieldName: (alertConfigs[0].GetMatchers()[0]["fieldName"]).(string),
Operator: (alertConfigs[0].GetMatchers()[0]["operator"]).(string),
Value: (alertConfigs[0].GetMatchers()[0]["value"]).(string),
},
}
expectedNotifications := []akov2.Notification{
Expand Down Expand Up @@ -1326,3 +1327,92 @@ func Test_firstElementOrEmpty(t *testing.T) {
assert.Equal(t, "1", firstElementOrZeroValue([]string{"1", "2", "3"}))
})
}

func TestToMatcherErrors(t *testing.T) {
testCases := []struct {
title string
m map[string]interface{}
expectedErrorMsg string
}{
{
title: "Nil map renders nil map error",
m: nil,
expectedErrorMsg: "empty map cannot be converted to Matcher",
},
{
title: "Empty map renders nil map error",
m: map[string]interface{}{},
expectedErrorMsg: "empty map cannot be converted to Matcher",
},
{
title: "Missing fieldName renders key not found error",
m: map[string]interface{}{"blah": 1},
expectedErrorMsg: "no key \"fieldName\"",
},
{
title: "Misnamed fieldName renders key not found error",
m: map[string]interface{}{"FieldName": 1},
expectedErrorMsg: "no key \"fieldName\"",
},
{
title: "Nil fieldName value renders conversion error",
m: map[string]interface{}{"fieldName": nil},
expectedErrorMsg: "\"fieldName\" is unset",
},
{
title: "No string fieldName renders conversion error",
m: map[string]interface{}{"fieldName": 1},
expectedErrorMsg: "\"fieldName\" is not a string",
},
{
title: "Missing operator renders key not found error",
m: map[string]interface{}{"fieldName": "blah"},
expectedErrorMsg: "no key \"operator\"",
},
{
title: "Non string operator renders conversion error",
m: map[string]interface{}{"fieldName": "blah", "operator": 1},
expectedErrorMsg: "\"operator\" is not a string",
},
{
title: "Missing value renders key not found error",
m: map[string]interface{}{"fieldName": "blah", "operator": "op"},
expectedErrorMsg: "no key \"value\"",
},
{
title: "Non string value renders conversion error",
m: map[string]interface{}{"fieldName": "blah", "operator": "op", "value": 0},
expectedErrorMsg: "\"value\" is not a string",
},
}
for _, tc := range testCases {
t.Run(tc.title, func(t *testing.T) {
_, err := toMatcher(tc.m)
log.Printf("err=%v", err)
assert.ErrorContains(t, err, tc.expectedErrorMsg)
})
}
}

func TestConvertMatchers(t *testing.T) {
maps := []map[string]interface{}{
nil,
{},
{"field": 1},
{"FieldName": 1},
{"fieldName": 1},
{"fieldName": nil},
{"fieldName": "field"},
{"fieldName": "field", "operator": 1},
{"fieldName": "field", "operator": "op"},
{"fieldName": "field", "operator": "op", "value": 0},
{"fieldName": "field", "operator": "op", "value": "value"},
{"fieldName": "field2", "operator": "op2", "value": "other-value"},
}
expected := []akov2.Matcher{
{FieldName: "field", Operator: "op", Value: "value"},
{FieldName: "field2", Operator: "op2", Value: "other-value"},
}
matchers := convertMatchers(maps)
assert.Equal(t, expected, matchers)
}
21 changes: 18 additions & 3 deletions test/e2e/atlas/kubernetes_config_generate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func TestProjectWithNonDefaultAlertConf(t *testing.T) {
newAlertConfig := akov2.AlertConfiguration{
Threshold: &akov2.Threshold{},
MetricThreshold: &akov2.MetricThreshold{},
EventTypeName: eventTypeName,
EventTypeName: "HOST_DOWN",
Enabled: true,
Notifications: []akov2.Notification{
{
Expand Down Expand Up @@ -270,6 +270,13 @@ func TestProjectWithNonDefaultAlertConf(t *testing.T) {
},
},
},
Matchers: []akov2.Matcher{
{
FieldName: "HOSTNAME",
Operator: "CONTAINS",
Value: "some-name",
},
},
}
expectedProject.Spec.AlertConfigurations = []akov2.AlertConfiguration{
newAlertConfig,
Expand All @@ -291,8 +298,11 @@ func TestProjectWithNonDefaultAlertConf(t *testing.T) {
strconv.Itoa(newAlertConfig.Notifications[0].IntervalMin),
"--notificationDelayMin",
strconv.Itoa(*newAlertConfig.Notifications[0].DelayMin),
fmt.Sprintf("--notificationSmsEnabled=%s", strconv.FormatBool(*newAlertConfig.Notifications[0].SMSEnabled)),
fmt.Sprintf("--notificationEmailEnabled=%s", strconv.FormatBool(*newAlertConfig.Notifications[0].EmailEnabled)),
fmt.Sprintf("--notificationSmsEnabled=%s", strconv.FormatBool(pointer.GetOrZero(newAlertConfig.Notifications[0].SMSEnabled))),
fmt.Sprintf("--notificationEmailEnabled=%s", strconv.FormatBool(pointer.GetOrZero(newAlertConfig.Notifications[0].EmailEnabled))),
fmt.Sprintf("--matcherFieldName=%s", newAlertConfig.Matchers[0].FieldName),
fmt.Sprintf("--matcherOperator=%s", newAlertConfig.Matchers[0].Operator),
fmt.Sprintf("--matcherValue=%s", newAlertConfig.Matchers[0].Value),
"-o=json")
cmd.Env = os.Environ()
_, err := cmd.CombinedOutput()
Expand Down Expand Up @@ -890,6 +900,11 @@ func checkProject(t *testing.T, output []runtime.Object, expected *akov2.AtlasPr
expected.Spec.AlertConfigurations[i].Notifications[j].ServiceKeyRef = p.Spec.AlertConfigurations[i].Notifications[j].ServiceKeyRef
expected.Spec.AlertConfigurations[i].Notifications[j].VictorOpsSecretRef = p.Spec.AlertConfigurations[i].Notifications[j].VictorOpsSecretRef
}
for k := range alertConfig.Matchers {
expected.Spec.AlertConfigurations[i].Matchers[k].FieldName = p.Spec.AlertConfigurations[i].Matchers[k].FieldName
expected.Spec.AlertConfigurations[i].Matchers[k].Operator = p.Spec.AlertConfigurations[i].Matchers[k].Operator
expected.Spec.AlertConfigurations[i].Matchers[k].Value = p.Spec.AlertConfigurations[i].Matchers[k].Value
}
}

asserts.Equal(expected, p)
Expand Down

0 comments on commit 6f46574

Please sign in to comment.