Merge pull request #2176 from robnester-rh/EC-1023

Revert "Merge pull request #2150 from zregvart/issue/EC-963"

cmd/fetch/fetch_policy.go

@@ -23,7 +23,6 @@ import (
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 
-	"github.com/enterprise-contract/ec-cli/internal/mutate"
 	"github.com/enterprise-contract/ec-cli/internal/policy/source"
 	"github.com/enterprise-contract/ec-cli/internal/utils"
 )
@@ -109,11 +108,11 @@ func fetchPolicyCmd() *cobra.Command {
 			sources := make([]*source.PolicyUrl, 0, len(sourceUrls)+len(dataSourceUrls))
 
 			for _, url := range sourceUrls {
-				sources = append(sources, &source.PolicyUrl{Url: mutate.Const(url), Kind: source.PolicyKind})
+				sources = append(sources, &source.PolicyUrl{Url: url, Kind: source.PolicyKind})
 			}
 
 			for _, url := range dataSourceUrls {
-				sources = append(sources, &source.PolicyUrl{Url: mutate.Const(url), Kind: source.DataKind})
+				sources = append(sources, &source.PolicyUrl{Url: url, Kind: source.DataKind})
 			}
 
 			for _, s := range sources {

cmd/inspect/inspect_policy.go

@@ -28,7 +28,6 @@ import (
 	"github.com/spf13/cobra"
 	"golang.org/x/exp/slices"
 
-	"github.com/enterprise-contract/ec-cli/internal/mutate"
 	"github.com/enterprise-contract/ec-cli/internal/opa"
 	opaRule "github.com/enterprise-contract/ec-cli/internal/opa/rule"
 	"github.com/enterprise-contract/ec-cli/internal/policy"
@@ -119,7 +118,7 @@ func inspectPolicyCmd() *cobra.Command {
 
 			allResults := make(map[string][]*ast.AnnotationsRef)
 			for _, url := range sourceUrls {
-				s := &source.PolicyUrl{Url: mutate.Const(url), Kind: source.PolicyKind}
+				s := &source.PolicyUrl{Url: url, Kind: source.PolicyKind}
 
 				// Download
 				policyDir, err := s.GetPolicy(ctx, destDir, false)

cmd/inspect/inspect_policy_data.go

@@ -31,7 +31,6 @@ import (
 	"golang.org/x/exp/slices"
 	"sigs.k8s.io/yaml"
 
-	"github.com/enterprise-contract/ec-cli/internal/mutate"
 	"github.com/enterprise-contract/ec-cli/internal/policy/source"
 	"github.com/enterprise-contract/ec-cli/internal/utils"
 )
@@ -89,7 +88,7 @@ func inspectPolicyDataCmd() *cobra.Command {
 
 			allData := make(map[string]interface{})
 			for _, url := range sourceUrls {
-				s := &source.PolicyUrl{Url: mutate.Const(url), Kind: source.PolicyKind}
+				s := &source.PolicyUrl{Url: url, Kind: source.PolicyKind}
 
 				// Download
 				policyDir, err := s.GetPolicy(ctx, destDir, false)

cmd/validate/image.go

@@ -230,63 +230,63 @@ func validateImageCmd(validate imageValidationFunc) *cobra.Command {
 				RekorURL:    data.rekorURL,
 			}
 
-			p, err := policy.NewPolicy(ctx, policyOptions)
-			if err != nil {
+			// We're not currently using the policyCache returned from PreProcessPolicy, but we could
+			// use it to cache the policy for future use.
+			if p, _, err := policy.PreProcessPolicy(ctx, policyOptions); err != nil {
 				allErrors = errors.Join(allErrors, err)
-				return
-			}
+			} else {
+				// inject extra variables into rule data per source
+				if len(data.extraRuleData) > 0 {
+					policySpec := p.Spec()
+					sources := policySpec.Sources
+					for i := range sources {
+						src := sources[i]
+						var rule_data_raw []byte
+						unmarshaled := make(map[string]interface{})
+
+						if src.RuleData != nil {
+							rule_data_raw, err = src.RuleData.MarshalJSON()
+							if err != nil {
+								log.Errorf("Unable to parse ruledata to raw data")
+							}
+							err = json.Unmarshal(rule_data_raw, &unmarshaled)
+							if err != nil {
+								log.Errorf("Unable to parse ruledata into standard JSON object")
+							}
+						} else {
+							sources[i].RuleData = new(extv1.JSON)
+						}
 
-			// inject extra variables into rule data per source
-			if len(data.extraRuleData) > 0 {
-				policySpec := p.Spec()
-				sources := policySpec.Sources
-				for i := range sources {
-					src := sources[i]
-					var rule_data_raw []byte
-					unmarshaled := make(map[string]interface{})
-
-					if src.RuleData != nil {
-						rule_data_raw, err = src.RuleData.MarshalJSON()
-						if err != nil {
-							log.Errorf("Unable to parse ruledata to raw data")
+						for j := range data.extraRuleData {
+							parts := strings.SplitN(data.extraRuleData[j], "=", 2)
+							if len(parts) < 2 {
+								log.Errorf("Incorrect syntax for --extra-rule-data")
+							}
+							extraRuleDataPolicyConfig, err := validate_utils.GetPolicyConfig(ctx, parts[1])
+							if err != nil {
+								log.Errorf("Unable to load data from extraRuleData: %s", err.Error())
+							}
+							unmarshaled[parts[0]] = extraRuleDataPolicyConfig
 						}
-						err = json.Unmarshal(rule_data_raw, &unmarshaled)
+						rule_data_raw, err = json.Marshal(unmarshaled)
 						if err != nil {
-							log.Errorf("Unable to parse ruledata into standard JSON object")
+							log.Errorf("Unable to parse updated ruledata: %s", err.Error())
 						}
-					} else {
-						sources[i].RuleData = new(extv1.JSON)
-					}
 
-					for j := range data.extraRuleData {
-						parts := strings.SplitN(data.extraRuleData[j], "=", 2)
-						if len(parts) < 2 {
-							log.Errorf("Incorrect syntax for --extra-rule-data")
+						if rule_data_raw == nil {
+							log.Errorf("Invalid rule data JSON")
 						}
-						extraRuleDataPolicyConfig, err := validate_utils.GetPolicyConfig(ctx, parts[1])
+
+						err = sources[i].RuleData.UnmarshalJSON(rule_data_raw)
 						if err != nil {
-							log.Errorf("Unable to load data from extraRuleData: %s", err.Error())
+							log.Errorf("Unable to marshal updated JSON: %s", err.Error())
 						}
-						unmarshaled[parts[0]] = extraRuleDataPolicyConfig
-					}
-					rule_data_raw, err = json.Marshal(unmarshaled)
-					if err != nil {
-						log.Errorf("Unable to parse updated ruledata: %s", err.Error())
-					}
-
-					if rule_data_raw == nil {
-						log.Errorf("Invalid rule data JSON")
-					}
-
-					err = sources[i].RuleData.UnmarshalJSON(rule_data_raw)
-					if err != nil {
-						log.Errorf("Unable to marshal updated JSON: %s", err.Error())
 					}
+					policySpec.Sources = sources
+					p = p.WithSpec(policySpec)
 				}
-				policySpec.Sources = sources
-				p = p.WithSpec(policySpec)
+				data.policy = p
 			}
-			data.policy = p
 
 			return
 		},

cmd/validate/image_integration_test.go

@@ -28,6 +28,7 @@ import (
 	"time"
 
 	"github.com/enterprise-contract/enterprise-contract-controller/api/v1alpha1"
+	ociMetadata "github.com/enterprise-contract/go-gather/metadata/oci"
 	app "github.com/konflux-ci/application-api/api/v1alpha1"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -51,10 +52,12 @@ func TestEvaluatorLifecycle(t *testing.T) {
 	commonMockClient(&client)
 	ctx = oci.WithClient(ctx, &client)
 	mdl := MockDownloader{}
+	downloaderCall := mdl.On("Download", mock.Anything, mock.Anything, false).Return(&ociMetadata.OCIMetadata{Digest: "sha256:da54bca5477bf4e3449bc37de1822888fa0fbb8d89c640218cb31b987374d357"}, nil).Times(noEvaluators)
 	ctx = context.WithValue(ctx, source.DownloaderFuncKey, &mdl)
 
 	evaluators := make([]*mockEvaluator, 0, noEvaluators)
 	expectations := make([]*mock.Call, 0, noEvaluators+1)
+	expectations = append(expectations, downloaderCall)
 
 	for i := 0; i < noEvaluators; i++ {
 		e := mockEvaluator{}
@@ -70,7 +73,7 @@ func TestEvaluatorLifecycle(t *testing.T) {
 
 	newConftestEvaluator = func(_ context.Context, s []source.PolicySource, _ evaluator.ConfigProvider, _ v1alpha1.Source) (evaluator.Evaluator, error) {
 		// We are splitting this url to get to the index of the evaluator.
-		idx, err := strconv.Atoi(s[0].PolicyUrl())
+		idx, err := strconv.Atoi(strings.Split(strings.Split(s[0].PolicyUrl(), "@")[0], "::")[1])
 		require.NoError(t, err)
 
 		return evaluators[idx], nil

cmd/validate/image_test.go

@@ -64,16 +64,7 @@ var rootArgs = []string{
 }
 
 func happyValidator() imageValidationFunc {
-	return func(ctx context.Context, component app.SnapshotComponent, _ *app.SnapshotSpec, p policy.Policy, _ []evaluator.Evaluator, _ bool) (*output.Output, error) {
-		// simulate fetching of sources
-		for _, src := range p.Spec().Sources {
-			for _, url := range source.PolicySourcesFrom(src) {
-				if _, err := url.GetPolicy(ctx, "dest", false); err != nil {
-					return nil, err
-				}
-			}
-		}
-
+	return func(_ context.Context, component app.SnapshotComponent, _ *app.SnapshotSpec, _ policy.Policy, _ []evaluator.Evaluator, _ bool) (*output.Output, error) {
 		return &output.Output{
 			ImageSignatureCheck: output.VerificationStatus{
 				Passed: true,

features/__snapshots__/validate_image.snap

@@ -1122,7 +1122,7 @@ Error: success criteria not met
     "sources": [
       {
         "policy": [
-          "git::https://${GITHOST}/git/unexpected-keyless-cert.git"
+          "git::${GITHOST}/git/unexpected-keyless-cert.git?ref=${LATEST_COMMIT}"
         ]
       }
     ]
@@ -1167,7 +1167,7 @@ Error: success criteria not met
     "sources": [
       {
         "policy": [
-          "git::https://${GITHOST}/git/invalid-image-signature.git"
+          "git::${GITHOST}/git/invalid-image-signature.git?ref=${LATEST_COMMIT}"
         ]
       }
     ],
@@ -1598,7 +1598,7 @@ Error: success criteria not met
     "sources": [
       {
         "policy": [
-          "git::https://${GITHOST}/git/mismatched-image-digest.git"
+          "git::${GITHOST}/git/mismatched-image-digest.git?ref=${LATEST_COMMIT}"
         ]
       }
     ],
@@ -2744,7 +2744,7 @@ ${__________known_PUBLIC_KEY}
     "sources": [
       {
         "policy": [
-          "git::https://${GITHOST}/git/rekor-by-default.git"
+          "git::${GITHOST}/git/rekor-by-default.git?ref=${LATEST_COMMIT}"
         ]
       }
     ],

features/__snapshots__/validate_input.snap

@@ -15,7 +15,7 @@
     "sources": [
       {
         "policy": [
-          "git::${GITHOST}/git/happy-day-policy.git?ref=${LATEST_COMMIT}"
+          "git::https://${GITHOST}/git/happy-day-policy.git"
         ]
       }
     ]
@@ -68,12 +68,12 @@ Error: error validating file pipeline_definition.yaml: evaluating policy: no reg
     "sources": [
       {
         "policy": [
-          "git::${GITHOST}/git/ham-policy?ref=${LATEST_COMMIT}"
+          "git::https://${GITHOST}/git/ham-policy"
         ]
       },
       {
         "policy": [
-          "git::${GITHOST}/git/spam-policy?ref=4707d251d08b466389705c121d84efa2683114cf"
+          "git::https://${GITHOST}/git/spam-policy"
         ]
       }
     ]

features/validate_image.feature

@@ -1121,25 +1121,26 @@ Feature: evaluate enterprise contract
     Then the exit status should be 0
     Then the output should match the snapshot
 
-  Scenario: many components and sources
-    Given a key pair named "known"
-      And a git repository named "multitude-policy" with
-      | main.rego | examples/happy_day.rego |
-      And policy configuration named "ec-policy" with 10 policy sources from "git::https://${GITHOST}/git/multitude-policy.git", patched with
-      | [{"op": "add", "path": "/sources/0/ruleData", "value": {"key": "value"}}]      |
-      | [{"op": "add", "path": "/sources/1/ruleData", "value": {"something": "here"}}] |
-      | [{"op": "add", "path": "/sources/2/ruleData", "value": {"key": "different"}}]  |
-      | [{"op": "add", "path": "/sources/3/ruleData", "value": {"hello": "world"}}]    |
-      | [{"op": "add", "path": "/sources/4/ruleData", "value": {"foo": "bar"}}]        |
-      | [{"op": "add", "path": "/sources/5/ruleData", "value": {"peek": "poke"}}]      |
-      | [{"op": "add", "path": "/sources/6/ruleData", "value": {"hide": "seek"}}]      |
-      | [{"op": "add", "path": "/sources/7/ruleData", "value": {"hokus": "pokus"}}]    |
-      | [{"op": "add", "path": "/sources/8/ruleData", "value": {"mr": "mxyzptlk"}}]    |
-      | [{"op": "add", "path": "/sources/9/ruleData", "value": {"more": "data"}}]      |
-      And an Snapshot named "multitude" with 10 components signed with "known" key
-    When ec command is run with "validate image --snapshot acceptance/multitude --policy acceptance/ec-policy --public-key ${known_PUBLIC_KEY} --rekor-url ${REKOR} --show-successes --output json"
-    Then the exit status should be 0
-      And the output should match the snapshot
+  # Commented out as part of EC-1023. This will be enabled once the issue is resolved.
+  # Scenario: many components and sources
+  #   Given a key pair named "known"
+  #     And a git repository named "multitude-policy" with
+  #     | main.rego | examples/happy_day.rego |
+  #     And policy configuration named "ec-policy" with 10 policy sources from "git::https://${GITHOST}/git/multitude-policy.git", patched with
+  #     | [{"op": "add", "path": "/sources/0/ruleData", "value": {"key": "value"}}]      |
+  #     | [{"op": "add", "path": "/sources/1/ruleData", "value": {"something": "here"}}] |
+  #     | [{"op": "add", "path": "/sources/2/ruleData", "value": {"key": "different"}}]  |
+  #     | [{"op": "add", "path": "/sources/3/ruleData", "value": {"hello": "world"}}]    |
+  #     | [{"op": "add", "path": "/sources/4/ruleData", "value": {"foo": "bar"}}]        |
+  #     | [{"op": "add", "path": "/sources/5/ruleData", "value": {"peek": "poke"}}]      |
+  #     | [{"op": "add", "path": "/sources/6/ruleData", "value": {"hide": "seek"}}]      |
+  #     | [{"op": "add", "path": "/sources/7/ruleData", "value": {"hokus": "pokus"}}]    |
+  #     | [{"op": "add", "path": "/sources/8/ruleData", "value": {"mr": "mxyzptlk"}}]    |
+  #     | [{"op": "add", "path": "/sources/9/ruleData", "value": {"more": "data"}}]      |
+  #     And an Snapshot named "multitude" with 10 components signed with "known" key
+  #   When ec command is run with "validate image --snapshot acceptance/multitude --policy acceptance/ec-policy --public-key ${known_PUBLIC_KEY} --rekor-url ${REKOR} --show-successes --output json"
+  #   Then the exit status should be 0
+  #    And the output should match the snapshot
 
   Scenario: Format options
     Given a key pair named "known"

internal/evaluation_target/input/input.go

@@ -36,7 +36,7 @@ type Input struct {
 
 // NewInput returns a Input struct with FPath and evaluator ready to use
 func NewInput(ctx context.Context, paths []string, p policy.Policy) (*Input, error) {
-	in := &Input{
+	i := &Input{
 		Paths: paths,
 	}
 
@@ -55,8 +55,8 @@ func NewInput(ctx context.Context, paths []string, p policy.Policy) (*Input, err
 		}
 
 		log.Debug("Conftest evaluator initialized")
-		in.Evaluators = append(in.Evaluators, c)
+		i.Evaluators = append(i.Evaluators, c)
 
 	}
-	return in, nil
+	return i, nil
 }

internal/evaluator/conftest_evaluator_test.go

@@ -45,7 +45,6 @@ import (
 	"k8s.io/kube-openapi/pkg/util/sets"
 
 	"github.com/enterprise-contract/ec-cli/internal/downloader"
-	"github.com/enterprise-contract/ec-cli/internal/mutate"
 	"github.com/enterprise-contract/ec-cli/internal/opa/rule"
 	"github.com/enterprise-contract/ec-cli/internal/policy"
 	"github.com/enterprise-contract/ec-cli/internal/policy/source"
@@ -1820,7 +1819,7 @@ func TestConftestEvaluatorEvaluate(t *testing.T) {
 
 	evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{
 		&source.PolicyUrl{
-			Url:  mutate.Const(rules),
+			Url:  rules,
 			Kind: source.PolicyKind,
 		},
 	}, config, ecc.Source{})
@@ -1883,7 +1882,7 @@ func TestUnconformingRule(t *testing.T) {
 
 	evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{
 		&source.PolicyUrl{
-			Url:  mutate.Const(rules),
+			Url:  rules,
 			Kind: source.PolicyKind,
 		},
 	}, p, ecc.Source{})
@@ -2099,7 +2098,7 @@ func TestNewConftestEvaluatorComputeIncludeExclude(t *testing.T) {
 
 			evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{
 				&source.PolicyUrl{
-					Url:  mutate.Const(path.Join(dir, "policy", "rules.tar")),
+					Url:  path.Join(dir, "policy", "rules.tar"),
 					Kind: source.PolicyKind,
 				},
 			}, p, tt.source)

internal/input/validate.go

@@ -47,14 +47,14 @@ func ValidateInput(ctx context.Context, fpath string, policy policy.Policy, deta
 		return nil, err
 	}
 
-	in, err := inputFile(ctx, inputFiles, policy)
+	p, err := inputFile(ctx, inputFiles, policy)
 	if err != nil {
 		log.Debug("Failed to create input!")
 		return nil, err
 	}
 
 	var allResults []evaluator.Outcome
-	for _, e := range in.Evaluators {
+	for _, e := range p.Evaluators {
 		results, _, err := e.Evaluate(ctx, evaluator.EvaluationTarget{Inputs: inputFiles})
 		if err != nil {
 			return nil, fmt.Errorf("evaluating policy: %w", err)