Skip to content

Commit af6d7bd

Browse files
mutianfmutianf
authored
fix: use service name as the default audience (#2579)
* fix: use service name as the default audience * fix * move code around * fix tests * format * add back public api * debug info * more debug * update * clean up debug logs * use settings * update * rename variables * update test:
1 parent 82d881f commit af6d7bd

File tree

7 files changed

+62
-25
lines changed

7 files changed

+62
-25
lines changed

‎google-cloud-bigtable/pom.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@
3434
<!-- Use client defined default endpoints -->
3535
<!-- Can be overriden on the commandline via `-Dbigtable.cfe-data-endpoint=bigtableadmin.googleapis.com:443` -->
3636
<bigtable.cfe-data-endpoint/>
37+
<bigtable.cfe-jwt-audience/>
3738
<bigtable.cfe-admin-endpoint/>
3839
<bigtable.directpath-data-endpoint/>
40+
<bigtable.directpath-jwt-audience/>
3941
<bigtable.directpath-admin-endpoint/>
4042

4143
<!-- This is used by bigtable-prod-batch-it profile to ensure that tests work on the batch endpoint.
@@ -425,6 +427,7 @@
425427
<bigtable.env>cloud</bigtable.env>
426428
<bigtable.data-endpoint>${bigtable.cfe-data-endpoint}</bigtable.data-endpoint>
427429
<bigtable.admin-endpoint>${bigtable.cfe-admin-endpoint}</bigtable.admin-endpoint>
430+
<bigtable.data-jwt-audience>${bigtable.cfe-jwt-audience}</bigtable.data-jwt-audience>
428431
<bigtable.enable-grpc-logs>${bigtable.enable-grpc-logs}</bigtable.enable-grpc-logs>
429432
<bigtable.grpc-log-dir>${project.build.directory}/test-grpc-logs/prod-it</bigtable.grpc-log-dir>
430433
</systemPropertyVariables>
@@ -475,6 +478,7 @@
475478
<bigtable.env>cloud</bigtable.env>
476479
<bigtable.data-endpoint>${bigtable.cfe-data-batch-endpoint}</bigtable.data-endpoint>
477480
<bigtable.admin-endpoint>${bigtable.cfe-admin-endpoint}</bigtable.admin-endpoint>
481+
<bigtable.data-jwt-audience>${bigtable.cfe-jwt-audience}</bigtable.data-jwt-audience>
478482
<bigtable.enable-grpc-logs>${bigtable.enable-grpc-logs}</bigtable.enable-grpc-logs>
479483
<bigtable.grpc-log-dir>${project.build.directory}/test-grpc-logs/prod-batch-it</bigtable.grpc-log-dir>
480484
</systemPropertyVariables>
@@ -510,6 +514,7 @@
510514
<bigtable.env>cloud</bigtable.env>
511515
<bigtable.data-endpoint>${bigtable.directpath-data-endpoint}</bigtable.data-endpoint>
512516
<bigtable.admin-endpoint>${bigtable.directpath-admin-endpoint}</bigtable.admin-endpoint>
517+
<bigtable.data-jwt-audience>${bigtable.directpath-jwt-audience}</bigtable.data-jwt-audience>
513518
<bigtable.enable-grpc-logs>${bigtable.enable-grpc-logs}</bigtable.enable-grpc-logs>
514519
<bigtable.grpc-log-dir>${project.build.directory}/test-grpc-logs/directpath-it</bigtable.grpc-log-dir>
515520
<bigtable.connection-mode>REQUIRE_DIRECT_PATH</bigtable.connection-mode>
@@ -551,6 +556,7 @@
551556
<bigtable.env>cloud</bigtable.env>
552557
<bigtable.data-endpoint>${bigtable.cfe-data-endpoint}</bigtable.data-endpoint>
553558
<bigtable.admin-endpoint>${bigtable.cfe-admin-endpoint}</bigtable.admin-endpoint>
559+
<bigtable.data-jwt-audience>${bigtable.cfe-jwt-audience}</bigtable.data-jwt-audience>
554560
<bigtable.enable-grpc-logs>${bigtable.enable-grpc-logs}</bigtable.enable-grpc-logs>
555561
<bigtable.grpc-log-dir>${project.build.directory}/test-grpc-logs/cfe-it</bigtable.grpc-log-dir>
556562
<bigtable.connection-mode>REQUIRE_CFE</bigtable.connection-mode>
@@ -589,6 +595,7 @@
589595
<bigtable.env>cloud</bigtable.env>
590596
<bigtable.data-endpoint>${bigtable.directpath-data-endpoint}</bigtable.data-endpoint>
591597
<bigtable.admin-endpoint>${bigtable.directpath-admin-endpoint}</bigtable.admin-endpoint>
598+
<bigtable.data-jwt-audience>${bigtable.directpath-jwt-audience}</bigtable.data-jwt-audience>
592599
<bigtable.enable-grpc-logs>${bigtable.enable-grpc-logs}</bigtable.enable-grpc-logs>
593600
<bigtable.grpc-log-dir>${project.build.directory}/test-grpc-logs/directpath-ipv4only-it</bigtable.grpc-log-dir>
594601
<bigtable.connection-mode>REQUIRE_DIRECT_PATH_IPV4</bigtable.connection-mode>

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -228,18 +228,13 @@ private static OpenTelemetry getOpenTelemetryFromMetricsProvider(
228228

229229
private static void patchCredentials(EnhancedBigtableStubSettings.Builder settings)
230230
throws IOException {
231-
int i = settings.getEndpoint().lastIndexOf(":");
232-
String host = settings.getEndpoint().substring(0, i);
233-
String audience = settings.getJwtAudienceMapping().get(host);
231+
String audience = settings.getJwtAudience();
234232

235-
if (audience == null) {
236-
return;
237-
}
238233
URI audienceUri = null;
239234
try {
240235
audienceUri = new URI(audience);
241236
} catch (URISyntaxException e) {
242-
throw new IllegalStateException("invalid JWT audience override", e);
237+
throw new IllegalStateException("invalid JWT audience", e);
243238
}
244239

245240
CredentialsProvider credentialsProvider = settings.getCredentialsProvider();

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -250,18 +250,16 @@ public class EnhancedBigtableStubSettings extends StubSettings<EnhancedBigtableS
250250
.build();
251251

252252
/**
253-
* In most cases, jwt audience == service name. However in some cases, this is not the case. The
254-
* following mapping is used to patch the audience in a JWT token.
253+
* Default jwt audience is always the service name unless it's override to test / staging for
254+
* testing.
255255
*/
256-
private static final Map<String, String> DEFAULT_JWT_AUDIENCE_MAPPING =
257-
ImmutableMap.of("batch-bigtable.googleapis.com", "https://bigtable.googleapis.com/");
256+
private static final String DEFAULT_DATA_JWT_AUDIENCE = "https://bigtable.googleapis.com/";
258257

259258
private final String projectId;
260259
private final String instanceId;
261260
private final String appProfileId;
262261
private final boolean isRefreshingChannel;
263262
private ImmutableList<String> primedTableIds;
264-
private final Map<String, String> jwtAudienceMapping;
265263
private final boolean enableRoutingCookie;
266264
private final boolean enableRetryInfo;
267265
private final boolean enableSkipTrailers;
@@ -287,6 +285,7 @@ public class EnhancedBigtableStubSettings extends StubSettings<EnhancedBigtableS
287285
private final MetricsProvider metricsProvider;
288286
@Nullable private final String metricsEndpoint;
289287
@Nonnull private final InternalMetricsProvider internalMetricsProvider;
288+
private final String jwtAudience;
290289

291290
private EnhancedBigtableStubSettings(Builder builder) {
292291
super(builder);
@@ -311,13 +310,13 @@ private EnhancedBigtableStubSettings(Builder builder) {
311310
appProfileId = builder.appProfileId;
312311
isRefreshingChannel = builder.isRefreshingChannel;
313312
primedTableIds = builder.primedTableIds;
314-
jwtAudienceMapping = builder.jwtAudienceMapping;
315313
enableRoutingCookie = builder.enableRoutingCookie;
316314
enableRetryInfo = builder.enableRetryInfo;
317315
enableSkipTrailers = builder.enableSkipTrailers;
318316
metricsProvider = builder.metricsProvider;
319317
metricsEndpoint = builder.metricsEndpoint;
320318
internalMetricsProvider = builder.internalMetricsProvider;
319+
jwtAudience = builder.jwtAudience;
321320

322321
// Per method settings.
323322
readRowsSettings = builder.readRowsSettings.build();
@@ -376,9 +375,14 @@ public List<String> getPrimedTableIds() {
376375
return primedTableIds;
377376
}
378377

378+
/**
379+
* @deprecated This is a no op and will always return an empty map. Audience is always set to
380+
* bigtable service name.
381+
*/
379382
@InternalApi("Used for internal testing")
383+
@Deprecated
380384
public Map<String, String> getJwtAudienceMapping() {
381-
return jwtAudienceMapping;
385+
return ImmutableMap.of();
382386
}
383387

384388
public MetricsProvider getMetricsProvider() {
@@ -748,7 +752,7 @@ public static class Builder extends StubSettings.Builder<EnhancedBigtableStubSet
748752
private String appProfileId;
749753
private boolean isRefreshingChannel;
750754
private ImmutableList<String> primedTableIds;
751-
private Map<String, String> jwtAudienceMapping;
755+
private String jwtAudience;
752756
private boolean enableRoutingCookie;
753757
private boolean enableRetryInfo;
754758
private boolean enableSkipTrailers;
@@ -789,13 +793,13 @@ private Builder() {
789793
this.appProfileId = SERVER_DEFAULT_APP_PROFILE_ID;
790794
this.isRefreshingChannel = true;
791795
primedTableIds = ImmutableList.of();
792-
jwtAudienceMapping = DEFAULT_JWT_AUDIENCE_MAPPING;
793796
setCredentialsProvider(defaultCredentialsProviderBuilder().build());
794797
this.enableRoutingCookie = true;
795798
this.enableRetryInfo = true;
796799
this.enableSkipTrailers = SKIP_TRAILERS;
797800
metricsProvider = DefaultMetricsProvider.INSTANCE;
798801
this.internalMetricsProvider = DEFAULT_INTERNAL_OTEL_PROVIDER;
802+
this.jwtAudience = DEFAULT_DATA_JWT_AUDIENCE;
799803

800804
// Defaults provider
801805
BigtableStubSettings.Builder baseDefaults = BigtableStubSettings.newBuilder();
@@ -928,12 +932,12 @@ private Builder(EnhancedBigtableStubSettings settings) {
928932
appProfileId = settings.appProfileId;
929933
isRefreshingChannel = settings.isRefreshingChannel;
930934
primedTableIds = settings.primedTableIds;
931-
jwtAudienceMapping = settings.jwtAudienceMapping;
932935
enableRoutingCookie = settings.enableRoutingCookie;
933936
enableRetryInfo = settings.enableRetryInfo;
934937
metricsProvider = settings.metricsProvider;
935938
metricsEndpoint = settings.getMetricsEndpoint();
936939
internalMetricsProvider = settings.internalMetricsProvider;
940+
jwtAudience = settings.jwtAudience;
937941

938942
// Per method settings.
939943
readRowsSettings = settings.readRowsSettings.toBuilder();
@@ -1075,9 +1079,20 @@ public List<String> getPrimedTableIds() {
10751079
return primedTableIds;
10761080
}
10771081

1082+
/**
1083+
* @deprecated This is a no op. Audience is always set to bigtable service name.
1084+
* @see #setJwtAudience(String) to override the audience.
1085+
*/
10781086
@InternalApi("Used for internal testing")
1087+
@Deprecated
10791088
public Builder setJwtAudienceMapping(Map<String, String> jwtAudienceMapping) {
1080-
this.jwtAudienceMapping = Preconditions.checkNotNull(jwtAudienceMapping);
1089+
return this;
1090+
}
1091+
1092+
/** Set the jwt audience override. */
1093+
@InternalApi("Used for internal testing")
1094+
public Builder setJwtAudience(String audience) {
1095+
this.jwtAudience = audience;
10811096
return this;
10821097
}
10831098

@@ -1140,9 +1155,20 @@ public boolean areInternalMetricsEnabled() {
11401155
return internalMetricsProvider == DISABLED_INTERNAL_OTEL_PROVIDER;
11411156
}
11421157

1158+
/**
1159+
* @deprecated This is a no op and will always return an empty map. Audience is always set to
1160+
* bigtable service name.
1161+
* @see #getJwtAudience() to get the audience.
1162+
*/
11431163
@InternalApi("Used for internal testing")
1164+
@Deprecated
11441165
public Map<String, String> getJwtAudienceMapping() {
1145-
return jwtAudienceMapping;
1166+
return ImmutableMap.of();
1167+
}
1168+
1169+
/** Return the jwt audience override. */
1170+
String getJwtAudience() {
1171+
return this.jwtAudience;
11461172
}
11471173

11481174
/**
@@ -1318,7 +1344,6 @@ public String toString() {
13181344
.add("appProfileId", appProfileId)
13191345
.add("isRefreshingChannel", isRefreshingChannel)
13201346
.add("primedTableIds", primedTableIds)
1321-
.add("jwtAudienceMapping", jwtAudienceMapping)
13221347
.add("enableRoutingCookie", enableRoutingCookie)
13231348
.add("enableRetryInfo", enableRetryInfo)
13241349
.add("enableSkipTrailers", enableSkipTrailers)
@@ -1340,6 +1365,7 @@ public String toString() {
13401365
.add("metricsProvider", metricsProvider)
13411366
.add("metricsEndpoint", metricsEndpoint)
13421367
.add("areInternalMetricsEnabled", internalMetricsProvider == DEFAULT_INTERNAL_OTEL_PROVIDER)
1368+
.add("jwtAudience", jwtAudience)
13431369
.add("parent", super.toString())
13441370
.toString();
13451371
}

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,9 @@ public void testNewClientsShareTransportChannel() throws Exception {
182182

183183
// Make sure that only 1 instance is created by each provider
184184
Mockito.verify(transportChannelProvider, Mockito.times(1)).getTransportChannel();
185-
Mockito.verify(credentialsProvider, Mockito.times(1)).getCredentials();
185+
// getCredentials was called twice, in patchCredentials and when creating the fixed
186+
// credentials in BigtableClientContext
187+
Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials();
186188
Mockito.verify(executorProvider, Mockito.times(1)).getExecutor();
187189
Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog();
188190
}
@@ -270,7 +272,9 @@ public void testCreateWithRefreshingChannel() throws Exception {
270272
factory.createForInstance("other-project", "other-instance");
271273

272274
// Make sure that only 1 instance is created by each provider
273-
Mockito.verify(credentialsProvider, Mockito.times(1)).getCredentials();
275+
// getCredentials was called twice, in patchCredentials and when creating the fixed credentials
276+
// in BigtableClientContext
277+
Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials();
274278
Mockito.verify(executorProvider, Mockito.times(1)).getExecutor();
275279
Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog();
276280

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,6 @@ public void enableRetryInfoFalseValueTest() throws IOException {
993993
"appProfileId",
994994
"isRefreshingChannel",
995995
"primedTableIds",
996-
"jwtAudienceMapping",
997996
"enableRoutingCookie",
998997
"enableRetryInfo",
999998
"enableSkipTrailers",
@@ -1013,6 +1012,7 @@ public void enableRetryInfoFalseValueTest() throws IOException {
10131012
"metricsProvider",
10141013
"metricsEndpoint",
10151014
"areInternalMetricsEnabled",
1015+
"jwtAudience",
10161016
};
10171017

10181018
@Test

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@
9595
import com.google.cloud.bigtable.data.v2.stub.sql.SqlProtoFactory;
9696
import com.google.cloud.bigtable.data.v2.stub.sql.SqlServerStream;
9797
import com.google.common.base.Preconditions;
98-
import com.google.common.collect.ImmutableMap;
9998
import com.google.common.collect.Queues;
10099
import com.google.common.io.BaseEncoding;
101100
import com.google.protobuf.ByteString;
@@ -224,8 +223,8 @@ public void testJwtAudience()
224223
String expectedAudience = "http://localaudience";
225224
EnhancedBigtableStubSettings settings =
226225
defaultSettings.toBuilder()
227-
.setJwtAudienceMapping(ImmutableMap.of("localhost", expectedAudience))
228226
.setCredentialsProvider(FixedCredentialsProvider.create(jwtCreds))
227+
.setJwtAudience(expectedAudience)
229228
.build();
230229
try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings)) {
231230
stub.readRowCallable().futureCall(Query.create("fake-table")).get();

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/CloudEnv.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ public boolean apply(InetSocketAddress input) {
7979

8080
private static final String DATA_ENDPOINT_PROPERTY_NAME = "bigtable.data-endpoint";
8181
private static final String ADMIN_ENDPOINT_PROPERTY_NAME = "bigtable.admin-endpoint";
82+
private static final String DATA_JWT_OVERRIDE_PROPERTY_NAME = "bigtable.data-jwt-audience";
8283

8384
private static final String PROJECT_PROPERTY_NAME = "bigtable.project";
8485
private static final String INSTANCE_PROPERTY_NAME = "bigtable.instance";
@@ -106,6 +107,7 @@ static CloudEnv fromSystemProperties() {
106107
return new CloudEnv(
107108
getOptionalProperty(DATA_ENDPOINT_PROPERTY_NAME, ""),
108109
getOptionalProperty(ADMIN_ENDPOINT_PROPERTY_NAME, ""),
110+
getOptionalProperty(DATA_JWT_OVERRIDE_PROPERTY_NAME, ""),
109111
getOptionalProperty(CMEK_KMS_KEY_PROPERTY_NAME, ""),
110112
getRequiredProperty(PROJECT_PROPERTY_NAME),
111113
getRequiredProperty(INSTANCE_PROPERTY_NAME),
@@ -117,6 +119,7 @@ static CloudEnv fromSystemProperties() {
117119
private CloudEnv(
118120
@Nullable String dataEndpoint,
119121
@Nullable String adminEndpoint,
122+
@Nullable String jwtAudienceOverride,
120123
@Nullable String kmsKeyName,
121124
String projectId,
122125
String instanceId,
@@ -137,6 +140,9 @@ private CloudEnv(
137140
if (!Strings.isNullOrEmpty(appProfileId)) {
138141
dataSettings.setAppProfileId(appProfileId);
139142
}
143+
if (!Strings.isNullOrEmpty(jwtAudienceOverride)) {
144+
dataSettings.stubSettings().setJwtAudience(jwtAudienceOverride);
145+
}
140146

141147
configureConnection(dataSettings.stubSettings());
142148
configureUserAgent(dataSettings.stubSettings());

0 commit comments

Comments
 (0)