Use given source URLs as keys in download cache

[zregvart]

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
Fixes: #2107

[zregvart]

I think I'd like to add more tests for this, not sure if I want to do this in this PR (which is already quite large) or in a followup.

[zregvart]

To reproduce the issue reliably add time.Sleep(2 * time.Seconds) here (in source.getPolicyThroughCache)

[zregvart]

The snapshots of these scenarios contain unpinned URLs because the url.GetPolicy doesn't occur due to errors from the builtin checks

[zregvart]

validate input now supports pinned source URLs

[codecov]

Codecov Report

Attention: Patch coverage is 73.41772% with 21 lines in your changes missing coverage. Please review.

Project coverage is 71.22%. Comparing base (247c44a) to head (087f465).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/validate/image.go	80.48%	8 Missing ⚠️
internal/policy/source/git_config.go	0.00%	4 Missing ⚠️
internal/evaluation_target/input/input.go	0.00%	3 Missing ⚠️
cmd/fetch/fetch_policy.go	0.00%	2 Missing ⚠️
internal/mutate/mutate.go	87.50%	2 Missing ⚠️
cmd/inspect/inspect_policy_data.go	0.00%	1 Missing ⚠️
internal/policy/source/source.go	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2150      +/-   ##
==========================================
- Coverage   71.25%   71.22%   -0.04%     
==========================================
  Files          88       89       +1     
  Lines        7512     7479      -33     
==========================================
- Hits         5353     5327      -26     
+ Misses       2159     2152       -7     

Flag	Coverage Δ
generative	?
integration	?
unit	71.22% <73.41%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cmd/inspect/inspect_policy.go	67.11% <100.00%> (ø)
internal/input/validate.go	77.52% <100.00%> (ø)
internal/policy/policy.go	80.53% <ø> (+0.78%)	⬆️
cmd/inspect/inspect_policy_data.go	35.45% <0.00%> (ø)
internal/policy/source/source.go	82.57% <88.88%> (-2.81%)	⬇️
cmd/fetch/fetch_policy.go	0.00% <0.00%> (ø)
internal/mutate/mutate.go	87.50% <87.50%> (ø)
internal/evaluation_target/input/input.go	0.00% <0.00%> (ø)
internal/policy/source/git_config.go	38.23% <0.00%> (ø)
cmd/validate/image.go	91.83% <80.48%> (ø)

[lcarva]

I really would prefer if we did have a pattern where something transforms an existing policy into a new policy with the pinned references. (I don't think we got that implementation quite right.) I'm hesitant of adding more pointer manipulation in the code base because that is harder to debug and maintain. But this is all internal and we should be able to refactor as needed later on.

[zregvart]

I really would prefer if we did have a pattern where something transforms an existing policy into a new policy with the pinned references. (I don't think we got that implementation quite right.) I'm hesitant of adding more pointer manipulation in the code base because that is harder to debug and maintain. But this is all internal and we should be able to refactor as needed later on.

Me too, would be a much bigger refactor though. The way we have it working now is that the download of sources is initiated from the evaluator, and the previous attempt of preprocessing the policy led us to a myriad of issues. Not saying it can't be done.