From 8e5b6de9b24f0aed9a12dbee79689722ad3a7653 Mon Sep 17 00:00:00 2001 From: William Beuil Date: Wed, 16 Dec 2020 13:02:02 +0100 Subject: [PATCH] Add alerting --- pkg/alerter/alert.go | 8 + pkg/alerter/alerter.go | 71 ++++ pkg/alerter/alerter_test.go | 210 +++++++++++ pkg/analyser/analysis.go | 13 + pkg/analyser/analyzer.go | 69 +++- pkg/analyser/analyzer_test.go | 341 +++++++++++++++++- pkg/analyser/testdata/input.json | 9 +- pkg/analyser/testdata/output.json | 9 +- pkg/cmd/scan.go | 9 +- pkg/cmd/scan/output/console.go | 18 +- pkg/cmd/scan/output/console_test.go | 6 + pkg/cmd/scan/output/json_test.go | 8 + pkg/cmd/scan/output/output_test.go | 72 ++++ pkg/cmd/scan/output/testdata/output.json | 3 +- .../testdata/output_computed_fields.json | 91 +++++ .../testdata/output_computed_fields.txt | 14 + pkg/driftctl.go | 6 +- pkg/remote/aws/init.go | 3 +- pkg/remote/remote.go | 5 +- pkg/scanner.go | 5 +- test/resource/resource.go | 6 +- 21 files changed, 954 insertions(+), 22 deletions(-) create mode 100644 pkg/alerter/alert.go create mode 100644 pkg/alerter/alerter.go create mode 100644 pkg/alerter/alerter_test.go create mode 100644 pkg/cmd/scan/output/testdata/output_computed_fields.json create mode 100644 pkg/cmd/scan/output/testdata/output_computed_fields.txt diff --git a/pkg/alerter/alert.go b/pkg/alerter/alert.go new file mode 100644 index 00000000..1db4d8f7 --- /dev/null +++ b/pkg/alerter/alert.go @@ -0,0 +1,8 @@ +package alerter + +type Alerts map[string][]Alert + +type Alert struct { + Message string `json:"message"` + ShouldIgnoreResource bool `json:"-"` +} diff --git a/pkg/alerter/alerter.go b/pkg/alerter/alerter.go new file mode 100644 index 00000000..c87ed923 --- /dev/null +++ b/pkg/alerter/alerter.go @@ -0,0 +1,71 @@ +package alerter + +import ( + "fmt" + + "github.com/cloudskiff/driftctl/pkg/resource" +) + +type Alerter struct { + alerts Alerts + alertsCh chan Alerts + doneCh chan bool +} + +func NewAlerter() *Alerter { + var alerter = &Alerter{ + alerts: make(Alerts), + alertsCh: make(chan Alerts), + doneCh: make(chan bool), + } + + go alerter.run() + + return alerter +} + +func (a *Alerter) run() { + defer func() { a.doneCh <- true }() + for alert := range a.alertsCh { + for k, v := range alert { + if val, ok := a.alerts[k]; ok { + a.alerts[k] = append(val, v...) + } else { + a.alerts[k] = v + } + } + } +} + +func (a *Alerter) SetAlerts(alerts Alerts) { + a.alerts = alerts +} + +func (a *Alerter) GetAlerts() Alerts { + close(a.alertsCh) + <-a.doneCh + return a.alerts +} + +func (a *Alerter) SendAlert(key string, alert Alert) { + a.alertsCh <- Alerts{ + key: []Alert{alert}, + } +} + +func (a *Alerter) IsResourceIgnored(res resource.Resource) bool { + alert, alertExists := a.alerts[fmt.Sprintf("%s.%s", res.TerraformType(), res.TerraformId())] + wildcardAlert, wildcardAlertExists := a.alerts[res.TerraformType()] + shouldIgnoreAlert := a.shouldBeIgnored(alert) + shouldIgnoreWildcardAlert := a.shouldBeIgnored(wildcardAlert) + return (alertExists && shouldIgnoreAlert) || (wildcardAlertExists && shouldIgnoreWildcardAlert) +} + +func (a *Alerter) shouldBeIgnored(alert []Alert) bool { + for _, a := range alert { + if a.ShouldIgnoreResource { + return true + } + } + return false +} diff --git a/pkg/alerter/alerter_test.go b/pkg/alerter/alerter_test.go new file mode 100644 index 00000000..1ff58754 --- /dev/null +++ b/pkg/alerter/alerter_test.go @@ -0,0 +1,210 @@ +package alerter + +import ( + "reflect" + "testing" + + "github.com/cloudskiff/driftctl/pkg/resource" + resource2 "github.com/cloudskiff/driftctl/test/resource" +) + +func TestAlerter_Alert(t *testing.T) { + cases := []struct { + name string + alerts Alerts + expected Alerts + }{ + { + name: "TestNoAlerts", + alerts: nil, + expected: Alerts{}, + }, + { + name: "TestWithSingleAlert", + alerts: Alerts{ + "fakeres.foobar": []Alert{ + { + Message: "This is an alert", + ShouldIgnoreResource: false, + }, + }, + }, + expected: Alerts{ + "fakeres.foobar": []Alert{ + { + Message: "This is an alert", + ShouldIgnoreResource: false, + }, + }, + }, + }, + { + name: "TestWithMultipleAlerts", + alerts: Alerts{ + "fakeres.foobar": []Alert{ + { + Message: "This is an alert", + ShouldIgnoreResource: false, + }, + { + Message: "This is a second alert", + ShouldIgnoreResource: true, + }, + }, + "fakeres.barfoo": []Alert{ + { + Message: "This is a third alert", + ShouldIgnoreResource: true, + }, + }, + }, + expected: Alerts{ + "fakeres.foobar": []Alert{ + { + Message: "This is an alert", + ShouldIgnoreResource: false, + }, + { + Message: "This is a second alert", + ShouldIgnoreResource: true, + }, + }, + "fakeres.barfoo": []Alert{ + { + Message: "This is a third alert", + ShouldIgnoreResource: true, + }, + }, + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + alerter := NewAlerter() + + for k, v := range c.alerts { + for _, a := range v { + alerter.SendAlert(k, a) + } + } + + if eq := reflect.DeepEqual(alerter.GetAlerts(), c.expected); !eq { + t.Errorf("Got %+v, expected %+v", alerter.GetAlerts(), c.expected) + } + }) + } +} + +func TestAlerter_IgnoreResources(t *testing.T) { + cases := []struct { + name string + alerts Alerts + resource resource.Resource + expected bool + }{ + { + name: "TestNoAlerts", + alerts: Alerts{}, + resource: &resource2.FakeResource{ + Type: "fakeres", + Id: "foobar", + }, + expected: false, + }, + { + name: "TestShouldNotBeIgnoredWithAlerts", + alerts: Alerts{ + "fakeres": { + { + Message: "Should not be ignored", + }, + }, + "fakeres.foobar": { + { + Message: "Should not be ignored", + }, + }, + "fakeres.barfoo": { + { + Message: "Should not be ignored", + }, + }, + "other.resource": { + { + Message: "Should not be ignored", + }, + }, + }, + resource: &resource2.FakeResource{ + Type: "fakeres", + Id: "foobar", + }, + expected: false, + }, + { + name: "TestShouldBeIgnoredWithAlertsOnWildcard", + alerts: Alerts{ + "fakeres": { + { + Message: "Should be ignored", + ShouldIgnoreResource: true, + }, + }, + "other.foobaz": { + { + Message: "Should be ignored", + ShouldIgnoreResource: true, + }, + }, + "other.resource": { + { + Message: "Should not be ignored", + }, + }, + }, + resource: &resource2.FakeResource{ + Type: "fakeres", + Id: "foobar", + }, + expected: true, + }, + { + name: "TestShouldBeIgnoredWithAlertsOnResource", + alerts: Alerts{ + "fakeres": { + { + Message: "Should be ignored", + ShouldIgnoreResource: true, + }, + }, + "other.foobaz": { + { + Message: "Should be ignored", + ShouldIgnoreResource: true, + }, + }, + "other.resource": { + { + Message: "Should not be ignored", + }, + }, + }, + resource: &resource2.FakeResource{ + Type: "other", + Id: "foobaz", + }, + expected: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + alerter := NewAlerter() + alerter.SetAlerts(c.alerts) + if got := alerter.IsResourceIgnored(c.resource); got != c.expected { + t.Errorf("Got %+v, expected %+v", got, c.expected) + } + }) + } +} diff --git a/pkg/analyser/analysis.go b/pkg/analyser/analysis.go index 8176bc5b..e08cb733 100644 --- a/pkg/analyser/analysis.go +++ b/pkg/analyser/analysis.go @@ -3,6 +3,7 @@ package analyser import ( "encoding/json" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/resource" "github.com/r3labs/diff/v2" ) @@ -26,6 +27,7 @@ type Analysis struct { deleted []resource.Resource differences []Difference summary Summary + alerts alerter.Alerts } type serializableDifference struct { @@ -40,6 +42,7 @@ type serializableAnalysis struct { Deleted []resource.SerializableResource `json:"deleted"` Differences []serializableDifference `json:"differences"` Coverage int `json:"coverage"` + Alerts alerter.Alerts `json:"alerts"` } func (a Analysis) MarshalJSON() ([]byte, error) { @@ -61,6 +64,7 @@ func (a Analysis) MarshalJSON() ([]byte, error) { } bla.Summary = a.summary bla.Coverage = a.Coverage() + bla.Alerts = a.alerts return json.Marshal(bla) } @@ -97,6 +101,7 @@ func (a *Analysis) UnmarshalJSON(bytes []byte) error { Changelog: di.Changelog, }) } + a.AddAlerts(bla.Alerts) return nil } @@ -127,6 +132,10 @@ func (a *Analysis) AddDifference(diffs ...Difference) { a.summary.TotalDrifted += len(diffs) } +func (a *Analysis) AddAlerts(alerts alerter.Alerts) { + a.alerts = alerts +} + func (a *Analysis) Coverage() int { if a.summary.TotalResources > 0 { return int((float32(a.summary.TotalManaged) / float32(a.summary.TotalResources)) * 100.0) @@ -153,3 +162,7 @@ func (a *Analysis) Differences() []Difference { func (a *Analysis) Summary() Summary { return a.summary } + +func (a *Analysis) Alerts() alerter.Alerts { + return a.alerts +} diff --git a/pkg/analyser/analyzer.go b/pkg/analyser/analyzer.go index f7366f68..2ca27d47 100644 --- a/pkg/analyser/analyzer.go +++ b/pkg/analyser/analyzer.go @@ -1,30 +1,36 @@ package analyser import ( + "fmt" + "reflect" "sort" + "strings" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/resource" "github.com/r3labs/diff/v2" ) -type Analyzer struct{} +type Analyzer struct { + alerter *alerter.Alerter +} type Filter interface { IsResourceIgnored(res resource.Resource) bool IsFieldIgnored(res resource.Resource, path []string) bool } -func NewAnalyzer() Analyzer { - return Analyzer{} +func NewAnalyzer(alerter *alerter.Alerter) Analyzer { + return Analyzer{alerter} } -func (a Analyzer) Analyze(remoteResources []resource.Resource, resourcesFromState []resource.Resource, filter Filter) (Analysis, error) { +func (a Analyzer) Analyze(remoteResources, resourcesFromState []resource.Resource, filter Filter) (Analysis, error) { analysis := Analysis{} // Iterate on remote resources and filter ignored resources filteredRemoteResource := make([]resource.Resource, 0, len(remoteResources)) for _, remoteRes := range remoteResources { - if filter.IsResourceIgnored(remoteRes) { + if filter.IsResourceIgnored(remoteRes) || a.alerter.IsResourceIgnored(remoteRes) { continue } filteredRemoteResource = append(filteredRemoteResource, remoteRes) @@ -33,7 +39,7 @@ func (a Analyzer) Analyze(remoteResources []resource.Resource, resourcesFromStat for _, stateRes := range resourcesFromState { i, remoteRes, found := findCorrespondingRes(filteredRemoteResource, stateRes) - if filter.IsResourceIgnored(stateRes) { + if filter.IsResourceIgnored(stateRes) || a.alerter.IsResourceIgnored(stateRes) { continue } @@ -63,6 +69,7 @@ func (a Analyzer) Analyze(remoteResources []resource.Resource, resourcesFromStat Res: stateRes, Changelog: changelog, }) + a.sendAlertOnComputedField(stateRes, delta) } } } @@ -70,6 +77,8 @@ func (a Analyzer) Analyze(remoteResources []resource.Resource, resourcesFromStat // Add remaining unmanaged resources analysis.AddUnmanaged(filteredRemoteResource...) + analysis.AddAlerts(a.alerter.GetAlerts()) + return analysis, nil } @@ -88,3 +97,51 @@ func removeResourceByIndex(i int, resources []resource.Resource) []resource.Reso } return append(resources[:i], resources[i+1:]...) } + +// sendAlertOnComputedField will send an alert to a channel if the field of a delta (e.g. diff) +// has a computed tag +func (a Analyzer) sendAlertOnComputedField(stateRes resource.Resource, delta diff.Changelog) { + for _, d := range delta { + if field, ok := a.getField(reflect.TypeOf(stateRes), d.Path); ok { + if computed := field.Tag.Get("computed") == "true"; computed { + path := strings.Join(d.Path, ".") + a.alerter.SendAlert(fmt.Sprintf("%s.%s", stateRes.TerraformType(), stateRes.TerraformId()), + alerter.Alert{ + Message: fmt.Sprintf("%s is a computed field", path), + }) + } + } + } +} + +// 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) { + switch t.Kind() { + case reflect.Ptr: + return a.getField(t.Elem(), path) + case reflect.Slice: + return a.getField(t.Elem(), path[1:]) + default: + { + if field, ok := t.FieldByName(path[0]); ok && a.hasNestedFields(field.Type) { + return a.getField(field.Type, path[1:]) + } else { + return field, ok + } + } + } +} + +// hasNestedFields will return true if the current field is either a struct +// or a slice of struct +func (a Analyzer) hasNestedFields(t reflect.Type) bool { + switch t.Kind() { + case reflect.Ptr: + return a.hasNestedFields(t.Elem()) + case reflect.Slice: + return t.Elem().Kind() == reflect.Struct + default: + return t.Kind() == reflect.Struct + } +} diff --git a/pkg/analyser/analyzer_test.go b/pkg/analyser/analyzer_test.go index 9f0b60ca..22a3a132 100644 --- a/pkg/analyser/analyzer_test.go +++ b/pkg/analyser/analyzer_test.go @@ -13,6 +13,7 @@ import ( "github.com/cloudskiff/driftctl/test/goldenfile" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/resource" "github.com/r3labs/diff/v2" @@ -30,6 +31,7 @@ func TestAnalyze(t *testing.T) { res resource.Resource path []string } + alerts alerter.Alerts expected Analysis hasDrifted bool }{ @@ -174,6 +176,10 @@ func TestAnalyze(t *testing.T) { Id: "foobar", FooBar: "foobar", BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, }, }, cloud: []resource.Resource{ @@ -181,6 +187,10 @@ func TestAnalyze(t *testing.T) { Id: "foobar", FooBar: "barfoo", BarFoo: "foobar", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"bar", "baz"}, }, }, expected: Analysis{ @@ -189,6 +199,10 @@ func TestAnalyze(t *testing.T) { Id: "foobar", FooBar: "foobar", BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, }, }, summary: Summary{ @@ -202,6 +216,10 @@ func TestAnalyze(t *testing.T) { Id: "foobar", FooBar: "foobar", BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, }, Changelog: diff.Changelog{ diff.Change{ @@ -220,6 +238,34 @@ func TestAnalyze(t *testing.T) { "BarFoo", }, }, + diff.Change{ + Type: "update", + From: "baz", + To: "bar", + Path: []string{ + "Struct", + "Baz", + }, + }, + diff.Change{ + Type: "update", + From: "bar", + To: "baz", + Path: []string{ + "Struct", + "Bar", + }, + }, + }, + }, + }, + alerts: alerter.Alerts{ + "FakeResource.foobar": { + { + Message: "BarFoo is a computed field", + }, + { + Message: "Struct.Baz is a computed field", }, }, }, @@ -231,6 +277,7 @@ func TestAnalyze(t *testing.T) { iac: []resource.Resource{ &testresource.FakeResource{ Id: "foobar", + Type: "fakeres", FooBar: "foobar", BarFoo: "barfoo", }, @@ -238,6 +285,7 @@ func TestAnalyze(t *testing.T) { cloud: []resource.Resource{ &testresource.FakeResource{ Id: "foobar", + Type: "fakeres", FooBar: "barfoo", BarFoo: "foobar", }, @@ -249,6 +297,7 @@ func TestAnalyze(t *testing.T) { { res: &testresource.FakeResource{ Id: "foobar", + Type: "fakeres", FooBar: "foobar", BarFoo: "barfoo", }, @@ -259,6 +308,7 @@ func TestAnalyze(t *testing.T) { managed: []resource.Resource{ &testresource.FakeResource{ Id: "foobar", + Type: "fakeres", FooBar: "foobar", BarFoo: "barfoo", }, @@ -272,6 +322,7 @@ func TestAnalyze(t *testing.T) { { Res: &testresource.FakeResource{ Id: "foobar", + Type: "fakeres", FooBar: "foobar", BarFoo: "barfoo", }, @@ -287,6 +338,13 @@ func TestAnalyze(t *testing.T) { }, }, }, + alerts: alerter.Alerts{ + "fakeres.foobar": { + { + Message: "BarFoo is a computed field", + }, + }, + }, }, hasDrifted: true, }, @@ -351,10 +409,260 @@ func TestAnalyze(t *testing.T) { }, hasDrifted: false, }, + { + name: "TestDiffWithAlertFiltering", + iac: []resource.Resource{ + &testresource.FakeResource{ + Id: "foobar", + Type: "fakeres", + FooBar: "foobar", + BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, + }, + &testresource.FakeResource{ + Id: "barfoo", + Type: "fakeres", + FooBar: "foobar", + BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, + }, + &testresource.FakeResource{ + Id: "foobaz", + Type: "other", + FooBar: "foobar", + BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, + }, + &testresource.FakeResource{ + Id: "resource", + Type: "other", + FooBar: "foobar", + BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, + StructSlice: []struct { + String string `computed:"true"` + Array []string `computed:"true"` + }{ + {"one", []string{"foo"}}, + }, + }, + }, + cloud: []resource.Resource{ + &testresource.FakeResource{ + Id: "foobar", + Type: "fakeres", + FooBar: "barfoo", + BarFoo: "foobar", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"bar", "baz"}, + }, + &testresource.FakeResource{ + Id: "barfoo", + Type: "fakeres", + FooBar: "barfoo", + BarFoo: "foobar", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"bar", "baz"}, + }, + &testresource.FakeResource{ + Id: "foobaz", + Type: "other", + FooBar: "barfoo", + BarFoo: "foobar", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"bar", "baz"}, + }, + &testresource.FakeResource{ + Id: "resource", + Type: "other", + FooBar: "barfoo", + BarFoo: "foobar", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"bar", "baz"}, + StructSlice: []struct { + String string `computed:"true"` + Array []string `computed:"true"` + }{ + {"two", []string{"oof"}}, + }, + }, + }, + alerts: alerter.Alerts{ + "fakeres": { + { + Message: "Should be ignored", + ShouldIgnoreResource: true, + }, + }, + "other.foobaz": { + { + Message: "Should be ignored", + ShouldIgnoreResource: true, + }, + }, + "other.resource": { + { + Message: "Should not be ignored", + }, + }, + }, + expected: Analysis{ + managed: []resource.Resource{ + &testresource.FakeResource{ + Id: "resource", + Type: "other", + FooBar: "foobar", + BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, + StructSlice: []struct { + String string `computed:"true"` + Array []string `computed:"true"` + }{ + {"one", []string{"foo"}}, + }, + }, + }, + summary: Summary{ + TotalResources: 1, + TotalDrifted: 1, + TotalManaged: 1, + }, + differences: []Difference{ + { + Res: &testresource.FakeResource{ + Id: "resource", + Type: "other", + FooBar: "foobar", + BarFoo: "barfoo", + Struct: struct { + Baz string `computed:"true"` + Bar string + }{"baz", "bar"}, + StructSlice: []struct { + String string `computed:"true"` + Array []string `computed:"true"` + }{ + {"one", []string{"foo"}}, + }, + }, + Changelog: diff.Changelog{ + diff.Change{ + Type: "update", + From: "foobar", + To: "barfoo", + Path: []string{ + "FooBar", + }, + }, + diff.Change{ + Type: "update", + From: "barfoo", + To: "foobar", + Path: []string{ + "BarFoo", + }, + }, + diff.Change{ + Type: "update", + From: "baz", + To: "bar", + Path: []string{ + "Struct", + "Baz", + }, + }, + diff.Change{ + Type: "update", + From: "bar", + To: "baz", + Path: []string{ + "Struct", + "Bar", + }, + }, + diff.Change{ + Type: "update", + From: "foo", + To: "oof", + Path: []string{ + "StructSlice", + "0", + "Array", + "0", + }, + }, + diff.Change{ + Type: "update", + From: "one", + To: "two", + Path: []string{ + "StructSlice", + "0", + "String", + }, + }, + }, + }, + }, + alerts: alerter.Alerts{ + "fakeres": { + { + Message: "Should be ignored", + ShouldIgnoreResource: true, + }, + }, + "other.foobaz": { + { + Message: "Should be ignored", + ShouldIgnoreResource: true, + }, + }, + "other.resource": { + { + Message: "Should not be ignored", + }, + { + Message: "BarFoo is a computed field", + }, + { + Message: "Struct.Baz is a computed field", + }, + { + Message: "StructSlice.0.String is a computed field", + }, + { + Message: "StructSlice.0.Array.0 is a computed field", + }, + }, + }, + }, + hasDrifted: true, + }, } - analyzer := NewAnalyzer() - for _, c := range cases { t.Run(c.name, func(t *testing.T) { filter := &mocks.Filter{} @@ -368,6 +676,12 @@ func TestAnalyze(t *testing.T) { } filter.On("IsFieldIgnored", mock.Anything, mock.Anything).Return(false) + alerter := alerter.NewAlerter() + if c.alerts != nil { + alerter.SetAlerts(c.alerts) + } + + analyzer := NewAnalyzer(alerter) result, err := analyzer.Analyze(c.cloud, c.iac, filter) if err != nil { @@ -429,6 +743,15 @@ func TestAnalyze(t *testing.T) { } } + alertsChanges, err := diff.Diff(result.Alerts(), c.expected.Alerts()) + if err != nil { + t.Fatalf("Unable to compare %+v", err) + } + if len(alertsChanges) > 0 { + for _, change := range alertsChanges { + t.Errorf("%+v", change) + } + } }) } } @@ -479,6 +802,13 @@ func TestAnalysis_MarshalJSON(t *testing.T) { }, }, }) + analysis.AddAlerts(alerter.Alerts{ + "aws_iam_access_key": { + { + Message: "This is an alert", + }, + }, + }) got, err := json.MarshalIndent(analysis, "", "\t") if err != nil { @@ -552,6 +882,13 @@ func TestAnalysis_UnmarshalJSON(t *testing.T) { }, }, }, + alerts: alerter.Alerts{ + "aws_iam_access_key": { + { + Message: "This is an alert", + }, + }, + }, } got := Analysis{} diff --git a/pkg/analyser/testdata/input.json b/pkg/analyser/testdata/input.json index 070bff6d..3f69e35b 100644 --- a/pkg/analyser/testdata/input.json +++ b/pkg/analyser/testdata/input.json @@ -54,5 +54,12 @@ ] } ], - "coverage": 33 + "coverage": 33, + "alerts": { + "aws_iam_access_key": [ + { + "message": "This is an alert" + } + ] + } } \ No newline at end of file diff --git a/pkg/analyser/testdata/output.json b/pkg/analyser/testdata/output.json index 3648b745..2c86d0bb 100644 --- a/pkg/analyser/testdata/output.json +++ b/pkg/analyser/testdata/output.json @@ -54,5 +54,12 @@ ] } ], - "coverage": 33 + "coverage": 33, + "alerts": { + "aws_iam_access_key": [ + { + "message": "This is an alert" + } + ] + } } \ No newline at end of file diff --git a/pkg/cmd/scan.go b/pkg/cmd/scan.go index a620fa40..0ee03c3f 100644 --- a/pkg/cmd/scan.go +++ b/pkg/cmd/scan.go @@ -9,6 +9,7 @@ import ( "syscall" "github.com/cloudskiff/driftctl/pkg" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/cmd/scan/output" "github.com/cloudskiff/driftctl/pkg/filter" "github.com/cloudskiff/driftctl/pkg/iac/config" @@ -123,7 +124,9 @@ func scanRun(opts *ScanOptions) error { c := make(chan os.Signal) signal.Notify(c, os.Interrupt, syscall.SIGTERM) - err := remote.Activate(opts.To) + alerter := alerter.NewAlerter() + + err := remote.Activate(opts.To, alerter) if err != nil { return err } @@ -135,7 +138,7 @@ func scanRun(opts *ScanOptions) error { logrus.Trace("Exited") }() - scanner := pkg.NewScanner(resource.Suppliers()) + scanner := pkg.NewScanner(resource.Suppliers(), alerter) iacSupplier, err := supplier.GetIACSupplier(opts.From) if err != nil { @@ -146,7 +149,7 @@ func scanRun(opts *ScanOptions) error { "backend": opts.From.Backend, "path": opts.From.Path, }).Debug("Found IAC provider") - ctl := pkg.NewDriftCTL(scanner, iacSupplier, opts.Filter) + ctl := pkg.NewDriftCTL(scanner, iacSupplier, opts.Filter, alerter) go func() { <-c diff --git a/pkg/cmd/scan/output/console.go b/pkg/cmd/scan/output/console.go index aff32a41..81706e03 100644 --- a/pkg/cmd/scan/output/console.go +++ b/pkg/cmd/scan/output/console.go @@ -28,6 +28,7 @@ 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") @@ -86,13 +87,28 @@ func (c *Console) Write(analysis *analyser.Analysis) error { continue } } - fmt.Printf(" %s %s => %s\n", pref, prettify(change.From), prettify(change.To)) + fmt.Printf(" %s %s => %s", pref, prettify(change.From), prettify(change.To)) + for key, val := range analysis.Alerts() { + if fmt.Sprintf("%s.%s", humanString, difference.Res.TerraformId()) == key { + for _, a := range val { + if fmt.Sprintf("%s is a computed field", path) == a.Message { + shouldWarnOnComputedFields = true + fmt.Printf(" %s", color.YellowString("(computed)")) + } + } + } + } + fmt.Printf("\n") } } } 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")) + } + return nil } diff --git a/pkg/cmd/scan/output/console_test.go b/pkg/cmd/scan/output/console_test.go index c42cde76..6127b8b1 100644 --- a/pkg/cmd/scan/output/console_test.go +++ b/pkg/cmd/scan/output/console_test.go @@ -49,6 +49,12 @@ func TestConsole_Write(t *testing.T) { args: args{analysis: fakeAnalysisWithStringerResources()}, wantErr: false, }, + { + name: "test console output with drift on computed fields", + goldenfile: "output_computed_fields.txt", + args: args{analysis: fakeAnalysisWithComputedFields()}, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/scan/output/json_test.go b/pkg/cmd/scan/output/json_test.go index 1b845ff8..7e3ce74c 100644 --- a/pkg/cmd/scan/output/json_test.go +++ b/pkg/cmd/scan/output/json_test.go @@ -30,6 +30,14 @@ func TestJSON_Write(t *testing.T) { }, wantErr: false, }, + { + name: "test json output with drift on computed fields", + goldenfile: "output_computed_fields.json", + args: args{ + analysis: fakeAnalysisWithComputedFields(), + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/scan/output/output_test.go b/pkg/cmd/scan/output/output_test.go index 4a5270bc..b35cd66b 100644 --- a/pkg/cmd/scan/output/output_test.go +++ b/pkg/cmd/scan/output/output_test.go @@ -3,6 +3,7 @@ package output import ( "fmt" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/analyser" testresource "github.com/cloudskiff/driftctl/test/resource" "github.com/r3labs/diff/v2" @@ -148,3 +149,74 @@ func fakeAnalysisWithStringerResources() *analyser.Analysis { }}) return &a } + +func fakeAnalysisWithComputedFields() *analyser.Analysis { + a := analyser.Analysis{} + a.AddManaged( + &testresource.FakeResource{ + Id: "diff-id-1", + Type: "aws_diff_resource", + }, + ) + a.AddDifference(analyser.Difference{Res: testresource.FakeResource{ + Id: "diff-id-1", + Type: "aws_diff_resource", + }, Changelog: []diff.Change{ + { + Type: diff.UPDATE, + Path: []string{"updated", "field"}, + From: "foobar", + To: "barfoo", + }, + { + Type: diff.CREATE, + Path: []string{"new", "field"}, + From: nil, + To: "newValue", + }, + { + Type: diff.DELETE, + Path: []string{"a"}, + From: "oldValue", + To: nil, + }, + { + Type: diff.UPDATE, + From: "foo", + To: "oof", + Path: []string{ + "struct", + "0", + "array", + "0", + }, + }, + { + Type: diff.UPDATE, + From: "one", + To: "two", + Path: []string{ + "struct", + "0", + "string", + }, + }, + }}) + a.AddAlerts(alerter.Alerts{ + "aws_diff_resource.diff-id-1": []alerter.Alert{ + { + Message: "updated.field is a computed field", + }, + { + Message: "a is a computed field", + }, + { + Message: "struct.0.array.0 is a computed field", + }, + { + Message: "struct.0.string is a computed field", + }, + }, + }) + return &a +} diff --git a/pkg/cmd/scan/output/testdata/output.json b/pkg/cmd/scan/output/testdata/output.json index c634a6a7..2f829b8b 100644 --- a/pkg/cmd/scan/output/testdata/output.json +++ b/pkg/cmd/scan/output/testdata/output.json @@ -72,5 +72,6 @@ ] } ], - "coverage": 33 + "coverage": 33, + "alerts": null } \ No newline at end of file diff --git a/pkg/cmd/scan/output/testdata/output_computed_fields.json b/pkg/cmd/scan/output/testdata/output_computed_fields.json new file mode 100644 index 00000000..5fee15bc --- /dev/null +++ b/pkg/cmd/scan/output/testdata/output_computed_fields.json @@ -0,0 +1,91 @@ +{ + "summary": { + "total_resources": 1, + "total_drifted": 1, + "total_unmanaged": 0, + "total_deleted": 0, + "total_managed": 1 + }, + "managed": [ + { + "id": "diff-id-1", + "type": "aws_diff_resource" + } + ], + "unmanaged": null, + "deleted": null, + "differences": [ + { + "res": { + "id": "diff-id-1", + "type": "aws_diff_resource" + }, + "changelog": [ + { + "type": "update", + "path": [ + "updated", + "field" + ], + "from": "foobar", + "to": "barfoo" + }, + { + "type": "create", + "path": [ + "new", + "field" + ], + "from": null, + "to": "newValue" + }, + { + "type": "delete", + "path": [ + "a" + ], + "from": "oldValue", + "to": null + }, + { + "type": "update", + "path": [ + "struct", + "0", + "array", + "0" + ], + "from": "foo", + "to": "oof" + }, + { + "type": "update", + "path": [ + "struct", + "0", + "string" + ], + "from": "one", + "to": "two" + } + ] + } + ], + "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.0 is a computed field" + }, + { + "message": "struct.0.string is a computed field" + } + ] + } +} \ No newline at end of file diff --git a/pkg/cmd/scan/output/testdata/output_computed_fields.txt b/pkg/cmd/scan/output/testdata/output_computed_fields.txt new file mode 100644 index 00000000..7eb5aea7 --- /dev/null +++ b/pkg/cmd/scan/output/testdata/output_computed_fields.txt @@ -0,0 +1,14 @@ +Found drifted resources: + - diff-id-1 (aws_diff_resource): + ~ updated.field: "foobar" => "barfoo" (computed) + + new.field: => "newValue" + - a: "oldValue" => (computed) + ~ struct.0.array.0: "foo" => "oof" (computed) + ~ struct.0.string: "one" => "two" (computed) +Found 1 resource(s) + - 100% coverage + - 1 covered by IaC + - 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 diff --git a/pkg/driftctl.go b/pkg/driftctl.go index 0bc1d7d0..5d82b7c7 100644 --- a/pkg/driftctl.go +++ b/pkg/driftctl.go @@ -3,6 +3,7 @@ package pkg import ( "fmt" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/analyser" "github.com/cloudskiff/driftctl/pkg/filter" "github.com/cloudskiff/driftctl/pkg/middlewares" @@ -18,8 +19,8 @@ type DriftCTL struct { filter *jmespath.JMESPath } -func NewDriftCTL(remoteSupplier resource.Supplier, iacSupplier resource.Supplier, filter *jmespath.JMESPath) *DriftCTL { - return &DriftCTL{remoteSupplier, iacSupplier, analyser.NewAnalyzer(), filter} +func NewDriftCTL(remoteSupplier resource.Supplier, iacSupplier resource.Supplier, filter *jmespath.JMESPath, alerter *alerter.Alerter) *DriftCTL { + return &DriftCTL{remoteSupplier, iacSupplier, analyser.NewAnalyzer(alerter), filter} } func (d DriftCTL) Run() *analyser.Analysis { @@ -65,6 +66,7 @@ func (d DriftCTL) Run() *analyser.Analysis { driftIgnore := filter.NewDriftIgnore() analysis, err := d.analyzer.Analyze(remoteResources, resourcesFromState, driftIgnore) + if err != nil { logrus.Errorf("Unable to analyse resources: %+v", err) return nil diff --git a/pkg/remote/aws/init.go b/pkg/remote/aws/init.go index 954aa8f3..21b559f6 100644 --- a/pkg/remote/aws/init.go +++ b/pkg/remote/aws/init.go @@ -2,6 +2,7 @@ package aws import ( "github.com/aws/aws-sdk-go/service/iam" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/resource" "github.com/cloudskiff/driftctl/pkg/terraform" @@ -17,7 +18,7 @@ const RemoteAWSTerraform = "aws+tf" * Initialize remote (configure credentials, launch tf providers and start gRPC clients) * Required to use Scanner */ -func Init() error { +func Init(alerter *alerter.Alerter) error { provider, err := NewTerraFormProvider() if err != nil { return err diff --git a/pkg/remote/remote.go b/pkg/remote/remote.go index 9c7d0f16..2179a57d 100644 --- a/pkg/remote/remote.go +++ b/pkg/remote/remote.go @@ -3,6 +3,7 @@ package remote import ( "fmt" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/remote/aws" ) @@ -19,10 +20,10 @@ func IsSupported(remote string) bool { return false } -func Activate(remote string) error { +func Activate(remote string, alerter *alerter.Alerter) error { switch remote { case aws.RemoteAWSTerraform: - return aws.Init() + return aws.Init(alerter) default: return fmt.Errorf("unsupported remote '%s'", remote) } diff --git a/pkg/scanner.go b/pkg/scanner.go index 6bb4ae05..178e3cbc 100644 --- a/pkg/scanner.go +++ b/pkg/scanner.go @@ -6,18 +6,21 @@ import ( "github.com/sirupsen/logrus" + "github.com/cloudskiff/driftctl/pkg/alerter" "github.com/cloudskiff/driftctl/pkg/resource" ) type Scanner struct { resourceSuppliers []resource.Supplier runner *ParallelRunner + alerter *alerter.Alerter } -func NewScanner(resourceSuppliers []resource.Supplier) *Scanner { +func NewScanner(resourceSuppliers []resource.Supplier, alerter *alerter.Alerter) *Scanner { return &Scanner{ resourceSuppliers: resourceSuppliers, runner: NewParallelRunner(context.TODO(), 10), + alerter: alerter, } } diff --git a/test/resource/resource.go b/test/resource/resource.go index b40a41b8..6de537bb 100644 --- a/test/resource/resource.go +++ b/test/resource/resource.go @@ -5,7 +5,7 @@ import "fmt" type FakeResource struct { Id string FooBar string - BarFoo string + BarFoo string `computed:"true"` Json string `jsonstring:"true"` Type string Tags map[string]string @@ -17,6 +17,10 @@ type FakeResource struct { Baz string `computed:"true"` Bar string } + StructSlice []struct { + String string `computed:"true"` + Array []string `computed:"true"` + } } func (d FakeResource) TerraformId() string {