From 2866fa8073ba8f4c9d10e6d1f514d81529420a32 Mon Sep 17 00:00:00 2001 From: Jose Fuentes Date: Tue, 28 Jan 2020 20:48:58 +0100 Subject: [PATCH 1/2] Make command to fail if missing rego definition Signed-off-by: Jose Fuentes --- cmd/check.go | 15 +++- pkg/exporter/json.go | 7 +- pkg/exporter/json_test.go | 3 +- pkg/packaging/eval.go | 7 +- pkg/reports/reports.go | 19 +++++ pkg/reports/reports_test.go | 158 +++++++++++++++++++++++------------- 6 files changed, 140 insertions(+), 69 deletions(-) diff --git a/cmd/check.go b/cmd/check.go index d1195db7..a53d1dcd 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -20,6 +20,7 @@ import ( "github.com/jetstack/preflight/pkg/output/gcs" "github.com/jetstack/preflight/pkg/packagesources/local" "github.com/jetstack/preflight/pkg/packaging" + "github.com/jetstack/preflight/pkg/reports" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -307,6 +308,7 @@ func check() { log.Fatal("No packages were enabled. Use 'enables-packages' option in configuration to enable the packages you want to use.") } + missingRules := false for _, pkgID := range enabledPackages { // Make sure we loaded the package for this. pkg := packages[pkgID] @@ -330,7 +332,12 @@ func check() { rc, err := packaging.EvalPackage(ctx, pkg, input) if err != nil { - log.Fatalf("Cannot evaluate package %q: %v", manifest.ID, err) + if _, ok := err.(*reports.MissingRegoDefinitionError); ok { + missingRules = true + log.Printf("%+v", err) + } else { + log.Fatalf("Cannot evaluate package %q: %v", manifest.ID, err) + } } intermediateBytes, err := json.Marshal(input) @@ -346,7 +353,11 @@ func check() { } } - log.Printf("Done.") + if missingRules { + log.Fatalf("Some of the rules are missing their corresponding Rego definition. See the rest of the log or the reports to see more details.") + } else { + log.Printf("Done.") + } } func homeDir() string { diff --git a/pkg/exporter/json.go b/pkg/exporter/json.go index 3b0f55f6..f1c94353 100644 --- a/pkg/exporter/json.go +++ b/pkg/exporter/json.go @@ -23,15 +23,18 @@ func NewJSONExporter() *JSONExporter { func (e *JSONExporter) Export(ctx context.Context, policyManifest *packaging.PolicyManifest, intermediateJSON []byte, rc *results.ResultCollection) (*bytes.Buffer, error) { report, err := reports.NewReport(policyManifest, rc) if err != nil { - return nil, err + if _, ok := err.(*reports.MissingRegoDefinitionError); !ok { + return nil, err + } } + missingRuleError := err b, err := json.Marshal(report) if err != nil { return nil, err } - return bytes.NewBuffer(b), nil + return bytes.NewBuffer(b), missingRuleError } // FileExtension returns the file extension for this exporter's format diff --git a/pkg/exporter/json_test.go b/pkg/exporter/json_test.go index 35d83b41..9eb8f856 100644 --- a/pkg/exporter/json_test.go +++ b/pkg/exporter/json_test.go @@ -9,6 +9,7 @@ import ( "github.com/yudai/gojsondiff/formatter" "github.com/jetstack/preflight/pkg/packaging" + "github.com/jetstack/preflight/pkg/reports" "github.com/jetstack/preflight/pkg/results" "github.com/jetstack/preflight/pkg/rules" ) @@ -178,7 +179,7 @@ func TestJSONExport(t *testing.T) { }` buf, err := jsonExporter.Export(context.Background(), pm, nil, rc) - if err != nil { + if _, ok := err.(*reports.MissingRegoDefinitionError); !ok { t.Fatalf("unexpected err: %+v", err) } diff --git a/pkg/packaging/eval.go b/pkg/packaging/eval.go index 86df9c28..8f1a2887 100644 --- a/pkg/packaging/eval.go +++ b/pkg/packaging/eval.go @@ -27,10 +27,5 @@ func EvalPackage(ctx context.Context, pkg Package, input interface{}) (*results. allResults = append(allResults, rs...) } - rc, err := results.NewResultCollectionFromRegoResultSet(&allResults) - if err != nil { - return nil, fmt.Errorf("cannot read results from rego: %s", err) - } - - return rc, nil + return results.NewResultCollectionFromRegoResultSet(&allResults) } diff --git a/pkg/reports/reports.go b/pkg/reports/reports.go index aec6d7ef..f4395e0e 100644 --- a/pkg/reports/reports.go +++ b/pkg/reports/reports.go @@ -2,6 +2,7 @@ package reports import ( "fmt" + "strings" "github.com/jetstack/preflight/api" "github.com/jetstack/preflight/pkg/packaging" @@ -91,6 +92,7 @@ func NewReport(pm *packaging.PolicyManifest, rc *results.ResultCollection) (api. Sections: make([]api.ReportSection, len(pm.Sections)), } + missingRules := []string{} for idxSection, section := range pm.Sections { report.Sections[idxSection] = api.ReportSection{ ID: section.ID, @@ -125,6 +127,7 @@ func NewReport(pm *packaging.PolicyManifest, rc *results.ResultCollection) (api. switch { case result == nil: missing = true + missingRules = append(missingRules, rule.ID) case result.IsFailureState(): success = false violations = result.Violations @@ -149,5 +152,21 @@ func NewReport(pm *packaging.PolicyManifest, rc *results.ResultCollection) (api. } } + if len(missingRules) > 0 { + return report, &MissingRegoDefinitionError{ + pkg: report.Package, + ids: missingRules, + } + } return report, nil } + +// MissingRegoDefinitionError error to be returned when a rule from the PolicyManifest was not found in Rego. +type MissingRegoDefinitionError struct { + pkg string + ids []string +} + +func (e *MissingRegoDefinitionError) Error() string { + return fmt.Sprintf("the following rules from the package %q are missing its Rego definition: %s", e.pkg, strings.Join(e.ids, ", ")) +} diff --git a/pkg/reports/reports_test.go b/pkg/reports/reports_test.go index fff015d6..16a4a224 100644 --- a/pkg/reports/reports_test.go +++ b/pkg/reports/reports_test.go @@ -245,74 +245,116 @@ func TestNewReport(t *testing.T) { ID: "b_rule", Name: "My Rule B", }, - { - ID: "c_rule", - Name: "My Rule C (missing)", - }, }, }, }, } - resultCollection := &results.ResultCollection{ - &results.Result{ID: rules.RuleToResult("a_rule"), Violations: []string{}}, - &results.Result{ID: rules.RuleToResult("b_rule"), Violations: []string{"violation"}}, - } + t.Run("returns a report", func(t *testing.T) { + resultCollection := &results.ResultCollection{ + &results.Result{ID: rules.RuleToResult("a_rule"), Violations: []string{}}, + &results.Result{ID: rules.RuleToResult("b_rule"), Violations: []string{"violation"}}, + } - got, err := NewReport(examplePackage, resultCollection) - if err != nil { - t.Fatalf("NewReport returned error: %v", err) - } + got, err := NewReport(examplePackage, resultCollection) + if err != nil { + t.Fatalf("NewReport returned error: %v", err) + } - want := api.Report{ - PreflightVersion: version.PreflightVersion, - Package: examplePackage.ID, - PackageInformation: api.PackageInformation{ - Namespace: examplePackage.Namespace, - ID: examplePackage.ID, - Version: examplePackage.PackageVersion, - SchemaVersion: examplePackage.SchemaVersion, - }, - Name: examplePackage.Name, - Description: examplePackage.Description, - Sections: []api.ReportSection{ - api.ReportSection{ - ID: "a_section", - Name: "My section", - Rules: []api.ReportRule{ - api.ReportRule{ - ID: "a_rule", - Name: "My Rule A", - Manual: false, - Success: true, - Missing: false, - Links: []string{}, - Violations: []string{}, - }, - api.ReportRule{ - ID: "b_rule", - Name: "My Rule B", - Manual: false, - Success: false, - Missing: false, - Links: []string{}, - Violations: []string{"violation"}, + want := api.Report{ + PreflightVersion: version.PreflightVersion, + Package: examplePackage.ID, + PackageInformation: api.PackageInformation{ + Namespace: examplePackage.Namespace, + ID: examplePackage.ID, + Version: examplePackage.PackageVersion, + SchemaVersion: examplePackage.SchemaVersion, + }, + Name: examplePackage.Name, + Description: examplePackage.Description, + Sections: []api.ReportSection{ + api.ReportSection{ + ID: "a_section", + Name: "My section", + Rules: []api.ReportRule{ + api.ReportRule{ + ID: "a_rule", + Name: "My Rule A", + Manual: false, + Success: true, + Missing: false, + Links: []string{}, + Violations: []string{}, + }, + api.ReportRule{ + ID: "b_rule", + Name: "My Rule B", + Manual: false, + Success: false, + Missing: false, + Links: []string{}, + Violations: []string{"violation"}, + }, }, - api.ReportRule{ - ID: "c_rule", - Name: "My Rule C (missing)", - Manual: false, - Success: false, - Missing: true, - Links: []string{}, - Violations: []string{}, + }, + }, + } + + if !reflect.DeepEqual(got, want) { + t.Fatalf("got != want; got=%+v, want=%+v", got, want) + } + }) + + t.Run("returns error MissingRegoDefinitionError if definition missing", func(t *testing.T) { + resultCollection := &results.ResultCollection{ + &results.Result{ID: rules.RuleToResult("a_rule"), Violations: []string{}}, + } + + got, err := NewReport(examplePackage, resultCollection) + if _, ok := err.(*MissingRegoDefinitionError); !ok { + t.Errorf("expected MissingRegoDefinitionError but got: %v", err) + } + + want := api.Report{ + PreflightVersion: version.PreflightVersion, + Package: examplePackage.ID, + PackageInformation: api.PackageInformation{ + Namespace: examplePackage.Namespace, + ID: examplePackage.ID, + Version: examplePackage.PackageVersion, + }, + Name: examplePackage.Name, + Description: examplePackage.Description, + Sections: []api.ReportSection{ + api.ReportSection{ + ID: "a_section", + Name: "My section", + Rules: []api.ReportRule{ + api.ReportRule{ + ID: "a_rule", + Name: "My Rule A", + Manual: false, + Success: true, + Missing: false, + Links: []string{}, + Violations: []string{}, + }, + api.ReportRule{ + ID: "b_rule", + Name: "My Rule B", + Manual: false, + Success: false, + Missing: true, + Links: []string{}, + Violations: []string{}, + }, }, }, }, - }, - } + } - if !reflect.DeepEqual(got, want) { - t.Fatalf("got != want; got=%+v, want=%+v", got, want) - } + if !reflect.DeepEqual(got, want) { + t.Fatalf("got != want; got=%+v, want=%+v", got, want) + } + }) } From 589ac40b5d03e1ce86ac20d2ce3c03f4f981b56e Mon Sep 17 00:00:00 2001 From: Jose Fuentes Castillo Date: Wed, 29 Jan 2020 15:41:32 +0100 Subject: [PATCH 2/2] Update pkg/reports/reports.go Co-Authored-By: Charlie Egan Signed-off-by: Jose Fuentes --- pkg/reports/reports.go | 2 +- pkg/reports/reports_test.go | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/reports/reports.go b/pkg/reports/reports.go index f4395e0e..cbc51a30 100644 --- a/pkg/reports/reports.go +++ b/pkg/reports/reports.go @@ -168,5 +168,5 @@ type MissingRegoDefinitionError struct { } func (e *MissingRegoDefinitionError) Error() string { - return fmt.Sprintf("the following rules from the package %q are missing its Rego definition: %s", e.pkg, strings.Join(e.ids, ", ")) + return fmt.Sprintf("the following rules from the package %q are missing their Rego definitions: %s", e.pkg, strings.Join(e.ids, ", ")) } diff --git a/pkg/reports/reports_test.go b/pkg/reports/reports_test.go index 16a4a224..02e26432 100644 --- a/pkg/reports/reports_test.go +++ b/pkg/reports/reports_test.go @@ -319,9 +319,10 @@ func TestNewReport(t *testing.T) { PreflightVersion: version.PreflightVersion, Package: examplePackage.ID, PackageInformation: api.PackageInformation{ - Namespace: examplePackage.Namespace, - ID: examplePackage.ID, - Version: examplePackage.PackageVersion, + Namespace: examplePackage.Namespace, + ID: examplePackage.ID, + Version: examplePackage.PackageVersion, + SchemaVersion: examplePackage.SchemaVersion, }, Name: examplePackage.Name, Description: examplePackage.Description,