From 5d6ffa3207b1f3340fd15afb8af692cc55a3c430 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 6 Feb 2025 12:37:23 -0700 Subject: [PATCH 1/5] dedup local bundle err handling Signed-off-by: Meredith Lancaster --- .../attestation/verification/attestation.go | 46 ++++++++----------- .../verification/attestation_test.go | 4 +- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index fdfd667bd..f613ba2d5 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -29,34 +29,28 @@ type FetchRemoteAttestationsParams struct { // GetLocalAttestations returns a slice of attestations read from a local bundle file. func GetLocalAttestations(path string) ([]*api.Attestation, error) { + var attestations []*api.Attestation + var err error fileExt := filepath.Ext(path) - switch fileExt { - case ".json": - attestations, err := loadBundleFromJSONFile(path) - if err != nil { - var pathErr *os.PathError - if errors.As(err, &pathErr) { - return nil, fmt.Errorf("bundle could not be loaded from JSON file at %s", path) - } else if errors.Is(err, bundle.ErrValidation) { - return nil, err - } - return nil, fmt.Errorf("bundle content could not be parsed") - } - return attestations, nil - case ".jsonl": - attestations, err := loadBundlesFromJSONLinesFile(path) - if err != nil { - var pathErr *os.PathError - if errors.As(err, &pathErr) { - return nil, fmt.Errorf("bundles could not be loaded from JSON lines file at %s", path) - } else if errors.Is(err, bundle.ErrValidation) { - return nil, err - } - return nil, fmt.Errorf("bundle content could not be parsed") - } - return attestations, nil + if fileExt == ".json" { + attestations, err = loadBundleFromJSONFile(path) + } else if fileExt == ".jsonl" { + attestations, err = loadBundlesFromJSONLinesFile(path) + } else { + return nil, ErrUnrecognisedBundleExtension } - return nil, ErrUnrecognisedBundleExtension + + if err != nil { + var pathErr *os.PathError + if errors.As(err, &pathErr) { + return nil, fmt.Errorf("could not load content from file path %s", path) + } else if errors.Is(err, bundle.ErrValidation) { + return nil, err + } + return nil, fmt.Errorf("bundle content could not be parsed") + } + + return attestations, nil } func loadBundleFromJSONFile(path string) ([]*api.Attestation, error) { diff --git a/pkg/cmd/attestation/verification/attestation_test.go b/pkg/cmd/attestation/verification/attestation_test.go index 08f0ccfef..8acff0c37 100644 --- a/pkg/cmd/attestation/verification/attestation_test.go +++ b/pkg/cmd/attestation/verification/attestation_test.go @@ -92,7 +92,7 @@ func TestGetLocalAttestations(t *testing.T) { path := "../test/data/not-found-bundle.json" attestations, err := GetLocalAttestations(path) - require.ErrorContains(t, err, "bundle could not be loaded from JSON file") + require.ErrorContains(t, err, "could not load content from file path") require.Nil(t, attestations) }) @@ -100,7 +100,7 @@ func TestGetLocalAttestations(t *testing.T) { path := "../test/data/not-found-bundle.jsonl" attestations, err := GetLocalAttestations(path) - require.ErrorContains(t, err, "bundles could not be loaded from JSON lines file") + require.ErrorContains(t, err, "could not load content from file path") require.Nil(t, attestations) }) From 84299b7d576994c05aef70ff1f7abc2aab10e23b Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 6 Feb 2025 12:50:30 -0700 Subject: [PATCH 2/5] var naming Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verification/attestation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index f613ba2d5..81c8c8813 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -54,12 +54,12 @@ func GetLocalAttestations(path string) ([]*api.Attestation, error) { } func loadBundleFromJSONFile(path string) ([]*api.Attestation, error) { - localAttestation, err := bundle.LoadJSONFromPath(path) + b, err := bundle.LoadJSONFromPath(path) if err != nil { return nil, err } - return []*api.Attestation{{Bundle: localAttestation}}, nil + return []*api.Attestation{{Bundle: b}}, nil } func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) { From 7fdb38028e1b52ce46b78e9e4528c019e9b91103 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 6 Feb 2025 16:06:43 -0700 Subject: [PATCH 3/5] remove custom transport Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/artifact/oci/client.go | 26 +--------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index bda114708..4e5acef3c 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "io" - "net/http" "strings" "github.com/google/go-containerregistry/pkg/authn" @@ -69,33 +68,10 @@ func (c LiveClient) GetImageDigest(imgName string) (*v1.Hash, name.Reference, er return &desc.Digest, name, nil } -type noncompliantRegistryTransport struct{} - -// RoundTrip will check if a request and associated response fulfill the following: -// 1. The response returns a 406 status code -// 2. The request path contains /referrers/ -// If both conditions are met, the response's status code will be overwritten to 404 -// This is a temporary solution to handle non compliant registries that return -// an unexpected status code 406 when the go-containerregistry library used -// by this code attempts to make a request to the referrers API. -// The go-containerregistry library can handle 404 response but not a 406 response. -// See the related go-containerregistry issue: https://github.com/google/go-containerregistry/issues/1962 -func (a *noncompliantRegistryTransport) RoundTrip(req *http.Request) (*http.Response, error) { - resp, err := http.DefaultTransport.RoundTrip(req) - if err != nil { - return resp, err - } - if resp.StatusCode == http.StatusNotAcceptable && strings.Contains(req.URL.Path, "/referrers/") { - resp.StatusCode = http.StatusNotFound - } - - return resp, err -} - func (c LiveClient) GetAttestations(ref name.Reference, digest string) ([]*api.Attestation, error) { attestations := make([]*api.Attestation, 0) - transportOpts := []remote.Option{remote.WithTransport(&noncompliantRegistryTransport{}), remote.WithAuthFromKeychain(authn.DefaultKeychain)} + transportOpts := []remote.Option{remote.WithAuthFromKeychain(authn.DefaultKeychain)} referrers, err := remote.Referrers(ref.Context().Digest(digest), transportOpts...) if err != nil { return attestations, fmt.Errorf("error getting referrers: %w", err) From ddc36c8a8e94410c458ffbca6647c1826c43a388 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 5 Mar 2025 07:31:28 -0700 Subject: [PATCH 4/5] Update pkg/cmd/attestation/verification/attestation.go Co-authored-by: Fredrik Skogman --- pkg/cmd/attestation/verification/attestation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index 81c8c8813..5c96d9eb5 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -43,7 +43,7 @@ func GetLocalAttestations(path string) ([]*api.Attestation, error) { if err != nil { var pathErr *os.PathError if errors.As(err, &pathErr) { - return nil, fmt.Errorf("could not load content from file path %s", path) + return nil, fmt.Errorf("could not load content from file path %s: %w", path, err) } else if errors.Is(err, bundle.ErrValidation) { return nil, err } From 917a00ddc109fef438f91b7778e59a932e9eaf69 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 5 Mar 2025 07:31:35 -0700 Subject: [PATCH 5/5] Update pkg/cmd/attestation/verification/attestation.go Co-authored-by: Fredrik Skogman --- pkg/cmd/attestation/verification/attestation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index 5c96d9eb5..db419ebac 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -47,7 +47,7 @@ func GetLocalAttestations(path string) ([]*api.Attestation, error) { } else if errors.Is(err, bundle.ErrValidation) { return nil, err } - return nil, fmt.Errorf("bundle content could not be parsed") + return nil, fmt.Errorf("bundle content could not be parsed: %w", err) } return attestations, nil