Skip to content

Commit d8544fd

Browse files
kannanjgithubkannanjgithub
committed
Address review comments.
1 parent 16e4bde commit d8544fd

17 files changed

+125
-83
lines changed

‎xds/src/main/java/io/grpc/internal/InternalAttributes.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Copyright 2025 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.internal;
18+
19+
import io.grpc.Attributes;
20+
import io.grpc.EquivalentAddressGroup;
21+
22+
public final class InternalAttributes {
23+
/** Name associated with individual address, if available (e.g., DNS name). */
24+
@EquivalentAddressGroup.Attr
25+
public static final Attributes.Key<String> ATTR_ADDRESS_NAME =
26+
Attributes.Key.create("io.grpc.xds.XdsAttributes.addressName");
27+
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import io.grpc.Status;
3737
import io.grpc.internal.ForwardingClientStreamTracer;
3838
import io.grpc.internal.GrpcUtil;
39+
import io.grpc.internal.InternalAttributes;
3940
import io.grpc.internal.ObjectPool;
4041
import io.grpc.services.MetricReport;
4142
import io.grpc.util.ForwardingLoadBalancerHelper;
@@ -241,9 +242,9 @@ public Subchannel createSubchannel(CreateSubchannelArgs args) {
241242
.set(ATTR_CLUSTER_LOCALITY, localityAtomicReference);
242243
if (GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE", false)) {
243244
String hostname = args.getAddresses().get(0).getAttributes()
244-
.get(XdsAttributes.ATTR_ADDRESS_NAME);
245+
.get(InternalAttributes.ATTR_ADDRESS_NAME);
245246
if (hostname != null) {
246-
attrsBuilder.set(XdsAttributes.ATTR_ADDRESS_NAME, hostname);
247+
attrsBuilder.set(InternalAttributes.ATTR_ADDRESS_NAME, hostname);
247248
}
248249
}
249250
args = args.toBuilder().setAddresses(addresses).setAttributes(attrsBuilder.build()).build();
@@ -439,7 +440,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) {
439440
result = PickResult.withSubchannel(result.getSubchannel(),
440441
result.getStreamTracerFactory(),
441442
result.getSubchannel().getAttributes().get(
442-
XdsAttributes.ATTR_ADDRESS_NAME));
443+
InternalAttributes.ATTR_ADDRESS_NAME));
443444
}
444445
}
445446
return result;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import io.grpc.LoadBalancerRegistry;
3030
import io.grpc.Status;
3131
import io.grpc.StatusOr;
32+
import io.grpc.internal.InternalAttributes;
3233
import io.grpc.util.GracefulSwitchLoadBalancer;
3334
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig;
3435
import io.grpc.xds.ClusterImplLoadBalancerProvider.ClusterImplConfig;
@@ -194,7 +195,7 @@ StatusOr<ClusterResolutionResult> edsUpdateToResult(
194195
.set(XdsAttributes.ATTR_LOCALITY_WEIGHT,
195196
localityLbInfo.localityWeight())
196197
.set(XdsAttributes.ATTR_SERVER_WEIGHT, weight)
197-
.set(XdsAttributes.ATTR_ADDRESS_NAME, endpoint.hostname())
198+
.set(InternalAttributes.ATTR_ADDRESS_NAME, endpoint.hostname())
198199
.build();
199200
EquivalentAddressGroup eag;
200201
if (config.isHttp11ProxyAvailable()) {

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,5 @@ public final class InternalXdsAttributes {
3232
public static final Attributes.Key<String> ATTR_CLUSTER_NAME =
3333
XdsAttributes.ATTR_CLUSTER_NAME;
3434

35-
/** Name associated with individual address, if available (e.g., DNS name). */
36-
@EquivalentAddressGroup.Attr
37-
public static final Attributes.Key<String> ATTR_ADDRESS_NAME = XdsAttributes.ATTR_ADDRESS_NAME;
38-
3935
private InternalXdsAttributes() {}
4036
}

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,6 @@ final class XdsAttributes {
8888
static final Attributes.Key<Long> ATTR_SERVER_WEIGHT =
8989
Attributes.Key.create("io.grpc.xds.XdsAttributes.serverWeight");
9090

91-
/** Name associated with individual address, if available (e.g., DNS name). */
92-
@EquivalentAddressGroup.Attr
93-
static final Attributes.Key<String> ATTR_ADDRESS_NAME =
94-
Attributes.Key.create("io.grpc.xds.XdsAttributes.addressName");
95-
9691
/**
9792
* Filter chain match for network filters.
9893
*/

‎xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.grpc.Attributes;
2424
import io.grpc.Grpc;
2525
import io.grpc.internal.GrpcUtil;
26+
import io.grpc.internal.InternalAttributes;
2627
import io.grpc.internal.ObjectPool;
2728
import io.grpc.netty.GrpcHttp2ConnectionHandler;
2829
import io.grpc.netty.InternalProtocolNegotiationEvent;
@@ -32,7 +33,6 @@
3233
import io.grpc.netty.ProtocolNegotiationEvent;
3334
import io.grpc.xds.EnvoyServerProtoData;
3435
import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
35-
import io.grpc.xds.InternalXdsAttributes;
3636
import io.grpc.xds.internal.security.trust.CertificateUtils;
3737
import io.netty.channel.ChannelHandler;
3838
import io.netty.channel.ChannelHandlerAdapter;
@@ -150,7 +150,7 @@ public ChannelHandler newHandler(GrpcHttp2ConnectionHandler grpcHandler) {
150150
return fallbackProtocolNegotiator.newHandler(grpcHandler);
151151
}
152152
return new ClientSecurityHandler(grpcHandler, localSslContextProviderSupplier,
153-
grpcHandler.getEagAttributes().get(InternalXdsAttributes.ATTR_ADDRESS_NAME));
153+
grpcHandler.getEagAttributes().get(InternalAttributes.ATTR_ADDRESS_NAME));
154154
}
155155

156156
@Override

‎xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,6 @@ public void checkClientTrusted(X509Certificate[] chain, String authType)
257257
}
258258

259259
@Override
260-
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
261260
public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket)
262261
throws CertificateException {
263262
List<StringMatcher> sniMatchers = null;
@@ -271,14 +270,15 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket
271270
sniMatchers = getAutoSniSanMatchers(sslParams);
272271
}
273272
if (sniMatchers.isEmpty() && certContext != null) {
274-
sniMatchers = certContext.getMatchSubjectAltNamesList();
273+
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
274+
List<StringMatcher> sniMatchersTmp = certContext.getMatchSubjectAltNamesList();
275+
sniMatchers = sniMatchersTmp;
275276
}
276277
chooseDelegate(chain).checkServerTrusted(chain, authType, socket);
277278
verifySubjectAltNameInChain(chain, sniMatchers);
278279
}
279280

280281
@Override
281-
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
282282
public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine)
283283
throws CertificateException {
284284
List<StringMatcher> sniMatchers = null;
@@ -289,7 +289,9 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngi
289289
sniMatchers = getAutoSniSanMatchers(sslParams);
290290
}
291291
if (sniMatchers.isEmpty() && certContext != null) {
292-
sniMatchers = certContext.getMatchSubjectAltNamesList();
292+
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
293+
List<StringMatcher> sniMatchersTmp = certContext.getMatchSubjectAltNamesList();
294+
sniMatchers = sniMatchersTmp;
293295
}
294296
chooseDelegate(chain).checkServerTrusted(chain, authType, sslEngine);
295297
verifySubjectAltNameInChain(chain, sniMatchers);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public class CdsLoadBalancer2Test {
117117
private static final String EDS_SERVICE_NAME = "backend-service-1.googleapis.com";
118118
private static final String NODE_ID = "node-id";
119119
private final io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext upstreamTlsContext =
120-
CommonTlsContextTestsUtil.buildUpstreamTlsContext("cert-instance-name", true, "", false);
120+
CommonTlsContextTestsUtil.buildUpstreamTlsContext("cert-instance-name", true);
121121
private static final Cluster EDS_CLUSTER = Cluster.newBuilder()
122122
.setName(CLUSTER)
123123
.setType(Cluster.DiscoveryType.EDS)

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
import io.grpc.Status.Code;
5656
import io.grpc.SynchronizationContext;
5757
import io.grpc.internal.FakeClock;
58-
import io.grpc.internal.ObjectPool;
58+
import io.grpc.internal.InternalAttributes;import io.grpc.internal.ObjectPool;
5959
import io.grpc.internal.PickFirstLoadBalancerProvider;
6060
import io.grpc.internal.PickSubchannelArgsImpl;
6161
import io.grpc.protobuf.ProtoUtils;
@@ -811,10 +811,10 @@ public void endpointAddressesAttachedWithClusterName() {
811811
new FixedResultPicker(PickResult.withSubchannel(subchannel)));
812812
}
813813
});
814-
assertThat(subchannel.getAttributes().get(XdsAttributes.ATTR_ADDRESS_NAME)).isEqualTo(
814+
assertThat(subchannel.getAttributes().get(InternalAttributes.ATTR_ADDRESS_NAME)).isEqualTo(
815815
"authority-host-name");
816816
for (EquivalentAddressGroup eag : subchannel.getAllAddresses()) {
817-
assertThat(eag.getAttributes().get(XdsAttributes.ATTR_ADDRESS_NAME))
817+
assertThat(eag.getAttributes().get(InternalAttributes.ATTR_ADDRESS_NAME))
818818
.isEqualTo("authority-host-name");
819819
}
820820

@@ -863,9 +863,9 @@ public void endpointAddressesAttachedWithClusterName() {
863863
}
864864
});
865865
// Sub Channel wrapper args won't have the address name although addresses will.
866-
assertThat(subchannel.getAttributes().get(XdsAttributes.ATTR_ADDRESS_NAME)).isNull();
866+
assertThat(subchannel.getAttributes().get(InternalAttributes.ATTR_ADDRESS_NAME)).isNull();
867867
for (EquivalentAddressGroup eag : subchannel.getAllAddresses()) {
868-
assertThat(eag.getAttributes().get(XdsAttributes.ATTR_ADDRESS_NAME))
868+
assertThat(eag.getAttributes().get(InternalAttributes.ATTR_ADDRESS_NAME))
869869
.isEqualTo("authority-host-name");
870870
}
871871

@@ -882,7 +882,7 @@ public void endpointAddressesAttachedWithClusterName() {
882882
public void endpointAddressesAttachedWithTlsConfig_securityEnabledByDefault() {
883883
UpstreamTlsContext upstreamTlsContext =
884884
CommonTlsContextTestsUtil.buildUpstreamTlsContext(
885-
"google_cloud_private_spiffe", true, "", false);
885+
"google_cloud_private_spiffe", true);
886886
LoadBalancerProvider weightedTargetProvider = new WeightedTargetLoadBalancerProvider();
887887
WeightedTargetConfig weightedTargetConfig =
888888
buildWeightedTargetConfig(ImmutableMap.of(locality, 10));
@@ -927,7 +927,7 @@ public void endpointAddressesAttachedWithTlsConfig_securityEnabledByDefault() {
927927

928928
// Config with a new UpstreamTlsContext.
929929
upstreamTlsContext = CommonTlsContextTestsUtil.buildUpstreamTlsContext(
930-
"google_cloud_private_spiffe1", true, "", false);
930+
"google_cloud_private_spiffe1", true);
931931
config = new ClusterImplConfig(CLUSTER, EDS_SERVICE_NAME, LRS_SERVER_INFO,
932932
null, Collections.<DropOverload>emptyList(),
933933
GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(
@@ -1020,7 +1020,7 @@ public String toString() {
10201020
// Unique but arbitrary string
10211021
.set(EquivalentAddressGroup.ATTR_LOCALITY_NAME, locality.toString());
10221022
if (authorityHostname != null) {
1023-
attributes.set(XdsAttributes.ATTR_ADDRESS_NAME, authorityHostname);
1023+
attributes.set(InternalAttributes.ATTR_ADDRESS_NAME, authorityHostname);
10241024
}
10251025
EquivalentAddressGroup eag = new EquivalentAddressGroup(new FakeSocketAddress(name),
10261026
attributes.build());

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
import io.grpc.inprocess.InProcessServerBuilder;
8383
import io.grpc.internal.FakeClock;
8484
import io.grpc.internal.GrpcUtil;
85-
import io.grpc.testing.GrpcCleanupRule;
85+
import io.grpc.internal.InternalAttributes;import io.grpc.testing.GrpcCleanupRule;
8686
import io.grpc.util.GracefulSwitchLoadBalancer;
8787
import io.grpc.util.GracefulSwitchLoadBalancerAccessor;
8888
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig;
@@ -408,7 +408,7 @@ public void edsClustersEndpointHostname_addedToAddressAttribute() {
408408

409409
assertThat(
410410
childBalancer.addresses.get(0).getAttributes()
411-
.get(XdsAttributes.ATTR_ADDRESS_NAME)).isEqualTo("hostname1");
411+
.get(InternalAttributes.ATTR_ADDRESS_NAME)).isEqualTo("hostname1");
412412
}
413413

414414
@Test
@@ -897,7 +897,7 @@ public void onlyLogicalDnsCluster_endpointsResolved() {
897897
newInetSocketAddress("127.0.2.1", 9000), newInetSocketAddress("127.0.2.2", 9000)))),
898898
childBalancer.addresses);
899899
assertThat(childBalancer.addresses.get(0).getAttributes()
900-
.get(XdsAttributes.ATTR_ADDRESS_NAME)).isEqualTo(DNS_HOST_NAME + ":9000");
900+
.get(InternalAttributes.ATTR_ADDRESS_NAME)).isEqualTo(DNS_HOST_NAME + ":9000");
901901
}
902902

903903
@Test
@@ -996,7 +996,7 @@ public void config_equalsTester() {
996996
ServerInfo.create("lrs.googleapis.com", InsecureChannelCredentials.create());
997997
UpstreamTlsContext tlsContext =
998998
CommonTlsContextTestsUtil.buildUpstreamTlsContext(
999-
"google_cloud_private_spiffe", true, "", false);
999+
"google_cloud_private_spiffe", true);
10001000
DiscoveryMechanism edsDiscoveryMechanism1 =
10011001
DiscoveryMechanism.forEds(CLUSTER, EDS_SERVICE_NAME, lrsServerInfo, 100L, tlsContext,
10021002
Collections.emptyMap(), null);

0 commit comments

Comments
 (0)