Skip to content

Commit 4bb0170

Browse files
authored
status: Fix status incompatibility introduced by #6919 and move non-regeneratable proto code into /testdata (#7724)
1 parent 80937a9 commit 4bb0170

File tree

21 files changed

+236
-52
lines changed

21 files changed

+236
-52
lines changed

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78
88
github.com/envoyproxy/go-control-plane v0.13.1
99
github.com/golang/glog v1.2.2
10+
github.com/golang/protobuf v1.5.4
1011
github.com/google/go-cmp v0.6.0
1112
github.com/google/uuid v1.6.0
1213
golang.org/x/net v0.30.0

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ github.com/envoyproxy/protoc-gen-validate v1.1.0 h1:tntQDh69XqOCOZsDz0lVJQez/2L6
1616
github.com/envoyproxy/protoc-gen-validate v1.1.0/go.mod h1:sXRDRVmzEbkM7CVcM06s9shE/m23dg3wzjl0UWqJ2q4=
1717
github.com/golang/glog v1.2.2 h1:1+mZ9upx1Dh6FmUTFR1naJ77miKiXgALjWOZ3NVFPmY=
1818
github.com/golang/glog v1.2.2/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w=
19+
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
20+
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
1921
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
2022
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
2123
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=

internal/status/status.go

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ func (s *Status) WithDetails(details ...protoadapt.MessageV1) (*Status, error) {
149149

150150
// Details returns a slice of details messages attached to the status.
151151
// If a detail cannot be decoded, the error is returned in place of the detail.
152+
// If the detail can be decoded, the proto message returned is of the same
153+
// type that was given to WithDetails().
152154
func (s *Status) Details() []any {
153155
if s == nil || s.s == nil {
154156
return nil
@@ -160,7 +162,38 @@ func (s *Status) Details() []any {
160162
details = append(details, err)
161163
continue
162164
}
163-
details = append(details, detail)
165+
// The call to MessageV1Of is required to unwrap the proto message if
166+
// it implemented only the MessageV1 API. The proto message would have
167+
// been wrapped in a V2 wrapper in Status.WithDetails. V2 messages are
168+
// added to a global registry used by any.UnmarshalNew().
169+
// MessageV1Of has the following behaviour:
170+
// 1. If the given message is a wrapped MessageV1, it returns the
171+
// unwrapped value.
172+
// 2. If the given message already implements MessageV1, it returns it
173+
// as is.
174+
// 3. Else, it wraps the MessageV2 in a MessageV1 wrapper.
175+
//
176+
// Since the Status.WithDetails() API only accepts MessageV1, calling
177+
// MessageV1Of ensures we return the same type that was given to
178+
// WithDetails:
179+
// * If the give type implemented only MessageV1, the unwrapping from
180+
// point 1 above will restore the type.
181+
// * If the given type implemented both MessageV1 and MessageV2, point 2
182+
// above will ensure no wrapping is performed.
183+
// * If the given type implemented only MessageV2 and was wrapped using
184+
// MessageV1Of before passing to WithDetails(), it would be unwrapped
185+
// in WithDetails by calling MessageV2Of(). Point 3 above will ensure
186+
// that the type is wrapped in a MessageV1 wrapper again before
187+
// returning. Note that protoc-gen-go doesn't generate code which
188+
// implements ONLY MessageV2 at the time of writing.
189+
//
190+
// NOTE: Status details can also be added using the FromProto method.
191+
// This could theoretically allow passing a Detail message that only
192+
// implements the V2 API. In such a case the message will be wrapped in
193+
// a MessageV1 wrapper when fetched using Details().
194+
// Since protoc-gen-go generates only code that implements both V1 and
195+
// V2 APIs for backward compatibility, this is not a concern.
196+
details = append(details, protoadapt.MessageV1Of(detail))
164197
}
165198
return details
166199
}

interop/xds/go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
2323
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
2424
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
2525
github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
26+
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
27+
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
2628
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
2729
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
2830
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=

reflection/test/go.mod

Lines changed: 0 additions & 18 deletions
This file was deleted.

reflection/test/go.sum

Lines changed: 0 additions & 14 deletions
This file was deleted.

reflection/test/serverreflection_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import (
4343
v1reflectionpb "google.golang.org/grpc/reflection/grpc_reflection_v1"
4444
v1alphareflectiongrpc "google.golang.org/grpc/reflection/grpc_reflection_v1alpha"
4545
pb "google.golang.org/grpc/reflection/grpc_testing"
46-
pbv3 "google.golang.org/grpc/reflection/test/grpc_testing_not_regenerate"
46+
pbv3 "google.golang.org/grpc/testdata/grpc_testing_not_regenerated"
4747
)
4848

4949
var (

scripts/regenerate.sh

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ export PATH="${GOBIN}:${PATH}"
2626
mkdir -p "${GOBIN}"
2727

2828
echo "removing existing generated files..."
29-
# grpc_testing_not_regenerate/*.pb.go is not re-generated,
30-
# see grpc_testing_not_regenerate/README.md for details.
31-
find . -name '*.pb.go' | grep -v 'grpc_testing_not_regenerate' | xargs rm -f || true
29+
# grpc_testing_not_regenerated/*.pb.go is not re-generated,
30+
# see grpc_testing_not_regenerated/README.md for details.
31+
find . -name '*.pb.go' | grep -v 'grpc_testing_not_regenerated' | xargs rm -f || true
3232

3333
echo "Executing: go install google.golang.org/protobuf/cmd/protoc-gen-go..."
3434
(cd test/tools && go install google.golang.org/protobuf/cmd/protoc-gen-go)
@@ -124,8 +124,8 @@ done
124124
mkdir -p "${WORKDIR}/out/google.golang.org/grpc/internal/proto/grpc_lookup_v1"
125125
mv "${WORKDIR}"/out/google.golang.org/grpc/lookup/grpc_lookup_v1/* "${WORKDIR}/out/google.golang.org/grpc/internal/proto/grpc_lookup_v1"
126126

127-
# grpc_testing_not_regenerate/*.pb.go are not re-generated,
128-
# see grpc_testing_not_regenerate/README.md for details.
129-
rm "${WORKDIR}"/out/google.golang.org/grpc/reflection/test/grpc_testing_not_regenerate/*.pb.go
127+
# grpc_testing_not_regenerated/*.pb.go are not re-generated,
128+
# see grpc_testing_not_regenerated/README.md for details.
129+
rm "${WORKDIR}"/out/google.golang.org/grpc/testdata/grpc_testing_not_regenerated/*.pb.go
130130

131131
cp -R "${WORKDIR}"/out/google.golang.org/grpc/* .

scripts/vet.sh

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,13 @@ git grep 'func [A-Z]' -- "*_test.go" | not grep -v 'func Test\|Benchmark\|Exampl
5959
git grep -l 'time.After(' -- "*.go" | not grep -v '_test.go\|test_utils\|testutils'
6060

6161
# - Do not use "interface{}"; use "any" instead.
62-
git grep -l 'interface{}' -- "*.go" 2>&1 | not grep -v '\.pb\.go\|protoc-gen-go-grpc\|grpc_testing_not_regenerate'
62+
git grep -l 'interface{}' -- "*.go" 2>&1 | not grep -v '\.pb\.go\|protoc-gen-go-grpc\|grpc_testing_not_regenerated'
6363

6464
# - Do not call grpclog directly. Use grpclog.Component instead.
6565
git grep -l -e 'grpclog.I' --or -e 'grpclog.W' --or -e 'grpclog.E' --or -e 'grpclog.F' --or -e 'grpclog.V' -- "*.go" | not grep -v '^grpclog/component.go\|^internal/grpctest/tlogger_test.go\|^internal/grpclog/prefix_logger.go'
6666

6767
# - Ensure that the deprecated protobuf dependency is not used.
68-
not git grep "\"github.com/golang/protobuf/*" -- "*.go" ':(exclude)reflection/test/grpc_testing_not_regenerate/*'
68+
not git grep "\"github.com/golang/protobuf/*" -- "*.go" ':(exclude)testdata/grpc_testing_not_regenerated/*'
6969

7070
# - Ensure all usages of grpc_testing package are renamed when importing.
7171
not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go"
@@ -112,7 +112,7 @@ for MOD_FILE in $(find . -name 'go.mod'); do
112112
noret_grep -v "(ST1000)" "${SC_OUT}" | noret_grep -v "(SA1019)" | noret_grep -v "(ST1003)" | noret_grep -v "(ST1019)\|\(other import of\)" | not grep -v "(SA4000)"
113113

114114
# Exclude underscore checks for generated code.
115-
noret_grep "(ST1003)" "${SC_OUT}" | not grep -v '\(.pb.go:\)\|\(code_string_test.go:\)\|\(grpc_testing_not_regenerate\)'
115+
noret_grep "(ST1003)" "${SC_OUT}" | not grep -v '\(.pb.go:\)\|\(code_string_test.go:\)\|\(grpc_testing_not_regenerated\)'
116116

117117
# Error for duplicate imports not including grpc protos.
118118
noret_grep "(ST1019)\|\(other import of\)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused
@@ -149,7 +149,7 @@ XXXXX PleaseIgnoreUnused'
149149
XXXXX Protobuf related deprecation errors:
150150
"github.com/golang/protobuf
151151
.pb.go:
152-
grpc_testing_not_regenerate
152+
grpc_testing_not_regenerated
153153
: ptypes.
154154
proto.RegisterType
155155
XXXXX gRPC internal usage deprecation errors:
@@ -191,7 +191,7 @@ done
191191
# Error for violation of enabled lint rules in config excluding generated code.
192192
revive \
193193
-set_exit_status=1 \
194-
-exclude "reflection/test/grpc_testing_not_regenerate/" \
194+
-exclude "testdata/grpc_testing_not_regenerated/" \
195195
-exclude "**/*.pb.go" \
196196
-formatter plain \
197197
-config "$(dirname "$0")/revive.toml" \

security/advancedtls/examples/go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
2+
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
13
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
24
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
35
golang.org/x/crypto v0.28.0 h1:GBDwsMXVQi34v5CCYUm2jkJvu4cbtru2U4TN2PSyQnw=

0 commit comments

Comments
 (0)