Merge pull request #1532 from zregvart/pr/evaluator-lifecycle

Ensure evaluators are not destroyed if reused

Makefile

@@ -83,7 +83,7 @@ clean: ## Delete build output
 .PHONY: test
 test: ## Run all unit tests
 	@echo "Unit tests:"
-	@go test -race -covermode=atomic -coverprofile=coverage-unit.out -timeout 500ms -tags=unit ./...
+	@go test -race -covermode=atomic -coverprofile=coverage-unit.out -timeout 1s -tags=unit ./...
 	@echo "Integration tests:"
 	@go test -race -covermode=atomic -coverprofile=coverage-integration.out -timeout 15s -tags=integration ./... | grep -v '\[no test files\]'
 # Given the nature of generative tests the test timeout is increased from 500ms

acceptance/image/image.go

@@ -132,11 +132,11 @@ func imageFrom(ctx context.Context, imageName string) (v1.Image, error) {
 	return remote.Image(ref)
 }
 
-// createAndPushImageSignature for a named image in the Context creates a signature
+// CreateAndPushImageSignature for a named image in the Context creates a signature
 // image, same as `cosign sign` or Tekton Chains would, of that named image and pushes it
 // to the stub registry as a new tag for that image akin to how cosign and Tekton Chains
 // do it
-func createAndPushImageSignature(ctx context.Context, imageName string, keyName string) (context.Context, error) {
+func CreateAndPushImageSignature(ctx context.Context, imageName string, keyName string) (context.Context, error) {
 	var state *imageState
 	ctx, err := testenv.SetupState(ctx, &state)
 	if err != nil {
@@ -218,10 +218,10 @@ func createAndPushImageSignature(ctx context.Context, imageName string, keyName
 	return ctx, nil
 }
 
-// createAndPushAttestation for a named image in the Context creates an attestation
+// CreateAndPushAttestation for a named image in the Context creates an attestation
 // image, same as `cosign attest` or Tekton Chains would, and pushes it to the stub
 // registry as a new tag for that image akin to how cosign and Tekton Chains do it
-func createAndPushAttestation(ctx context.Context, imageName, keyName string) (context.Context, error) {
+func CreateAndPushAttestation(ctx context.Context, imageName, keyName string) (context.Context, error) {
 	return createAndPushAttestationWithPatches(ctx, imageName, keyName, nil)
 }
 
@@ -326,8 +326,8 @@ func createAndPushAttestationWithPatches(ctx context.Context, imageName, keyName
 	return ctx, nil
 }
 
-// createAndPushImageWithParent creates a parent image and a test image for the given imageName.
-func createAndPushImageWithParent(ctx context.Context, imageName string) (context.Context, error) {
+// CreateAndPushImageWithParent creates a parent image and a test image for the given imageName.
+func CreateAndPushImageWithParent(ctx context.Context, imageName string) (context.Context, error) {
 	var err error
 
 	parentName := fmt.Sprintf("%s/parent", imageName)
@@ -897,7 +897,7 @@ func applyPatches(statement *in_toto.ProvenanceStatementSLSA02, patches *godog.T
 // ("sig") or attestation ("att")
 func steal(what string) func(context.Context, string, string) (context.Context, error) {
 	return func(ctx context.Context, imageName string, signatureFrom string) (context.Context, error) {
-		ctx, err := createAndPushImageWithParent(ctx, imageName)
+		ctx, err := CreateAndPushImageWithParent(ctx, imageName)
 		if err != nil {
 			return ctx, err
 		}
@@ -984,11 +984,11 @@ func copyAllImages(ctx context.Context, source, destination string) (context.Con
 
 // AddStepsTo adds Gherkin steps to the godog ScenarioContext
 func AddStepsTo(sc *godog.ScenarioContext) {
-	sc.Step(`^an image named "([^"]*)"$`, createAndPushImageWithParent)
+	sc.Step(`^an image named "([^"]*)"$`, CreateAndPushImageWithParent)
 	sc.Step(`^an image named "([^"]*)" containing a layer with:$`, createAndPushImageWithLayer)
 	sc.Step(`^the image "([^"]*)" has labels:$`, labelImage)
-	sc.Step(`^a valid image signature of "([^"]*)" image signed by the "([^"]*)" key$`, createAndPushImageSignature)
-	sc.Step(`^a valid attestation of "([^"]*)" signed by the "([^"]*)" key$`, createAndPushAttestation)
+	sc.Step(`^a valid image signature of "([^"]*)" image signed by the "([^"]*)" key$`, CreateAndPushImageSignature)
+	sc.Step(`^a valid attestation of "([^"]*)" signed by the "([^"]*)" key$`, CreateAndPushAttestation)
 	sc.Step(`^a valid attestation of "([^"]*)" signed by the "([^"]*)" key, patched with$`, createAndPushAttestationWithPatches)
 	sc.Step(`^a signed and attested keyless image named "([^"]*)"$`, createAndPushKeylessImage)
 	sc.Step(`^a OCI policy bundle named "([^"]*)" with$`, createAndPushPolicyBundle)

acceptance/kubernetes/kubernetes.go

@@ -35,6 +35,7 @@ import (
 	"github.com/enterprise-contract/ec-cli/acceptance/kubernetes/stub"
 	"github.com/enterprise-contract/ec-cli/acceptance/kubernetes/types"
 	"github.com/enterprise-contract/ec-cli/acceptance/registry"
+	"github.com/enterprise-contract/ec-cli/acceptance/rekor"
 	"github.com/enterprise-contract/ec-cli/acceptance/snaps"
 	"github.com/enterprise-contract/ec-cli/acceptance/testenv"
 )
@@ -122,6 +123,23 @@ func createNamedPolicy(ctx context.Context, name string, specification *godog.Do
 	return c.cluster.CreateNamedPolicy(ctx, name, specification.Content)
 }
 
+func createNamedPolicyWithManySources(ctx context.Context, name string, amount int, source string) error {
+	c := testenv.FetchState[ClusterState](ctx)
+
+	if err := mustBeUp(ctx, *c); err != nil {
+		return err
+	}
+
+	sources := make([]string, 0, amount)
+	for i := 0; i < amount; i++ {
+		sources = append(sources, fmt.Sprintf(`{"policy": ["%s"]}`, source))
+	}
+
+	policy := fmt.Sprintf(`{"sources": [%s]}`, strings.Join(sources, ", "))
+
+	return c.cluster.CreateNamedPolicy(ctx, name, policy)
+}
+
 func createNamedSnapshot(ctx context.Context, name string, specification *godog.DocString) error {
 	c := testenv.FetchState[ClusterState](ctx)
 
@@ -132,6 +150,50 @@ func createNamedSnapshot(ctx context.Context, name string, specification *godog.
 	return c.cluster.CreateNamedSnapshot(ctx, name, specification.Content)
 }
 
+func createNamedSnapshotWithManyComponents(ctx context.Context, name string, amount int, key string) (context.Context, error) {
+	c := testenv.FetchState[ClusterState](ctx)
+
+	if err := mustBeUp(ctx, *c); err != nil {
+		return ctx, err
+	}
+
+	components := make([]string, 0, amount)
+	for i := 0; i < amount; i++ {
+		imageRef := fmt.Sprintf("%s/image-%d", name, i)
+		var err error
+		ctx, err = image.CreateAndPushImageWithParent(ctx, imageRef)
+		if err != nil {
+			return ctx, err
+		}
+
+		ctx, err = image.CreateAndPushImageSignature(ctx, imageRef, key)
+		if err != nil {
+			return ctx, err
+		}
+
+		err = rekor.RekorEntryForImageSignature(ctx, imageRef)
+		if err != nil {
+			return ctx, err
+		}
+
+		ctx, err = image.CreateAndPushAttestation(ctx, imageRef, key)
+		if err != nil {
+			return ctx, err
+		}
+
+		err = rekor.RekorEntryForAttestation(ctx, imageRef)
+		if err != nil {
+			return ctx, err
+		}
+
+		components = append(components, fmt.Sprintf(`{"name": "component%d", "containerImage": "${REGISTRY}/%s"}`, i, imageRef))
+	}
+
+	snapshot := fmt.Sprintf(`{"components": [%s]}`, strings.Join(components, ", "))
+
+	return ctx, c.cluster.CreateNamedSnapshot(ctx, name, snapshot)
+}
+
 func createPolicy(ctx context.Context, specification *godog.DocString) error {
 	c := testenv.FetchState[ClusterState](ctx)
 
@@ -331,8 +393,10 @@ func AddStepsTo(sc *godog.ScenarioContext) {
 	sc.Step(`^the task should succeed$`, theTaskShouldSucceed)
 	sc.Step(`^the task should fail$`, theTaskShouldFail)
 	sc.Step(`^an Snapshot named "([^"]*)" with specification$`, createNamedSnapshot)
+	sc.Step(`^an Snapshot named "([^"]*)" with (\d+) components signed with "([^"]*)" key`, createNamedSnapshotWithManyComponents)
 	sc.Step(`^the task logs for step "([^"]*)" should match the snapshot$`, taskLogsShouldMatchTheSnapshot)
 	sc.Step(`^the task results should match the snapshot$`, taskResultsShouldMatchTheSnapshot)
+	sc.Step(`^policy configuration named "([^"]*)" with (\d+) policy sources from "([^"]*)"$`, createNamedPolicyWithManySources)
 	// stop usage of the cluster once a test is done, godog will call this
 	// function on failure and on the last step, so more than once if the
 	// failure is not on the last step and once if there was no failure or the

acceptance/rekor/rekor.go

@@ -311,10 +311,10 @@ func jsonPathFromAttestation(data []byte) (string, error) {
 	return fmt.Sprintf("$..[?(@.value=='%x')]", sha256.Sum256(data)), nil
 }
 
-// rekorEntryForAttestation given an image name for which attestation has been
+// RekorEntryForAttestation given an image name for which attestation has been
 // previously performed via image.createAndPushAttestation, creates stub for a
 // mostly empty attestation log entry in Rekor
-func rekorEntryForAttestation(ctx context.Context, imageName string) error {
+func RekorEntryForAttestation(ctx context.Context, imageName string) error {
 	attestation, err := image.AttestationFrom(ctx, imageName)
 	if err != nil {
 		return err
@@ -323,10 +323,10 @@ func rekorEntryForAttestation(ctx context.Context, imageName string) error {
 	return stubRekorEntryFor(ctx, attestation, jsonPathFromAttestation)
 }
 
-// rekorEntryForImageSignature given an image name for which signature has been
+// RekorEntryForImageSignature given an image name for which signature has been
 // previously performed via image.createAndPushImageSignature, creates stub
 // log entry in Rekor
-func rekorEntryForImageSignature(ctx context.Context, imageName string) error {
+func RekorEntryForImageSignature(ctx context.Context, imageName string) error {
 	signature, err := image.ImageSignatureFrom(ctx, imageName)
 	if err != nil {
 		return err
@@ -359,6 +359,6 @@ func IsRunning(ctx context.Context) bool {
 // AddStepsTo adds Gherkin steps to the godog ScenarioContext
 func AddStepsTo(sc *godog.ScenarioContext) {
 	sc.Step(`^stub rekord running$`, stubRekordRunning)
-	sc.Step(`^a valid Rekor entry for attestation of "([^"]*)"$`, rekorEntryForAttestation)
-	sc.Step(`^a valid Rekor entry for image signature of "([^"]*)"$`, rekorEntryForImageSignature)
+	sc.Step(`^a valid Rekor entry for attestation of "([^"]*)"$`, RekorEntryForAttestation)
+	sc.Step(`^a valid Rekor entry for image signature of "([^"]*)"$`, RekorEntryForImageSignature)
 }

cmd/validate/image.go

@@ -251,6 +251,7 @@ func validateImageCmd(validate imageValidationFunc) *cobra.Command {
 				}
 
 				evaluators = append(evaluators, c)
+				defer c.Destroy()
 			}
 
 			// worker is responsible for processing one component at a time from the jobs channel,

cmd/validate/image_test.go

@@ -23,22 +23,28 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strconv"
+	"strings"
 	"testing"
 	"time"
 
 	hd "github.com/MakeNowJust/heredoc"
+	"github.com/enterprise-contract/enterprise-contract-controller/api/v1alpha1"
 	"github.com/gkampitakis/go-snaps/snaps"
 	app "github.com/redhat-appstudio/application-api/api/v1alpha1"
 	"github.com/sigstore/cosign/v2/pkg/cosign"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 
 	"github.com/enterprise-contract/ec-cli/cmd/root"
 	"github.com/enterprise-contract/ec-cli/internal/applicationsnapshot"
 	"github.com/enterprise-contract/ec-cli/internal/evaluator"
 	"github.com/enterprise-contract/ec-cli/internal/output"
 	"github.com/enterprise-contract/ec-cli/internal/policy"
+	"github.com/enterprise-contract/ec-cli/internal/policy/source"
 	"github.com/enterprise-contract/ec-cli/internal/utils"
 )
 
@@ -1107,3 +1113,92 @@ func TestValidateImageCommand_RunE(t *testing.T) {
 		}
 	  }`, effectiveTimeTest, utils.TestPublicKeyJSON, utils.TestPublicKeyJSON), out.String())
 }
+
+type mockEvaluator struct {
+	mock.Mock
+}
+
+func (e *mockEvaluator) Evaluate(ctx context.Context, inputs []string) ([]evaluator.Outcome, evaluator.Data, error) {
+	args := e.Called(ctx, inputs)
+
+	return args.Get(0).([]evaluator.Outcome), args.Get(1).(evaluator.Data), args.Error(2)
+}
+
+func (e *mockEvaluator) Destroy() {
+	e.Called()
+}
+
+func (e *mockEvaluator) CapabilitiesPath() string {
+	args := e.Called()
+
+	return args.String(0)
+}
+
+func TestEvaluatorLifecycle(t *testing.T) {
+	ctx := utils.WithFS(context.Background(), afero.NewMemMapFs())
+
+	noEvaluators := 100
+
+	evaluators := make([]*mockEvaluator, 0, noEvaluators)
+	expectations := make([]*mock.Call, 0, noEvaluators)
+
+	for i := 0; i < noEvaluators; i++ {
+		e := mockEvaluator{}
+		call := e.On("Evaluate", ctx, mock.Anything).Return([]evaluator.Outcome{}, evaluator.Data{}, nil)
+
+		evaluators = append(evaluators, &e)
+		expectations = append(expectations, call)
+	}
+
+	for i := 0; i < noEvaluators; i++ {
+		evaluators[i].On("Destroy").NotBefore(expectations...)
+	}
+
+	newConftestEvaluator = func(_ context.Context, s []source.PolicySource, _ evaluator.ConfigProvider, _ v1alpha1.Source) (evaluator.Evaluator, error) {
+		idx, err := strconv.Atoi(s[0].PolicyUrl())
+		require.NoError(t, err)
+
+		return evaluators[idx], nil
+	}
+	t.Cleanup(func() {
+		newConftestEvaluator = evaluator.NewConftestEvaluator
+	})
+
+	validate := func(_ context.Context, component app.SnapshotComponent, _ policy.Policy, evaluators []evaluator.Evaluator, _ bool) (*output.Output, error) {
+		for _, e := range evaluators {
+			_, _, err := e.Evaluate(ctx, []string{})
+			require.NoError(t, err)
+		}
+
+		return &output.Output{ImageURL: component.ContainerImage}, nil
+	}
+
+	validateImageCmd := validateImageCmd(validate)
+	cmd := setUpCobra(validateImageCmd)
+
+	cmd.SetContext(ctx)
+
+	effectiveTimeTest := time.Now().UTC().Format(time.RFC3339Nano)
+
+	sources := make([]string, 0, noEvaluators)
+	for i := 0; i < noEvaluators; i++ {
+		sources = append(sources, fmt.Sprintf(`{"policy": ["%d"]}`, i))
+	}
+
+	cmd.SetArgs([]string{
+		"validate",
+		"image",
+		"--image",
+		"registry/image:tag",
+		"--policy",
+		fmt.Sprintf(`{"publicKey": %s, "sources": [%s]}`, utils.TestPublicKeyJSON, strings.Join(sources, ", ")),
+		"--effective-time",
+		effectiveTimeTest,
+	})
+
+	var out bytes.Buffer
+	cmd.SetOut(&out)
+
+	err := cmd.Execute()
+	assert.NoError(t, err)
+}