Skip to content

Commit a79ba96

Browse files
arjan-balpurnesh42H
authored andcommitted
status: Fix status incompatibility introduced by grpc#6919 and move non-regeneratable proto code into /testdata (grpc#7724)
1 parent b79fb61 commit a79ba96

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.0
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.29.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
@@ -56,13 +56,13 @@ git grep 'func [A-Z]' -- "*_test.go" | not grep -v 'func Test\|Benchmark\|Exampl
5656
git grep -l 'time.After(' -- "*.go" | not grep -v '_test.go\|test_utils\|testutils'
5757

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

6161
# - Do not call grpclog directly. Use grpclog.Component instead.
6262
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'
6363

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

6767
# - Ensure all usages of grpc_testing package are renamed when importing.
6868
not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go"
@@ -109,7 +109,7 @@ for MOD_FILE in $(find . -name 'go.mod'); do
109109
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)"
110110

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

114114
# Error for duplicate imports not including grpc protos.
115115
noret_grep "(ST1019)\|\(other import of\)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused
@@ -146,7 +146,7 @@ XXXXX PleaseIgnoreUnused'
146146
XXXXX Protobuf related deprecation errors:
147147
"github.com/golang/protobuf
148148
.pb.go:
149-
grpc_testing_not_regenerate
149+
grpc_testing_not_regenerated
150150
: ptypes.
151151
proto.RegisterType
152152
XXXXX gRPC internal usage deprecation errors:
@@ -188,7 +188,7 @@ done
188188
# Error for violation of enabled lint rules in config excluding generated code.
189189
revive \
190190
-set_exit_status=1 \
191-
-exclude "reflection/test/grpc_testing_not_regenerate/" \
191+
-exclude "testdata/grpc_testing_not_regenerated/" \
192192
-exclude "**/*.pb.go" \
193193
-formatter plain \
194194
-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.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A=

0 commit comments

Comments
 (0)