Use given source URLs as keys in download cache

We ran into an issue when we started mixing given and pinned source URLs
in the download cache. This has happened because we cached the given
URLs first, mutated the policy to contain the pinned URLs and later
tried to download through the cache via pinned URLs.

This changes so we only use the given source URLs in the download cache
by allowing the source URLs to mutate the policy URLs after they are
downloaded. That is, for each source URL in the policy we have two
states: one containing the given value and used in the download cache,
and second after the download with the resulting resolved/pinned value.

There are two repercussions of this: any resolving/pinning of URLs is
transparent to the callers if they use `source.PolicySourcesFrom`
followed by `url.GetPolicy` -- this allows any use of `Policy` to be
automatically resolved/pinned (no need for Policy pre-processing); and
secondly the Policy is mutated while in execution via `url.GetPolicy`
and careful consideration, hopefully/effectively shielded by using
`source.PolicySourcesFrom`, of the slice mutation -- making sure that
the mutation doesn't happen in the temporary value of the iteration is
required.

Resolves: https://issues.redhat.com/browse/EC-963

cmd/fetch/fetch_policy.go

@@ -23,6 +23,7 @@ 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"
 )
@@ -108,11 +109,11 @@ func fetchPolicyCmd() *cobra.Command {
 			sources := make([]*source.PolicyUrl, 0, len(sourceUrls)+len(dataSourceUrls))
 
 			for _, url := range sourceUrls {
-				sources = append(sources, &source.PolicyUrl{Url: url, Kind: source.PolicyKind})
+				sources = append(sources, &source.PolicyUrl{Url: mutate.Const(url), Kind: source.PolicyKind})
 			}
 
 			for _, url := range dataSourceUrls {
-				sources = append(sources, &source.PolicyUrl{Url: url, Kind: source.DataKind})
+				sources = append(sources, &source.PolicyUrl{Url: mutate.Const(url), Kind: source.DataKind})
 			}
 
 			for _, s := range sources {

cmd/inspect/inspect_policy.go

@@ -28,6 +28,7 @@ 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"
@@ -118,7 +119,7 @@ func inspectPolicyCmd() *cobra.Command {
 
 			allResults := make(map[string][]*ast.AnnotationsRef)
 			for _, url := range sourceUrls {
-				s := &source.PolicyUrl{Url: url, Kind: source.PolicyKind}
+				s := &source.PolicyUrl{Url: mutate.Const(url), Kind: source.PolicyKind}
 
 				// Download
 				policyDir, err := s.GetPolicy(ctx, destDir, false)

cmd/inspect/inspect_policy_data.go

@@ -31,6 +31,7 @@ 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"
 )
@@ -88,7 +89,7 @@ func inspectPolicyDataCmd() *cobra.Command {
 
 			allData := make(map[string]interface{})
 			for _, url := range sourceUrls {
-				s := &source.PolicyUrl{Url: url, Kind: source.PolicyKind}
+				s := &source.PolicyUrl{Url: mutate.Const(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,
 			}
 
-			// 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 {
+			p, err := policy.NewPolicy(ctx, policyOptions)
+			if err != nil {
 				allErrors = errors.Join(allErrors, err)
-			} 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)
-						}
+				return
+			}
 
-						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
+			// 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")
 						}
-						rule_data_raw, err = json.Marshal(unmarshaled)
+						err = json.Unmarshal(rule_data_raw, &unmarshaled)
 						if err != nil {
-							log.Errorf("Unable to parse updated ruledata: %s", err.Error())
+							log.Errorf("Unable to parse ruledata into standard JSON object")
 						}
+					} else {
+						sources[i].RuleData = new(extv1.JSON)
+					}
 
-						if rule_data_raw == nil {
-							log.Errorf("Invalid rule data 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")
 						}
-
-						err = sources[i].RuleData.UnmarshalJSON(rule_data_raw)
+						extraRuleDataPolicyConfig, err := validate_utils.GetPolicyConfig(ctx, parts[1])
 						if err != nil {
-							log.Errorf("Unable to marshal updated JSON: %s", err.Error())
+							log.Errorf("Unable to load data from extraRuleData: %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)
 				}
-				data.policy = p
+				policySpec.Sources = sources
+				p = p.WithSpec(policySpec)
 			}
+			data.policy = p
 
 			return
 		},

cmd/validate/image_integration_test.go

@@ -28,7 +28,6 @@ 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"
@@ -52,12 +51,10 @@ 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{}
@@ -73,7 +70,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(strings.Split(strings.Split(s[0].PolicyUrl(), "@")[0], "::")[1])
+		idx, err := strconv.Atoi(s[0].PolicyUrl())
 		require.NoError(t, err)
 
 		return evaluators[idx], nil

cmd/validate/image_test.go

@@ -64,7 +64,16 @@ var rootArgs = []string{
 }
 
 func happyValidator() imageValidationFunc {
-	return func(_ context.Context, component app.SnapshotComponent, _ *app.SnapshotSpec, _ policy.Policy, _ []evaluator.Evaluator, _ bool) (*output.Output, error) {
+	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 &output.Output{
 			ImageSignatureCheck: output.VerificationStatus{
 				Passed: true,

features/__snapshots__/validate_image.snap

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

features/__snapshots__/validate_input.snap

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

features/validate_image.feature

@@ -1137,9 +1137,8 @@ Feature: evaluate enterprise contract
       | [{"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"
+    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 --trace"
     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) {
-	i := &Input{
+	in := &Input{
 		Paths: paths,
 	}
 
@@ -55,8 +55,8 @@ func NewInput(ctx context.Context, paths []string, p policy.Policy) (*Input, err
 		}
 
 		log.Debug("Conftest evaluator initialized")
-		i.Evaluators = append(i.Evaluators, c)
+		in.Evaluators = append(in.Evaluators, c)
 
 	}
-	return i, nil
+	return in, nil
 }

internal/evaluator/conftest_evaluator_test.go

@@ -45,6 +45,7 @@ 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"
@@ -1819,7 +1820,7 @@ func TestConftestEvaluatorEvaluate(t *testing.T) {
 
 	evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{
 		&source.PolicyUrl{
-			Url:  rules,
+			Url:  mutate.Const(rules),
 			Kind: source.PolicyKind,
 		},
 	}, config, ecc.Source{})
@@ -1882,7 +1883,7 @@ func TestUnconformingRule(t *testing.T) {
 
 	evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{
 		&source.PolicyUrl{
-			Url:  rules,
+			Url:  mutate.Const(rules),
 			Kind: source.PolicyKind,
 		},
 	}, p, ecc.Source{})
@@ -2098,7 +2099,7 @@ func TestNewConftestEvaluatorComputeIncludeExclude(t *testing.T) {
 
 			evaluator, err := NewConftestEvaluator(ctx, []source.PolicySource{
 				&source.PolicyUrl{
-					Url:  path.Join(dir, "policy", "rules.tar"),
+					Url:  mutate.Const(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
 	}
 
-	p, err := inputFile(ctx, inputFiles, policy)
+	in, err := inputFile(ctx, inputFiles, policy)
 	if err != nil {
 		log.Debug("Failed to create input!")
 		return nil, err
 	}
 
 	var allResults []evaluator.Outcome
-	for _, e := range p.Evaluators {
+	for _, e := range in.Evaluators {
 		results, _, err := e.Evaluate(ctx, evaluator.EvaluationTarget{Inputs: inputFiles})
 		if err != nil {
 			return nil, fmt.Errorf("evaluating policy: %w", err)