Skip to content

Commit 73fc7fd

Browse files
xmenxkxmenxk
authored
fix(transport): not enable s2a when there is endpoint override (#2368)
1 parent 2d69d97 commit 73fc7fd

File tree

2 files changed

+22
-76
lines changed

2 files changed

+22
-76
lines changed

‎internal/cba.go

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,6 @@ func getTransportConfig(settings *DialSettings) (*transportConfig, error) {
111111
return nil, errUniverseNotSupportedMTLS
112112
}
113113

114-
s2aMTLSEndpoint := settings.DefaultMTLSEndpoint
115-
// If there is endpoint override, honor it.
116-
if settings.Endpoint != "" {
117-
s2aMTLSEndpoint = endpoint
118-
}
119114
s2aAddress := GetS2AAddress()
120115
if s2aAddress == "" {
121116
return &defaultTransportConfig, nil
@@ -124,7 +119,7 @@ func getTransportConfig(settings *DialSettings) (*transportConfig, error) {
124119
clientCertSource: clientCertSource,
125120
endpoint: endpoint,
126121
s2aAddress: s2aAddress,
127-
s2aMTLSEndpoint: s2aMTLSEndpoint,
122+
s2aMTLSEndpoint: settings.DefaultMTLSEndpoint,
128123
}, nil
129124
}
130125

@@ -293,12 +288,8 @@ func shouldUseS2A(clientCertSource cert.Source, settings *DialSettings) bool {
293288
if !isGoogleS2AEnabled() {
294289
return false
295290
}
296-
// If DefaultMTLSEndpoint is not set and no endpoint override, skip S2A.
297-
if settings.DefaultMTLSEndpoint == "" && settings.Endpoint == "" {
298-
return false
299-
}
300-
// If MTLS is not enabled for this endpoint, skip S2A.
301-
if !mtlsEndpointEnabledForS2A() {
291+
// If DefaultMTLSEndpoint is not set or has endpoint override, skip S2A.
292+
if settings.DefaultMTLSEndpoint == "" || settings.Endpoint != "" {
302293
return false
303294
}
304295
// If custom HTTP client is provided, skip S2A.
@@ -308,12 +299,6 @@ func shouldUseS2A(clientCertSource cert.Source, settings *DialSettings) bool {
308299
return true
309300
}
310301

311-
// mtlsEndpointEnabledForS2A checks if the endpoint is indeed MTLS-enabled, so that we can use S2A for MTLS connection.
312-
var mtlsEndpointEnabledForS2A = func() bool {
313-
// TODO(xmenxk): determine this via discovery config.
314-
return true
315-
}
316-
317302
func isGoogleS2AEnabled() bool {
318303
return strings.ToLower(os.Getenv(googleAPIUseS2AEnv)) == "true"
319304
}

‎internal/cba_test.go

Lines changed: 19 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -141,67 +141,60 @@ func TestGetGRPCTransportConfigAndEndpoint(t *testing.T) {
141141
Desc string
142142
InputSettings *DialSettings
143143
S2ARespFunc func() (string, error)
144-
MTLSEnabled func() bool
145144
WantEndpoint string
146145
}{
147146
{
148-
"no client cert, endpoint is MTLS enabled, S2A address not empty",
147+
"has client cert",
149148
&DialSettings{
150149
DefaultMTLSEndpoint: testMTLSEndpoint,
151150
DefaultEndpoint: testRegularEndpoint,
151+
ClientCertSource: dummyClientCertSource,
152152
},
153153
validConfigResp,
154-
func() bool { return true },
155154
testMTLSEndpoint,
156155
},
157156
{
158-
"has client cert",
157+
"no client cert, S2A address not empty",
159158
&DialSettings{
160159
DefaultMTLSEndpoint: testMTLSEndpoint,
161160
DefaultEndpoint: testRegularEndpoint,
162-
ClientCertSource: dummyClientCertSource,
163161
},
164162
validConfigResp,
165-
func() bool { return true },
166163
testMTLSEndpoint,
167164
},
168165
{
169-
"no client cert, endpoint is not MTLS enabled",
166+
"no client cert, S2A address empty",
170167
&DialSettings{
171168
DefaultMTLSEndpoint: testMTLSEndpoint,
172169
DefaultEndpoint: testRegularEndpoint,
173170
},
174-
validConfigResp,
175-
func() bool { return false },
171+
invalidConfigResp,
176172
testRegularEndpoint,
177173
},
178174
{
179-
"no client cert, endpoint is MTLS enabled, S2A address empty",
175+
"no client cert, S2A address not empty, override endpoint",
180176
&DialSettings{
181177
DefaultMTLSEndpoint: testMTLSEndpoint,
182178
DefaultEndpoint: testRegularEndpoint,
179+
Endpoint: testOverrideEndpoint,
183180
},
184-
invalidConfigResp,
185-
func() bool { return true },
186-
testRegularEndpoint,
181+
validConfigResp,
182+
testOverrideEndpoint,
187183
},
188184
{
189-
"no client cert, endpoint is MTLS enabled, S2A address not empty, override endpoint",
185+
"no client cert, S2A address not empty, DefaultMTLSEndpoint not set",
190186
&DialSettings{
191-
DefaultMTLSEndpoint: testMTLSEndpoint,
187+
DefaultMTLSEndpoint: "",
192188
DefaultEndpoint: testRegularEndpoint,
193-
Endpoint: testOverrideEndpoint,
194189
},
195190
validConfigResp,
196-
func() bool { return true },
197-
testOverrideEndpoint,
191+
testRegularEndpoint,
198192
},
199193
}
200194
defer setupTest()()
201195

202196
for _, tc := range testCases {
203197
httpGetMetadataMTLSConfig = tc.S2ARespFunc
204-
mtlsEndpointEnabledForS2A = tc.MTLSEnabled
205198
if tc.InputSettings.ClientCertSource != nil {
206199
os.Setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "true")
207200
} else {
@@ -221,21 +214,9 @@ func TestGetHTTPTransportConfigAndEndpoint_s2a(t *testing.T) {
221214
Desc string
222215
InputSettings *DialSettings
223216
S2ARespFunc func() (string, error)
224-
MTLSEnabled func() bool
225217
WantEndpoint string
226218
DialFuncNil bool
227219
}{
228-
{
229-
"no client cert, endpoint is MTLS enabled, S2A address not empty",
230-
&DialSettings{
231-
DefaultMTLSEndpoint: testMTLSEndpoint,
232-
DefaultEndpoint: testRegularEndpoint,
233-
},
234-
validConfigResp,
235-
func() bool { return true },
236-
testMTLSEndpoint,
237-
false,
238-
},
239220
{
240221
"has client cert",
241222
&DialSettings{
@@ -244,43 +225,39 @@ func TestGetHTTPTransportConfigAndEndpoint_s2a(t *testing.T) {
244225
ClientCertSource: dummyClientCertSource,
245226
},
246227
validConfigResp,
247-
func() bool { return true },
248228
testMTLSEndpoint,
249229
true,
250230
},
251231
{
252-
"no client cert, endpoint is not MTLS enabled",
232+
"no client cert, S2A address not empty",
253233
&DialSettings{
254234
DefaultMTLSEndpoint: testMTLSEndpoint,
255235
DefaultEndpoint: testRegularEndpoint,
256236
},
257237
validConfigResp,
258-
func() bool { return false },
259-
testRegularEndpoint,
260-
true,
238+
testMTLSEndpoint,
239+
false,
261240
},
262241
{
263-
"no client cert, endpoint is MTLS enabled, S2A address empty",
242+
"no client cert, S2A address empty",
264243
&DialSettings{
265244
DefaultMTLSEndpoint: testMTLSEndpoint,
266245
DefaultEndpoint: testRegularEndpoint,
267246
},
268247
invalidConfigResp,
269-
func() bool { return true },
270248
testRegularEndpoint,
271249
true,
272250
},
273251
{
274-
"no client cert, endpoint is MTLS enabled, S2A address not empty, override endpoint",
252+
"no client cert, S2A address not empty, override endpoint",
275253
&DialSettings{
276254
DefaultMTLSEndpoint: testMTLSEndpoint,
277255
DefaultEndpoint: testRegularEndpoint,
278256
Endpoint: testOverrideEndpoint,
279257
},
280258
validConfigResp,
281-
func() bool { return true },
282259
testOverrideEndpoint,
283-
false,
260+
true,
284261
},
285262
{
286263
"no client cert, S2A address not empty, but DefaultMTLSEndpoint is not set",
@@ -289,30 +266,17 @@ func TestGetHTTPTransportConfigAndEndpoint_s2a(t *testing.T) {
289266
DefaultEndpoint: testRegularEndpoint,
290267
},
291268
validConfigResp,
292-
func() bool { return true },
293269
testRegularEndpoint,
294270
true,
295271
},
296272
{
297-
"no client cert, S2A address not empty, override endpoint is set",
298-
&DialSettings{
299-
DefaultMTLSEndpoint: "",
300-
Endpoint: testOverrideEndpoint,
301-
},
302-
validConfigResp,
303-
func() bool { return true },
304-
testOverrideEndpoint,
305-
false,
306-
},
307-
{
308-
"no client cert, endpoint is MTLS enabled, S2A address not empty, custom HTTP client",
273+
"no client cert, S2A address not empty, custom HTTP client",
309274
&DialSettings{
310275
DefaultMTLSEndpoint: testMTLSEndpoint,
311276
DefaultEndpoint: testRegularEndpoint,
312277
HTTPClient: http.DefaultClient,
313278
},
314279
validConfigResp,
315-
func() bool { return true },
316280
testRegularEndpoint,
317281
true,
318282
},
@@ -322,7 +286,6 @@ func TestGetHTTPTransportConfigAndEndpoint_s2a(t *testing.T) {
322286

323287
for _, tc := range testCases {
324288
httpGetMetadataMTLSConfig = tc.S2ARespFunc
325-
mtlsEndpointEnabledForS2A = tc.MTLSEnabled
326289
if tc.InputSettings.ClientCertSource != nil {
327290
os.Setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "true")
328291
} else {
@@ -344,7 +307,6 @@ func TestGetHTTPTransportConfigAndEndpoint_s2a(t *testing.T) {
344307
}
345308

346309
func setupTest() func() {
347-
oldDefaultMTLSEnabled := mtlsEndpointEnabledForS2A
348310
oldHTTPGet := httpGetMetadataMTLSConfig
349311
oldExpiry := configExpiry
350312
oldUseS2A := os.Getenv(googleAPIUseS2AEnv)
@@ -355,7 +317,6 @@ func setupTest() func() {
355317

356318
return func() {
357319
httpGetMetadataMTLSConfig = oldHTTPGet
358-
mtlsEndpointEnabledForS2A = oldDefaultMTLSEnabled
359320
configExpiry = oldExpiry
360321
os.Setenv(googleAPIUseS2AEnv, oldUseS2A)
361322
os.Setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", oldUseClientCert)

0 commit comments

Comments
 (0)