Skip to content

Commit b28bd3c

Browse files
sauravzgsauravzg
committed
Fixup 12492: Refactor allowedGrpcService to be non optional and fix bug
Makes `allowedGrpcServices` to be a non-optional struct instead of an `Optional<Map<str,AllowedService>>` since it's essentially an immuatable hash map, making it preferable to use an empty instance instead of null. Change a small bug where we continued instead of return when parsing bootstrap credentials.
1 parent c1b95f1 commit b28bd3c

File tree

9 files changed

+107
-50
lines changed

9 files changed

+107
-50
lines changed

‎xds/src/main/java/io/grpc/xds/GrpcBootstrapperImpl.java‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@
2525
import io.grpc.xds.client.BootstrapperImpl;
2626
import io.grpc.xds.client.XdsInitializationException;
2727
import io.grpc.xds.client.XdsLogger;
28+
import io.grpc.xds.internal.grpcservice.AllowedGrpcService;
29+
import io.grpc.xds.internal.grpcservice.AllowedGrpcServices;
2830
import io.grpc.xds.internal.grpcservice.ChannelCredsConfig;
2931
import io.grpc.xds.internal.grpcservice.ConfiguredChannelCredentials;
30-
import io.grpc.xds.internal.grpcservice.GrpcServiceXdsContext;
3132
import java.io.IOException;
3233
import java.util.List;
3334
import java.util.Map;
@@ -163,7 +164,7 @@ private static ConfiguredChannelCredentials parseChannelCredentials(List<Map<Str
163164

164165
ChannelCredentials creds = provider.newChannelCredentials(config);
165166
if (creds == null) {
166-
continue;
167+
return null;
167168
}
168169
return ConfiguredChannelCredentials.create(creds, new JsonChannelCredsConfig(type, config));
169170
}
@@ -172,10 +173,14 @@ private static ConfiguredChannelCredentials parseChannelCredentials(List<Map<Str
172173
}
173174

174175
@Override
175-
protected Optional<Object> parseAllowedGrpcServices(
176+
protected Object parseAllowedGrpcServices(
176177
Map<String, ?> rawAllowedGrpcServices)
177178
throws XdsInitializationException {
178-
ImmutableMap.Builder<String, GrpcServiceXdsContext.AllowedGrpcService> builder =
179+
if (rawAllowedGrpcServices == null || rawAllowedGrpcServices.isEmpty()) {
180+
return AllowedGrpcServices.empty();
181+
}
182+
183+
ImmutableMap.Builder<String, AllowedGrpcService> builder =
179184
ImmutableMap.builder();
180185
for (String targetUri : rawAllowedGrpcServices.keySet()) {
181186
Map<String, ?> serviceConfig = JsonUtil.getObject(rawAllowedGrpcServices, targetUri);
@@ -193,13 +198,12 @@ protected Optional<Object> parseAllowedGrpcServices(
193198
parseCallCredentials(JsonUtil.checkObjectList(rawCallCredsList), targetUri);
194199
}
195200

196-
GrpcServiceXdsContext.AllowedGrpcService.Builder b = GrpcServiceXdsContext.AllowedGrpcService
197-
.builder().configuredChannelCredentials(configuredChannel);
201+
AllowedGrpcService.Builder b = AllowedGrpcService.builder()
202+
.configuredChannelCredentials(configuredChannel);
198203
callCredentials.ifPresent(b::callCredentials);
199204
builder.put(targetUri, b.build());
200205
}
201-
ImmutableMap<String, GrpcServiceXdsContext.AllowedGrpcService> parsed = builder.buildOrThrow();
202-
return parsed.isEmpty() ? Optional.empty() : Optional.of(parsed);
206+
return AllowedGrpcServices.create(builder.build());
203207
}
204208

205209
@SuppressWarnings("unused")

‎xds/src/main/java/io/grpc/xds/client/Bootstrapper.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
import com.google.common.collect.ImmutableMap;
2525
import io.grpc.Internal;
2626
import io.grpc.xds.client.EnvoyProtoData.Node;
27+
import io.grpc.xds.internal.grpcservice.AllowedGrpcServices;
2728
import java.util.List;
2829
import java.util.Map;
29-
import java.util.Optional;
3030
import javax.annotation.Nullable;
3131

3232
/**
@@ -210,13 +210,14 @@ public abstract static class BootstrapInfo {
210210
* Parsed allowed_grpc_services configuration.
211211
* Returns an opaque object containing the parsed configuration.
212212
*/
213-
public abstract Optional<Object> allowedGrpcServices();
213+
public abstract Object allowedGrpcServices();
214214

215215
@VisibleForTesting
216216
public static Builder builder() {
217217
return new AutoValue_Bootstrapper_BootstrapInfo.Builder()
218218
.clientDefaultListenerResourceNameTemplate("%s")
219-
.authorities(ImmutableMap.<String, AuthorityInfo>of());
219+
.authorities(ImmutableMap.<String, AuthorityInfo>of())
220+
.allowedGrpcServices(AllowedGrpcServices.empty());
220221
}
221222

222223
@AutoValue.Builder
@@ -238,7 +239,7 @@ public abstract Builder clientDefaultListenerResourceNameTemplate(
238239

239240
public abstract Builder authorities(Map<String, AuthorityInfo> authorities);
240241

241-
public abstract Builder allowedGrpcServices(Optional<Object> allowedGrpcServices);
242+
public abstract Builder allowedGrpcServices(Object allowedGrpcServices);
242243

243244
public abstract BootstrapInfo build();
244245
}

‎xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,17 +240,15 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map<String, ?> rawData)
240240
}
241241

242242
Map<String, ?> rawAllowedGrpcServices = JsonUtil.getObject(rawData, "allowed_grpc_services");
243-
if (rawAllowedGrpcServices != null) {
244-
builder.allowedGrpcServices(parseAllowedGrpcServices(rawAllowedGrpcServices));
245-
}
243+
builder.allowedGrpcServices(parseAllowedGrpcServices(rawAllowedGrpcServices));
246244

247245
return builder;
248246
}
249247

250-
protected java.util.Optional<Object> parseAllowedGrpcServices(
248+
protected Object parseAllowedGrpcServices(
251249
Map<String, ?> rawAllowedGrpcServices)
252250
throws XdsInitializationException {
253-
return java.util.Optional.empty();
251+
return io.grpc.xds.internal.grpcservice.AllowedGrpcServices.empty();
254252
}
255253

256254
private List<ServerInfo> parseServerInfos(List<?> rawServerConfigs, XdsLogger logger)

‎xds/src/main/java/io/grpc/xds/internal/grpcservice/AllowedGrpcService.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright 2026 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc.xds.internal.grpcservice;
18+
19+
import com.google.auto.value.AutoValue;
20+
import io.grpc.CallCredentials;
21+
import java.util.Optional;
22+
23+
/**
24+
* Represents an allowed gRPC service configuration with local credentials.
25+
*/
26+
@AutoValue
27+
public abstract class AllowedGrpcService {
28+
public abstract ConfiguredChannelCredentials configuredChannelCredentials();
29+
30+
public abstract Optional<CallCredentials> callCredentials();
31+
32+
public static Builder builder() {
33+
return new AutoValue_AllowedGrpcService.Builder();
34+
}
35+
36+
@AutoValue.Builder
37+
public abstract static class Builder {
38+
public abstract Builder configuredChannelCredentials(ConfiguredChannelCredentials credentials);
39+
40+
public abstract Builder callCredentials(CallCredentials callCredentials);
41+
42+
public abstract AllowedGrpcService build();
43+
}
44+
}

‎xds/src/main/java/io/grpc/xds/internal/grpcservice/AllowedGrpcServices.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2026 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc.xds.internal.grpcservice;
18+
19+
import com.google.auto.value.AutoValue;
20+
import com.google.common.collect.ImmutableMap;
21+
import java.util.Map;
22+
23+
/**
24+
* Wrapper for allowed gRPC services keyed by target URI.
25+
*/
26+
@AutoValue
27+
public abstract class AllowedGrpcServices {
28+
public abstract ImmutableMap<String, AllowedGrpcService> services();
29+
30+
public static AllowedGrpcServices create(Map<String, AllowedGrpcService> services) {
31+
return new AutoValue_AllowedGrpcServices(ImmutableMap.copyOf(services));
32+
}
33+
34+
public static AllowedGrpcServices empty() {
35+
return create(ImmutableMap.of());
36+
}
37+
}

‎xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfigParser.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public static GrpcServiceConfig.GoogleGrpcConfig parseGoogleGrpcConfig(
130130
}
131131

132132
if (!context.isTrustedControlPlane()) {
133-
Optional<GrpcServiceXdsContext.AllowedGrpcService> override =
133+
Optional<AllowedGrpcService> override =
134134
context.validAllowedGrpcService();
135135
if (!override.isPresent()) {
136136
throw new GrpcServiceParseException(

‎xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceXdsContext.java‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package io.grpc.xds.internal.grpcservice;
1818

1919
import com.google.auto.value.AutoValue;
20-
import io.grpc.CallCredentials;
2120
import io.grpc.Internal;
2221
import java.util.Optional;
2322

@@ -45,27 +44,4 @@ public static GrpcServiceXdsContext create(
4544
isTargetUriSchemeSupported);
4645
}
4746

48-
/**
49-
* Represents an allowed gRPC service configuration with local credentials.
50-
*/
51-
@AutoValue
52-
public abstract static class AllowedGrpcService {
53-
public abstract ConfiguredChannelCredentials configuredChannelCredentials();
54-
55-
public abstract Optional<CallCredentials> callCredentials();
56-
57-
public static Builder builder() {
58-
return new AutoValue_GrpcServiceXdsContext_AllowedGrpcService.Builder();
59-
}
60-
61-
@AutoValue.Builder
62-
public abstract static class Builder {
63-
public abstract Builder configuredChannelCredentials(
64-
ConfiguredChannelCredentials credentials);
65-
66-
public abstract Builder callCredentials(CallCredentials callCredentials);
67-
68-
public abstract AllowedGrpcService build();
69-
}
70-
}
7147
}

‎xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@
3737
import io.grpc.xds.client.EnvoyProtoData.Node;
3838
import io.grpc.xds.client.Locality;
3939
import io.grpc.xds.client.XdsInitializationException;
40-
import io.grpc.xds.internal.grpcservice.GrpcServiceXdsContext.AllowedGrpcService;
40+
import io.grpc.xds.internal.grpcservice.AllowedGrpcService;
41+
import io.grpc.xds.internal.grpcservice.AllowedGrpcServices;
4142
import java.io.IOException;
4243
import java.util.List;
4344
import java.util.Map;
@@ -117,13 +118,10 @@ public void parseBootstrap_allowedGrpcServices() throws XdsInitializationExcepti
117118

118119
bootstrapper.setFileReader(createFileReader(BOOTSTRAP_FILE_PATH, rawData));
119120
BootstrapInfo info = bootstrapper.bootstrap();
120-
@SuppressWarnings("unchecked")
121-
Map<String, AllowedGrpcService> allowed =
122-
(Map<String, AllowedGrpcService>) info.allowedGrpcServices().get();
123-
121+
AllowedGrpcServices allowed = (AllowedGrpcServices) info.allowedGrpcServices();
124122
assertThat(allowed).isNotNull();
125-
assertThat(allowed).containsKey("dns:///foo.com:443");
126-
AllowedGrpcService service = allowed.get("dns:///foo.com:443");
123+
assertThat(allowed.services()).containsKey("dns:///foo.com:443");
124+
AllowedGrpcService service = allowed.services().get("dns:///foo.com:443");
127125
assertThat(service.configuredChannelCredentials().channelCredentials())
128126
.isInstanceOf(InsecureChannelCredentials.class);
129127
assertThat(service.callCredentials().isPresent()).isFalse();

‎xds/src/test/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfigParserTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import io.envoyproxy.envoy.extensions.grpc_service.channel_credentials.local.v3.LocalCredentials;
3030
import io.envoyproxy.envoy.extensions.grpc_service.channel_credentials.xds.v3.XdsCredentials;
3131
import io.grpc.InsecureChannelCredentials;
32-
import io.grpc.xds.internal.grpcservice.GrpcServiceXdsContext.AllowedGrpcService;
3332
import java.nio.charset.StandardCharsets;
3433
import org.junit.Test;
3534
import org.junit.runner.RunWith;

0 commit comments

Comments
 (0)