refactor(notation): move outcome checker to own func

Signed-off-by: Jason <[email protected]>

Author: JasonTheDeveloper

internal/controller/ocirepository_controller.go

@@ -437,26 +437,16 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 
 		result, err := r.verifySignature(ctx, obj, ref, keychain, auth, opts...)
 		if err != nil {
-			if result == soci.VerificationResultFailed {
-				provider := obj.Spec.Verify.Provider
-				if obj.Spec.Verify.SecretRef == nil && obj.Spec.Verify.Provider == "cosign" {
-					provider = fmt.Sprintf("%s keyless", provider)
-				}
-				e := serror.NewGeneric(
-					fmt.Errorf("failed to verify the signature using provider '%s': %w", provider, err),
-					sourcev1.VerificationError,
-				)
-				conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error())
-				return sreconcile.ResultEmpty, e
-			}
-
-			if result == soci.VerificationResultIgnored {
-				r.eventLogf(ctx, obj, corev1.EventTypeWarning, sourcev1.VerificationError,
-					"validation ignored for '%s' with message: %s", ref, err)
+			provider := obj.Spec.Verify.Provider
+			if obj.Spec.Verify.SecretRef == nil && obj.Spec.Verify.Provider == "cosign" {
+				provider = fmt.Sprintf("%s keyless", provider)
 			}
-		} else if result == soci.VerificationResultIgnored {
-			r.eventLogf(ctx, obj, corev1.EventTypeWarning, sourcev1.VerificationError,
-				"validation ignored for '%s'", ref)
+			e := serror.NewGeneric(
+				fmt.Errorf("failed to verify the signature using provider '%s': %w", provider, err),
+				sourcev1.VerificationError,
+			)
+			conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error())
+			return sreconcile.ResultEmpty, e
 		}
 
 		if result == soci.VerificationResultSuccess {

internal/oci/notation/notation.go

@@ -236,6 +236,18 @@ func (v *NotationVerifier) Verify(ctx context.Context, ref name.Reference) (oci.
 		return oci.VerificationResultFailed, err
 	}
 
+	return v.checkOutcome(outcomes, url)
+}
+
+// checkOutcome checks the verification outcomes for a given URL and returns the corresponding OCI verification result.
+// It takes a slice of verification outcomes and a URL as input parameters.
+// If there are no verification outcomes, it returns a failed verification result with an error message.
+// If the first verification outcome has a verification level of "trustpolicy.LevelSkip", it returns an ignored verification result.
+// If any of the verification results have an error, it logs the error message and sets the "ignore" flag to true if the error type is "trustpolicy.TypeAuthenticity".
+// If the "ignore" flag is true, it returns an ignored verification result.
+// Otherwise, it returns a successful verification result.
+// The function returns the OCI verification result and an error, if any.
+func (v *NotationVerifier) checkOutcome(outcomes []*notation.VerificationOutcome, url string) (oci.VerificationResult, error) {
 	if len(outcomes) == 0 {
 		return oci.VerificationResultFailed, fmt.Errorf("signature verification failed for all the signatures associated with %s", url)
 	}
@@ -246,18 +258,22 @@ func (v *NotationVerifier) Verify(ctx context.Context, ref name.Reference) (oci.
 		return oci.VerificationResultIgnored, nil
 	}
 
+	ignore := false
+
 	for _, i := range outcome.VerificationResults {
 		if i.Error != nil {
 			if i.Type == trustpolicy.TypeAuthenticity {
-				return oci.VerificationResultIgnored, i.Error
+				ignore = true
 			}
 
-			if i.Action == trustpolicy.ActionLog {
-				v.logger.Info(fmt.Sprintf("verification check for type %s failed for %s with message %s", i.Type, url, i.Error.Error()))
-			}
+			v.logger.Info(fmt.Sprintf("verification check for type %s failed for %s with message %s", i.Type, url, i.Error.Error()))
 		}
 	}
 
+	if ignore {
+		return oci.VerificationResultIgnored, nil
+	}
+
 	return oci.VerificationResultSuccess, nil
 }
 

internal/oci/notation/notation_test.go

@@ -17,15 +17,19 @@ limitations under the License.
 package notation
 
 import (
+	"fmt"
 	"net/http"
 	"reflect"
 	"testing"
 
 	"github.com/go-logr/logr"
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
+	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/verifier/trustpolicy"
 	. "github.com/onsi/gomega"
+
+	"github.com/fluxcd/source-controller/internal/oci"
 )
 
 func TestOptions(t *testing.T) {
@@ -351,6 +355,116 @@ func TestCleanTrustPolicy(t *testing.T) {
 	}
 }
 
+func TestOutcomeChecker(t *testing.T) {
+	g := NewWithT(t)
+
+	testCases := []struct {
+		name               string
+		outcome            []*notation.VerificationOutcome
+		wantErr            bool
+		errMessage         string
+		logMessage         []string
+		verificationResult oci.VerificationResult
+	}{
+		{
+			name:               "no outcome failed with error message",
+			verificationResult: oci.VerificationResultFailed,
+			wantErr:            true,
+			errMessage:         "signature verification failed for all the signatures associated with example.com/podInfo",
+		},
+		{
+			name: "verification result ignored with log message",
+			outcome: []*notation.VerificationOutcome{
+				{
+					VerificationLevel: trustpolicy.LevelAudit,
+					VerificationResults: []*notation.ValidationResult{
+						{
+							Type:   trustpolicy.TypeAuthenticity,
+							Action: trustpolicy.ActionLog,
+							Error:  fmt.Errorf("123"),
+						},
+					},
+				},
+			},
+			verificationResult: oci.VerificationResultIgnored,
+			logMessage:         []string{"verification check for type authenticity failed for example.com/podInfo with message 123"},
+		},
+		{
+			name: "verification result ignored with no log message (skip)",
+			outcome: []*notation.VerificationOutcome{
+				{
+					VerificationLevel:   trustpolicy.LevelSkip,
+					VerificationResults: []*notation.ValidationResult{},
+				},
+			},
+			verificationResult: oci.VerificationResultIgnored,
+		},
+		{
+			name: "verification result success with log message",
+			outcome: []*notation.VerificationOutcome{
+				{
+					VerificationLevel: trustpolicy.LevelAudit,
+					VerificationResults: []*notation.ValidationResult{
+						{
+							Type:   trustpolicy.TypeAuthenticTimestamp,
+							Action: trustpolicy.ActionLog,
+							Error:  fmt.Errorf("456"),
+						},
+						{
+							Type:   trustpolicy.TypeExpiry,
+							Action: trustpolicy.ActionLog,
+							Error:  fmt.Errorf("789"),
+						},
+					},
+				},
+			},
+			verificationResult: oci.VerificationResultSuccess,
+			logMessage: []string{
+				"verification check for type authenticTimestamp failed for example.com/podInfo with message 456",
+				"verification check for type expiry failed for example.com/podInfo with message 789",
+			},
+		},
+		{
+			name: "verification result success with no log message",
+			outcome: []*notation.VerificationOutcome{
+				{
+					VerificationLevel:   trustpolicy.LevelAudit,
+					VerificationResults: []*notation.ValidationResult{},
+				},
+			},
+			verificationResult: oci.VerificationResultSuccess,
+		},
+	}
+
+	// Run the test cases
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			l := &testLogger{[]string{}, logr.RuntimeInfo{CallDepth: 1}}
+			logger := logr.New(l)
+
+			v := NotationVerifier{
+				logger: logger,
+			}
+
+			result, err := v.checkOutcome(tc.outcome, "example.com/podInfo")
+
+			if tc.wantErr {
+				g.Expect(err).ToNot(BeNil())
+				g.Expect(err.Error()).Should(Equal(tc.errMessage))
+			} else {
+				g.Expect(err).To(BeNil())
+			}
+
+			g.Expect(result).Should(Equal(tc.verificationResult))
+			g.Expect(len(l.Output)).Should(Equal(len(tc.logMessage)))
+
+			for i, j := range tc.logMessage {
+				g.Expect(l.Output[i]).Should(Equal(j))
+			}
+		})
+	}
+}
+
 func dummyPolicyDocument() (policyDoc *trustpolicy.Document) {
 	policyDoc = &trustpolicy.Document{
 		Version:       "1.0",