From f92d7035541bfa62eb33488adc047ce81215b1d3 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 21 Nov 2024 15:40:15 -0700 Subject: [PATCH] pr feedback Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/inspect/inspect_test.go | 14 ++++----- .../attestation/verification/mock_verifier.go | 15 +++++----- .../verify/attestation_integration_test.go | 4 +-- pkg/cmd/attestation/verify/verify_test.go | 30 +++++++++---------- 4 files changed, 32 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/attestation/inspect/inspect_test.go b/pkg/cmd/attestation/inspect/inspect_test.go index 29ca9aec7..368cc54f5 100644 --- a/pkg/cmd/attestation/inspect/inspect_test.go +++ b/pkg/cmd/attestation/inspect/inspect_test.go @@ -58,7 +58,7 @@ func TestNewInspectCmd(t *testing.T) { BundlePath: bundlePath, DigestAlgorithm: "sha384", OCIClient: oci.MockClient{}, - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: true, }, @@ -70,7 +70,7 @@ func TestNewInspectCmd(t *testing.T) { BundlePath: bundlePath, DigestAlgorithm: "sha256", OCIClient: oci.MockClient{}, - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: false, }, @@ -82,7 +82,7 @@ func TestNewInspectCmd(t *testing.T) { BundlePath: bundlePath, DigestAlgorithm: "sha512", OCIClient: oci.MockClient{}, - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: false, }, @@ -93,7 +93,7 @@ func TestNewInspectCmd(t *testing.T) { ArtifactPath: artifactPath, DigestAlgorithm: "sha256", OCIClient: oci.MockClient{}, - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: true, }, @@ -105,7 +105,7 @@ func TestNewInspectCmd(t *testing.T) { BundlePath: bundlePath, DigestAlgorithm: "sha256", OCIClient: oci.MockClient{}, - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsExporter: true, }, @@ -148,7 +148,7 @@ func TestRunInspect(t *testing.T) { DigestAlgorithm: "sha512", Logger: io.NewTestHandler(), OCIClient: oci.MockClient{}, - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), } t.Run("with valid artifact and bundle", func(t *testing.T) { @@ -176,7 +176,7 @@ func TestJSONOutput(t *testing.T) { DigestAlgorithm: "sha512", Logger: io.NewHandler(testIO), OCIClient: oci.MockClient{}, - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), exporter: cmdutil.NewJSONExporter(), } require.Nil(t, runInspect(&opts)) diff --git a/pkg/cmd/attestation/verification/mock_verifier.go b/pkg/cmd/attestation/verification/mock_verifier.go index 44ed9405e..be828e66f 100644 --- a/pkg/cmd/attestation/verification/mock_verifier.go +++ b/pkg/cmd/attestation/verification/mock_verifier.go @@ -50,14 +50,15 @@ func (v *MockSigstoreVerifier) Verify([]*api.Attestation, verify.PolicyBuilder) return results, nil } -func NewMockSigstoreVerifier(t *testing.T, mockResults []*AttestationProcessingResult) *MockSigstoreVerifier { - return &MockSigstoreVerifier{t, mockResults} +func NewMockSigstoreVerifier(t *testing.T) *MockSigstoreVerifier { + result := BuildSigstoreJsMockResult(t) + results := []*AttestationProcessingResult{&result} + + return &MockSigstoreVerifier{t, results} } -func NewDefaultMockSigstoreVerifier(t *testing.T) *MockSigstoreVerifier { - result := BuildDefaultMockResult(t) - results := []*AttestationProcessingResult{&result} - return &MockSigstoreVerifier{t, results} +func NewMockSigstoreVerifierWithMockResults(t *testing.T, mockResults []*AttestationProcessingResult) *MockSigstoreVerifier { + return &MockSigstoreVerifier{t, mockResults} } type FailSigstoreVerifier struct{} @@ -90,7 +91,7 @@ func BuildMockResult(b *bundle.Bundle, buildSignerURI, sourceRepoOwnerURI, sourc } } -func BuildDefaultMockResult(t *testing.T) AttestationProcessingResult { +func BuildSigstoreJsMockResult(t *testing.T) AttestationProcessingResult { bundle := data.SigstoreBundle(t) buildSignerURI := "https://github.com/github/example/.github/workflows/release.yml@refs/heads/main" sourceRepoOwnerURI := "https://github.com/sigstore" diff --git a/pkg/cmd/attestation/verify/attestation_integration_test.go b/pkg/cmd/attestation/verify/attestation_integration_test.go index f07fb2c1e..caa02281f 100644 --- a/pkg/cmd/attestation/verify/attestation_integration_test.go +++ b/pkg/cmd/attestation/verify/attestation_integration_test.go @@ -84,9 +84,9 @@ func TestVerifyAttestations(t *testing.T) { require.Len(t, attestations, 3) rwfResult := verification.BuildMockResult(reusableWorkflowAttestations[0].Bundle, "", "https://github.com/malancas", "", verification.GitHubOIDCIssuer) - sgjResult := verification.BuildDefaultMockResult(t) + sgjResult := verification.BuildSigstoreJsMockResult(t) mockResults := []*verification.AttestationProcessingResult{&sgjResult, &rwfResult, &sgjResult} - mockSgVerifier := verification.NewMockSigstoreVerifier(t, mockResults) + mockSgVerifier := verification.NewMockSigstoreVerifierWithMockResults(t, mockResults) // we want to test that attestations that pass Sigstore verification but fail // cert extension verification are filtered out properly in the second step diff --git a/pkg/cmd/attestation/verify/verify_test.go b/pkg/cmd/attestation/verify/verify_test.go index 75fd18c44..9a2e9f18c 100644 --- a/pkg/cmd/attestation/verify/verify_test.go +++ b/pkg/cmd/attestation/verify/verify_test.go @@ -75,7 +75,7 @@ func TestNewVerifyCmd(t *testing.T) { OIDCIssuer: verification.GitHubOIDCIssuer, Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: true, }, @@ -92,7 +92,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: false, }, @@ -109,7 +109,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://foo.ghe.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: false, }, @@ -126,7 +126,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: true, }, @@ -143,7 +143,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: false, }, @@ -159,7 +159,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: true, }, @@ -175,7 +175,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, Repo: "sigstore/sigstore-js", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: true, }, @@ -191,7 +191,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: false, }, @@ -207,7 +207,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: false, }, @@ -223,7 +223,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: true, }, @@ -240,7 +240,7 @@ func TestNewVerifyCmd(t *testing.T) { PredicateType: verification.SLSAPredicateV1, SAN: "https://github.com/sigstore/", SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsErr: true, }, @@ -257,7 +257,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsExporter: true, }, @@ -274,7 +274,7 @@ func TestNewVerifyCmd(t *testing.T) { Owner: "sigstore", PredicateType: "https://spdx.dev/Document/v2.3", SANRegex: "(?i)^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), }, wantsExporter: true, }, @@ -365,7 +365,7 @@ func TestJSONOutput(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), exporter: cmdutil.NewJSONExporter(), } require.NoError(t, runVerify(&opts)) @@ -389,7 +389,7 @@ func TestRunVerify(t *testing.T) { Owner: "sigstore", PredicateType: verification.SLSAPredicateV1, SANRegex: "^https://github.com/sigstore/", - SigstoreVerifier: verification.NewDefaultMockSigstoreVerifier(t), + SigstoreVerifier: verification.NewMockSigstoreVerifier(t), } t.Run("with valid artifact and bundle", func(t *testing.T) {