From 8a71cb4aeca58ca9e412851fe092ab8b1482296c Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Thu, 2 Apr 2026 16:13:40 +0200 Subject: [PATCH 1/4] Add gravitational_trace.nocrypto build tag --- .github/workflows/test.yaml | 2 + errors.go | 3 +- errors_crypto.go | 26 +++++ errors_nocrypto.go | 35 ++++++ httplib.go | 16 +++ httplib_test.go | 2 + internal/traverse.go | 14 +++ trace_crypto_test.go | 214 ++++++++++++++++++++++++++++++++++++ trace_test.go | 187 ------------------------------- 9 files changed, 310 insertions(+), 189 deletions(-) create mode 100644 errors_crypto.go create mode 100644 errors_nocrypto.go create mode 100644 trace_crypto_test.go diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 6de5056..d23e47f 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -20,3 +20,5 @@ jobs: go-version-file: go.mod - name: Test run: go test -race ./... + - name: Test (nocrypto) + run: go test -race -tags gravitational_trace.nocrypto ./... diff --git a/errors.go b/errors.go index ab92b7a..847217d 100644 --- a/errors.go +++ b/errors.go @@ -17,7 +17,6 @@ limitations under the License. package trace import ( - "crypto/x509" "errors" "fmt" "io" @@ -347,7 +346,7 @@ func ConvertSystemError(err error) error { return newTrace(&AccessDeniedError{ Message: message, }) - case x509.SystemRootsError, x509.UnknownAuthorityError: + case x509SystemRootsError, x509UnknownAuthorityError: return newTrace(&TrustError{Err: innerError}) } if _, ok := innerError.(net.Error); ok { diff --git a/errors_crypto.go b/errors_crypto.go new file mode 100644 index 0000000..518e44a --- /dev/null +++ b/errors_crypto.go @@ -0,0 +1,26 @@ +//go:build !gravitational_trace.nocrypto + +// Copyright 2026 Gravitational, Inc. +// +// 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 trace + +import "crypto/x509" + +// these type aliases are used by [ConvertSystemError]; builds with the nocrypto +// tag will define them differently +type ( + x509SystemRootsError = x509.SystemRootsError + x509UnknownAuthorityError = x509.UnknownAuthorityError +) diff --git a/errors_nocrypto.go b/errors_nocrypto.go new file mode 100644 index 0000000..f6380ec --- /dev/null +++ b/errors_nocrypto.go @@ -0,0 +1,35 @@ +//go:build gravitational_trace.nocrypto + +// Copyright 2026 Gravitational, Inc. +// +// 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 trace + +// these unexported error types are never constructed but they are referenced by +// [ConvertSystemError]; in builds without the nocrypto tag they are aliases of +// errors in [crypto/x509] +type ( + x509SystemRootsError struct{} + x509UnknownAuthorityError struct{} +) + +// Error implements [error]. +func (x509SystemRootsError) Error() string { + return "trace.x509SystemRootsError (this is a bug)" +} + +// Error implements [error]. +func (x509UnknownAuthorityError) Error() string { + return "trace.x509UnknownAuthorityError (this is a bug)" +} diff --git a/httplib.go b/httplib.go index 8f85c0b..5f7628b 100644 --- a/httplib.go +++ b/httplib.go @@ -1,3 +1,19 @@ +//go:build !gravitational_trace.nocrypto + +// Copyright 2026 Gravitational, Inc. +// +// 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 trace import ( diff --git a/httplib_test.go b/httplib_test.go index 56b2865..b9b3adc 100644 --- a/httplib_test.go +++ b/httplib_test.go @@ -1,3 +1,5 @@ +//go:build !gravitational_trace.nocrypto + /* Copyright 2020 Gravitational, Inc. diff --git a/internal/traverse.go b/internal/traverse.go index 2df44a6..159a551 100644 --- a/internal/traverse.go +++ b/internal/traverse.go @@ -1,3 +1,17 @@ +// Copyright 2026 Gravitational, Inc. +// +// 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 internal // TraverseErr traverses the err error chain until fn returns true. diff --git a/trace_crypto_test.go b/trace_crypto_test.go new file mode 100644 index 0000000..7c58b6e --- /dev/null +++ b/trace_crypto_test.go @@ -0,0 +1,214 @@ +//go:build !gravitational_trace.nocrypto + +// Copyright 2026 Gravitational, Inc. +// +// 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 trace + +import ( + "fmt" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetFields(t *testing.T) { + testErr := fmt.Errorf("description") + assert.Equal(t, map[string]interface{}{}, GetFields(testErr)) + + fields := map[string]interface{}{ + "test_key": "test_value", + } + err := WithFields(Wrap(testErr), fields) + assert.Equal(t, fields, GetFields(err)) + + // ensure that you can get fields from a proxyError + e := roundtripError(err) + assert.Equal(t, fields, GetFields(e)) +} + +func roundtripError(err error) error { + w := newTestWriter() + WriteError(w, err) + + outErr := ReadError(w.StatusCode, w.Body) + return outErr +} + +func TestGenericErrors(t *testing.T) { + testCases := []struct { + Err Error + Predicate func(error) bool + StatusCode int + comment string + }{ + { + Err: NotFound("not found"), + Predicate: IsNotFound, + StatusCode: http.StatusNotFound, + comment: "not found error", + }, + { + Err: AlreadyExists("already exists"), + Predicate: IsAlreadyExists, + StatusCode: http.StatusConflict, + comment: "already exists error", + }, + { + Err: BadParameter("is bad"), + Predicate: IsBadParameter, + StatusCode: http.StatusBadRequest, + comment: "bad parameter error", + }, + { + Err: CompareFailed("is bad"), + Predicate: IsCompareFailed, + StatusCode: http.StatusPreconditionFailed, + comment: "comparison failed error", + }, + { + Err: AccessDenied("denied"), + Predicate: IsAccessDenied, + StatusCode: http.StatusForbidden, + comment: "access denied error", + }, + { + Err: ConnectionProblem(nil, "prob"), + Predicate: IsConnectionProblem, + StatusCode: http.StatusRequestTimeout, + comment: "connection error", + }, + { + Err: LimitExceeded("limit exceeded"), + Predicate: IsLimitExceeded, + StatusCode: http.StatusTooManyRequests, + comment: "limit exceeded error", + }, + { + Err: NotImplemented("not implemented"), + Predicate: IsNotImplemented, + StatusCode: http.StatusNotImplemented, + comment: "not implemented error", + }, + } + + for _, testCase := range testCases { + SetDebug(true) + err := testCase.Err + + var traceErr *TraceErr + var ok bool + if traceErr, ok = err.(*TraceErr); !ok { + t.Fatalf("Expected error to be of type *TraceErr: %#v", err) + } + + assert.NotEmpty(t, traceErr.Traces, testCase.comment) + assert.Regexp(t, ".*.trace_crypto_test\\.go.*", line(DebugReport(err)), testCase.comment) + assert.NotRegexp(t, ".*.errors\\.go.*", line(DebugReport(err)), testCase.comment) + assert.NotRegexp(t, ".*.trace\\.go.*", line(DebugReport(err)), testCase.comment) + assert.True(t, testCase.Predicate(err), testCase.comment) + + w := newTestWriter() + WriteError(w, err) + + outErr := ReadError(w.StatusCode, w.Body) + if _, ok := outErr.(proxyError); !ok { + t.Fatalf("Expected error to be of type proxyError: %#v", outErr) + } + assert.True(t, testCase.Predicate(outErr), testCase.comment) + + SetDebug(false) + w = newTestWriter() + WriteError(w, err) + outErr = ReadError(w.StatusCode, w.Body) + assert.True(t, testCase.Predicate(outErr), testCase.comment) + } +} + +// Make sure we write some output produced by standard errors +func TestWriteExternalErrors(t *testing.T) { + err := Wrap(fmt.Errorf("snap!")) + + SetDebug(true) + w := newTestWriter() + WriteError(w, err) + extErr := ReadError(w.StatusCode, w.Body) + assert.Equal(t, http.StatusInternalServerError, w.StatusCode) + assert.Regexp(t, ".*.snap.*", strings.Replace(string(w.Body), "\n", "", -1)) + require.NotNil(t, extErr) + assert.EqualError(t, err, extErr.Error()) + + SetDebug(false) + w = newTestWriter() + WriteError(w, err) + extErr = ReadError(w.StatusCode, w.Body) + assert.Equal(t, http.StatusInternalServerError, w.StatusCode) + assert.Regexp(t, ".*.snap.*", strings.Replace(string(w.Body), "\n", "", -1)) + require.NotNil(t, extErr) + assert.EqualError(t, err, extErr.Error()) +} + +func TestAggregateConvertsToCommonErrors(t *testing.T) { + testCases := []struct { + Err error + Predicate func(error) bool + RoundtripPredicate func(error) bool + StatusCode int + comment string + }{ + { + comment: "Aggregate unwraps to first aggregated error", + Err: NewAggregate( + BadParameter("invalid value of foo"), + LimitExceeded("limit exceeded"), + ), + Predicate: IsAggregate, + RoundtripPredicate: IsBadParameter, + StatusCode: http.StatusBadRequest, + }, + { + comment: "Nested aggregate unwraps recursively", + Err: NewAggregate( + NewAggregate( + BadParameter("invalid value of foo"), + LimitExceeded("limit exceeded"), + ), + ), + Predicate: IsAggregate, + RoundtripPredicate: IsBadParameter, + StatusCode: http.StatusBadRequest, + }, + } + for _, testCase := range testCases { + SetDebug(true) + err := testCase.Err + + assert.Regexp(t, ".*.trace_crypto_test.go.*", line(DebugReport(err)), testCase.comment) + assert.True(t, testCase.Predicate(err), testCase.comment) + + w := newTestWriter() + WriteError(w, err) + outErr := ReadError(w.StatusCode, w.Body) + assert.True(t, testCase.RoundtripPredicate(outErr), testCase.comment) + + SetDebug(false) + w = newTestWriter() + WriteError(w, err) + outErr = ReadError(w.StatusCode, w.Body) + assert.True(t, testCase.RoundtripPredicate(outErr), testCase.comment) + } +} diff --git a/trace_test.go b/trace_test.go index ae04206..eed9ffe 100644 --- a/trace_test.go +++ b/trace_test.go @@ -333,29 +333,6 @@ func TestUserMessageWithFields(t *testing.T) { assert.Equal(t, "test_key=\"test_value\" user message\tdescription", line(UserMessageWithFields(err))) } -func TestGetFields(t *testing.T) { - testErr := fmt.Errorf("description") - assert.Equal(t, map[string]interface{}{}, GetFields(testErr)) - - fields := map[string]interface{}{ - "test_key": "test_value", - } - err := WithFields(Wrap(testErr), fields) - assert.Equal(t, fields, GetFields(err)) - - // ensure that you can get fields from a proxyError - e := roundtripError(err) - assert.Equal(t, fields, GetFields(e)) -} - -func roundtripError(err error) error { - w := newTestWriter() - WriteError(w, err) - - outErr := ReadError(w.StatusCode, w.Body) - return outErr -} - func TestWrapNil(t *testing.T) { err1 := Wrap(nil, "message: %v", "extra") assert.Nil(t, err1) @@ -409,119 +386,6 @@ func TestWrapStdlibErrors(t *testing.T) { assert.True(t, IsNotFound(os.ErrNotExist)) } -func TestGenericErrors(t *testing.T) { - testCases := []struct { - Err Error - Predicate func(error) bool - StatusCode int - comment string - }{ - { - Err: NotFound("not found"), - Predicate: IsNotFound, - StatusCode: http.StatusNotFound, - comment: "not found error", - }, - { - Err: AlreadyExists("already exists"), - Predicate: IsAlreadyExists, - StatusCode: http.StatusConflict, - comment: "already exists error", - }, - { - Err: BadParameter("is bad"), - Predicate: IsBadParameter, - StatusCode: http.StatusBadRequest, - comment: "bad parameter error", - }, - { - Err: CompareFailed("is bad"), - Predicate: IsCompareFailed, - StatusCode: http.StatusPreconditionFailed, - comment: "comparison failed error", - }, - { - Err: AccessDenied("denied"), - Predicate: IsAccessDenied, - StatusCode: http.StatusForbidden, - comment: "access denied error", - }, - { - Err: ConnectionProblem(nil, "prob"), - Predicate: IsConnectionProblem, - StatusCode: http.StatusRequestTimeout, - comment: "connection error", - }, - { - Err: LimitExceeded("limit exceeded"), - Predicate: IsLimitExceeded, - StatusCode: http.StatusTooManyRequests, - comment: "limit exceeded error", - }, - { - Err: NotImplemented("not implemented"), - Predicate: IsNotImplemented, - StatusCode: http.StatusNotImplemented, - comment: "not implemented error", - }, - } - - for _, testCase := range testCases { - SetDebug(true) - err := testCase.Err - - var traceErr *TraceErr - var ok bool - if traceErr, ok = err.(*TraceErr); !ok { - t.Fatalf("Expected error to be of type *TraceErr: %#v", err) - } - - assert.NotEmpty(t, traceErr.Traces, testCase.comment) - assert.Regexp(t, ".*.trace_test\\.go.*", line(DebugReport(err)), testCase.comment) - assert.NotRegexp(t, ".*.errors\\.go.*", line(DebugReport(err)), testCase.comment) - assert.NotRegexp(t, ".*.trace\\.go.*", line(DebugReport(err)), testCase.comment) - assert.True(t, testCase.Predicate(err), testCase.comment) - - w := newTestWriter() - WriteError(w, err) - - outErr := ReadError(w.StatusCode, w.Body) - if _, ok := outErr.(proxyError); !ok { - t.Fatalf("Expected error to be of type proxyError: %#v", outErr) - } - assert.True(t, testCase.Predicate(outErr), testCase.comment) - - SetDebug(false) - w = newTestWriter() - WriteError(w, err) - outErr = ReadError(w.StatusCode, w.Body) - assert.True(t, testCase.Predicate(outErr), testCase.comment) - } -} - -// Make sure we write some output produced by standard errors -func TestWriteExternalErrors(t *testing.T) { - err := Wrap(fmt.Errorf("snap!")) - - SetDebug(true) - w := newTestWriter() - WriteError(w, err) - extErr := ReadError(w.StatusCode, w.Body) - assert.Equal(t, http.StatusInternalServerError, w.StatusCode) - assert.Regexp(t, ".*.snap.*", strings.Replace(string(w.Body), "\n", "", -1)) - require.NotNil(t, extErr) - assert.EqualError(t, err, extErr.Error()) - - SetDebug(false) - w = newTestWriter() - WriteError(w, err) - extErr = ReadError(w.StatusCode, w.Body) - assert.Equal(t, http.StatusInternalServerError, w.StatusCode) - assert.Regexp(t, ".*.snap.*", strings.Replace(string(w.Body), "\n", "", -1)) - require.NotNil(t, extErr) - assert.EqualError(t, err, extErr.Error()) -} - type netError struct{} func (e *netError) Error() string { return "net" } @@ -572,57 +436,6 @@ func TestWithFields(t *testing.T) { assert.Regexp(t, ".*.testfield2: value2.*", line(DebugReport(err))) } -func TestAggregateConvertsToCommonErrors(t *testing.T) { - testCases := []struct { - Err error - Predicate func(error) bool - RoundtripPredicate func(error) bool - StatusCode int - comment string - }{ - { - comment: "Aggregate unwraps to first aggregated error", - Err: NewAggregate( - BadParameter("invalid value of foo"), - LimitExceeded("limit exceeded"), - ), - Predicate: IsAggregate, - RoundtripPredicate: IsBadParameter, - StatusCode: http.StatusBadRequest, - }, - { - comment: "Nested aggregate unwraps recursively", - Err: NewAggregate( - NewAggregate( - BadParameter("invalid value of foo"), - LimitExceeded("limit exceeded"), - ), - ), - Predicate: IsAggregate, - RoundtripPredicate: IsBadParameter, - StatusCode: http.StatusBadRequest, - }, - } - for _, testCase := range testCases { - SetDebug(true) - err := testCase.Err - - assert.Regexp(t, ".*.trace_test.go.*", line(DebugReport(err)), testCase.comment) - assert.True(t, testCase.Predicate(err), testCase.comment) - - w := newTestWriter() - WriteError(w, err) - outErr := ReadError(w.StatusCode, w.Body) - assert.True(t, testCase.RoundtripPredicate(outErr), testCase.comment) - - SetDebug(false) - w = newTestWriter() - WriteError(w, err) - outErr = ReadError(w.StatusCode, w.Body) - assert.True(t, testCase.RoundtripPredicate(outErr), testCase.comment) - } -} - func TestAggregateThrowAwayNils(t *testing.T) { err := NewAggregate(fmt.Errorf("error1"), nil, fmt.Errorf("error2")) require.NotNil(t, err) From b21667655011b95279e5d9e533bc183c374840c3 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Thu, 2 Apr 2026 16:27:09 +0200 Subject: [PATCH 2/4] Add workflow to check dependencies --- .github/workflows/test.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index d23e47f..db8fab5 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -22,3 +22,15 @@ jobs: run: go test -race ./... - name: Test (nocrypto) run: go test -race -tags gravitational_trace.nocrypto ./... + nocrypto: + name: Check nocrypto dependencies + runs-on: ubuntu-22.04 + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Check dependencies with the nocrypto build tag + run: "go list -tags gravitational_trace.nocrypto -deps ./... | sort | tee /dev/stderr | (! grep -q -e ^crypto$ -e ^crypto/ )" From d3fcabaf2aed05e28c7d6e15d93946b240cf5a7e Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Thu, 2 Apr 2026 18:03:04 +0200 Subject: [PATCH 3/4] Explicitly specify bash to get pipefail in the new gha job --- .github/workflows/test.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index db8fab5..9ef38b4 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -33,4 +33,5 @@ jobs: with: go-version-file: go.mod - name: Check dependencies with the nocrypto build tag + shell: bash # for pipefail run: "go list -tags gravitational_trace.nocrypto -deps ./... | sort | tee /dev/stderr | (! grep -q -e ^crypto$ -e ^crypto/ )" From 0567671f0c435083b55e53fc8c4a549408fb9854 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Tue, 7 Apr 2026 16:24:39 +0200 Subject: [PATCH 4/4] Add build tag to README.md --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index f4dd360..8d6d077 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,8 @@ func main() { } ``` +### Build tags +The functions `WriteError`, `ReadError` and `ErrorToCode` require including the standard library package `net/http`, which transitively depends on `crypto/tls`, `crypto/x509`, and a lot of the `crypto/...` package tree, and the `ConvertSystemError` function depends on `crypto/x509` to wrap the `x509.SystemRootsError` and `x509.UnknownAuthorityError` errors into a `trace.TrustError`. - +As a size optimization for binaries that don't otherwise make use of `net/http` or `crypto/x509`, builds with the `gravitational_trace.nocrypto` build tag will exclude the `WriteError`, `ReadError` and `ErrorToCode` functions, and will not match against the `x509.SystemRootsError` and `x509.UnknownAuthorityError` errors in `ConvertSystemError`, which will get rid of the requirement for `net/http` or `crypto/...`.