Skip to content

Commit 5ee4f6b

Browse files
committed
client: don't reuse a pick's authority override on a later attempt
1 parent 89d4d61 commit 5ee4f6b

2 files changed

Lines changed: 170 additions & 3 deletions

File tree

stream.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,12 @@ func (a *csAttempt) getTransport() error {
618618

619619
func (a *csAttempt) newStream() error {
620620
cs := a.cs
621-
cs.callHdr.PreviousAttempts = cs.numRetries
621+
// The header is copied because the fields set below, notably the authority
622+
// override taken from the pick result, describe the endpoint picked for
623+
// this attempt only. Mutating the clientStream's header would carry them
624+
// into a later attempt.
625+
callHdr := *cs.callHdr
626+
callHdr.PreviousAttempts = cs.numRetries
622627

623628
// Merge metadata stored in PickResult, if any, with existing call metadata.
624629
// It is safe to overwrite the csAttempt's context here, since all state
@@ -642,11 +647,11 @@ func (a *csAttempt) newStream() error {
642647
// apply it, as specified in gRFC A81.
643648
if cs.callInfo.authority == "" {
644649
if authMD := a.pickResult.Metadata.Get(":authority"); len(authMD) > 0 {
645-
cs.callHdr.Authority = authMD[0]
650+
callHdr.Authority = authMD[0]
646651
}
647652
}
648653
}
649-
s, err := a.transport.NewStream(a.ctx, cs.callHdr, a.statsHandler)
654+
s, err := a.transport.NewStream(a.ctx, &callHdr, a.statsHandler)
650655
if err != nil {
651656
nse, ok := err.(*transport.NewStreamError)
652657
if !ok {
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/*
2+
*
3+
* Copyright 2026 gRPC authors.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package test
20+
21+
import (
22+
"context"
23+
"fmt"
24+
"sync"
25+
"sync/atomic"
26+
"testing"
27+
28+
"google.golang.org/grpc"
29+
"google.golang.org/grpc/balancer"
30+
"google.golang.org/grpc/codes"
31+
"google.golang.org/grpc/credentials/insecure"
32+
"google.golang.org/grpc/internal/balancer/stub"
33+
"google.golang.org/grpc/internal/stubserver"
34+
"google.golang.org/grpc/metadata"
35+
"google.golang.org/grpc/resolver"
36+
"google.golang.org/grpc/resolver/manual"
37+
"google.golang.org/grpc/status"
38+
39+
testgrpc "google.golang.org/grpc/interop/grpc_testing"
40+
testpb "google.golang.org/grpc/interop/grpc_testing"
41+
)
42+
43+
// authorityOverridePicker returns an authority override in the pick result
44+
// metadata for the first pick only, and no metadata for subsequent picks. This
45+
// mirrors an xDS cluster (gRFC A81) where only some of the endpoints carry a
46+
// hostname, so only some picks rewrite the authority.
47+
type authorityOverridePicker struct {
48+
sc balancer.SubConn
49+
authority string
50+
picks atomic.Int32
51+
}
52+
53+
func (p *authorityOverridePicker) Pick(balancer.PickInfo) (balancer.PickResult, error) {
54+
res := balancer.PickResult{SubConn: p.sc}
55+
if p.picks.Add(1) == 1 {
56+
res.Metadata = metadata.Pairs(":authority", p.authority)
57+
}
58+
return res, nil
59+
}
60+
61+
// TestAuthorityOverrideNotReusedAcrossAttempts verifies that an authority
62+
// override supplied by the LB picker applies only to the attempt it was picked
63+
// for. A retry attempt whose pick carries no override must fall back to the
64+
// channel's authority instead of reusing the previous attempt's override.
65+
func (s) TestAuthorityOverrideNotReusedAcrossAttempts(t *testing.T) {
66+
const (
67+
balancerName = "authority-override-retry-balancer"
68+
overrideAuthority = "picked-endpoint.example.com"
69+
wantAuthority = "test.server"
70+
)
71+
72+
bf := stub.BalancerFuncs{
73+
UpdateClientConnState: func(bd *stub.BalancerData, ccs balancer.ClientConnState) error {
74+
addrs := ccs.ResolverState.Addresses
75+
if len(addrs) == 0 {
76+
return nil
77+
}
78+
var sc balancer.SubConn
79+
sc, err := bd.ClientConn.NewSubConn(addrs[:1], balancer.NewSubConnOptions{
80+
StateListener: func(state balancer.SubConnState) {
81+
bd.ClientConn.UpdateState(balancer.State{
82+
ConnectivityState: state.ConnectivityState,
83+
Picker: &authorityOverridePicker{sc: sc, authority: overrideAuthority},
84+
})
85+
},
86+
})
87+
if err != nil {
88+
return err
89+
}
90+
sc.Connect()
91+
return nil
92+
},
93+
}
94+
stub.Register(balancerName, bf)
95+
96+
var mu sync.Mutex
97+
var authorities []string
98+
ss := &stubserver.StubServer{
99+
EmptyCallF: func(ctx context.Context, _ *testpb.Empty) (*testpb.Empty, error) {
100+
md, _ := metadata.FromIncomingContext(ctx)
101+
mu.Lock()
102+
authorities = append(authorities, md.Get(":authority")...)
103+
attempt := len(authorities)
104+
mu.Unlock()
105+
// Fail the first attempt with a retryable code so that the RPC is
106+
// retried, and let the second attempt succeed.
107+
if attempt == 1 {
108+
return nil, status.Error(codes.Unavailable, "forcing a retry")
109+
}
110+
return &testpb.Empty{}, nil
111+
},
112+
}
113+
if err := ss.StartServer(); err != nil {
114+
t.Fatalf("Failed to start server: %v", err)
115+
}
116+
defer ss.Stop()
117+
118+
r := manual.NewBuilderWithScheme("whatever")
119+
r.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: ss.Address}}})
120+
121+
sc := fmt.Sprintf(`{
122+
"loadBalancingConfig": [{%q: {}}],
123+
"methodConfig": [{
124+
"name": [{"service": "grpc.testing.TestService"}],
125+
"retryPolicy": {
126+
"maxAttempts": 2,
127+
"initialBackoff": "0.01s",
128+
"maxBackoff": "0.01s",
129+
"backoffMultiplier": 1.0,
130+
"retryableStatusCodes": ["UNAVAILABLE"]
131+
}
132+
}]
133+
}`, balancerName)
134+
135+
cc, err := grpc.NewClient(r.Scheme()+":///"+wantAuthority,
136+
grpc.WithTransportCredentials(insecure.NewCredentials()),
137+
grpc.WithResolvers(r),
138+
grpc.WithDefaultServiceConfig(sc),
139+
)
140+
if err != nil {
141+
t.Fatalf("grpc.NewClient() failed: %v", err)
142+
}
143+
defer cc.Close()
144+
145+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
146+
defer cancel()
147+
if _, err := testgrpc.NewTestServiceClient(cc).EmptyCall(ctx, &testpb.Empty{}); err != nil {
148+
t.Fatalf("EmptyCall() failed: %v", err)
149+
}
150+
151+
mu.Lock()
152+
defer mu.Unlock()
153+
if len(authorities) != 2 {
154+
t.Fatalf("Server saw %d attempts (%q), want 2", len(authorities), authorities)
155+
}
156+
if authorities[0] != overrideAuthority {
157+
t.Errorf("First attempt used authority %q, want %q", authorities[0], overrideAuthority)
158+
}
159+
if authorities[1] != wantAuthority {
160+
t.Errorf("Retry attempt used authority %q, want %q", authorities[1], wantAuthority)
161+
}
162+
}

0 commit comments

Comments
 (0)