diff --git a/app/controlplane/internal/oidcauthenticator/auth.go b/app/controlplane/internal/oidcauthenticator/auth.go index f89e87e9f..0e279b0ca 100644 --- a/app/controlplane/internal/oidcauthenticator/auth.go +++ b/app/controlplane/internal/oidcauthenticator/auth.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -17,8 +17,8 @@ package authenticator import ( "context" - "errors" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/coreos/go-oidc/v3/oidc" "golang.org/x/oauth2" ) @@ -34,7 +34,7 @@ type OIDC struct { func NewOIDC(discoveryDomain, clientID, clientSecret, redirectURL string) (*OIDC, error) { provider, err := oidc.NewProvider(context.Background(), discoveryDomain) if err != nil { - return nil, err + return nil, errorsx.WithStack(err) } conf := oauth2.Config{ @@ -56,11 +56,16 @@ func NewOIDC(discoveryDomain, clientID, clientSecret, redirectURL string) (*OIDC func (a *OIDC) VerifyIDToken(ctx context.Context, token *oauth2.Token) (*oidc.IDToken, error) { rawIDToken, ok := token.Extra("id_token").(string) if !ok { - return nil, errors.New("no id_token field in oauth2 token") + return nil, errorsx.New("no id_token field in oauth2 token") } // default settings oidcConfig := &oidc.Config{ClientID: a.ClientID} - return a.Verifier(oidcConfig).Verify(ctx, rawIDToken) + idToken, err := a.Verifier(oidcConfig).Verify(ctx, rawIDToken) + if err != nil { + return nil, errorsx.WithStack(err) + } + + return idToken, nil } diff --git a/app/controlplane/internal/oidcauthenticator/auth_test.go b/app/controlplane/internal/oidcauthenticator/auth_test.go new file mode 100644 index 000000000..d75f8eda2 --- /dev/null +++ b/app/controlplane/internal/oidcauthenticator/auth_test.go @@ -0,0 +1,78 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// 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. + +package authenticator + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" +) + +func TestNewOIDCDiscoveryFailureRecordsStack(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + t.Cleanup(srv.Close) + + _, err := NewOIDC(srv.URL, "client", "secret", "http://localhost/callback") + require.Error(t, err) + assert.True(t, errorsx.HasStack(err)) +} + +func TestVerifyIDTokenErrorsRecordStack(t *testing.T) { + testCases := []struct { + name string + token *oauth2.Token + }{ + {name: "missing id_token", token: &oauth2.Token{}}, + {name: "invalid id_token", token: (&oauth2.Token{}).WithExtra(map[string]any{"id_token": "not-a-jwt"})}, + } + + srv := newDiscoveryServer(t) + a, err := NewOIDC(srv.URL, "client", "secret", "http://localhost/callback") + require.NoError(t, err) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := a.VerifyIDToken(context.Background(), tc.token) + require.Error(t, err) + assert.True(t, errorsx.HasStack(err)) + }) + } +} + +// newDiscoveryServer serves a minimal OIDC discovery document for its own URL. +func newDiscoveryServer(t *testing.T) *httptest.Server { + t.Helper() + + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/.well-known/openid-configuration" { + http.NotFound(w, r) + return + } + + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"issuer":"` + srv.URL + `","authorization_endpoint":"` + srv.URL + `/auth","token_endpoint":"` + srv.URL + `/token","jwks_uri":"` + srv.URL + `/keys"}`)) + })) + t.Cleanup(srv.Close) + + return srv +} diff --git a/app/controlplane/pkg/auditor/nats.go b/app/controlplane/pkg/auditor/nats.go index a11349c00..89deacade 100644 --- a/app/controlplane/pkg/auditor/nats.go +++ b/app/controlplane/pkg/auditor/nats.go @@ -22,6 +22,7 @@ import ( "strings" "time" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/natsconn" "github.com/go-kratos/kratos/v2/log" "github.com/nats-io/nats.go/jetstream" @@ -85,7 +86,7 @@ func newPublisher(rc *natsconn.ReloadableConnection, logger log.Logger) *AuditLo func (p *AuditLogPublisher) initJetStream() error { js, err := jetstream.New(p.rc.Conn) if err != nil { - return fmt.Errorf("creating jetstream context: %w", err) + return errorsx.Errorf("creating jetstream context: %w", err) } ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) @@ -96,7 +97,7 @@ func (p *AuditLogPublisher) initJetStream() error { Subjects: []string{subjectName}, Replicas: p.rc.Replicas, }); err != nil { - return fmt.Errorf("creating stream: %w", err) + return errorsx.Errorf("creating stream: %w", err) } p.logger.Infow("msg", "stream created or updated", "name", streamName, "subject", subjectName, "replicas", p.rc.Replicas) @@ -124,5 +125,9 @@ func (p *AuditLogPublisher) Publish(data *EventPayload) error { } specificSubject := fmt.Sprintf("%s.%s.%s", baseSubjectName, strings.ToLower(string(data.Data.TargetType)), strings.ToLower(data.Data.ActionType)) - return p.rc.Publish(specificSubject, jsonPayload) + if err := p.rc.Publish(specificSubject, jsonPayload); err != nil { + return errorsx.Errorf("publishing audit event to %s: %w", specificSubject, err) + } + + return nil } diff --git a/app/controlplane/pkg/biz/signing.go b/app/controlplane/pkg/biz/signing.go index b1d13b48f..0d3d53c4c 100644 --- a/app/controlplane/pkg/biz/signing.go +++ b/app/controlplane/pkg/biz/signing.go @@ -23,13 +23,13 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/hex" - "errors" "fmt" "net/url" "os" conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/ca" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/otelx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/go-kratos/kratos/v2/log" @@ -188,24 +188,24 @@ func (s *SigningUseCase) CreateSigningCert(ctx context.Context, orgID string, cs var publicKey crypto.PublicKey if len(csrRaw) == 0 { - return nil, errors.New("csr cannot be empty") + return nil, errorsx.New("csr cannot be empty") } // Parse CSR csr, err := cryptoutils.ParseCSR(csrRaw) if err != nil { - return nil, fmt.Errorf("parsing csr: %w", err) + return nil, errorsx.Errorf("parsing csr: %w", err) } // Parse public key and check for weak key parameters publicKey = csr.PublicKey if err := cryptoutils.ValidatePubKey(publicKey); err != nil { - return nil, fmt.Errorf("invalid public key: %w", err) + return nil, errorsx.Errorf("invalid public key: %w", err) } // Check the CSR signature is valid if err := csr.CheckSignature(); err != nil { - return nil, fmt.Errorf("invalid signature: %w", err) + return nil, errorsx.Errorf("invalid signature: %w", err) } // Create certificate from CA provider (no Signed Certificate Timestamps for now) @@ -221,13 +221,13 @@ func (s *SigningUseCase) CreateSigningCert(ctx context.Context, orgID string, cs // Generated certificate finalPEM, err := csc.CertPEM() if err != nil { - return nil, fmt.Errorf("marshaling certificate to PEM: %w", err) + return nil, errorsx.Errorf("marshaling certificate to PEM: %w", err) } // Certificate chain finalChainPEM, err := csc.ChainPEM() if err != nil { - return nil, fmt.Errorf("marshaling chain to PEM: %w", err) + return nil, errorsx.Errorf("marshaling chain to PEM: %w", err) } return append([]string{finalPEM}, finalChainPEM...), nil @@ -254,7 +254,7 @@ func (s *SigningUseCase) GetTrustedRoot(ctx context.Context) (*TrustedRoot, erro for _, cert := range chain { pemCert, err := cryptoutils.MarshalCertificateToPEM(cert) if err != nil { - return nil, fmt.Errorf("marshaling certificate to PEM: %w", err) + return nil, errorsx.Errorf("marshaling certificate to PEM: %w", err) } trustedRoot.Keys[keyID] = append(trustedRoot.Keys[keyID], string(pemCert)) } @@ -270,7 +270,7 @@ func (s *SigningUseCase) GetTrustedRoot(ctx context.Context) (*TrustedRoot, erro for _, cert := range authority.CertChain { pemCert, err := cryptoutils.MarshalCertificateToPEM(cert) if err != nil { - return nil, fmt.Errorf("marshaling certificate to PEM: %w", err) + return nil, errorsx.Errorf("marshaling certificate to PEM: %w", err) } trustedRoot.TimestampAuthorities[authorityKeyID] = append(trustedRoot.TimestampAuthorities[authorityKeyID], string(pemCert)) } diff --git a/app/controlplane/pkg/ca/ejbca/ejbca.go b/app/controlplane/pkg/ca/ejbca/ejbca.go index 2d8e1aaf8..3fd55d4be 100644 --- a/app/controlplane/pkg/ca/ejbca/ejbca.go +++ b/app/controlplane/pkg/ca/ejbca/ejbca.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ import ( "net/http" "os" + "github.com/chainloop-dev/chainloop/pkg/errorsx" fulcioca "github.com/sigstore/fulcio/pkg/ca" "github.com/sigstore/fulcio/pkg/identity" "github.com/sigstore/sigstore/pkg/cryptoutils" @@ -35,6 +36,20 @@ import ( const CAName = "ejbCA" +// EnrollStatusError is returned when EJBCA answers a certificate enrollment +// with an unexpected HTTP status. It is a separate type so that error trackers +// do not group it with transport or decoding failures. +type EnrollStatusError struct { + // Status is the HTTP status line, for example "403 Forbidden". + Status string + // StatusCode is the HTTP status code. + StatusCode int +} + +func (e *EnrollStatusError) Error() string { + return fmt.Sprintf("wrong status creating certificate: %v", e.Status) +} + type EJBCA struct { // Connection settings // EJBCA installation. ie https://localhost/ejbca/ @@ -55,7 +70,7 @@ type EJBCA struct { func New(serverURL, keyPath, certPath, rootCAPath, certProfileName, endEntityProfileName, caName string) (*EJBCA, error) { if serverURL == "" || keyPath == "" || certPath == "" || certProfileName == "" || endEntityProfileName == "" || caName == "" { - return nil, fmt.Errorf("ejbca: invalid arguments") + return nil, errorsx.New("ejbca: invalid arguments") } return &EJBCA{ @@ -102,12 +117,12 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. } ejbcaReqBytes, err := json.Marshal(ejbcaReq) if err != nil { - return nil, fmt.Errorf("failed to marshal EJBCA Request: %w", err) + return nil, errorsx.Errorf("failed to marshal EJBCA Request: %w", err) } cert, err := tls.LoadX509KeyPair(e.certPath, e.keyPath) if err != nil { - return nil, fmt.Errorf("error loading X509 key pair: %w", err) + return nil, errorsx.Errorf("error loading X509 key pair: %w", err) } tlsConfig := &tls.Config{ @@ -118,7 +133,7 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. if e.rootCAPath != "" { caCert, err := os.ReadFile(e.rootCAPath) if err != nil { - return nil, fmt.Errorf("error reading root CA: %w", err) + return nil, errorsx.Errorf("error reading root CA: %w", err) } caCertPool := x509.NewCertPool() caCertPool.AppendCertsFromPEM(caCert) @@ -131,7 +146,7 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. req, err := http.NewRequest("POST", fmt.Sprintf("%s/ejbca-rest-api/v1/certificate/pkcs10enroll", e.serverURL), bytes.NewReader(ejbcaReqBytes)) if err != nil { - return nil, fmt.Errorf("unable to create request: %w", err) + return nil, errorsx.Errorf("unable to create request: %w", err) } req.Header.Set("Content-Type", "application/json") @@ -139,26 +154,27 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. resp, err := client.Do(req) if err != nil { - return nil, fmt.Errorf("unable to send request: %w", err) + return nil, errorsx.Errorf("unable to send request: %w", err) } + defer resp.Body.Close() if resp.StatusCode != http.StatusCreated { - return nil, fmt.Errorf("wrong status creating certificate: %v", resp.Status) + return nil, errorsx.WithStack(&EnrollStatusError{Status: resp.Status, StatusCode: resp.StatusCode}) } var rsp response body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("unable to read response body: %w", err) + return nil, errorsx.Errorf("unable to read response body: %w", err) } if err := json.Unmarshal(body, &rsp); err != nil { - return nil, fmt.Errorf("unable to parse response body: %w", err) + return nil, errorsx.Errorf("unable to parse response body: %w", err) } // Decode certificate derCert, err := base64.StdEncoding.DecodeString(rsp.Certificate) if err != nil { - return nil, fmt.Errorf("unable to decode response body: %w", err) + return nil, errorsx.Errorf("unable to decode response body: %w", err) } // Decode chain @@ -166,16 +182,21 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. for _, c := range rsp.CertificateChain { decodedCert, err := base64.StdEncoding.DecodeString(c) if err != nil { - return nil, fmt.Errorf("unable to decode response body: %w", err) + return nil, errorsx.Errorf("unable to decode response body: %w", err) } cert, err := x509.ParseCertificate(decodedCert) if err != nil { - return nil, fmt.Errorf("unable to parse cert chain: %w", err) + return nil, errorsx.Errorf("unable to parse cert chain: %w", err) } chain = append(chain, cert) } - return fulcioca.CreateCSCFromDER(derCert, chain) + csc, err := fulcioca.CreateCSCFromDER(derCert, chain) + if err != nil { + return nil, errorsx.Errorf("unable to create code signing certificate: %w", err) + } + + return csc, nil } func (e EJBCA) GetRootChain(_ context.Context) ([]*x509.Certificate, error) { @@ -184,9 +205,13 @@ func (e EJBCA) GetRootChain(_ context.Context) ([]*x509.Certificate, error) { if e.rootCAPath != "" { caCert, err := os.ReadFile(e.rootCAPath) if err != nil { - return nil, fmt.Errorf("error reading root CA: %w", err) + return nil, errorsx.Errorf("error reading root CA: %w", err) + } + certs, err := cryptoutils.LoadCertificatesFromPEM(bytes.NewReader(caCert)) + if err != nil { + return nil, errorsx.Errorf("error loading root CA: %w", err) } - return cryptoutils.LoadCertificatesFromPEM(bytes.NewReader(caCert)) + return certs, nil } return nil, nil } diff --git a/app/controlplane/pkg/ca/ejbca/ejbca_test.go b/app/controlplane/pkg/ca/ejbca/ejbca_test.go new file mode 100644 index 000000000..21bbeb7df --- /dev/null +++ b/app/controlplane/pkg/ca/ejbca/ejbca_test.go @@ -0,0 +1,30 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// 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. + +package ejbca + +import ( + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewInvalidArguments(t *testing.T) { + _, err := New("", "", "", "", "", "", "") + require.EqualError(t, err, "ejbca: invalid arguments") + assert.True(t, errorsx.HasStack(err)) +} diff --git a/app/controlplane/pkg/ca/fileca/fileca.go b/app/controlplane/pkg/ca/fileca/fileca.go index 3a996627b..82368cd46 100644 --- a/app/controlplane/pkg/ca/fileca/fileca.go +++ b/app/controlplane/pkg/ca/fileca/fileca.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -20,11 +20,10 @@ import ( "context" "crypto" "crypto/x509" - "errors" - "fmt" "os" "path/filepath" + "github.com/chainloop-dev/chainloop/pkg/errorsx" fulcioca "github.com/sigstore/fulcio/pkg/ca" "github.com/sigstore/fulcio/pkg/ca/baseca" "github.com/sigstore/fulcio/pkg/identity" @@ -50,7 +49,7 @@ func New(certPath, keyPath, keyPass string, verify bool) (*FileCA, error) { // if the CA is a signer, verify the chain chain, signer := baseCA.GetSignerWithChain() if err := fulcioca.VerifyCertChain(chain, signer); err != nil { - return nil, err + return nil, errorsx.Errorf("fileca: verifying certificate chain: %w", err) } } @@ -68,35 +67,40 @@ func loadKeyPair(certPath, keyPath, keyPass string) (*fulcioca.SignerCerts, erro data, err := os.ReadFile(filepath.Clean(certPath)) if err != nil { - return nil, err + return nil, errorsx.Errorf("fileca: reading certificate: %w", err) } certs, err = cryptoutils.LoadCertificatesFromPEM(bytes.NewReader(data)) if err != nil { - return nil, err + return nil, errorsx.Errorf("fileca: loading certificate: %w", err) } opaqueKey, err := pemutil.Read(keyPath, pemutil.WithPassword([]byte(keyPass))) if err != nil { - return nil, err + return nil, errorsx.Errorf("fileca: reading private key: %w", err) } var ok bool key, ok = opaqueKey.(crypto.Signer) if !ok { - return nil, errors.New(`fileca: loaded private key can't be used to sign`) + return nil, errorsx.New(`fileca: loaded private key can't be used to sign`) } return &fulcioca.SignerCerts{Certs: certs, Signer: key}, nil } func (f FileCA) CreateCertificateFromCSR(ctx context.Context, principal identity.Principal, csr *x509.CertificateRequest) (*fulcioca.CodeSigningCertificate, error) { - return f.ca.CreateCertificate(ctx, principal, csr.PublicKey) + cert, err := f.ca.CreateCertificate(ctx, principal, csr.PublicKey) + if err != nil { + return nil, errorsx.Errorf("fileca: creating certificate: %w", err) + } + + return cert, nil } func (f FileCA) GetRootChain(ctx context.Context) ([]*x509.Certificate, error) { tb, err := f.ca.TrustBundle(ctx) if err != nil { - return nil, fmt.Errorf("failed to load trust bundle: %w", err) + return nil, errorsx.Errorf("failed to load trust bundle: %w", err) } return tb[0], nil } diff --git a/app/controlplane/pkg/ca/fileca/fileca_test.go b/app/controlplane/pkg/ca/fileca/fileca_test.go new file mode 100644 index 000000000..343b8b291 --- /dev/null +++ b/app/controlplane/pkg/ca/fileca/fileca_test.go @@ -0,0 +1,51 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// 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. + +package fileca + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/chainloop-dev/chainloop/pkg/errorsx/errorsxtest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewErrorsRecordOriginStack(t *testing.T) { + invalidPEM := filepath.Join(t.TempDir(), "invalid.pem") + require.NoError(t, os.WriteFile(invalidPEM, []byte("not a pem"), 0o600)) + + testCases := []struct { + name string + certPath string + wantErr string + }{ + {name: "missing certificate", certPath: filepath.Join(t.TempDir(), "missing.pem"), wantErr: "fileca: reading certificate"}, + {name: "invalid certificate", certPath: invalidPEM, wantErr: "fileca: loading certificate"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := New(tc.certPath, "key.pem", "", false) + require.ErrorContains(t, err, tc.wantErr) + assert.True(t, errorsx.HasStack(err)) + assert.True(t, strings.HasSuffix(errorsxtest.ErrorOriginFunction(err), "loadKeyPair")) + }) + } +} diff --git a/app/controlplane/pkg/policies/policyprovider.go b/app/controlplane/pkg/policies/policyprovider.go index 6a043aa5c..6dbcdda98 100644 --- a/app/controlplane/pkg/policies/policyprovider.go +++ b/app/controlplane/pkg/policies/policyprovider.go @@ -25,6 +25,7 @@ import ( schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/unmarshal" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/grpcconn" "github.com/chainloop-dev/chainloop/pkg/policies" "google.golang.org/protobuf/encoding/protojson" @@ -93,6 +94,29 @@ var ( ErrUnauthorized = fmt.Errorf("unauthorized request to policy provider") ) +// UnexpectedStatusError is returned when the policy provider answers with an +// HTTP status that the client does not handle. It has its own type so that +// Sentry does not group it with transport or decode failures. +type UnexpectedStatusError struct { + // StatusCode is the HTTP status code of the response. + StatusCode int +} + +func (e *UnexpectedStatusError) Error() string { + return fmt.Sprintf("expected status code 200 but got %d", e.StatusCode) +} + +// doRequest sends req to the policy provider. A transport failure records the +// stack of the caller, so Sentry groups it by the request that failed. +func doRequest(req *http.Request) (*http.Response, error) { + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, errorsx.Errorf("error executing policy request: %w", err) + } + + return resp, nil +} + // ResolveOption configures a policy or policy-group resolution request. type ResolveOption func(*resolveOptions) @@ -186,10 +210,11 @@ func (p *PolicyProvider) ValidateAttachment(att *schemaapi.PolicyAttachment, aut } // make the request - resp, err := http.DefaultClient.Do(req) + resp, err := doRequest(req) if err != nil { - return fmt.Errorf("error executing policy request: %w", err) + return err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { switch resp.StatusCode { @@ -197,23 +222,21 @@ func (p *PolicyProvider) ValidateAttachment(att *schemaapi.PolicyAttachment, aut // Ignore endpoint not found as it might not be implemented by the provider return nil case http.StatusUnauthorized, http.StatusForbidden: - return fmt.Errorf("%w: %s", ErrUnauthorized, readBodyMsg(resp)) + return errorsx.Errorf("%w: %s", ErrUnauthorized, readBodyMsg(resp)) default: - return fmt.Errorf("expected status code 200 but got %d", resp.StatusCode) + return errorsx.WithStack(&UnexpectedStatusError{StatusCode: resp.StatusCode}) } } resBytes, err := io.ReadAll(resp.Body) if err != nil { - return fmt.Errorf("error reading validation response: %w", err) + return errorsx.Errorf("error reading validation response: %w", err) } - defer resp.Body.Close() - // unmarshall response var response ValidateResponse if err := json.Unmarshal(resBytes, &response); err != nil { - return fmt.Errorf("error unmarshalling validation response: %w", err) + return errorsx.Errorf("error unmarshalling validation response: %w", err) } if !response.Valid { @@ -285,37 +308,38 @@ func (p *PolicyProvider) queryProvider(url *url.URL, digest, orgName string, aut } // make the request - resp, err := http.DefaultClient.Do(req) + resp, err := doRequest(req) if err != nil { - return "", "", fmt.Errorf("error executing policy request: %w", err) + return "", "", err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { switch resp.StatusCode { case http.StatusNotFound: return "", "", ErrNotFound case http.StatusUnauthorized, http.StatusForbidden: - return "", "", fmt.Errorf("%w: %s", ErrUnauthorized, readBodyMsg(resp)) + return "", "", errorsx.Errorf("%w: %s", ErrUnauthorized, readBodyMsg(resp)) default: - return "", "", fmt.Errorf("expected status code 200 but got %d", resp.StatusCode) + return "", "", errorsx.WithStack(&UnexpectedStatusError{StatusCode: resp.StatusCode}) } } resBytes, err := io.ReadAll(resp.Body) if err != nil { - return "", "", fmt.Errorf("error reading policy response: %w", err) + return "", "", errorsx.Errorf("error reading policy response: %w", err) } // unmarshall response var response ProviderResponse if err := json.Unmarshal(resBytes, &response); err != nil { - return "", "", fmt.Errorf("error unmarshalling policy response: %w", err) + return "", "", errorsx.Errorf("error unmarshalling policy response: %w", err) } // if raw message is provided, just interpret it as a base64 encoded string if response.Raw != nil { if err := unmarshalFromRaw(response.Raw, out); err != nil { - return "", "", fmt.Errorf("error unmarshalling policy response: %w", err) + return "", "", errorsx.Errorf("error unmarshalling policy response: %w", err) } } else if response.Data != nil { // extract the policy payload from the query response @@ -325,7 +349,7 @@ func (p *PolicyProvider) queryProvider(url *url.URL, digest, orgName string, aut } if err := protojson.Unmarshal(jsonPolicy, out); err != nil { - return "", "", fmt.Errorf("error unmarshalling policy response: %w", err) + return "", "", errorsx.Errorf("error unmarshalling policy response: %w", err) } } diff --git a/app/controlplane/pkg/policies/policyprovider_http_test.go b/app/controlplane/pkg/policies/policyprovider_http_test.go index dc1dd86eb..1bea8a533 100644 --- a/app/controlplane/pkg/policies/policyprovider_http_test.go +++ b/app/controlplane/pkg/policies/policyprovider_http_test.go @@ -16,6 +16,7 @@ package policies import ( + "errors" "fmt" "net/http" "net/http/httptest" @@ -23,6 +24,7 @@ import ( "testing" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -99,14 +101,34 @@ func TestResolveHTTPStatusHandling(t *testing.T) { assert.NotErrorIs(t, err, ErrUnauthorized) assert.NotErrorIs(t, err, ErrNotFound) assert.Contains(t, err.Error(), fmt.Sprintf("got %d", tc.statusCode)) + + var statusErr *UnexpectedStatusError + require.ErrorAs(t, err, &statusErr) + assert.Equal(t, tc.statusCode, statusErr.StatusCode) } if tc.wantMsg != "" { assert.Contains(t, err.Error(), tc.wantMsg) } + if !errors.Is(tc.wantErr, ErrNotFound) { + assert.True(t, errorsx.HasStack(err), "a provider failure must record a stack") + } }) } } +func TestResolveTransportFailureRecordsStack(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + // Close the server so that the request fails in the transport. + server.Close() + + provider := &PolicyProvider{name: "test", url: server.URL} + + _, _, err := provider.Resolve("test-policy", "", ProviderAuthOpts{Token: "test-token"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "error executing policy request") + assert.True(t, errorsx.HasStack(err)) +} + func TestResolveGroupHTTPStatusHandling(t *testing.T) { testCases := []struct { name string diff --git a/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go b/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go index f58a726b2..c8a8e0cfc 100644 --- a/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go +++ b/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go @@ -27,6 +27,8 @@ import ( "strings" "encoding/json" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) type base struct { @@ -222,18 +224,17 @@ func teamPermissionsRequest(httpClient *http.Client, host *url.URL, apiKey strin // Check the response if res.StatusCode != http.StatusOK { - err = fmt.Errorf("bad status: %s", res.Status) - return nil, err + return nil, errorsx.WithStack(&StatusError{Status: res.Status, StatusCode: res.StatusCode}) } resBody, err := io.ReadAll(res.Body) if err != nil { - return nil, err + return nil, errorsx.Errorf("reading the response body: %w", err) } resp := &teamPermissionsResponse{} if err := json.Unmarshal(resBody, resp); err != nil { - return nil, err + return nil, errorsx.Errorf("decoding the team permissions response: %w", err) } return resp, nil @@ -286,18 +287,17 @@ func uploadSBOMRequest(httpClient *http.Client, host *url.URL, apiKey string, va // Check the response if res.StatusCode != http.StatusOK { - err = fmt.Errorf("bad status: %s", res.Status) - return nil, err + return nil, errorsx.WithStack(&StatusError{Status: res.Status, StatusCode: res.StatusCode}) } resBody, err := io.ReadAll(res.Body) if err != nil { - return nil, err + return nil, errorsx.Errorf("reading the response body: %w", err) } resp := &uploadSBOMResponse{} if err := json.Unmarshal(resBody, resp); err != nil { - return nil, err + return nil, errorsx.Errorf("decoding the SBOM upload response: %w", err) } return resp, nil @@ -327,6 +327,18 @@ func projectExists(httpClient *http.Client, host *url.URL, apiKey string, projec case http.StatusNotFound: return false, nil default: - return false, fmt.Errorf("bad status: %s", res.Status) + return false, errorsx.WithStack(&StatusError{Status: res.Status, StatusCode: res.StatusCode}) } } + +// StatusError is returned when Dependency-Track answers with an unexpected +// HTTP status. It has its own type so that Sentry does not group it with +// transport or decode failures. +type StatusError struct { + // Status is the HTTP status line of the response, for example "403 Forbidden". + Status string + // StatusCode is the HTTP status code of the response. + StatusCode int +} + +func (e *StatusError) Error() string { return "bad status: " + e.Status } diff --git a/app/controlplane/plugins/core/dependency-track/v1/client/sbom_test.go b/app/controlplane/plugins/core/dependency-track/v1/client/sbom_test.go index d0b1ddecd..678843a4f 100644 --- a/app/controlplane/plugins/core/dependency-track/v1/client/sbom_test.go +++ b/app/controlplane/plugins/core/dependency-track/v1/client/sbom_test.go @@ -19,10 +19,13 @@ import ( "bytes" "io" "net/http" + "net/http/httptest" "net/url" "testing" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const hostname = "http://example.com" @@ -135,3 +138,52 @@ func TestDoCheck(t *testing.T) { } } } + +func TestBadStatusIsTypedWithStack(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer server.Close() + + host, err := url.Parse(server.URL) + require.NoError(t, err) + + testCases := []struct { + name string + call func() error + }{ + { + name: "team permissions", + call: func() error { + _, err := teamPermissionsRequest(server.Client(), host, "key") + return err + }, + }, + { + name: "upload SBOM", + call: func() error { + _, err := uploadSBOMRequest(server.Client(), host, "key", map[string]io.Reader{}) + return err + }, + }, + { + name: "project exists", + call: func() error { + _, err := projectExists(server.Client(), host, "key", "id") + return err + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.call() + + var statusErr *StatusError + require.ErrorAs(t, err, &statusErr) + assert.Equal(t, http.StatusForbidden, statusErr.StatusCode) + assert.EqualError(t, err, "bad status: 403 Forbidden") + assert.True(t, errorsx.HasStack(err)) + }) + } +} diff --git a/app/controlplane/plugins/core/discord-webhook/v1/discord.go b/app/controlplane/plugins/core/discord-webhook/v1/discord.go index 5a1283327..048a2bfbf 100644 --- a/app/controlplane/plugins/core/discord-webhook/v1/discord.go +++ b/app/controlplane/plugins/core/discord-webhook/v1/discord.go @@ -26,6 +26,7 @@ import ( "net/http" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" ) @@ -108,12 +109,12 @@ func (i *Integration) Register(ctx context.Context, req *sdk.RegistrationRequest } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("invalid webhook URL, status: %s", resp.Status) + return nil, errorsx.Errorf("invalid webhook URL, status: %s", resp.Status) } var webHookInfo webhookResponse if err := json.NewDecoder(resp.Body).Decode(&webHookInfo); err != nil { - return nil, fmt.Errorf("invalid webhook URL: %w", err) + return nil, errorsx.Errorf("invalid webhook URL: %w", err) } // Configuration State @@ -236,7 +237,7 @@ func executeWebhook(client *http.Client, webhookURL, usernameOverride string, st if r.StatusCode != http.StatusOK { b, _ := io.ReadAll(r.Body) - return fmt.Errorf("non-OK HTTP status while calling the webhook: %d, body: %s", r.StatusCode, string(b)) + return errorsx.Errorf("non-OK HTTP status while calling the webhook: %d, body: %s", r.StatusCode, string(b)) } return nil diff --git a/app/controlplane/plugins/core/guac/v1/guac.go b/app/controlplane/plugins/core/guac/v1/guac.go index b31ed61ec..fd90ee902 100644 --- a/app/controlplane/plugins/core/guac/v1/guac.go +++ b/app/controlplane/plugins/core/guac/v1/guac.go @@ -29,6 +29,7 @@ import ( "code.cloudfoundry.org/bytefmt" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" "google.golang.org/api/option" ) @@ -124,7 +125,7 @@ func (i *Integration) Register(ctx context.Context, req *sdk.RegistrationRequest // Check that the credentials are valid and the bucket exists client, err := storage.NewClient(ctx, option.WithCredentialsJSON([]byte(request.Credentials))) if err != nil { - return nil, fmt.Errorf("creating storage client: %w", err) + return nil, errorsx.Errorf("creating storage client: %w", err) } _, err = loadBucket(ctx, client, request.Bucket) @@ -175,7 +176,7 @@ func (i *Integration) Execute(ctx context.Context, req *sdk.ExecutionRequest) er client, err := storage.NewClient(ctx, option.WithCredentialsJSON([]byte(req.RegistrationInfo.Credentials.Password))) if err != nil { - return fmt.Errorf("creating storage client: %w", err) + return errorsx.Errorf("creating storage client: %w", err) } bucket, err := loadBucket(ctx, client, registrationConfig.Bucket) @@ -228,7 +229,7 @@ const pathPrefix = "chainloop" func uploadToBucket(ctx context.Context, bucket *storage.BucketHandle, filename string, content []byte, md *sdk.ChainloopMetadata, logger *log.Helper) error { bucketInfo, err := bucket.Attrs(ctx) if err != nil { - return fmt.Errorf("getting bucket info: %w", err) + return errorsx.Errorf("getting bucket info: %w", err) } buf := bytes.NewBuffer(content) @@ -255,7 +256,7 @@ func uploadToBucket(ctx context.Context, bucket *storage.BucketHandle, filename } if _, err := io.Copy(w, buf); err != nil { - return fmt.Errorf("writing to the bucket: %w", err) + return errorsx.Errorf("writing to the bucket: %w", err) } logger.Infow("msg", "uploaded to the bucket", "file", filename, "bucket", bucketInfo.Name, "size", bytefmt.ByteSize(fileSize)) @@ -280,13 +281,13 @@ func loadBucket(ctx context.Context, client *storage.Client, bucket string) (*st return nil, fmt.Errorf("the bucket %s does not exist", bucket) } - return nil, fmt.Errorf("checking the bucket: %w", err) + return nil, errorsx.Errorf("checking the bucket: %w", err) } // Write test file to make sure credentials have write permissions testFile := b.Object("chainloop-test-write") if err := testFile.NewWriter(ctx).Close(); err != nil { - return nil, fmt.Errorf("we can't write to the bucket %s: %w", bucket, err) + return nil, errorsx.Errorf("we can't write to the bucket %s: %w", bucket, err) } _ = testFile.Delete(ctx) diff --git a/app/controlplane/plugins/core/slack-webhook/v1/slack_webhook.go b/app/controlplane/plugins/core/slack-webhook/v1/slack_webhook.go index ecca50ae9..59e1935d2 100644 --- a/app/controlplane/plugins/core/slack-webhook/v1/slack_webhook.go +++ b/app/controlplane/plugins/core/slack-webhook/v1/slack_webhook.go @@ -27,6 +27,7 @@ import ( "text/template" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" ) @@ -154,7 +155,7 @@ func executeWebhook(client *http.Client, webhookURL, msgContent string) error { if r.StatusCode != http.StatusOK { b, _ := io.ReadAll(r.Body) - return fmt.Errorf("non-OK HTTP status while calling the webhook: %d, body: %s", r.StatusCode, string(b)) + return errorsx.Errorf("non-OK HTTP status while calling the webhook: %d, body: %s", r.StatusCode, string(b)) } return nil diff --git a/app/controlplane/plugins/core/smtp/v1/extension.go b/app/controlplane/plugins/core/smtp/v1/extension.go index 6f0a2f6de..f06ae69e6 100644 --- a/app/controlplane/plugins/core/smtp/v1/extension.go +++ b/app/controlplane/plugins/core/smtp/v1/extension.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import ( nsmtp "net/smtp" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" ) @@ -204,7 +205,7 @@ func sendEmail(host string, port string, user, password, from, to, cc, subject, auth := nsmtp.PlainAuth("", user, password, host) err := nsmtp.SendMail(host+":"+port, auth, from, []string{to}, []byte(message)) if err != nil { - return fmt.Errorf("error sending email: %w", err) + return errorsx.Errorf("error sending email: %w", err) } return nil diff --git a/app/controlplane/plugins/core/webhook/v1/webhook.go b/app/controlplane/plugins/core/webhook/v1/webhook.go index f3059c666..55d6a7174 100644 --- a/app/controlplane/plugins/core/webhook/v1/webhook.go +++ b/app/controlplane/plugins/core/webhook/v1/webhook.go @@ -27,6 +27,7 @@ import ( schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" ) @@ -278,7 +279,7 @@ func (i *Integration) sendWebhook(ctx context.Context, url, kind string, payload if resp.StatusCode < 200 || resp.StatusCode >= 300 { i.Logger.Errorw("webhook responded with non-success status code", "status", resp.StatusCode, "body", string(body)) - return fmt.Errorf("webhook responded with status code %d: %s", resp.StatusCode, string(body)) + return errorsx.Errorf("webhook responded with status code %d: %s", resp.StatusCode, string(body)) } return nil diff --git a/app/controlplane/plugins/sdk/v1/httpclient.go b/app/controlplane/plugins/sdk/v1/httpclient.go index 7aa8430de..48b474d41 100644 --- a/app/controlplane/plugins/sdk/v1/httpclient.go +++ b/app/controlplane/plugins/sdk/v1/httpclient.go @@ -20,6 +20,7 @@ import ( "net/http" "time" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/netguard" ) @@ -85,5 +86,31 @@ func NewHTTPClient(opts HTTPClientOptions) *http.Client { netguard.RestrictTransport(transport) } - return &http.Client{Timeout: opts.Timeout, Transport: transport} + return &http.Client{Timeout: opts.Timeout, Transport: &stackTransport{base: transport}} +} + +// stackTransport records the call stack of the plugin code that sent a +// request when the request fails, so Sentry groups a failed outbound call by +// the code that made it and not by the place that reports the error. +type stackTransport struct { + // base is the transport that sends the requests. + base http.RoundTripper +} + +// RoundTrip sends req with the base transport and adds a stack to its error. +func (t *stackTransport) RoundTrip(req *http.Request) (*http.Response, error) { + resp, err := t.base.RoundTrip(req) + if err != nil { + return nil, errorsx.WithStack(err) + } + + return resp, nil +} + +// CloseIdleConnections forwards to the base transport, so +// http.Client.CloseIdleConnections still releases pooled connections. +func (t *stackTransport) CloseIdleConnections() { + if c, ok := t.base.(interface{ CloseIdleConnections() }); ok { + c.CloseIdleConnections() + } } diff --git a/app/controlplane/plugins/sdk/v1/httpclient_test.go b/app/controlplane/plugins/sdk/v1/httpclient_test.go index 2d0980510..c9d50ef53 100644 --- a/app/controlplane/plugins/sdk/v1/httpclient_test.go +++ b/app/controlplane/plugins/sdk/v1/httpclient_test.go @@ -20,6 +20,7 @@ import ( "net/http/httptest" "testing" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -48,6 +49,7 @@ func TestNewHTTPClientPublicTargetsOnly(t *testing.T) { resp, err := client.Get(server.URL) if tc.wantBlocked { require.ErrorIs(t, err, ErrBlockedTarget) + assert.True(t, errorsx.HasStack(err), "a failed request must record a stack") return } @@ -76,7 +78,9 @@ func TestNewHTTPClientProxyUse(t *testing.T) { t.Run(tc.name, func(t *testing.T) { client := NewHTTPClient(HTTPClientOptions{PublicTargetsOnly: tc.publicTargetsOnly}) - transport, ok := client.Transport.(*http.Transport) + st, ok := client.Transport.(*stackTransport) + require.True(t, ok) + transport, ok := st.base.(*http.Transport) require.True(t, ok) if tc.wantProxy { diff --git a/pkg/blobmanager/azureblob/backend.go b/pkg/blobmanager/azureblob/backend.go index edc3f72b4..9a16c86a4 100644 --- a/pkg/blobmanager/azureblob/backend.go +++ b/pkg/blobmanager/azureblob/backend.go @@ -17,7 +17,6 @@ package azureblob import ( "context" - "errors" "fmt" "io" "os" @@ -29,6 +28,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror" pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) type Backend struct { @@ -43,7 +43,7 @@ var _ backend.UploaderDownloader = (*Backend)(nil) func NewBackend(creds *Credentials) (*Backend, error) { credential, err := azidentity.NewClientSecretCredential(creds.TenantID, creds.ClientID, creds.ClientSecret, nil) if err != nil { - return nil, fmt.Errorf("failed to create Azure Service principal Credential: %w", err) + return nil, errorsx.Errorf("failed to create Azure Service principal Credential: %w", err) } return &Backend{ @@ -60,7 +60,7 @@ func (b *Backend) client() (*azblob.Client, error) { // Top level client client, err := azblob.NewClient(url, b.credentials, nil) if err != nil { - return nil, fmt.Errorf("failed to create Azure Blob Storage client: %w", err) + return nil, errorsx.Errorf("failed to create Azure Blob Storage client: %w", err) } return client, nil @@ -78,7 +78,7 @@ func (b *Backend) getServiceURL() string { func (b *Backend) blobClient(digest string) (*blob.Client, error) { blobClient, err := blob.NewClient(b.resourcePath(digest), b.credentials, nil) if err != nil { - return nil, fmt.Errorf("failed to create Azure Blob Storage client: %w", err) + return nil, errorsx.Errorf("failed to create Azure Blob Storage client: %w", err) } return blobClient, nil @@ -135,14 +135,14 @@ func (b *Backend) Upload(ctx context.Context, r io.Reader, resource *pb.CASResou _, err = client.UploadFile(ctx, b.container, resourceName(resource.Digest), f, &azblob.UploadFileOptions{ Metadata: metadata, }) - return err + return errorsx.WithStack(err) } _, err = client.UploadStream(ctx, b.container, resourceName(resource.Digest), r, &azblob.UploadStreamOptions{ Metadata: metadata, }) - return err + return errorsx.WithStack(err) } func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, error) { @@ -157,19 +157,19 @@ func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, return nil, backend.NewErrNotFound("artifact") } - return nil, fmt.Errorf("failed to get blob properties: %w", err) + return nil, errorsx.Errorf("failed to get blob properties: %w", err) } // Check asset author is chainloop that way we can ignore files uploaded by other tools // note: this is not a security mechanism, an additional check will be put in place for tamper check author, ok := properties.Metadata[annotationNameAuthor] if !ok || *author != backend.AuthorAnnotation { - return nil, errors.New("asset not uploaded by Chainloop") + return nil, errorsx.New("asset not uploaded by Chainloop") } fileName, ok := properties.Metadata[annotationNameFilename] if !ok { - return nil, fmt.Errorf("couldn't find file metadata") + return nil, errorsx.Errorf("couldn't find file metadata") } return &pb.CASResource{ @@ -181,7 +181,7 @@ func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, func (b *Backend) Download(ctx context.Context, w io.Writer, digest string) error { exists, err := b.Exists(ctx, digest) if err != nil { - return fmt.Errorf("invalid artifact: %w", err) + return errorsx.Errorf("invalid artifact: %w", err) } else if !exists { return backend.NewErrNotFound("artifact") } @@ -194,14 +194,14 @@ func (b *Backend) Download(ctx context.Context, w io.Writer, digest string) erro // Download the blob downloadStream, err := client.DownloadStream(ctx, b.container, resourceName(digest), nil) if err != nil { - return fmt.Errorf("failed to download blob: %w", err) + return errorsx.Errorf("failed to download blob: %w", err) } retryReader := downloadStream.NewRetryReader(ctx, &azblob.RetryReaderOptions{}) defer retryReader.Close() if _, err := io.Copy(w, retryReader); err != nil { - return fmt.Errorf("failed to copy blob to writer: %w", err) + return errorsx.Errorf("failed to copy blob to writer: %w", err) } return nil @@ -222,7 +222,7 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error { // Touch a file _, err = client.UploadBuffer(ctx, b.container, "healthCheck", nil, nil) if err != nil { - return fmt.Errorf("failed write to Blob Storage: %w", err) + return errorsx.Errorf("failed write to Blob Storage: %w", err) } return nil @@ -232,7 +232,7 @@ func (b *Backend) createContainer(ctx context.Context, client *azblob.Client) er // Create container name _, err := client.CreateContainer(ctx, b.container, nil) if err != nil && !bloberror.HasCode(err, bloberror.ContainerAlreadyExists) { - return fmt.Errorf("failed to create Blob storage Container: %w", err) + return errorsx.Errorf("failed to create Blob storage Container: %w", err) } return nil diff --git a/pkg/blobmanager/oci/backend.go b/pkg/blobmanager/oci/backend.go index 53b266e35..7d96a018c 100644 --- a/pkg/blobmanager/oci/backend.go +++ b/pkg/blobmanager/oci/backend.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -36,6 +36,7 @@ import ( pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) type Backend struct { @@ -78,12 +79,12 @@ func NewBackend(repository string, regOpts *RegistryOptions, opts ...NewBackendO // This method is very naive so signatures will be used in future releases func (b *Backend) Exists(_ context.Context, digest string) (bool, error) { if digest == "" { - return false, errors.New("digest is empty") + return false, errorsx.New("digest is empty") } ref, err := name.ParseReference(b.resourcePath(digest)) if err != nil { - return false, err + return false, errorsx.WithStack(err) } // It's not trivial to catch if the error is a 404 (yeah I know...) so we will assume that @@ -110,12 +111,12 @@ func (b *Backend) Upload(_ context.Context, r io.Reader, resource *pb.CASResourc // TODO: Split content in multiple layers and do concurrent uploads/downloads data, err := io.ReadAll(r) if err != nil { - return fmt.Errorf("reading content: %w", err) + return errorsx.Errorf("reading content: %w", err) } ref, err := name.ParseReference(b.resourcePath(resource.Digest)) if err != nil { - return fmt.Errorf("parsing reference: %w", err) + return errorsx.Errorf("parsing reference: %w", err) } img, err := craftImage(data, resource) @@ -129,7 +130,7 @@ func (b *Backend) Upload(_ context.Context, r io.Reader, resource *pb.CASResourc err = remote.Write(ref, img, remote.WithAuthFromKeychain(b.keychain)) if err != nil { - return fmt.Errorf("writing image: %w", err) + return errorsx.Errorf("writing image: %w", err) } return nil @@ -141,11 +142,11 @@ func (b *Backend) resourcePath(resourceName string) string { func craftImage(content []byte, resource *pb.CASResource) (v1.Image, error) { if len(content) == 0 { - return nil, errors.New("content is empty") + return nil, errorsx.New("content is empty") } if resource == nil || resource.FileName == "" || resource.Digest == "" { - return nil, errors.New("resource metadata is not valid") + return nil, errorsx.New("resource metadata is not valid") } base := mutate.MediaType(empty.Image, types.OCIManifestSchema1) @@ -159,7 +160,7 @@ func craftImage(content []byte, resource *pb.CASResource) (v1.Image, error) { layer := static.NewLayer(content, backend.DetectedMediaType(content)) img, err := mutate.Append(base, mutate.Addendum{Layer: layer}) if err != nil { - return nil, err + return nil, errorsx.WithStack(err) } return img, nil @@ -167,12 +168,12 @@ func craftImage(content []byte, resource *pb.CASResource) (v1.Image, error) { func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, error) { if digest == "" { - return nil, errors.New("digest is empty") + return nil, errorsx.New("digest is empty") } ref, err := name.ParseReference(b.resourcePath(digest)) if err != nil { - return nil, fmt.Errorf("parsing reference: %w", err) + return nil, errorsx.Errorf("parsing reference: %w", err) } img, err := remote.Image(ref, remote.WithAuthFromKeychain(b.keychain)) @@ -182,7 +183,7 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e return nil, backend.NewErrNotFound("image") } - return nil, fmt.Errorf("getting image: %w", err) + return nil, errorsx.Errorf("getting image: %w", err) } if err := validateImage(img, digest); err != nil { @@ -191,7 +192,7 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e manifest, err := img.Manifest() if err != nil { - return nil, fmt.Errorf("extracting manifest: %w", err) + return nil, errorsx.Errorf("extracting manifest: %w", err) } // Validate image already checked that the manifest has exactly one layer @@ -199,7 +200,7 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e filename, ok := manifest.Annotations[ocispec.AnnotationTitle] if !ok { - return nil, errors.New("couldn't find file metadata") + return nil, errorsx.New("couldn't find file metadata") } return &pb.CASResource{Digest: digest, FileName: filename, Size: size}, nil @@ -207,17 +208,17 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e func (b *Backend) Download(_ context.Context, w io.Writer, digest string) error { if digest == "" { - return errors.New("digest is empty") + return errorsx.New("digest is empty") } ref, err := name.ParseReference(b.resourcePath(digest)) if err != nil { - return fmt.Errorf("parsing reference: %w", err) + return errorsx.Errorf("parsing reference: %w", err) } img, err := remote.Image(ref, remote.WithAuthFromKeychain(b.keychain)) if err != nil { - return fmt.Errorf("getting image: %w", err) + return errorsx.Errorf("getting image: %w", err) } if err := validateImage(img, digest); err != nil { @@ -227,13 +228,13 @@ func (b *Backend) Download(_ context.Context, w io.Writer, digest string) error // Download the layer with the same digest, not relying on the image name l, err := img.LayerByDiffID(v1.Hash{Algorithm: "sha256", Hex: digest}) if err != nil { - return fmt.Errorf("getting layer with hash sha256:%s: %w", digest, err) + return errorsx.Errorf("getting layer with hash sha256:%s: %w", digest, err) } // Do not uncompress since we want the raw stored data rc, err := l.Compressed() if err != nil { - return fmt.Errorf("extracting data from layer: %w", err) + return errorsx.Errorf("extracting data from layer: %w", err) } defer rc.Close() @@ -241,7 +242,7 @@ func (b *Backend) Download(_ context.Context, w io.Writer, digest string) error buf := make([]byte, 1<<20) _, err = io.CopyBuffer(w, rc, buf) if err != nil { - return fmt.Errorf("copying data from layer: %w", err) + return errorsx.Errorf("copying data from layer: %w", err) } return nil @@ -252,35 +253,35 @@ func validateImage(img v1.Image, digest string) error { // Review required annotations m, err := img.Manifest() if err != nil { - return fmt.Errorf("getting manifest: %w", err) + return errorsx.Errorf("getting manifest: %w", err) } if v, ok := m.Annotations[ocispec.AnnotationAuthors]; !ok || v != backend.AuthorAnnotation { - return errors.New("image not uploaded by chainloop") + return errorsx.New("image not uploaded by chainloop") } if v, ok := m.Annotations[ocispec.AnnotationTitle]; !ok && v != "" { - return errors.New("image does not contain filename information") + return errorsx.New("image does not contain filename information") } // NOTE: we use img.Layers instead of LayerByDiffID because the latter does not compute the image manifest layers, err := img.Layers() if err != nil { - return fmt.Errorf("getting layers: %w", err) + return errorsx.Errorf("getting layers: %w", err) } if len(layers) != 1 { - return errors.New("image does not contain a single layer") + return errorsx.New("image does not contain a single layer") } // Check the actual layer digest content meets the expected one d, err := layers[0].Digest() if err != nil { - return fmt.Errorf("getting layer digest: %w", err) + return errorsx.Errorf("getting layer digest: %w", err) } if d.Hex != digest { - return errors.New("layer digest does not match the expected one") + return errorsx.New("layer digest does not match the expected one") } return nil @@ -290,8 +291,8 @@ func validateImage(img v1.Image, digest string) error { func (b *Backend) CheckWritePermissions(_ context.Context) error { ref, err := name.ParseReference(fmt.Sprintf("%s/chainloop-test", b.repo)) if err != nil { - return fmt.Errorf("parsing the reference image for validation: %w", err) + return errorsx.Errorf("parsing the reference image for validation: %w", err) } - return remote.CheckPushPermission(ref, b.keychain, http.DefaultTransport) + return errorsx.WithStack(remote.CheckPushPermission(ref, b.keychain, http.DefaultTransport)) } diff --git a/pkg/blobmanager/s3/backend.go b/pkg/blobmanager/s3/backend.go index f9b8c8f3e..01e8c22a3 100644 --- a/pkg/blobmanager/s3/backend.go +++ b/pkg/blobmanager/s3/backend.go @@ -33,6 +33,7 @@ import ( "github.com/aws/smithy-go" pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) const ( @@ -52,11 +53,11 @@ const defaultRegion = "us-east-1" func NewBackend(creds *Credentials) (*Backend, error) { if creds == nil { - return nil, errors.New("credentials cannot be nil") + return nil, errorsx.New("credentials cannot be nil") } if err := creds.Validate(); err != nil { - return nil, fmt.Errorf("invalid credentials: %w", err) + return nil, errorsx.Errorf("invalid credentials: %w", err) } // Set a default region if not provided @@ -68,7 +69,7 @@ func NewBackend(creds *Credentials) (*Backend, error) { // Bucket might contain the not only the bucket name but also the endpoint endpoint, bucket, err := extractLocationAndBucket(creds) if err != nil { - return nil, fmt.Errorf("failed to parse bucket name: %w", err) + return nil, errorsx.Errorf("failed to parse bucket name: %w", err) } // Using AWS config directly instead of using config.LoadDefaultConfig @@ -124,7 +125,7 @@ func extractLocationAndBucket(creds *Credentials) (string, string, error) { // so we override the bucket and set the endpoint if needed parsedLocation, err := url.Parse(creds.Location) if err != nil { - return "", "", fmt.Errorf("failed to parse location: %w", err) + return "", "", errorsx.Errorf("failed to parse location: %w", err) } host := parsedLocation.Host @@ -139,7 +140,7 @@ func extractLocationAndBucket(creds *Credentials) (string, string, error) { return endpoint, pathSegments[1], nil } - return "", "", fmt.Errorf("the location doesn't contain a bucket name") + return "", "", errorsx.Errorf("the location doesn't contain a bucket name") } // Exists check that the artifact is already present in the repository @@ -169,7 +170,7 @@ func (b *Backend) Upload(ctx context.Context, r io.Reader, resource *pb.CASResou // variants), and some S3-compatible endpoints such as R2 reject it outright. if _, err := uploader.Upload(ctx, input); err != nil { - return fmt.Errorf("failed to upload to bucket: %w", err) + return errorsx.Errorf("failed to upload to bucket: %w", err) } return nil @@ -196,24 +197,24 @@ func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, return nil, backend.NewErrNotFound("artifact") } - return nil, fmt.Errorf("failed to read from bucket: %w", err) + return nil, errorsx.Errorf("failed to read from bucket: %w", err) } // Check integrity of the remote object if resp.ChecksumSHA256 != nil && *resp.ChecksumSHA256 != hexSha256ToBinaryB64(digest) { - return nil, fmt.Errorf("failed to validate integrity of object, got=%s, want=%s", *resp.ChecksumSHA256, hexSha256ToBinaryB64(digest)) + return nil, errorsx.Errorf("failed to validate integrity of object, got=%s, want=%s", *resp.ChecksumSHA256, hexSha256ToBinaryB64(digest)) } // Check asset author is Chainloop that way we can ignore files uploaded by other tools // note: this is not a security mechanism, an additional check will be put in place for tamper check author, ok := resp.Metadata[annotationNameAuthor] if !ok || author != backend.AuthorAnnotation { - return nil, errors.New("asset not uploaded by Chainloop") + return nil, errorsx.New("asset not uploaded by Chainloop") } fileName, ok := resp.Metadata[annotationNameFilename] if !ok { - return nil, fmt.Errorf("couldn't find file metadata") + return nil, errorsx.Errorf("couldn't find file metadata") } return &pb.CASResource{ @@ -243,7 +244,7 @@ func (b *Backend) Download(ctx context.Context, w io.Writer, digest string) erro Key: aws.String(resourceName(digest)), }) - return err + return errorsx.WithStack(err) } // CheckWritePermissions performs an actual write to the repository to check that the credentials @@ -258,7 +259,7 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error { // Write to the bucket if _, err := b.client.PutObject(ctx, input); err != nil { - return fmt.Errorf("failed to write to bucket: %w", err) + return errorsx.Errorf("failed to write to bucket: %w", err) } // and read the object back @@ -268,7 +269,7 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error { }) if err != nil { - return fmt.Errorf("failed to read from bucket: %w", err) + return errorsx.Errorf("failed to read from bucket: %w", err) } return nil diff --git a/pkg/blobmanager/s3accesspoint/backend.go b/pkg/blobmanager/s3accesspoint/backend.go index 6c30e663a..04b3d2930 100644 --- a/pkg/blobmanager/s3accesspoint/backend.go +++ b/pkg/blobmanager/s3accesspoint/backend.go @@ -35,6 +35,7 @@ import ( pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" robotaccount "github.com/chainloop-dev/chainloop/internal/robotaccount/cas" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) const ( @@ -88,7 +89,7 @@ func NewBackend(ctx context.Context, creds *Credentials) (*Backend, error) { // against the pod identity is necessary. awsCfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(creds.Region)) if err != nil { - return nil, fmt.Errorf("loading aws config: %w", err) + return nil, errorsx.Errorf("loading aws config: %w", err) } stsClient := sts.NewFromConfig(awsCfg) @@ -160,7 +161,7 @@ func (b *Backend) Upload(ctx context.Context, r io.Reader, resource *pb.CASResou }, }) if err != nil { - return fmt.Errorf("uploading to access point: %w", err) + return errorsx.Errorf("uploading to access point: %w", err) } return nil } @@ -180,23 +181,23 @@ func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, if errors.As(err, &apiErr) && apiErr.ErrorCode() == "NotFound" { return nil, backend.NewErrNotFound("artifact") } - return nil, fmt.Errorf("reading from access point: %w", err) + return nil, errorsx.Errorf("reading from access point: %w", err) } // Integrity check: when S3 returned a checksum, make sure the digest // the caller asked for matches the server's recorded value. if resp.ChecksumSHA256 != nil && *resp.ChecksumSHA256 != hexSha256ToBinaryB64(digest) { - return nil, fmt.Errorf("failed to validate integrity of object, got=%s, want=%s", + return nil, errorsx.Errorf("failed to validate integrity of object, got=%s, want=%s", *resp.ChecksumSHA256, hexSha256ToBinaryB64(digest)) } author, ok := resp.Metadata[annotationNameAuthor] if !ok || author != backend.AuthorAnnotation { - return nil, errors.New("asset not uploaded by Chainloop") + return nil, errorsx.New("asset not uploaded by Chainloop") } fileName, ok := resp.Metadata[annotationNameFilename] if !ok { - return nil, errors.New("couldn't find file metadata") + return nil, errorsx.New("couldn't find file metadata") } var size int64 @@ -227,7 +228,7 @@ func (b *Backend) Download(ctx context.Context, w io.Writer, digest string) erro Bucket: aws.String(b.creds.AccessPointARN), Key: aws.String(key), }) - return err + return errorsx.WithStack(err) } // CheckWritePermissions verifies that the calling org can actually mint a @@ -249,13 +250,13 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error { Bucket: aws.String(b.creds.AccessPointARN), Key: aws.String(key), }); err != nil { - return fmt.Errorf("writing healthcheck object: %w", err) + return errorsx.Errorf("writing healthcheck object: %w", err) } if _, err := b.s3Client.GetObject(ctx, &s3.GetObjectInput{ Bucket: aws.String(b.creds.AccessPointARN), Key: aws.String(key), }); err != nil { - return fmt.Errorf("reading healthcheck object: %w", err) + return errorsx.Errorf("reading healthcheck object: %w", err) } return nil } @@ -299,9 +300,10 @@ func (p *sessionCredentialsProvider) Retrieve(ctx context.Context) (aws.Credenti // SDK's default credential chain directly. if p.useAmbientForRetrieve { if p.ambientCreds == nil { - return aws.Credentials{}, errors.New("s3accesspoint: dev mode requested but no ambient credentials available") + return aws.Credentials{}, errorsx.New("s3accesspoint: dev mode requested but no ambient credentials available") } - return p.ambientCreds.Retrieve(ctx) + creds, err := p.ambientCreds.Retrieve(ctx) + return creds, errorsx.WithStack(err) } // The session policy intersects with the base role's permissions and @@ -333,10 +335,10 @@ func (p *sessionCredentialsProvider) Retrieve(ctx context.Context) (aws.Credenti out, err := p.stsClient.AssumeRole(ctx, input) if err != nil { - return aws.Credentials{}, fmt.Errorf("sts:AssumeRole for org %s: %w", info.OrgID, err) + return aws.Credentials{}, errorsx.Errorf("sts:AssumeRole for org %s: %w", info.OrgID, err) } if out.Credentials == nil { - return aws.Credentials{}, errors.New("sts:AssumeRole returned no credentials") + return aws.Credentials{}, errorsx.New("sts:AssumeRole returned no credentials") } return aws.Credentials{ diff --git a/pkg/cache/natskv.go b/pkg/cache/natskv.go index 4d508c1cd..14c8b95c7 100644 --- a/pkg/cache/natskv.go +++ b/pkg/cache/natskv.go @@ -24,6 +24,7 @@ import ( "sync" "time" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" ) @@ -110,7 +111,7 @@ func isRetryableInitError(err error) bool { func (c *natsKVCache[T]) initBucket() error { js, err := jetstream.New(c.conn) if err != nil { - return err + return errorsx.Errorf("cache: creating jetstream context: %w", err) } kv, err := js.CreateOrUpdateKeyValue(context.Background(), jetstream.KeyValueConfig{ @@ -122,7 +123,7 @@ func (c *natsKVCache[T]) initBucket() error { Storage: jetstream.MemoryStorage, }) if err != nil { - return err + return errorsx.Errorf("cache: creating or updating bucket %s: %w", c.bucket, err) } // NATS KV hardcodes DiscardNew; we want DiscardOld so the cache evicts @@ -148,7 +149,7 @@ func (c *natsKVCache[T]) ensureDiscardOld(js jetstream.JetStream) error { stream, err := js.Stream(ctx, streamName) if err != nil { - return fmt.Errorf("cache: failed to get backing stream %s: %w", streamName, err) + return errorsx.Errorf("cache: failed to get backing stream %s: %w", streamName, err) } cfg := stream.CachedInfo().Config @@ -158,7 +159,7 @@ func (c *natsKVCache[T]) ensureDiscardOld(js jetstream.JetStream) error { cfg.Discard = jetstream.DiscardOld if _, err := js.UpdateStream(ctx, cfg); err != nil { - return fmt.Errorf("cache: failed to set DiscardOld on stream %s: %w", streamName, err) + return errorsx.Errorf("cache: failed to set DiscardOld on stream %s: %w", streamName, err) } return nil } diff --git a/pkg/credentials/aws/secretmanager.go b/pkg/credentials/aws/secretmanager.go index 6e0f6f6a4..062fdbc26 100644 --- a/pkg/credentials/aws/secretmanager.go +++ b/pkg/credentials/aws/secretmanager.go @@ -30,6 +30,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/sso/types" "github.com/aws/smithy-go" "github.com/chainloop-dev/chainloop/pkg/credentials" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/google/uuid" @@ -57,7 +58,7 @@ type NewManagerOpts struct { func NewManager(opts *NewManagerOpts) (*Manager, error) { if opts.Region == "" || opts.AccessKey == "" || opts.SecretKey == "" { - return nil, errors.New("region, accessKey and the secretKey are required") + return nil, errorsx.New("region, accessKey and the secretKey are required") } l := opts.Logger @@ -92,7 +93,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, // Store the credentials as json key pairs c, err := json.Marshal(creds) if err != nil { - return "", fmt.Errorf("marshaling credentials to be stored: %w", err) + return "", errorsx.Errorf("marshaling credentials to be stored: %w", err) } if o.SecretName != "" { @@ -117,7 +118,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, } if err != nil { - return "", fmt.Errorf("saving secret in AWS: %w", err) + return "", errorsx.Errorf("saving secret in AWS: %w", err) } return secretName, nil @@ -135,11 +136,11 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretID string, creds an case (&types.ResourceNotFoundException{}).ErrorCode(): return fmt.Errorf("%w: path=%s", credentials.ErrNotFound, secretID) default: - return fmt.Errorf("getting AWS Secret Value: %w", err) + return errorsx.Errorf("getting AWS Secret Value: %w", err) } } - return fmt.Errorf("getting AWS Secret Value: %w", err) + return errorsx.Errorf("getting AWS Secret Value: %w", err) } return json.Unmarshal([]byte(*resp.SecretString), creds) @@ -150,5 +151,5 @@ func (m *Manager) DeleteCredentials(ctx context.Context, secretID string) error SecretId: aws.String(secretID), }) - return err + return errorsx.WithStack(err) } diff --git a/pkg/credentials/azurekv/keyvault.go b/pkg/credentials/azurekv/keyvault.go index 6d7a1a8c3..3975c3c2e 100644 --- a/pkg/credentials/azurekv/keyvault.go +++ b/pkg/credentials/azurekv/keyvault.go @@ -29,6 +29,7 @@ import ( "github.com/chainloop-dev/chainloop/pkg/credentials" "github.com/google/uuid" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/go-kratos/kratos/v2/log" ) @@ -69,19 +70,19 @@ var ErrValidation = errors.New("credentials validation error") func (o *NewManagerOpts) Validate() error { if o.TenantID == "" { - return fmt.Errorf("%w: missing tenant ID", ErrValidation) + return errorsx.Errorf("%w: missing tenant ID", ErrValidation) } if o.ClientID == "" { - return fmt.Errorf("%w: missing client ID", ErrValidation) + return errorsx.Errorf("%w: missing client ID", ErrValidation) } if o.ClientSecret == "" { - return fmt.Errorf("%w: missing client secret", ErrValidation) + return errorsx.Errorf("%w: missing client secret", ErrValidation) } if o.VaultURI == "" { - return fmt.Errorf("%w: missing VAULT URI", ErrValidation) + return errorsx.Errorf("%w: missing VAULT URI", ErrValidation) } return nil @@ -102,7 +103,7 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { credential, err := azidentity.NewClientSecretCredential(opts.TenantID, opts.ClientID, opts.ClientSecret, nil) if err != nil { - return nil, fmt.Errorf("failed to create Azure Service principal Credential: %w", err) + return nil, errorsx.Errorf("failed to create Azure Service principal Credential: %w", err) } // Establish a connection to the Key Vault client @@ -131,11 +132,11 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, // Store the credentials as json key pairs c, err := json.Marshal(creds) if err != nil { - return "", fmt.Errorf("marshaling credentials to be stored: %w", err) + return "", errorsx.Errorf("marshaling credentials to be stored: %w", err) } if _, err := m.client.SetSecret(ctx, secretName, azsecrets.SetSecretParameters{Value: strPtr(string(c))}, nil); err != nil { - return "", fmt.Errorf("failed to set secret: %w", err) + return "", errorsx.Errorf("failed to set secret: %w", err) } return secretName, nil @@ -151,7 +152,7 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretName string, creds return fmt.Errorf("%w: path=%s", credentials.ErrNotFound, secretName) } - return fmt.Errorf("failed to get secret: %w", err) + return errorsx.Errorf("failed to get secret: %w", err) } return json.Unmarshal([]byte(*resp.Value), creds) @@ -161,7 +162,7 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretName string, creds func (m *Manager) DeleteCredentials(ctx context.Context, secretName string) error { _, err := m.client.DeleteSecret(ctx, secretName, nil) if err != nil { - return fmt.Errorf("failed to delete secret: %w", err) + return errorsx.Errorf("failed to delete secret: %w", err) } return nil @@ -174,11 +175,11 @@ func ValidateWriterClient(m *Manager, pathPrefix string) error { ctx := context.Background() if _, err := m.client.SetSecret(ctx, secretName, azsecrets.SetSecretParameters{Value: strPtr("")}, nil); err != nil { - return fmt.Errorf("failed to set secret: %w", err) + return errorsx.Errorf("failed to set secret: %w", err) } if _, err := m.client.DeleteSecret(ctx, secretName, nil); err != nil { - return fmt.Errorf("failed to delete secret: %w", err) + return errorsx.Errorf("failed to delete secret: %w", err) } return nil @@ -201,8 +202,8 @@ func ValidateReaderClient(m *Manager, pathPrefix string) error { return nil } - return fmt.Errorf("failed to get secret: %w", err) + return errorsx.Errorf("failed to get secret: %w", err) } - return errors.New("expected error") + return errorsx.New("expected error") } diff --git a/pkg/credentials/gcp/secretmanager.go b/pkg/credentials/gcp/secretmanager.go index b0c9cc784..1a22fa0d2 100644 --- a/pkg/credentials/gcp/secretmanager.go +++ b/pkg/credentials/gcp/secretmanager.go @@ -18,7 +18,6 @@ package gcp import ( "context" "encoding/json" - "errors" "fmt" "io" "strings" @@ -30,6 +29,7 @@ import ( "google.golang.org/grpc/status" "github.com/chainloop-dev/chainloop/pkg/credentials" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/docker/distribution/uuid" "github.com/go-kratos/kratos/v2/log" @@ -59,7 +59,7 @@ type NewManagerOpts struct { func NewManager(opts *NewManagerOpts) (*Manager, error) { if opts.ProjectID == "" || opts.ServiceAccountKey == "" { - return nil, errors.New("projectID and serviceAccountKey are required") + return nil, errorsx.New("projectID and serviceAccountKey are required") } l := opts.Logger @@ -72,7 +72,7 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { cli, err := secretmanager.NewClient(context.TODO(), option.WithCredentialsFile(opts.ServiceAccountKey)) if err != nil { - return nil, fmt.Errorf("error while creating the client: %w", err) + return nil, errorsx.Errorf("error while creating the client: %w", err) } logger.Infow("msg", "created GCP connection", "projectID", opts.ProjectID, "role", opts.Role, "prefix", opts.SecretPrefix) @@ -90,7 +90,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, // store creds in key-value pair c, err := json.Marshal(creds) if err != nil { - return "", fmt.Errorf("marshaling credentials to be stored: %w", err) + return "", errorsx.Errorf("marshaling credentials to be stored: %w", err) } o := credentials.ApplySaveOptions(opts...) @@ -124,18 +124,18 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, return secretID, nil } if !isNotFoundErr(addErr) { - return "", fmt.Errorf("adding secret version in GCP: %w", addErr) + return "", errorsx.Errorf("adding secret version in GCP: %w", addErr) } // Secret container doesn't exist yet — create it and fall through to AddSecretVersion below. if _, err = m.client.CreateSecret(ctx, secretReq); err != nil { - return "", fmt.Errorf("creating secret in GCP: %w", err) + return "", errorsx.Errorf("creating secret in GCP: %w", err) } m.logger.Infow("msg", "created new secret", "secretID", secretID) } else { // New secret: create the container and use the canonical name from the API response. secret, createErr := m.client.CreateSecret(ctx, secretReq) if createErr != nil { - return "", fmt.Errorf("creating secret in GCP: %w", createErr) + return "", errorsx.Errorf("creating secret in GCP: %w", createErr) } parent = secret.Name m.logger.Infow("msg", "created new secret", "secretID", secretID) @@ -147,7 +147,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, Payload: &secretmanagerpb.SecretPayload{Data: c}, }) if err != nil { - return "", fmt.Errorf("creating secret version in GCP: %w", err) + return "", errorsx.Errorf("creating secret version in GCP: %w", err) } m.logger.Infow("msg", "added new secret version", "secretID", secretID, "versionID", v.Name) @@ -181,5 +181,5 @@ func (m *Manager) DeleteCredentials(ctx context.Context, secretID string) error } m.logger.Infow("msg", "deleting secret", "secretID", secretID) - return m.client.DeleteSecret(ctx, &deleteRequest) + return errorsx.WithStack(m.client.DeleteSecret(ctx, &deleteRequest)) } diff --git a/pkg/credentials/vault/keyval.go b/pkg/credentials/vault/keyval.go index 6321f4940..7b820780e 100644 --- a/pkg/credentials/vault/keyval.go +++ b/pkg/credentials/vault/keyval.go @@ -26,6 +26,7 @@ import ( "time" "github.com/chainloop-dev/chainloop/pkg/credentials" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/docker/distribution/uuid" "github.com/go-kratos/kratos/v2/log" @@ -60,7 +61,7 @@ const healthCheckNonExisting = "chainloop-non-existing" // SecretPrefix is used to namespace secrets in the KVv2 engine during write operations. func NewManager(opts *NewManagerOpts) (*Manager, error) { if opts.AuthToken == "" || opts.Address == "" { - return nil, errors.New("auth token and instance address are required") + return nil, errorsx.New("auth token and instance address are required") } config := vault.DefaultConfig() @@ -69,7 +70,7 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { client, err := vault.NewClient(config) if err != nil { - return nil, err + return nil, errorsx.Errorf("creating Vault client: %w", err) } client.SetToken(opts.AuthToken) @@ -108,11 +109,11 @@ func validateWriterClient(kv *vault.KVv2, pathPrefix string) error { ctx := context.Background() keyPath := strings.Join([]string{pathPrefix, healthCheckSecret}, "/") if _, err := kv.Put(ctx, keyPath, nil); err != nil { - return err + return errorsx.Errorf("writing health check secret: %w", err) } if err := kv.DeleteMetadata(ctx, keyPath); err != nil { - return fmt.Errorf("deleting health check secret: %w", err) + return errorsx.Errorf("deleting health check secret: %w", err) } return nil @@ -130,7 +131,7 @@ func validateReaderClient(kv *vault.KVv2, pathPrefix string) error { return nil } - return err + return errorsx.Errorf("reading health check secret: %w", err) } return nil @@ -139,7 +140,7 @@ func validateReaderClient(kv *vault.KVv2, pathPrefix string) error { func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, opts ...credentials.SaveOption) (string, error) { credsM, err := structToMap(creds) if err != nil { - return "", fmt.Errorf("converting struct to map: %w", err) + return "", errorsx.Errorf("converting struct to map: %w", err) } o := credentials.ApplySaveOptions(opts...) @@ -151,7 +152,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, _, err = m.client.Put(ctx, secretName, credsM) if err != nil { - return "", fmt.Errorf("storing secret in Vault: %w", err) + return "", errorsx.Errorf("storing secret in Vault: %w", err) } return secretName, nil @@ -166,11 +167,11 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretID string, creds an return fmt.Errorf("%w: path=%s", credentials.ErrNotFound, secretID) } - return fmt.Errorf("reading secret from Vault: %w", err) + return errorsx.Errorf("reading secret from Vault: %w", err) } if err := mapToStruct(s.Data, creds); err != nil { - return fmt.Errorf("converting secret to struct: %w", err) + return errorsx.Errorf("converting secret to struct: %w", err) } return nil @@ -178,7 +179,7 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretID string, creds an func (m *Manager) DeleteCredentials(ctx context.Context, secretID string) error { m.logger.Infow("msg", "deleting credentials", "path", secretID) - return m.client.DeleteMetadata(ctx, secretID) + return errorsx.WithStack(m.client.DeleteMetadata(ctx, secretID)) } // convert from struct to map[string]interface{} diff --git a/pkg/grpcconn/grpcconn.go b/pkg/grpcconn/grpcconn.go index e548e5319..4b9e55bbf 100644 --- a/pkg/grpcconn/grpcconn.go +++ b/pkg/grpcconn/grpcconn.go @@ -147,8 +147,10 @@ func New(uri, authToken string, opt ...Option) (*grpc.ClientConn, error) { } var opts []grpc.DialOption - unaryInterceptors := []grpc.UnaryClientInterceptor{} - streamInterceptors := []grpc.StreamClientInterceptor{} + // The stack interceptors come first, so they are outermost: inner + // interceptors such as retry see the errors as gRPC returns them. + unaryInterceptors := []grpc.UnaryClientInterceptor{StackUnaryClientInterceptor()} + streamInterceptors := []grpc.StreamClientInterceptor{StackStreamClientInterceptor()} if authToken != "" || optionalArgs.tokenProvider != nil { grpcCreds := newTokenAuth(authToken, optionalArgs.insecure, optionalArgs.orgName) @@ -165,12 +167,10 @@ func New(uri, authToken string, opt ...Option) (*grpc.ClientConn, error) { unaryInterceptors = append(unaryInterceptors, optionalArgs.unaryInterceptors...) streamInterceptors = append(streamInterceptors, optionalArgs.streamInterceptors...) - if len(unaryInterceptors) > 0 { - opts = append(opts, grpc.WithChainUnaryInterceptor(unaryInterceptors...)) - } - if len(streamInterceptors) > 0 { - opts = append(opts, grpc.WithChainStreamInterceptor(streamInterceptors...)) - } + opts = append(opts, + grpc.WithChainUnaryInterceptor(unaryInterceptors...), + grpc.WithChainStreamInterceptor(streamInterceptors...), + ) // Currently we only support system tls certs var tlsDialOption grpc.DialOption diff --git a/pkg/grpcconn/stack.go b/pkg/grpcconn/stack.go new file mode 100644 index 000000000..ba14cd16d --- /dev/null +++ b/pkg/grpcconn/stack.go @@ -0,0 +1,79 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// 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. + +package grpcconn + +import ( + "context" + "io" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "google.golang.org/grpc" + "google.golang.org/grpc/metadata" +) + +// StackUnaryClientInterceptor records the stack of the caller on each error of +// a unary call, so Sentry groups a failed call by the code that made it. +// The error keeps its message and its gRPC status. +func StackUnaryClientInterceptor() grpc.UnaryClientInterceptor { + return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + return errorsx.WithStack(invoker(ctx, method, req, reply, cc, opts...)) + } +} + +// StackStreamClientInterceptor records the stack of the caller on each error of +// a streaming call: on stream creation, Header, SendMsg, RecvMsg and CloseSend. +// It does not wrap io.EOF, because callers compare it with ==. +func StackStreamClientInterceptor() grpc.StreamClientInterceptor { + return func(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { + cs, err := streamer(ctx, desc, cc, method, opts...) + if err != nil { + return nil, errorsx.WithStack(err) + } + + return &stackClientStream{ClientStream: cs}, nil + } +} + +// stackClientStream is a client stream that records the stack of the caller on each error. +type stackClientStream struct { + grpc.ClientStream +} + +func (s *stackClientStream) SendMsg(m any) error { + return streamErr(s.ClientStream.SendMsg(m)) +} + +func (s *stackClientStream) RecvMsg(m any) error { + return streamErr(s.ClientStream.RecvMsg(m)) +} + +func (s *stackClientStream) CloseSend() error { + return streamErr(s.ClientStream.CloseSend()) +} + +func (s *stackClientStream) Header() (metadata.MD, error) { + md, err := s.ClientStream.Header() + return md, streamErr(err) +} + +// streamErr adds a stack to err, except to io.EOF, which ends a stream normally. +func streamErr(err error) error { + if err == io.EOF { //nolint:errorlint // gRPC returns io.EOF unwrapped and callers compare it with ==. + return err + } + + return errorsx.WithStack(err) +} diff --git a/pkg/grpcconn/stack_test.go b/pkg/grpcconn/stack_test.go new file mode 100644 index 000000000..d152f6ca8 --- /dev/null +++ b/pkg/grpcconn/stack_test.go @@ -0,0 +1,175 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// 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. + +package grpcconn + +import ( + "context" + "errors" + "io" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" +) + +func TestStackUnaryClientInterceptor(t *testing.T) { + statusErr := status.Error(codes.NotFound, "missing") + + testCases := []struct { + name string + invokeErr error + wantStack bool + }{ + {name: "nil stays nil", invokeErr: nil}, + {name: "status error gets a stack", invokeErr: statusErr, wantStack: true}, + {name: "plain error gets a stack", invokeErr: errors.New("boom"), wantStack: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + invoker := func(context.Context, string, any, any, *grpc.ClientConn, ...grpc.CallOption) error { + return tc.invokeErr + } + + err := StackUnaryClientInterceptor()(context.Background(), "/svc/Method", nil, nil, nil, invoker) + if tc.invokeErr == nil { + assert.NoError(t, err) + return + } + + require.ErrorIs(t, err, tc.invokeErr) + assert.Equal(t, tc.invokeErr.Error(), err.Error()) + assert.Equal(t, tc.wantStack, errorsx.HasStack(err)) + assert.Equal(t, status.Code(tc.invokeErr), status.Code(err)) + }) + } +} + +// fakeClientStream returns the configured errors from its stream methods. +type fakeClientStream struct { + grpc.ClientStream + sendErr, recvErr, closeErr, headerErr error +} + +func (s *fakeClientStream) SendMsg(any) error { return s.sendErr } +func (s *fakeClientStream) RecvMsg(any) error { return s.recvErr } +func (s *fakeClientStream) CloseSend() error { return s.closeErr } +func (s *fakeClientStream) Header() (metadata.MD, error) { return nil, s.headerErr } + +func TestStackStreamClientInterceptor(t *testing.T) { + statusErr := status.Error(codes.Unavailable, "down") + + t.Run("stream creation error gets a stack", func(t *testing.T) { + streamer := func(context.Context, *grpc.StreamDesc, *grpc.ClientConn, string, ...grpc.CallOption) (grpc.ClientStream, error) { + return nil, statusErr + } + + cs, err := StackStreamClientInterceptor()(context.Background(), &grpc.StreamDesc{}, nil, "/svc/Stream", streamer) + assert.Nil(t, cs) + require.ErrorIs(t, err, statusErr) + assert.True(t, errorsx.HasStack(err)) + assert.Equal(t, codes.Unavailable, status.Code(err)) + }) + + testCases := []struct { + name string + stream *fakeClientStream + call func(grpc.ClientStream) error + wantErr error + wantStack bool + }{ + { + name: "SendMsg error gets a stack", + stream: &fakeClientStream{sendErr: statusErr}, + call: func(cs grpc.ClientStream) error { return cs.SendMsg(nil) }, + wantErr: statusErr, + wantStack: true, + }, + { + name: "RecvMsg error gets a stack", + stream: &fakeClientStream{recvErr: statusErr}, + call: func(cs grpc.ClientStream) error { return cs.RecvMsg(nil) }, + wantErr: statusErr, + wantStack: true, + }, + { + name: "CloseSend error gets a stack", + stream: &fakeClientStream{closeErr: statusErr}, + call: func(cs grpc.ClientStream) error { return cs.CloseSend() }, + wantErr: statusErr, + wantStack: true, + }, + { + name: "Header error gets a stack", + stream: &fakeClientStream{headerErr: statusErr}, + call: func(cs grpc.ClientStream) error { + _, err := cs.Header() + return err + }, + wantErr: statusErr, + wantStack: true, + }, + { + name: "RecvMsg io.EOF stays io.EOF", + stream: &fakeClientStream{recvErr: io.EOF}, + call: func(cs grpc.ClientStream) error { return cs.RecvMsg(nil) }, + wantErr: io.EOF, + }, + { + name: "SendMsg io.EOF stays io.EOF", + stream: &fakeClientStream{sendErr: io.EOF}, + call: func(cs grpc.ClientStream) error { return cs.SendMsg(nil) }, + wantErr: io.EOF, + }, + { + name: "nil stays nil", + stream: &fakeClientStream{}, + call: func(cs grpc.ClientStream) error { return cs.RecvMsg(nil) }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + streamer := func(context.Context, *grpc.StreamDesc, *grpc.ClientConn, string, ...grpc.CallOption) (grpc.ClientStream, error) { + return tc.stream, nil + } + + cs, err := StackStreamClientInterceptor()(context.Background(), &grpc.StreamDesc{}, nil, "/svc/Stream", streamer) + require.NoError(t, err) + + err = tc.call(cs) + if tc.wantErr == nil { + assert.NoError(t, err) + return + } + + if !tc.wantStack { + // Callers compare io.EOF with ==, so it must not be wrapped. + assert.Same(t, tc.wantErr, err) //nolint:testifylint + return + } + + require.ErrorIs(t, err, tc.wantErr) + assert.True(t, errorsx.HasStack(err)) + assert.Equal(t, status.Code(tc.wantErr), status.Code(err)) + }) + } +} diff --git a/pkg/natsconn/natsconn.go b/pkg/natsconn/natsconn.go index 8ba7aecbb..c96ec9d99 100644 --- a/pkg/natsconn/natsconn.go +++ b/pkg/natsconn/natsconn.go @@ -17,10 +17,10 @@ package natsconn import ( "context" - "fmt" "sync" "time" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" "github.com/nats-io/nats.go" ) @@ -78,7 +78,7 @@ func New(cfg *Config, logger log.Logger) (*ReloadableConnection, func(), error) nc, err := nats.Connect(cfg.URI, opts...) if err != nil { - return nil, noop, fmt.Errorf("connecting to NATS: %w", err) + return nil, noop, errorsx.Errorf("connecting to NATS: %w", err) } rc.Conn = nc diff --git a/pkg/policies/fetch_remote_test.go b/pkg/policies/fetch_remote_test.go new file mode 100644 index 000000000..629a43cd4 --- /dev/null +++ b/pkg/policies/fetch_remote_test.go @@ -0,0 +1,73 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// 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. + +package policies + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFetchRemoteErrorsRecordStack(t *testing.T) { + notFound := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer notFound.Close() + + closed := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + // Close the server so that the request fails in the transport. + closed.Close() + + testCases := []struct { + name string + url string + wantStatus int + wantMsg string + }{ + { + name: "non-2xx status returns a typed error", + url: notFound.URL, + wantStatus: http.StatusNotFound, + wantMsg: "requesting remote policy: server returned HTTP 404", + }, + { + name: "transport failure", + url: closed.URL, + wantMsg: "requesting remote policy: ", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := fetchRemote(nil, tc.url) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantMsg) + assert.True(t, errorsx.HasStack(err)) + + var statusErr *RemoteStatusError + if tc.wantStatus == 0 { + assert.NotErrorAs(t, err, &statusErr) + return + } + require.ErrorAs(t, err, &statusErr) + assert.Equal(t, tc.wantStatus, statusErr.StatusCode) + }) + } +} diff --git a/pkg/policies/loader.go b/pkg/policies/loader.go index 1e7e104d4..957026669 100644 --- a/pkg/policies/loader.go +++ b/pkg/policies/loader.go @@ -32,6 +32,7 @@ import ( v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/unmarshal" "github.com/chainloop-dev/chainloop/pkg/cache" + "github.com/chainloop-dev/chainloop/pkg/errorsx" crv1 "github.com/google/go-containerregistry/pkg/v1" "golang.org/x/sync/singleflight" "google.golang.org/protobuf/encoding/protojson" @@ -171,22 +172,34 @@ func fetchRemote(client *http.Client, ref string) ([]byte, error) { if errors.As(err, &urlErr) { err = urlErr.Err } - return nil, fmt.Errorf("requesting remote policy: %w", err) + return nil, errorsx.Errorf("requesting remote policy: %w", err) } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, fmt.Errorf("requesting remote policy: server returned HTTP %d", resp.StatusCode) + return nil, errorsx.Errorf("requesting remote policy: %w", &RemoteStatusError{StatusCode: resp.StatusCode}) } raw, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("reading remote policy: %w", err) + return nil, errorsx.Errorf("reading remote policy: %w", err) } return raw, nil } +// RemoteStatusError is returned when the server of a remote policy or policy +// group answers with a status other than 2xx. It has its own type so that +// Sentry does not group it with transport failures. +type RemoteStatusError struct { + // StatusCode is the HTTP status code of the response. + StatusCode int +} + +func (e *RemoteStatusError) Error() string { + return fmt.Sprintf("server returned HTTP %d", e.StatusCode) +} + // ChainloopLoader loads policies referenced with chainloop://provider/name URLs type ChainloopLoader struct { Client pb.AttestationServiceClient