From e291d15aa1c3ff44d1f7affa1da2edce38c99188 Mon Sep 17 00:00:00 2001 From: Luiz Carvalho Date: Mon, 18 Sep 2023 16:59:28 -0400 Subject: [PATCH] Process include/exclude from policy source https://issues.redhat.com/browse/HACBS-2428 Signed-off-by: Luiz Carvalho --- go.mod | 2 +- go.sum | 4 +- .../application_snapshot_image.go | 2 +- .../definition/definition.go | 2 +- internal/evaluator/conftest_evaluator.go | 57 ++++++----- internal/evaluator/conftest_evaluator_test.go | 94 +++++++++++++++++-- 6 files changed, 127 insertions(+), 34 deletions(-) diff --git a/go.mod b/go.mod index f3d38ac86..71c2f6134 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( cuelang.org/go v0.6.0 github.com/MakeNowJust/heredoc v1.0.0 github.com/enterprise-contract/ec-cli/application v0.0.0-00010101000000-000000000000 - github.com/enterprise-contract/enterprise-contract-controller/api v0.0.0-20230725143429-4731fc7d3b41 + github.com/enterprise-contract/enterprise-contract-controller/api v0.0.0-20230920125001-2d598fbbe40c github.com/evanphx/json-patch v5.7.0+incompatible github.com/gkampitakis/go-snaps v0.4.10 github.com/go-logr/logr v1.2.4 diff --git a/go.sum b/go.sum index 0f70ef1a7..418b5aa2a 100644 --- a/go.sum +++ b/go.sum @@ -466,8 +466,8 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp github.com/emicklei/go-restful/v3 v3.10.2 h1:hIovbnmBTLjHXkqEBUz3HGpXZdM7ZrE9fJIZIqlJLqE= github.com/emicklei/go-restful/v3 v3.10.2/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/emicklei/proto v1.10.0 h1:pDGyFRVV5RvV+nkBK9iy3q67FBy9Xa7vwrOTE+g5aGw= -github.com/enterprise-contract/enterprise-contract-controller/api v0.0.0-20230725143429-4731fc7d3b41 h1:C/hm4M7092Q+5fffsHmdL/+yrpQDuDNX8036qxFacEs= -github.com/enterprise-contract/enterprise-contract-controller/api v0.0.0-20230725143429-4731fc7d3b41/go.mod h1:ET4FRiAdAk3+mtkFQF/rGxZHpwx0RT4zh6k7IUWs23U= +github.com/enterprise-contract/enterprise-contract-controller/api v0.0.0-20230920125001-2d598fbbe40c h1:R2jbKNboD2qFaGX6saKq6wp8ehL/NJKFCfbkc3HQzPE= +github.com/enterprise-contract/enterprise-contract-controller/api v0.0.0-20230920125001-2d598fbbe40c/go.mod h1:A5Nr4ZbDDR+HuGhezHsbOsBfvyXrV4jEtv0JhhdDe34= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= diff --git a/internal/evaluation_target/application_snapshot_image/application_snapshot_image.go b/internal/evaluation_target/application_snapshot_image/application_snapshot_image.go index 4697c1179..e99fa5157 100644 --- a/internal/evaluation_target/application_snapshot_image/application_snapshot_image.go +++ b/internal/evaluation_target/application_snapshot_image/application_snapshot_image.go @@ -107,7 +107,7 @@ func NewApplicationSnapshotImage(ctx context.Context, component app.SnapshotComp log.Debugf("policySource: %#v", policySource) } - c, err := newConftestEvaluator(ctx, policySources, p) + c, err := newConftestEvaluator(ctx, policySources, p, sourceGroup.Config) if err != nil { log.Debug("Failed to initialize the conftest evaluator!") return nil, err diff --git a/internal/evaluation_target/definition/definition.go b/internal/evaluation_target/definition/definition.go index db0f4fc82..ddde98223 100644 --- a/internal/evaluation_target/definition/definition.go +++ b/internal/evaluation_target/definition/definition.go @@ -43,7 +43,7 @@ func NewDefinition(ctx context.Context, fpath []string, sources []source.PolicyS return nil, err } - c, err := newConftestEvaluator(ctx, sources, pol, namespace) + c, err := newConftestEvaluator(ctx, sources, pol, nil, namespace) if err != nil { return nil, err diff --git a/internal/evaluator/conftest_evaluator.go b/internal/evaluator/conftest_evaluator.go index ba5ca9b35..de73b0ed3 100644 --- a/internal/evaluator/conftest_evaluator.go +++ b/internal/evaluator/conftest_evaluator.go @@ -26,6 +26,7 @@ import ( "strings" "time" + ecc "github.com/enterprise-contract/enterprise-contract-controller/api/v1alpha1" "github.com/open-policy-agent/conftest/output" conftest "github.com/open-policy-agent/conftest/policy" "github.com/open-policy-agent/conftest/runner" @@ -135,6 +136,8 @@ type conftestEvaluator struct { dataDir string policyDir string policy policy.Policy + include []string + exclude []string fs afero.Fs namespace []string } @@ -199,13 +202,13 @@ func (r conftestRunner) Run(ctx context.Context, fileList []string) (result []Ou // NewConftestEvaluator returns initialized conftestEvaluator implementing // Evaluator interface -func NewConftestEvaluator(ctx context.Context, policySources []source.PolicySource, p policy.Policy) (Evaluator, error) { - return NewConftestEvaluatorWithNamespace(ctx, policySources, p, nil) +func NewConftestEvaluator(ctx context.Context, policySources []source.PolicySource, p policy.Policy, sc *ecc.SourceConfig) (Evaluator, error) { + return NewConftestEvaluatorWithNamespace(ctx, policySources, p, sc, nil) } // set the policy namespace -func NewConftestEvaluatorWithNamespace(ctx context.Context, policySources []source.PolicySource, p policy.Policy, namespace []string) (Evaluator, error) { +func NewConftestEvaluatorWithNamespace(ctx context.Context, policySources []source.PolicySource, p policy.Policy, sc *ecc.SourceConfig, namespace []string) (Evaluator, error) { fs := utils.FS(ctx) c := conftestEvaluator{ policySources: policySources, @@ -215,6 +218,8 @@ func NewConftestEvaluatorWithNamespace(ctx context.Context, policySources []sour namespace: namespace, } + c.include, c.exclude = computeIncludeExclude(sc, p) + dir, err := utils.CreateWorkDir(fs) if err != nil { log.Debug("Failed to create work dir!") @@ -698,26 +703,8 @@ func isResultEffective(failure Result, now time.Time) bool { // discarded based on the policy configuration. func (c conftestEvaluator) isResultIncluded(result Result) bool { ruleMatchers := makeMatchers(result) - var includes, excludes []string - - spec := c.policy.Spec() - cfg := spec.Configuration - if cfg != nil { - for _, c := range cfg.Collections { - // If the old way of specifying collections are used, convert them. - includes = append(includes, "@"+c) - } - includes = append(includes, cfg.Include...) - excludes = append(excludes, cfg.Exclude...) - } - - if len(includes) == 0 { - includes = []string{"*"} - } - - includeScore := scoreMatches(ruleMatchers, includes) - excludeScore := scoreMatches(ruleMatchers, excludes) - + includeScore := scoreMatches(ruleMatchers, c.include) + excludeScore := scoreMatches(ruleMatchers, c.exclude) return includeScore > excludeScore } @@ -868,3 +855,27 @@ func strictCapabilities(ctx context.Context) (string, error) { } return string(blob), nil } + +func computeIncludeExclude(sc *ecc.SourceConfig, p policy.Policy) ([]string, []string) { + var include, exclude []string + + // The lines below take care to make a copy of the includes/excludes slices in order + // to ensure mutations are not unexpectedly propagated. + if sc != nil && (len(sc.Include) != 0 || len(sc.Exclude) != 0) { + include = append(include, sc.Include...) + exclude = append(exclude, sc.Exclude...) + } else if policyConfig := p.Spec().Configuration; policyConfig != nil { + include = append(include, policyConfig.Include...) + exclude = append(exclude, policyConfig.Exclude...) + // If the old way of specifying collections are used, convert them. + for _, collection := range policyConfig.Collections { + include = append(include, fmt.Sprintf("@%s", collection)) + } + } + + if len(include) == 0 { + include = []string{"*"} + } + + return include, exclude +} diff --git a/internal/evaluator/conftest_evaluator_test.go b/internal/evaluator/conftest_evaluator_test.go index 1b5215d47..685c29410 100644 --- a/internal/evaluator/conftest_evaluator_test.go +++ b/internal/evaluator/conftest_evaluator_test.go @@ -196,7 +196,7 @@ func TestConftestEvaluatorEvaluateTimeBased(t *testing.T) { evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{ testPolicySource{}, - }, pol) + }, pol, nil) assert.NoError(t, err) actualResults, data, err := evaluator.Evaluate(ctx, inputs) @@ -236,7 +236,7 @@ func TestConftestEvaluatorCapabilities(t *testing.T) { evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{ testPolicySource{}, - }, p) + }, p, nil) assert.NoError(t, err) blob, err := afero.ReadFile(fs, evaluator.CapabilitiesPath()) @@ -285,7 +285,7 @@ func TestConftestEvaluatorEvaluateNoSuccessWarningsOrFailures(t *testing.T) { evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{ testPolicySource{}, - }, p) + }, p, nil) assert.NoError(t, err) actualResults, data, err := evaluator.Evaluate(ctx, inputs) @@ -1107,7 +1107,7 @@ func TestConftestEvaluatorIncludeExclude(t *testing.T) { evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{ testPolicySource{}, - }, p) + }, p, nil) assert.NoError(t, err) got, data, err := evaluator.Evaluate(ctx, inputs) @@ -1604,7 +1604,7 @@ func TestConftestEvaluatorEvaluate(t *testing.T) { Url: rules, Kind: source.PolicyKind, }, - }, p) + }, p, nil) require.NoError(t, err) results, data, err := evaluator.Evaluate(ctx, []string{path.Join(dir, "inputs")}) @@ -1648,13 +1648,95 @@ func TestUnconformingRule(t *testing.T) { Url: rules, Kind: source.PolicyKind, }, - }, p) + }, p, nil) require.NoError(t, err) _, _, err = evaluator.Evaluate(ctx, []string{path.Join(dir, "inputs")}) assert.EqualError(t, err, `the rule "deny = true { true }" returns an unsupported value, at no_msg.rego:3`) } +func TestNewConftestEvaluatorComputeIncludeExclude(t *testing.T) { + cases := []struct { + name string + globalConfig *ecc.EnterpriseContractPolicyConfiguration + sourceConfig *ecc.SourceConfig + expectedInclude []string + expectedExclude []string + }{ + {name: "no config", expectedInclude: []string{"*"}}, + { + name: "empty global config", + globalConfig: &ecc.EnterpriseContractPolicyConfiguration{}, + expectedInclude: []string{"*"}, + }, + { + name: "global config", + globalConfig: &ecc.EnterpriseContractPolicyConfiguration{ + Include: []string{"include-me"}, + Exclude: []string{"exclude-me"}, + Collections: []string{"collect-me"}, + }, + expectedInclude: []string{"include-me", "@collect-me"}, + expectedExclude: []string{"exclude-me"}, + }, + { + name: "empty source config", + sourceConfig: &ecc.SourceConfig{}, + expectedInclude: []string{"*"}, + }, + { + name: "source config", + sourceConfig: &ecc.SourceConfig{ + Include: []string{"include-me"}, + Exclude: []string{"exclude-me"}, + }, + expectedInclude: []string{"include-me"}, + expectedExclude: []string{"exclude-me"}, + }, + { + name: "source config over global config", + globalConfig: &ecc.EnterpriseContractPolicyConfiguration{ + Include: []string{"include-ignored"}, + Exclude: []string{"exclude-ignored"}, + Collections: []string{"collection-ignored"}, + }, + sourceConfig: &ecc.SourceConfig{ + Include: []string{"include-me"}, + Exclude: []string{"exclude-me"}, + }, + expectedInclude: []string{"include-me"}, + expectedExclude: []string{"exclude-me"}, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + ctx := withCapabilities(context.Background(), testCapabilities) + + p, err := policy.NewOfflinePolicy(ctx, "2014-05-31") + require.NoError(t, err) + + p = p.WithSpec(ecc.EnterpriseContractPolicySpec{ + Configuration: tt.globalConfig, + }) + + evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{ + &source.PolicyUrl{ + Url: path.Join(dir, "policy", "rules.tar"), + Kind: source.PolicyKind, + }, + }, p, tt.sourceConfig) + require.NoError(t, err) + + ce := evaluator.(conftestEvaluator) + require.Equal(t, tt.expectedInclude, ce.include) + require.Equal(t, tt.expectedExclude, ce.exclude) + }) + } + +} + var testCapabilities string func init() {