From 087f4659b10ada36b092d3f599e024d5c79ac955 Mon Sep 17 00:00:00 2001 From: Zoran Regvart Date: Fri, 8 Nov 2024 13:22:23 +0100 Subject: [PATCH] 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 | 5 +- cmd/inspect/inspect_policy.go | 3 +- cmd/inspect/inspect_policy_data.go | 3 +- cmd/validate/image.go | 90 +++++++++---------- cmd/validate/image_integration_test.go | 5 +- cmd/validate/image_test.go | 11 ++- features/__snapshots__/validate_image.snap | 8 +- features/__snapshots__/validate_input.snap | 6 +- features/validate_image.feature | 2 +- internal/evaluation_target/input/input.go | 6 +- internal/evaluator/conftest_evaluator_test.go | 7 +- internal/input/validate.go | 4 +- internal/mutate/mutate.go | 65 ++++++++++++++ internal/mutate/mutate_test.go | 63 +++++++++++++ internal/policy/policy.go | 65 -------------- internal/policy/policy_test.go | 11 +-- internal/policy/source/git_config.go | 10 ++- internal/policy/source/source.go | 21 ++--- internal/policy/source/source_test.go | 34 ++++--- 19 files changed, 252 insertions(+), 167 deletions(-) create mode 100644 internal/mutate/mutate.go create mode 100644 internal/mutate/mutate_test.go diff --git a/cmd/fetch/fetch_policy.go b/cmd/fetch/fetch_policy.go index 9b8bd6a2c..7a04fdc1f 100644 --- a/cmd/fetch/fetch_policy.go +++ b/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 { diff --git a/cmd/inspect/inspect_policy.go b/cmd/inspect/inspect_policy.go index 70b53bb2c..c59a55b83 100644 --- a/cmd/inspect/inspect_policy.go +++ b/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) diff --git a/cmd/inspect/inspect_policy_data.go b/cmd/inspect/inspect_policy_data.go index f51df593d..a3237d8b4 100644 --- a/cmd/inspect/inspect_policy_data.go +++ b/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) diff --git a/cmd/validate/image.go b/cmd/validate/image.go index 2538ada80..2e19d99b2 100644 --- a/cmd/validate/image.go +++ b/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 }, diff --git a/cmd/validate/image_integration_test.go b/cmd/validate/image_integration_test.go index c530904e8..666a5300c 100644 --- a/cmd/validate/image_integration_test.go +++ b/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 diff --git a/cmd/validate/image_test.go b/cmd/validate/image_test.go index be44dada2..2b0209a2f 100644 --- a/cmd/validate/image_test.go +++ b/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, diff --git a/features/__snapshots__/validate_image.snap b/features/__snapshots__/validate_image.snap index dfb60cc31..c9b458d41 100755 --- a/features/__snapshots__/validate_image.snap +++ b/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" ] } ], diff --git a/features/__snapshots__/validate_input.snap b/features/__snapshots__/validate_input.snap index 7f301a3e0..b59fc2f9c 100755 --- a/features/__snapshots__/validate_input.snap +++ b/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" ] } ] diff --git a/features/validate_image.feature b/features/validate_image.feature index e03b482a8..93cded8e2 100644 --- a/features/validate_image.feature +++ b/features/validate_image.feature @@ -1139,7 +1139,7 @@ Feature: evaluate enterprise contract 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 + And the output should match the snapshot Scenario: Format options Given a key pair named "known" diff --git a/internal/evaluation_target/input/input.go b/internal/evaluation_target/input/input.go index 4e980dd0c..79eddfe62 100644 --- a/internal/evaluation_target/input/input.go +++ b/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 } diff --git a/internal/evaluator/conftest_evaluator_test.go b/internal/evaluator/conftest_evaluator_test.go index 6202d92b7..908186c6c 100644 --- a/internal/evaluator/conftest_evaluator_test.go +++ b/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) diff --git a/internal/input/validate.go b/internal/input/validate.go index 00e8ac04d..e3ab3135f 100644 --- a/internal/input/validate.go +++ b/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) diff --git a/internal/mutate/mutate.go b/internal/mutate/mutate.go new file mode 100644 index 000000000..faffed037 --- /dev/null +++ b/internal/mutate/mutate.go @@ -0,0 +1,65 @@ +// Copyright The Enterprise Contract Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.` +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package mutate + +import "fmt" + +type Mut[T any] interface { + Value() T + Set(T) +} + +type mutableValue[T any] struct { + v *T +} + +func (m *mutableValue[T]) Value() T { + return *m.v +} + +func (m *mutableValue[T]) Set(value T) { + *m.v = value +} + +func (m *mutableValue[T]) String() string { + return fmt.Sprint("%", *m.v) +} + +func Value[T any](value *T) Mut[T] { + return &mutableValue[T]{value} +} + +func Const[T any](value T) Mut[T] { + return &mutableValue[T]{&value} +} + +type mutableSlice[T any] struct { + s []T + idx int +} + +func (m *mutableSlice[T]) Value() T { + return m.s[m.idx] +} + +func (m *mutableSlice[T]) Set(value T) { + m.s[m.idx] = value +} + +func Slice[T any](s []T, i int) Mut[T] { + return &mutableSlice[T]{s, i} +} diff --git a/internal/mutate/mutate_test.go b/internal/mutate/mutate_test.go new file mode 100644 index 000000000..cf75af44b --- /dev/null +++ b/internal/mutate/mutate_test.go @@ -0,0 +1,63 @@ +// Copyright The Enterprise Contract Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.` +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package mutate + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMutatesStructFields(t *testing.T) { + s := struct { + some string + }{ + "hello", + } + + m := Value(&s.some) + + assert.Equal(t, "hello", s.some) + assert.Equal(t, "hello", m.Value()) + + m.Set("hi") + assert.Equal(t, "hi", s.some) + assert.Equal(t, "hi", m.Value()) +} + +func TestMutatesSliceItems(t *testing.T) { + s := []string{ + "hello", + "world", + } + + m := Slice(s, 1) + + assert.Equal(t, []string{"hello", "world"}, s) + assert.Equal(t, "world", m.Value()) + + m.Set("universe") + assert.Equal(t, []string{"hello", "universe"}, s) + assert.Equal(t, "universe", m.Value()) +} + +func TestEquality(t *testing.T) { + x := "value" + + assert.Equal(t, Value(&x), Value(&x)) + assert.Equal(t, Value(&x), Const("value")) +} diff --git a/internal/policy/policy.go b/internal/policy/policy.go index c12807ed5..d9a1fcece 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -27,7 +27,6 @@ import ( "strings" "time" - "github.com/enterprise-contract/enterprise-contract-controller/api/v1alpha1" ecc "github.com/enterprise-contract/enterprise-contract-controller/api/v1alpha1" schemaExporter "github.com/invopop/jsonschema" "github.com/santhosh-tekuri/jsonschema/v5" @@ -582,70 +581,6 @@ func validatePolicyConfig(policyConfig string) error { return nil } -// PreProcessPolicy fetches policy sources and returns a policy object with -// pinned SHA/image digest URL where applicable, along with a policy cache object. -func PreProcessPolicy(ctx context.Context, policyOptions Options) (Policy, *cache.PolicyCache, error) { - var policyCache *cache.PolicyCache - pinnedPolicyUrls := map[string][]string{} - policyCache, err := cache.NewPolicyCache(ctx) - if err != nil { - return nil, nil, err - } - - p, err := NewPolicy(ctx, policyOptions) - if err != nil { - return nil, nil, err - } - - sources := p.Spec().Sources - for i, sourceGroup := range sources { - log.Debugf("Fetching policy source group '%+v'\n", sourceGroup.Name) - policySources := PolicySourcesFrom(sourceGroup) - - fs := utils.FS(ctx) - dir, err := utils.CreateWorkDir(fs) - if err != nil { - log.Debug("Failed to create work dir!") - return nil, nil, err - } - - for _, policySource := range policySources { - if strings.HasPrefix(policySource.PolicyUrl(), "data:") { - continue - } - - destDir, err := policySource.GetPolicy(ctx, dir, false) - if err != nil { - log.Debugf("Unable to download source from %s!", policySource.PolicyUrl()) - return nil, nil, err - } - log.Debugf("Downloaded policy source from %s to %s\n", policySource.PolicyUrl(), destDir) - - url := policySource.PolicyUrl() - - if _, found := policyCache.Get(policySource.PolicyUrl()); !found { - log.Debugf("Cache miss for: %s, adding to cache", url) - policyCache.Set(url, destDir, nil) - pinnedPolicyUrls[policySource.Subdir()] = append(pinnedPolicyUrls[policySource.Subdir()], url) - log.Debugf("Added %s to the pinnedPolicyUrls in \"%s\"", url, policySource.Subdir()) - } else { - log.Debugf("Cache hit for: %s", url) - } - } - - sources[i] = v1alpha1.Source{ - Name: sourceGroup.Name, - Policy: urls(policySources, source.PolicyKind), - Data: urls(policySources, source.DataKind), - RuleData: sourceGroup.RuleData, - Config: sourceGroup.Config, - VolatileConfig: sourceGroup.VolatileConfig, - } - } - - return p, policyCache, err -} - func urls(s []source.PolicySource, kind source.PolicyType) []string { ret := make([]string, 0, len(s)) for _, u := range s { diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index acb7a456b..fa2b2776a 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -39,6 +39,7 @@ import ( "sigs.k8s.io/yaml" "github.com/enterprise-contract/ec-cli/internal/kubernetes" + "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" ) @@ -831,9 +832,9 @@ func TestUrls(t *testing.T) { { name: "Returns URLs of the specified kind", s: []source.PolicySource{ - &source.PolicyUrl{Url: "http://example.com/policy1", Kind: source.PolicyKind}, - &source.PolicyUrl{Url: "http://example.com/data1", Kind: source.DataKind}, - &source.PolicyUrl{Url: "http://example.com/policy2", Kind: source.PolicyKind}, + &source.PolicyUrl{Url: mutate.Const("http://example.com/policy1"), Kind: source.PolicyKind}, + &source.PolicyUrl{Url: mutate.Const("http://example.com/data1"), Kind: source.DataKind}, + &source.PolicyUrl{Url: mutate.Const("http://example.com/policy2"), Kind: source.PolicyKind}, }, kind: source.PolicyKind, want: []string{"http://example.com/policy1", "http://example.com/policy2"}, @@ -841,8 +842,8 @@ func TestUrls(t *testing.T) { { name: "Returns empty slice when no URLs of the specified kind", s: []source.PolicySource{ - &source.PolicyUrl{Url: "http://example.com/data1", Kind: source.PolicyType("data")}, - &source.PolicyUrl{Url: "http://example.com/data2", Kind: source.PolicyType("data")}, + &source.PolicyUrl{Url: mutate.Const("http://example.com/data1"), Kind: source.PolicyType("data")}, + &source.PolicyUrl{Url: mutate.Const("http://example.com/data2"), Kind: source.PolicyType("data")}, }, kind: source.PolicyKind, want: []string{}, diff --git a/internal/policy/source/git_config.go b/internal/policy/source/git_config.go index d615a2f50..d534b52df 100644 --- a/internal/policy/source/git_config.go +++ b/internal/policy/source/git_config.go @@ -28,6 +28,8 @@ import ( getter "github.com/hashicorp/go-getter" log "github.com/sirupsen/logrus" + + "github.com/enterprise-contract/ec-cli/internal/mutate" ) // SourceIsFile returns true if go-getter thinks the src looks like a file path. @@ -57,21 +59,21 @@ func SourceIsHttp(src string) bool { func GoGetterDownload(ctx context.Context, tmpDir, src string) (string, error) { // Download the config from a url c := PolicyUrl{ - Url: src, + Url: mutate.Value(&src), Kind: ConfigKind, } configDir, err := c.GetPolicy(ctx, tmpDir, false) if err != nil { - log.Debugf("Failed to download policy config from %s", c.Url) + log.Debugf("Failed to download policy config from %s", c.Url.Value()) return "", err } - log.Debugf("Downloaded policy config from %s to %s", c.Url, configDir) + log.Debugf("Downloaded policy config from %s to %s", c.Url.Value(), configDir) // Look for a suitable file to use for the config configFile, err := choosePolicyFile(ctx, configDir) if err != nil { // A more useful error message: - return "", fmt.Errorf("no suitable config file found at %s", c.Url) + return "", fmt.Errorf("no suitable config file found at %s", c.Url.Value()) } log.Debugf("Chose file %s to use for the policy config", configFile) return configFile, nil diff --git a/internal/policy/source/source.go b/internal/policy/source/source.go index e49a8fccc..4971cad30 100644 --- a/internal/policy/source/source.go +++ b/internal/policy/source/source.go @@ -40,6 +40,7 @@ import ( "github.com/spf13/afero" "github.com/enterprise-contract/ec-cli/internal/downloader" + "github.com/enterprise-contract/ec-cli/internal/mutate" "github.com/enterprise-contract/ec-cli/internal/utils" ) @@ -70,8 +71,7 @@ type PolicySource interface { } type PolicyUrl struct { - // A string containing a go-getter style source url compatible with conftest pull - Url string + Url mutate.Mut[string] Kind PolicyType } @@ -144,7 +144,7 @@ func (p *PolicyUrl) GetPolicy(ctx context.Context, workDir string, showMsg bool) if trace.IsEnabled() { region := trace.StartRegion(ctx, "ec:get-policy") defer region.End() - trace.Logf(ctx, "", "policy=%q", p.Url) + trace.Logf(ctx, "", "policy=%q", p.Url.Value()) } dl := func(source string, dest string) (metadata.Metadata, error) { @@ -160,17 +160,18 @@ func (p *PolicyUrl) GetPolicy(ctx context.Context, workDir string, showMsg bool) return "", err } - p.Url, err = metadata.GetPinnedURL(p.Url) - log.Debug("Pinned URL: ", p.Url) + pinned, err := metadata.GetPinnedURL(p.Url.Value()) if err != nil { return "", err } + log.Debug("Pinned URL: ", pinned) + p.Url.Set(pinned) return dest, err } func (p *PolicyUrl) PolicyUrl() string { - return p.Url + return p.Url.Value() } func (p *PolicyUrl) Subdir() string { @@ -253,13 +254,13 @@ func (inlineData) Type() PolicyType { func PolicySourcesFrom(s ecc.Source) []PolicySource { policySources := make([]PolicySource, 0, len(s.Policy)+len(s.Data)) - for _, policySourceUrl := range s.Policy { - url := PolicyUrl{Url: policySourceUrl, Kind: PolicyKind} + for i := range s.Policy { + url := PolicyUrl{Url: mutate.Slice(s.Policy, i), Kind: PolicyKind} policySources = append(policySources, &url) } - for _, dataSourceUrl := range s.Data { - url := PolicyUrl{Url: dataSourceUrl, Kind: DataKind} + for i := range s.Data { + url := PolicyUrl{Url: mutate.Slice(s.Data, i), Kind: DataKind} policySources = append(policySources, &url) } diff --git a/internal/policy/source/source_test.go b/internal/policy/source/source_test.go index 87bc36127..768388ffc 100644 --- a/internal/policy/source/source_test.go +++ b/internal/policy/source/source_test.go @@ -37,6 +37,7 @@ import ( "github.com/stretchr/testify/require" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "github.com/enterprise-contract/ec-cli/internal/mutate" "github.com/enterprise-contract/ec-cli/internal/utils" ) @@ -86,7 +87,7 @@ func TestGetPolicy(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - p := PolicyUrl{Url: tt.sourceUrl, Kind: PolicyKind} + p := PolicyUrl{Url: mutate.Value(&tt.sourceUrl), Kind: PolicyKind} dl := mockDownloader{} dl.On("Download", mock.MatchedBy(func(dest string) bool { @@ -137,6 +138,13 @@ func TestInlineDataSource(t *testing.T) { } func TestPolicySourcesFrom(t *testing.T) { + + policies1 := []string{"github.com/org/repo1//policy/", "github.com/org/repo2//policy/", "github.com/org/repo3//policy/"} + data1 := []string{"github.com/org/repo1//data/", "github.com/org/repo2//data/", "github.com/org/repo3//data/"} + + policies2 := []string{"github.com/org/repo1//policy/"} + data2 := []string{"github.com/org/repo1//data/"} + // var ruleData = &extv1.JSON{Raw: []byte("foo")} tests := []struct { name string @@ -147,29 +155,29 @@ func TestPolicySourcesFrom(t *testing.T) { name: "fetches policy configs", source: ecc.Source{ Name: "policy1", - Policy: []string{"github.com/org/repo1//policy/", "github.com/org/repo2//policy/", "github.com/org/repo3//policy/"}, - Data: []string{"github.com/org/repo1//data/", "github.com/org/repo2//data/", "github.com/org/repo3//data/"}, + Policy: policies1, + Data: data1, }, expected: []PolicySource{ - &PolicyUrl{Url: "github.com/org/repo1//policy/", Kind: PolicyKind}, - &PolicyUrl{Url: "github.com/org/repo2//policy/", Kind: PolicyKind}, - &PolicyUrl{Url: "github.com/org/repo3//policy/", Kind: PolicyKind}, - &PolicyUrl{Url: "github.com/org/repo1//data/", Kind: DataKind}, - &PolicyUrl{Url: "github.com/org/repo2//data/", Kind: DataKind}, - &PolicyUrl{Url: "github.com/org/repo3//data/", Kind: DataKind}, + &PolicyUrl{Url: mutate.Slice(policies1, 0), Kind: PolicyKind}, + &PolicyUrl{Url: mutate.Slice(policies1, 1), Kind: PolicyKind}, + &PolicyUrl{Url: mutate.Slice(policies1, 2), Kind: PolicyKind}, + &PolicyUrl{Url: mutate.Slice(data1, 0), Kind: DataKind}, + &PolicyUrl{Url: mutate.Slice(data1, 1), Kind: DataKind}, + &PolicyUrl{Url: mutate.Slice(data1, 2), Kind: DataKind}, }, }, { name: "handles rule data", source: ecc.Source{ Name: "policy2", - Policy: []string{"github.com/org/repo1//policy/"}, - Data: []string{"github.com/org/repo1//data/"}, + Policy: policies2, + Data: data2, RuleData: &extv1.JSON{Raw: []byte(`"foo":"bar"`)}, }, expected: []PolicySource{ - &PolicyUrl{Url: "github.com/org/repo1//policy/", Kind: PolicyKind}, - &PolicyUrl{Url: "github.com/org/repo1//data/", Kind: DataKind}, + &PolicyUrl{Url: mutate.Slice(policies2, 0), Kind: PolicyKind}, + &PolicyUrl{Url: mutate.Slice(data2, 0), Kind: DataKind}, inlineData{source: []byte("{\"rule_data__configuration__\":\"foo\":\"bar\"}")}, }, },