From 346737027775cf76265945fe7168c92d67babb15 Mon Sep 17 00:00:00 2001 From: Hamish Hutchings Date: Fri, 25 Jun 2021 12:27:15 +0200 Subject: [PATCH 1/5] Initial Commit on fine-grained excludes Signed-off-by: Hamish Hutchings --- examples/exceptions2/main.tf | 8 ++++++++ examples/exceptions2/policy/deny.rego | 29 +++++++++++++++++++++++++++ output/result.go | 1 + output/standard.go | 4 ++++ policy/engine.go | 17 ++++++++++++++++ 5 files changed, 59 insertions(+) create mode 100644 examples/exceptions2/main.tf create mode 100644 examples/exceptions2/policy/deny.rego diff --git a/examples/exceptions2/main.tf b/examples/exceptions2/main.tf new file mode 100644 index 000000000..0f092e947 --- /dev/null +++ b/examples/exceptions2/main.tf @@ -0,0 +1,8 @@ + + + +resource "null_resource" "exception-name" {} + +resource "null_resource" "invalid-name" {} + +resource "invalid_type" "valid_name" {} diff --git a/examples/exceptions2/policy/deny.rego b/examples/exceptions2/policy/deny.rego new file mode 100644 index 000000000..ee7c8df22 --- /dev/null +++ b/examples/exceptions2/policy/deny.rego @@ -0,0 +1,29 @@ +package main + +exceptions = {"exception-name"} + +func_name_msg(name) = ret { + ret := sprintf("Resource Name '%s' contains dashes", [name]) +} + +deny_name[msg] { + input.resource[_][name] + contains(name, "-") + msg := func_name_msg(name) +} + +deny_resource_type[msg] { + input.resource[type] + type == "invalid_type" + msg := sprintf("Resource Type '%s' is invalid", [type]) +} + +exclude_name[rules] { + input.resource[_][name] + exceptions[name] + rules := [func_name_msg(name)] +} + +exception[rules] { + rules := ["resource_type"] +} diff --git a/output/result.go b/output/result.go index 1f4f1828a..b06f0f86b 100644 --- a/output/result.go +++ b/output/result.go @@ -77,6 +77,7 @@ type CheckResult struct { Warnings []Result `json:"warnings,omitempty"` Failures []Result `json:"failures,omitempty"` Exceptions []Result `json:"exceptions,omitempty"` + Excludes []Result `json:"excludes,omitempty"` Queries []QueryResult `json:"queries,omitempty"` } diff --git a/output/standard.go b/output/standard.go index 73b5beae3..48eea994d 100644 --- a/output/standard.go +++ b/output/standard.go @@ -89,6 +89,10 @@ func (s *Standard) Output(results []CheckResult) error { } } + for _, exclude := range result.Excludes { + fmt.Fprintln(s.Writer, colorizer.Colorize("EXCL", aurora.BlueFg), indicator, namespace, exclude.Message) + } + totalFailures += len(result.Failures) totalExceptions += len(result.Exceptions) totalWarnings += len(result.Warnings) diff --git a/policy/engine.go b/policy/engine.go index 4e888e4fe..c4b58be9e 100644 --- a/policy/engine.go +++ b/policy/engine.go @@ -298,6 +298,7 @@ func (e *Engine) check(ctx context.Context, path string, config interface{}, nam var failures []output.Result var warnings []output.Result + var excludes []output.Result for _, ruleResult := range ruleQueryResult.Results { // Exceptions have already been accounted for in the exception query so @@ -311,6 +312,21 @@ func (e *Engine) check(ctx context.Context, path string, config interface{}, nam continue } + localExcludeQuery := fmt.Sprintf("data.%s.exclude_%s[_][_] = %q", namespace, removeRulePrefix(rule), ruleResult.Message) + localExcludeQueryResult, err := e.query(ctx, config, localExcludeQuery) + if err != nil { + return output.CheckResult{}, fmt.Errorf("query exception: %w", err) + } + + // If the query was a failure, let's have a look & see if an exception was written for it. + if len(localExcludeQueryResult.Results) > 0 { + // append an exception & continue + localExcludeResult := localExcludeQueryResult.Results[0] + localExcludeResult.Message = localExcludeQuery + excludes = append(excludes, localExcludeResult) + continue + } + if isFailure(rule) { failures = append(failures, ruleResult) } else { @@ -321,6 +337,7 @@ func (e *Engine) check(ctx context.Context, path string, config interface{}, nam checkResult.Failures = append(checkResult.Failures, failures...) checkResult.Warnings = append(checkResult.Warnings, warnings...) checkResult.Exceptions = append(checkResult.Exceptions, exceptions...) + checkResult.Excludes = append(checkResult.Excludes, excludes...) checkResult.Queries = append(checkResult.Queries, exceptionQueryResult) checkResult.Queries = append(checkResult.Queries, ruleQueryResult) From 23f5a01632c1c8dbfa6b2be06a240de0f9aacc23 Mon Sep 17 00:00:00 2001 From: Hamish Hutchings Date: Fri, 25 Jun 2021 12:42:31 +0200 Subject: [PATCH 2/5] Clean up reporting Signed-off-by: Hamish Hutchings --- output/standard.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/output/standard.go b/output/standard.go index 48eea994d..141cf0d2e 100644 --- a/output/standard.go +++ b/output/standard.go @@ -51,6 +51,7 @@ func (s *Standard) Output(results []CheckResult) error { var totalFailures int var totalExceptions int + var totalExclusions int var totalWarnings int var totalSuccesses int var totalSkipped int @@ -96,11 +97,12 @@ func (s *Standard) Output(results []CheckResult) error { totalFailures += len(result.Failures) totalExceptions += len(result.Exceptions) totalWarnings += len(result.Warnings) + totalExclusions += len(result.Excludes) totalSkipped += len(result.Skipped) totalSuccesses += result.Successes } - totalTests := totalFailures + totalExceptions + totalWarnings + totalSuccesses + totalSkipped + totalTests := totalFailures + totalExceptions + totalWarnings + totalSuccesses + totalExclusions + totalSkipped var pluralSuffixTests string if totalTests != 1 { @@ -122,12 +124,18 @@ func (s *Standard) Output(results []CheckResult) error { pluralSuffixExceptions = "s" } - outputText := fmt.Sprintf("%v test%s, %v passed, %v warning%s, %v failure%s, %v exception%s", + var pluralSuffixExclusions string + if totalExclusions != 1 { + pluralSuffixExclusions = "s" + } + + outputText := fmt.Sprintf("%v test%s, %v passed, %v warning%s, %v failure%s, %v exception%s, %v exclusion%s", totalTests, pluralSuffixTests, totalSuccesses, totalWarnings, pluralSuffixWarnings, totalFailures, pluralSuffixFailures, totalExceptions, pluralSuffixExceptions, + totalExclusions, pluralSuffixExclusions, ) if s.ShowSkipped { From 4486092bb2232ff7fcc50061099a42a6d17d116a Mon Sep 17 00:00:00 2001 From: Hamish Hutchings Date: Wed, 30 Jun 2021 15:37:59 +0200 Subject: [PATCH 3/5] Refactor to use structured output for excludes - Add New Example based on ticket (excludes2) - Refactor to allow for structured content being returned Signed-off-by: Hamish Hutchings --- examples/{exceptions2 => excludes}/main.tf | 0 .../policy/deny.rego | 0 examples/excludes2/deployment.yaml | 35 +++++++++++++++++++ examples/excludes2/policy/deny.rego | 29 +++++++++++++++ policy/engine.go | 6 +++- 5 files changed, 69 insertions(+), 1 deletion(-) rename examples/{exceptions2 => excludes}/main.tf (100%) rename examples/{exceptions2 => excludes}/policy/deny.rego (100%) create mode 100644 examples/excludes2/deployment.yaml create mode 100644 examples/excludes2/policy/deny.rego diff --git a/examples/exceptions2/main.tf b/examples/excludes/main.tf similarity index 100% rename from examples/exceptions2/main.tf rename to examples/excludes/main.tf diff --git a/examples/exceptions2/policy/deny.rego b/examples/excludes/policy/deny.rego similarity index 100% rename from examples/exceptions2/policy/deny.rego rename to examples/excludes/policy/deny.rego diff --git a/examples/excludes2/deployment.yaml b/examples/excludes2/deployment.yaml new file mode 100644 index 000000000..446403b8b --- /dev/null +++ b/examples/excludes2/deployment.yaml @@ -0,0 +1,35 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: mydep +spec: + template: + spec: + containers: + - name: web + image: nginx + ports: + - containerPort: 8080 + securityContext: + runAsNonRoot: false + - name: host-agent + image: host-agent +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: not-mydep +spec: + template: + spec: + containers: + - name: web + image: nginx + ports: + - containerPort: 8080 + securityContext: + runAsNonRoot: true + - name: host-agent + image: host-agent + securityContext: + runAsNonRoot: true diff --git a/examples/excludes2/policy/deny.rego b/examples/excludes2/policy/deny.rego new file mode 100644 index 000000000..820bdd199 --- /dev/null +++ b/examples/excludes2/policy/deny.rego @@ -0,0 +1,29 @@ +package main + +deny_root[result] { + input.kind == "Deployment" + c = input.spec.template.spec.containers[_] + not c.securityContext.runAsNonRoot + + # key "msg" is required to be set. + result := { + "container": c.name, + "deployment": input.metadata.name, + "msg": sprintf("container %s in deployment %s doesn't set runAsNonRoot", [c.name, input.metadata.name]), + } +} + +root_exceptions = [{"deployment": "mydep", "containers": ["host-agent"]}] + +# Here the exception I want to be able to express is "mydep can run host-agent as root". +# But not web as root +exclude_root[attrs] { + deployment := input.metadata.name + container := input.spec.template.spec.containers[_].name + exception := root_exceptions[_] + + deployment == exception.deployment + container == exception.containers[_] + + attrs = [{"container": container, "deployment": deployment}] +} diff --git a/policy/engine.go b/policy/engine.go index c4b58be9e..b1ac96f1f 100644 --- a/policy/engine.go +++ b/policy/engine.go @@ -3,6 +3,7 @@ package policy import ( "bytes" "context" + "encoding/json" "fmt" "io/ioutil" "os" @@ -137,6 +138,7 @@ func (e *Engine) Check(ctx context.Context, configs map[string]interface{}, name checkResult.Failures = append(checkResult.Failures, result.Failures...) checkResult.Warnings = append(checkResult.Warnings, result.Warnings...) checkResult.Exceptions = append(checkResult.Exceptions, result.Exceptions...) + checkResult.Excludes = append(checkResult.Excludes, result.Excludes...) checkResult.Queries = append(checkResult.Queries, result.Queries...) } checkResults = append(checkResults, checkResult) @@ -233,6 +235,7 @@ func (e *Engine) Runtime() *ast.Term { func (e *Engine) check(ctx context.Context, path string, config interface{}, namespace string) (output.CheckResult, error) { var rules []string var ruleCount int + for _, module := range e.Modules() { currentNamespace := strings.Replace(module.Package.Path.String(), "data.", "", 1) if currentNamespace != namespace { @@ -312,7 +315,8 @@ func (e *Engine) check(ctx context.Context, path string, config interface{}, nam continue } - localExcludeQuery := fmt.Sprintf("data.%s.exclude_%s[_][_] = %q", namespace, removeRulePrefix(rule), ruleResult.Message) + result, err := json.Marshal(ruleResult.Metadata) + localExcludeQuery := fmt.Sprintf("data.%s.exclude_%s[_][_] = %s", namespace, removeRulePrefix(rule), result) localExcludeQueryResult, err := e.query(ctx, config, localExcludeQuery) if err != nil { return output.CheckResult{}, fmt.Errorf("query exception: %w", err) From 2251f80b6450b84a784a1b3d6ed218a44954086c Mon Sep 17 00:00:00 2001 From: Hamish Hutchings Date: Wed, 30 Jun 2021 15:53:15 +0200 Subject: [PATCH 4/5] Clean up the engine code to be a bit clearer. - Refactor the terraform example Signed-off-by: Hamish Hutchings --- examples/excludes/policy/deny.rego | 17 ++++++++--------- policy/engine.go | 30 +++++++++++++++++++----------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/examples/excludes/policy/deny.rego b/examples/excludes/policy/deny.rego index ee7c8df22..249dc2cb2 100644 --- a/examples/excludes/policy/deny.rego +++ b/examples/excludes/policy/deny.rego @@ -2,14 +2,14 @@ package main exceptions = {"exception-name"} -func_name_msg(name) = ret { - ret := sprintf("Resource Name '%s' contains dashes", [name]) -} - -deny_name[msg] { +deny_name[result] { input.resource[_][name] contains(name, "-") - msg := func_name_msg(name) + msg := sprintf("Resource Name '%s' contains dashes", [name]) + result := { + "msg": msg, + "resource-name": name, + } } deny_resource_type[msg] { @@ -18,10 +18,9 @@ deny_resource_type[msg] { msg := sprintf("Resource Type '%s' is invalid", [type]) } -exclude_name[rules] { - input.resource[_][name] +exclude_name[attrs] { exceptions[name] - rules := [func_name_msg(name)] + attrs := [{"resource-name": name}] } exception[rules] { diff --git a/policy/engine.go b/policy/engine.go index b1ac96f1f..72ec21627 100644 --- a/policy/engine.go +++ b/policy/engine.go @@ -316,21 +316,29 @@ func (e *Engine) check(ctx context.Context, path string, config interface{}, nam } result, err := json.Marshal(ruleResult.Metadata) - localExcludeQuery := fmt.Sprintf("data.%s.exclude_%s[_][_] = %s", namespace, removeRulePrefix(rule), result) - localExcludeQueryResult, err := e.query(ctx, config, localExcludeQuery) if err != nil { - return output.CheckResult{}, fmt.Errorf("query exception: %w", err) + return output.CheckResult{}, fmt.Errorf("json marshal: %w", err) } - // If the query was a failure, let's have a look & see if an exception was written for it. - if len(localExcludeQueryResult.Results) > 0 { - // append an exception & continue - localExcludeResult := localExcludeQueryResult.Results[0] - localExcludeResult.Message = localExcludeQuery - excludes = append(excludes, localExcludeResult) - continue - } + // If we have a non-null metadata response, then we are eligible to exclude the policy. + // Otherwise we can just skip & process the policy violation + if string(result) != "null" { + localExcludeQuery := fmt.Sprintf("data.%s.exclude_%s[_][_] = %s", namespace, removeRulePrefix(rule), result) + localExcludeQueryResult, err := e.query(ctx, config, localExcludeQuery) + if err != nil { + return output.CheckResult{}, fmt.Errorf("query exception: %w", err) + } + // If the query was a failure, let's have a look & see if an exception was written for it. + if len(localExcludeQueryResult.Results) > 0 { + // append an exception & continue + localExcludeResult := localExcludeQueryResult.Results[0] + localExcludeResult.Message = localExcludeQuery + excludes = append(excludes, localExcludeResult) + continue + } + + } if isFailure(rule) { failures = append(failures, ruleResult) } else { From 373e658cb36e75536d354d5207c62d3c9a76083c Mon Sep 17 00:00:00 2001 From: Hamish Hutchings Date: Wed, 30 Jun 2021 16:01:08 +0200 Subject: [PATCH 5/5] Update tests to accomodate new output Signed-off-by: Hamish Hutchings --- acceptance.bats | 8 ++++---- output/standard_test.go | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/acceptance.bats b/acceptance.bats index 4ea5a917e..63abc1a3c 100755 --- a/acceptance.bats +++ b/acceptance.bats @@ -74,7 +74,7 @@ @test "Test command works with nested namespaces" { run ./conftest test --namespace main.gke -p examples/hcl1/policy/ examples/hcl1/gke.tf --no-color [ "$status" -eq 1 ] - [ "${lines[1]}" = "1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions" ] + [ "${lines[1]}" = "1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions, 0 exclusions" ] } @test "Verify command has trace flag" { @@ -267,13 +267,13 @@ @test "The number of tests run is accurate" { run ./conftest test -p examples/kubernetes/policy examples/kubernetes/service.yaml --no-color [ "$status" -eq 0 ] - [ "${lines[1]}" = "5 tests, 4 passed, 1 warning, 0 failures, 0 exceptions" ] + [ "${lines[1]}" = "5 tests, 4 passed, 1 warning, 0 failures, 0 exceptions, 0 exclusions" ] } @test "Exceptions reported correctly" { run ./conftest test -p examples/exceptions/policy examples/exceptions/deployments.yaml --no-color [ "$status" -eq 1 ] - [ "${lines[2]}" = "2 tests, 0 passed, 0 warnings, 1 failure, 1 exception" ] + [ "${lines[2]}" = "2 tests, 0 passed, 0 warnings, 1 failure, 1 exception, 0 exclusions" ] } @test "Exceptions output" { @@ -285,7 +285,7 @@ @test "Suppress exceptions output" { run ./conftest test -p examples/exceptions/policy examples/exceptions/deployments.yaml --no-color --suppress-exceptions [ "$status" -eq 1 ] - [ "${lines[1]}" = "2 tests, 0 passed, 0 warnings, 1 failure, 1 exception" ] + [ "${lines[1]}" = "2 tests, 0 passed, 0 warnings, 1 failure, 1 exception, 0 exclusions" ] } @test "Can combine yaml files" { diff --git a/output/standard_test.go b/output/standard_test.go index 86780109d..9a8af0d30 100644 --- a/output/standard_test.go +++ b/output/standard_test.go @@ -28,7 +28,7 @@ func TestStandard(t *testing.T) { "WARN - foo.yaml - namespace - first warning", "FAIL - foo.yaml - namespace - first failure", "", - "2 tests, 0 passed, 1 warning, 1 failure, 0 exceptions", + "2 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 0 exclusions", "", }, }, @@ -46,7 +46,7 @@ func TestStandard(t *testing.T) { "WARN - - namespace - first warning", "FAIL - - namespace - first failure", "", - "2 tests, 0 passed, 1 warning, 1 failure, 0 exceptions", + "2 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 0 exclusions", "", }, }, @@ -66,7 +66,7 @@ func TestStandard(t *testing.T) { "WARN - foo.yaml - namespace - first warning", "FAIL - foo.yaml - namespace - first failure", "", - "3 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 1 skipped", + "3 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 0 exclusions, 1 skipped", "", }, }, @@ -85,7 +85,7 @@ func TestStandard(t *testing.T) { "WARN - - namespace - first warning", "FAIL - - namespace - first failure", "", - "2 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 0 skipped", + "2 tests, 0 passed, 1 warning, 1 failure, 0 exceptions, 0 exclusions, 0 skipped", "", }, },