Skip to content

Commit

Permalink
only send one alert for computed fields
Browse files Browse the repository at this point in the history
  • Loading branch information
Martin Guibert committed Jan 22, 2021
1 parent 7db31a1 commit e470757
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 56 deletions.
30 changes: 8 additions & 22 deletions pkg/analyser/analyzer.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package analyser

import (
"fmt"
"reflect"
"sort"

Expand Down Expand Up @@ -35,7 +34,7 @@ func (a Analyzer) Analyze(remoteResources, resourcesFromState []resource.Resourc
filteredRemoteResource = append(filteredRemoteResource, remoteRes)
}

nbComputed := 0
haveComputedDiff := false
for _, stateRes := range resourcesFromState {
i, remoteRes, found := findCorrespondingRes(filteredRemoteResource, stateRes)

Expand Down Expand Up @@ -65,7 +64,7 @@ func (a Analyzer) Analyze(remoteResources, resourcesFromState []resource.Resourc
c := Change{Change: change}
c.Computed = a.isComputedField(stateRes, c)
if c.Computed {
nbComputed++
haveComputedDiff = true
}
changelog = append(changelog, c)
}
Expand All @@ -75,8 +74,12 @@ func (a Analyzer) Analyze(remoteResources, resourcesFromState []resource.Resourc
Changelog: changelog,
})
}

a.sendAlertOnComputedField(nbComputed)
}
if haveComputedDiff {
a.alerter.SendAlert("",
alerter.Alert{
Message: "You have diffs on computed fields, check the documentation for potential false positive drifts",
})
}
}

Expand Down Expand Up @@ -113,23 +116,6 @@ func (a Analyzer) isComputedField(stateRes resource.Resource, change Change) boo
return false
}

// sendAlertOnComputedField will send an alert if we found computed fields
func (a Analyzer) sendAlertOnComputedField(nbComputed int) {
if nbComputed <= 0 {
return // no computed field
}

computedFields := "1 computed field"
if nbComputed > 1 {
computedFields = fmt.Sprintf("%d computed fields", nbComputed)
}

a.alerter.SendAlert("*",
alerter.Alert{
Message: fmt.Sprintf("You have diffs on %s, check the documentation for potential false positive drifts", computedFields),
})
}

// getField recursively finds the deepest field inside a resource depending on
// its path and its type
func (a Analyzer) getField(t reflect.Type, path []string) (reflect.StructField, bool) {
Expand Down
12 changes: 6 additions & 6 deletions pkg/analyser/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ func TestAnalyze(t *testing.T) {
},
},
alerts: alerter.Alerts{
"*": {
"": {
{
Message: "You have diffs on 2 computed fields, check the documentation for potential false positive drifts",
Message: "You have diffs on computed fields, check the documentation for potential false positive drifts",
},
},
},
Expand Down Expand Up @@ -349,9 +349,9 @@ func TestAnalyze(t *testing.T) {
},
},
alerts: alerter.Alerts{
"*": {
"": {
{
Message: "You have diffs on 1 computed field, check the documentation for potential false positive drifts",
Message: "You have diffs on computed fields, check the documentation for potential false positive drifts",
},
},
},
Expand Down Expand Up @@ -671,9 +671,9 @@ func TestAnalyze(t *testing.T) {
Message: "Should not be ignored",
},
},
"*": {
"": {
{
Message: "You have diffs on 4 computed fields, check the documentation for potential false positive drifts",
Message: "You have diffs on computed fields, check the documentation for potential false positive drifts",
},
},
},
Expand Down
9 changes: 4 additions & 5 deletions pkg/cmd/scan/output/console.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ func NewConsole() *Console {
}

func (c *Console) Write(analysis *analyser.Analysis) error {
var shouldWarnOnComputedFields bool

if analysis.Summary().TotalDeleted > 0 {
fmt.Printf("Found deleted resources:\n")
deletedByType := groupByType(analysis.Deleted())
Expand Down Expand Up @@ -89,7 +87,6 @@ func (c *Console) Write(analysis *analyser.Analysis) error {
}
fmt.Printf(" %s %s => %s", pref, prettify(change.From), prettify(change.To))
if change.Computed {
shouldWarnOnComputedFields = true
fmt.Printf(" %s", color.YellowString("(computed)"))
}
fmt.Printf("\n")
Expand All @@ -99,8 +96,10 @@ func (c *Console) Write(analysis *analyser.Analysis) error {

c.writeSummary(analysis)

if shouldWarnOnComputedFields {
fmt.Printf("%s\n", color.YellowString("You have diffs on computed field, check the documentation for potential false positive drifts"))
for _, alerts := range analysis.Alerts() {
for _, alert := range alerts {
fmt.Printf("%s\n", color.YellowString(alert.Message))
}
}

return nil
Expand Down
13 changes: 2 additions & 11 deletions pkg/cmd/scan/output/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,18 +229,9 @@ func fakeAnalysisWithComputedFields() *analyser.Analysis {
},
}})
a.SetAlerts(alerter.Alerts{
"aws_diff_resource.diff-id-1": []alerter.Alert{
"": []alerter.Alert{
{
Message: "updated.field is a computed field",
},
{
Message: "a is a computed field",
},
{
Message: "struct.0.array is a computed field",
},
{
Message: "struct.0.string is a computed field",
Message: "You have diffs on computed fields, check the documentation for potential false positive drifts",
},
},
})
Expand Down
13 changes: 2 additions & 11 deletions pkg/cmd/scan/output/testdata/output_computed_fields.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,9 @@
],
"coverage": 100,
"alerts": {
"aws_diff_resource.diff-id-1": [
"": [
{
"message": "updated.field is a computed field"
},
{
"message": "a is a computed field"
},
{
"message": "struct.0.array is a computed field"
},
{
"message": "struct.0.string is a computed field"
"message": "You have diffs on computed fields, check the documentation for potential false positive drifts"
}
]
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/scan/output/testdata/output_computed_fields.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ Found 1 resource(s)
- 0 not covered by IaC
- 0 deleted on cloud provider
- 1/1 drifted from IaC
You have diffs on computed field, check the documentation for potential false positive drifts
You have diffs on computed fields, check the documentation for potential false positive drifts

0 comments on commit e470757

Please sign in to comment.