From 52c06c0a0a15e4fc7ffeaa8cc0ae655eb7babd8f Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 10 Sep 2025 12:47:37 +0000 Subject: [PATCH 01/10] New aggregate type for `ChannelCredentials` --- .../ResourceAllocatingChannelCredentials.java | 55 +++++++++++++++++++ ...ourceAllocatingChannelCredentialsTest.java | 43 +++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 api/src/main/java/io/grpc/ResourceAllocatingChannelCredentials.java create mode 100644 api/src/test/java/io/grpc/ResourceAllocatingChannelCredentialsTest.java diff --git a/api/src/main/java/io/grpc/ResourceAllocatingChannelCredentials.java b/api/src/main/java/io/grpc/ResourceAllocatingChannelCredentials.java new file mode 100644 index 00000000000..c0443feaac4 --- /dev/null +++ b/api/src/main/java/io/grpc/ResourceAllocatingChannelCredentials.java @@ -0,0 +1,55 @@ +/* + * Copyright 2025 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import java.io.Closeable; + +/** + * {@code ChannelCredentials} which holds allocated resources (e.g. file watchers) upon + * instantiation of a given {@code ChannelCredentials} object, which must be closed once + * mentioned {@code ChannelCredentials} are no longer in use. + */ +public final class ResourceAllocatingChannelCredentials extends ChannelCredentials { + public static ChannelCredentials create( + ChannelCredentials channelCreds, ImmutableList resources) { + return new ResourceAllocatingChannelCredentials(channelCreds, resources); + } + + private final ChannelCredentials channelCreds; + private final ImmutableList resources; + + private ResourceAllocatingChannelCredentials( + ChannelCredentials channelCreds, ImmutableList resources) { + this.channelCreds = Preconditions.checkNotNull(channelCreds, "channelCreds"); + this.resources = Preconditions.checkNotNull(resources, "resources"); + } + + public ChannelCredentials getChannelCredentials() { + return channelCreds; + } + + public ImmutableList getAllocatedResources() { + return resources; + } + + @Override + public ChannelCredentials withoutBearerTokens() { + return channelCreds.withoutBearerTokens(); + } +} diff --git a/api/src/test/java/io/grpc/ResourceAllocatingChannelCredentialsTest.java b/api/src/test/java/io/grpc/ResourceAllocatingChannelCredentialsTest.java new file mode 100644 index 00000000000..2647c3ee0a0 --- /dev/null +++ b/api/src/test/java/io/grpc/ResourceAllocatingChannelCredentialsTest.java @@ -0,0 +1,43 @@ +/* + * Copyright 2025 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import java.io.Closeable; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link ResourceAllocatingChannelCredentials}. */ +@RunWith(JUnit4.class) +public class ResourceAllocatingChannelCredentialsTest { + @Test + public void withoutBearerTokenDelegatesCall() { + ChannelCredentials channelChreds = new ChannelCredentials() { + @Override + public ChannelCredentials withoutBearerTokens() { + return this; + } + }; + ImmutableList resources = ImmutableList.of(); + ChannelCredentials creds = + ResourceAllocatingChannelCredentials.create(channelChreds, resources); + assertThat(creds.withoutBearerTokens()).isEqualTo(channelChreds); + } +} From 82021c24277f3c235e5794f18de9d3b0d9aae11e Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 10 Sep 2025 12:53:08 +0000 Subject: [PATCH 02/10] Postponed `ChannelCredentials` instantiation. Lifetime of `ChannelCredentials` is now bounded to `GrpcXdsTransport` --- .../io/grpc/xds/GrpcBootstrapperImpl.java | 34 +++++++--------- .../io/grpc/xds/GrpcXdsTransportFactory.java | 39 +++++++++++++++++-- .../io/grpc/xds/XdsCredentialsRegistry.java | 9 +++++ .../java/io/grpc/xds/client/Bootstrapper.java | 7 ++-- .../io/grpc/xds/client/BootstrapperImpl.java | 6 +-- .../grpc/xds/client/XdsTransportFactory.java | 2 +- .../grpc/xds/ClusterImplLoadBalancerTest.java | 3 +- .../xds/ClusterResolverLoadBalancerTest.java | 3 +- .../java/io/grpc/xds/CsdsServiceTest.java | 3 +- .../io/grpc/xds/GrpcBootstrapperImplTest.java | 22 +++++------ .../grpc/xds/GrpcXdsClientImplDataTest.java | 5 +-- .../grpc/xds/GrpcXdsClientImplTestBase.java | 28 +++++++------ .../grpc/xds/GrpcXdsTransportFactoryTest.java | 4 +- .../xds/SharedXdsClientPoolProviderTest.java | 12 +++--- .../io/grpc/xds/XdsClientFallbackTest.java | 12 +++++- .../java/io/grpc/xds/XdsNameResolverTest.java | 15 ++++--- .../java/io/grpc/xds/XdsServerTestHelper.java | 3 +- .../io/grpc/xds/XdsServerWrapperTest.java | 7 ++-- .../client/CommonBootstrapperTestUtils.java | 18 +++++---- 19 files changed, 134 insertions(+), 98 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GrpcBootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/GrpcBootstrapperImpl.java index f61fab42cae..05e4f49f1d2 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcBootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/GrpcBootstrapperImpl.java @@ -18,7 +18,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; -import io.grpc.ChannelCredentials; +import com.google.common.collect.ImmutableSet; import io.grpc.internal.JsonUtil; import io.grpc.xds.client.BootstrapperImpl; import io.grpc.xds.client.XdsInitializationException; @@ -90,31 +90,32 @@ protected String getJsonContent() throws XdsInitializationException, IOException } @Override - protected Object getImplSpecificConfig(Map serverConfig, String serverUri) + protected ImmutableMap getImplSpecificConfig(Map serverConfig, + String serverUri) throws XdsInitializationException { - return getChannelCredentials(serverConfig, serverUri); + return getChannelCredentialsConfig(serverConfig, serverUri); } - private static ChannelCredentials getChannelCredentials(Map serverConfig, - String serverUri) + private static ImmutableMap getChannelCredentialsConfig(Map serverConfig, + String serverUri) throws XdsInitializationException { List rawChannelCredsList = JsonUtil.getList(serverConfig, "channel_creds"); if (rawChannelCredsList == null || rawChannelCredsList.isEmpty()) { throw new XdsInitializationException( "Invalid bootstrap: server " + serverUri + " 'channel_creds' required"); } - ChannelCredentials channelCredentials = + ImmutableMap channelCredentialsConfig = parseChannelCredentials(JsonUtil.checkObjectList(rawChannelCredsList), serverUri); - if (channelCredentials == null) { + if (channelCredentialsConfig == null) { throw new XdsInitializationException( "Server " + serverUri + ": no supported channel credentials found"); } - return channelCredentials; + return channelCredentialsConfig; } @Nullable - private static ChannelCredentials parseChannelCredentials(List> jsonList, - String serverUri) + private static ImmutableMap parseChannelCredentials(List> jsonList, + String serverUri) throws XdsInitializationException { for (Map channelCreds : jsonList) { String type = JsonUtil.getString(channelCreds, "type"); @@ -122,15 +123,10 @@ private static ChannelCredentials parseChannelCredentials(List> j throw new XdsInitializationException( "Invalid bootstrap: server " + serverUri + " with 'channel_creds' type unspecified"); } - XdsCredentialsProvider provider = XdsCredentialsRegistry.getDefaultRegistry() - .getProvider(type); - if (provider != null) { - Map config = JsonUtil.getObject(channelCreds, "config"); - if (config == null) { - config = ImmutableMap.of(); - } - - return provider.newChannelCredentials(config); + ImmutableSet supportedNames = XdsCredentialsRegistry.getDefaultRegistry() + .getSupportedCredentialNames(); + if (supportedNames.contains(type)) { + return ImmutableMap.copyOf(channelCreds); } } return null; diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index 0da51bf47f7..7192e3b8459 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -19,6 +19,8 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import io.grpc.CallCredentials; import io.grpc.CallOptions; import io.grpc.ChannelCredentials; @@ -28,9 +30,15 @@ import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.ResourceAllocatingChannelCredentials; import io.grpc.Status; +import io.grpc.internal.GrpcUtil; +import io.grpc.internal.JsonUtil; import io.grpc.xds.client.Bootstrapper; +import io.grpc.xds.client.XdsInitializationException; import io.grpc.xds.client.XdsTransportFactory; +import java.io.Closeable; +import java.util.Map; import java.util.concurrent.TimeUnit; final class GrpcXdsTransportFactory implements XdsTransportFactory { @@ -42,7 +50,7 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory { } @Override - public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { + public XdsTransport create(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException { return new GrpcXdsTransport(serverInfo, callCredentials); } @@ -56,8 +64,9 @@ static class GrpcXdsTransport implements XdsTransport { private final ManagedChannel channel; private final CallCredentials callCredentials; + private final ImmutableList resources; - public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo) { + public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException { this(serverInfo, null); } @@ -66,9 +75,27 @@ public GrpcXdsTransport(ManagedChannel channel) { this(channel, null); } - public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials) { + public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials) + throws XdsInitializationException { String target = serverInfo.target(); - ChannelCredentials channelCredentials = (ChannelCredentials) serverInfo.implSpecificConfig(); + Map implSpecificConfig = serverInfo.implSpecificConfig(); + String type = JsonUtil.getString(implSpecificConfig, "type"); + XdsCredentialsProvider provider = XdsCredentialsRegistry.getDefaultRegistry() + .getProvider(type); + Map config = JsonUtil.getObject(implSpecificConfig, "config"); + if (config == null) { + config = ImmutableMap.of(); + } + ChannelCredentials channelCredentials = provider.newChannelCredentials(config); + if (channelCredentials == null) { + throw new XdsInitializationException( + "Cannot create channel credentials of type " + type + " for target " + target); + } + // if {@code ChannelCredentials} instance has allocated resource of any type, save them to be + // released once the channel is shutdown + this.resources = (channelCredentials instanceof ResourceAllocatingChannelCredentials) + ? ((ResourceAllocatingChannelCredentials) channelCredentials).getAllocatedResources() + : ImmutableList.of(); this.channel = Grpc.newChannelBuilder(target, channelCredentials) .keepAliveTime(5, TimeUnit.MINUTES) .build(); @@ -79,6 +106,7 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials call public GrpcXdsTransport(ManagedChannel channel, CallCredentials callCredentials) { this.channel = checkNotNull(channel, "channel"); this.callCredentials = callCredentials; + this.resources = ImmutableList.of(); } @Override @@ -99,6 +127,9 @@ public StreamingCall createStreamingCall( @Override public void shutdown() { channel.shutdown(); + for (Closeable resource : resources) { + GrpcUtil.closeQuietly(resource); + } } private class XdsStreamingCall implements diff --git a/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java b/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java index 9dfefaf1a65..b78bbff6a23 100644 --- a/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java +++ b/xds/src/main/java/io/grpc/xds/XdsCredentialsRegistry.java @@ -21,6 +21,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.annotations.concurrent.GuardedBy; import io.grpc.InternalServiceProviders; import java.util.ArrayList; @@ -147,6 +148,14 @@ public synchronized XdsCredentialsProvider getProvider(String name) { return effectiveProviders.get(checkNotNull(name, "name")); } + /** + * Returns list of registered xds credential names. Each provider declares its name via + * {@link XdsCredentialsProvider#getName}. + */ + public synchronized ImmutableSet getSupportedCredentialNames() { + return effectiveProviders.keySet(); + } + @VisibleForTesting static List> getHardCodedClasses() { // Class.forName(String) is used to remove the need for ProGuard configuration. Note that diff --git a/xds/src/main/java/io/grpc/xds/client/Bootstrapper.java b/xds/src/main/java/io/grpc/xds/client/Bootstrapper.java index 4fa75f6b335..c3ae86fa9bf 100644 --- a/xds/src/main/java/io/grpc/xds/client/Bootstrapper.java +++ b/xds/src/main/java/io/grpc/xds/client/Bootstrapper.java @@ -57,7 +57,7 @@ public BootstrapInfo bootstrap(Map rawData) throws XdsInitializationE public abstract static class ServerInfo { public abstract String target(); - public abstract Object implSpecificConfig(); + public abstract ImmutableMap implSpecificConfig(); public abstract boolean ignoreResourceDeletion(); @@ -66,14 +66,15 @@ public abstract static class ServerInfo { public abstract boolean resourceTimerIsTransientError(); @VisibleForTesting - public static ServerInfo create(String target, @Nullable Object implSpecificConfig) { + public static ServerInfo create( + String target, @Nullable ImmutableMap implSpecificConfig) { return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig, false, false, false); } @VisibleForTesting public static ServerInfo create( - String target, Object implSpecificConfig, boolean ignoreResourceDeletion, + String target, ImmutableMap implSpecificConfig, boolean ignoreResourceDeletion, boolean isTrustedXdsServer, boolean resourceTimerIsTransientError) { return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig, ignoreResourceDeletion, isTrustedXdsServer, resourceTimerIsTransientError); diff --git a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java index 423c1a118e8..ead67640a75 100644 --- a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java @@ -76,8 +76,8 @@ protected BootstrapperImpl() { protected abstract String getJsonContent() throws IOException, XdsInitializationException; - protected abstract Object getImplSpecificConfig(Map serverConfig, String serverUri) - throws XdsInitializationException; + protected abstract ImmutableMap getImplSpecificConfig( + Map serverConfig, String serverUri) throws XdsInitializationException; /** @@ -253,7 +253,7 @@ private List parseServerInfos(List rawServerConfigs, XdsLogger lo } logger.log(XdsLogLevel.INFO, "xDS server URI: {0}", serverUri); - Object implSpecificConfig = getImplSpecificConfig(serverConfig, serverUri); + ImmutableMap implSpecificConfig = getImplSpecificConfig(serverConfig, serverUri); boolean resourceTimerIsTransientError = false; boolean ignoreResourceDeletion = false; diff --git a/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java index ec700bd6dc9..8ad48098842 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java @@ -25,7 +25,7 @@ */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/10823") public interface XdsTransportFactory { - XdsTransport create(Bootstrapper.ServerInfo serverInfo); + XdsTransport create(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException; /** * Represents transport for xDS communication (e.g., a gRPC channel). diff --git a/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java index 7df0630b779..1dcec49f9ca 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java @@ -34,7 +34,6 @@ import io.grpc.ConnectivityState; import io.grpc.ConnectivityStateInfo; import io.grpc.EquivalentAddressGroup; -import io.grpc.InsecureChannelCredentials; import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.CreateSubchannelArgs; import io.grpc.LoadBalancer.FixedResultPicker; @@ -116,7 +115,7 @@ public class ClusterImplLoadBalancerTest { private static final String CLUSTER = "cluster-foo.googleapis.com"; private static final String EDS_SERVICE_NAME = "service.googleapis.com"; private static final ServerInfo LRS_SERVER_INFO = - ServerInfo.create("api.google.com", InsecureChannelCredentials.create()); + ServerInfo.create("api.google.com", ImmutableMap.of("type", "insecure")); private static final Metadata.Key ORCA_ENDPOINT_LOAD_METRICS_KEY = Metadata.Key.of( "endpoint-load-metrics-bin", diff --git a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java index 982677d24da..d9304e25f06 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java @@ -38,7 +38,6 @@ import io.grpc.ConnectivityState; import io.grpc.EquivalentAddressGroup; import io.grpc.HttpConnectProxiedSocketAddress; -import io.grpc.InsecureChannelCredentials; import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancer.PickResult; @@ -126,7 +125,7 @@ public class ClusterResolverLoadBalancerTest { private static final String EDS_SERVICE_NAME2 = "backend-service-bar.googleapis.com"; private static final String DNS_HOST_NAME = "dns-service.googleapis.com"; private static final ServerInfo LRS_SERVER_INFO = - ServerInfo.create("lrs.googleapis.com", InsecureChannelCredentials.create()); + ServerInfo.create("lrs.googleapis.com", ImmutableMap.of("type", "insecure")); private final Locality locality1 = Locality.create("test-region-1", "test-zone-1", "test-subzone-1"); private final Locality locality2 = diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index 573aef7ca1e..6fe56efa214 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -38,7 +38,6 @@ import io.envoyproxy.envoy.service.status.v3.ClientStatusResponse; import io.envoyproxy.envoy.type.matcher.v3.NodeMatcher; import io.grpc.Deadline; -import io.grpc.InsecureChannelCredentials; import io.grpc.MetricRecorder; import io.grpc.Status; import io.grpc.Status.Code; @@ -79,7 +78,7 @@ public class CsdsServiceTest { EnvoyProtoData.Node.newBuilder().setId(NODE_ID).build(); private static final BootstrapInfo BOOTSTRAP_INFO = BootstrapInfo.builder() .servers(ImmutableList.of( - ServerInfo.create(SERVER_URI, InsecureChannelCredentials.create()))) + ServerInfo.create(SERVER_URI, ImmutableMap.of("type", "insecure")))) .node(BOOTSTRAP_NODE) .build(); private static final FakeXdsClient XDS_CLIENT_NO_RESOURCES = new FakeXdsClient(); diff --git a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java index 3f93cc6f191..b8bcbf3c1b2 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java @@ -24,8 +24,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; -import io.grpc.InsecureChannelCredentials; -import io.grpc.TlsChannelCredentials; import io.grpc.internal.GrpcUtil; import io.grpc.internal.GrpcUtil.GrpcBuildVersion; import io.grpc.xds.client.Bootstrapper; @@ -115,7 +113,7 @@ public void parseBootstrap_singleXdsServer() throws XdsInitializationException { assertThat(info.servers()).hasSize(1); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); assertThat(serverInfo.target()).isEqualTo(SERVER_URI); - assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class); + assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure")); assertThat(info.node()).isEqualTo( getNodeBuilder() .setId("ENVOY_NODE_ID") @@ -169,11 +167,11 @@ public void parseBootstrap_multipleXdsServers() throws XdsInitializationExceptio assertThat(serverInfoList.get(0).target()) .isEqualTo("trafficdirector-foo.googleapis.com:443"); assertThat(serverInfoList.get(0).implSpecificConfig()) - .isInstanceOf(TlsChannelCredentials.class); + .isEqualTo(ImmutableMap.of("type", "tls")); assertThat(serverInfoList.get(1).target()) .isEqualTo("trafficdirector-bar.googleapis.com:443"); assertThat(serverInfoList.get(1).implSpecificConfig()) - .isInstanceOf(InsecureChannelCredentials.class); + .isEqualTo(ImmutableMap.of("type", "insecure")); assertThat(info.node()).isEqualTo( getNodeBuilder() .setId("ENVOY_NODE_ID") @@ -217,7 +215,7 @@ public void parseBootstrap_IgnoreIrrelevantFields() throws XdsInitializationExce assertThat(info.servers()).hasSize(1); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); assertThat(serverInfo.target()).isEqualTo(SERVER_URI); - assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class); + assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));; assertThat(info.node()).isEqualTo( getNodeBuilder() .setId("ENVOY_NODE_ID") @@ -288,7 +286,7 @@ public void parseBootstrap_useFirstSupportedChannelCredentials() assertThat(info.servers()).hasSize(1); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); assertThat(serverInfo.target()).isEqualTo(SERVER_URI); - assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class); + assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));; assertThat(info.node()).isEqualTo(getNodeBuilder().build()); } @@ -583,7 +581,7 @@ public void useV2ProtocolByDefault() throws XdsInitializationException { BootstrapInfo info = bootstrapper.bootstrap(); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); assertThat(serverInfo.target()).isEqualTo(SERVER_URI); - assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class); + assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure")); assertThat(serverInfo.ignoreResourceDeletion()).isFalse(); } @@ -605,7 +603,7 @@ public void useV3ProtocolIfV3FeaturePresent() throws XdsInitializationException BootstrapInfo info = bootstrapper.bootstrap(); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); assertThat(serverInfo.target()).isEqualTo(SERVER_URI); - assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class); + assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure")); assertThat(serverInfo.ignoreResourceDeletion()).isFalse(); } @@ -627,7 +625,7 @@ public void serverFeatureIgnoreResourceDeletion() throws XdsInitializationExcept BootstrapInfo info = bootstrapper.bootstrap(); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); assertThat(serverInfo.target()).isEqualTo(SERVER_URI); - assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class); + assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure")); // Only ignore_resource_deletion feature enabled: confirm it's on, and xds_v3 is off. assertThat(serverInfo.ignoreResourceDeletion()).isTrue(); } @@ -650,7 +648,7 @@ public void serverFeatureTrustedXdsServer() throws XdsInitializationException { BootstrapInfo info = bootstrapper.bootstrap(); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); assertThat(serverInfo.target()).isEqualTo(SERVER_URI); - assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class); + assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure")); assertThat(serverInfo.isTrustedXdsServer()).isTrue(); } @@ -672,7 +670,7 @@ public void serverFeatureIgnoreResourceDeletion_xdsV3() throws XdsInitialization BootstrapInfo info = bootstrapper.bootstrap(); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); assertThat(serverInfo.target()).isEqualTo(SERVER_URI); - assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class); + assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure")); // ignore_resource_deletion features enabled: confirm both are on. assertThat(serverInfo.ignoreResourceDeletion()).isTrue(); } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 3650e5d5bb9..2ab8b833db5 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -113,7 +113,6 @@ import io.envoyproxy.envoy.type.v3.FractionalPercent.DenominatorType; import io.envoyproxy.envoy.type.v3.Int64Range; import io.grpc.EquivalentAddressGroup; -import io.grpc.InsecureChannelCredentials; import io.grpc.LoadBalancerRegistry; import io.grpc.Status.Code; import io.grpc.internal.JsonUtil; @@ -168,7 +167,7 @@ public class GrpcXdsClientImplDataTest { private static final RouterFilter.Provider ROUTER_FILTER_PROVIDER = new RouterFilter.Provider(); private static final ServerInfo LRS_SERVER_INFO = - ServerInfo.create("lrs.googleapis.com", InsecureChannelCredentials.create()); + ServerInfo.create("lrs.googleapis.com", ImmutableMap.of("type", "insecure")); private static final String GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE = "GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE"; @@ -3549,7 +3548,7 @@ private static Filter buildHttpConnectionManagerFilter(HttpFilter... httpFilters private XdsResourceType.Args getXdsResourceTypeArgs(boolean isTrustedServer) { return new XdsResourceType.Args( - ServerInfo.create("http://td", "", false, isTrustedServer, false), "1.0", null, null, null, null + ServerInfo.create("http://td", ImmutableMap.of(), false, isTrustedServer, false), "1.0", null, null, null, null ); } } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index 9ff19c6d1b0..b560c8c2994 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -48,10 +48,8 @@ import io.envoyproxy.envoy.extensions.filters.http.router.v3.Router; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateProviderPluginInstance; import io.grpc.BindableService; -import io.grpc.ChannelCredentials; import io.grpc.Context; import io.grpc.Context.CancellableContext; -import io.grpc.InsecureChannelCredentials; import io.grpc.ManagedChannel; import io.grpc.Server; import io.grpc.Status; @@ -160,7 +158,6 @@ public abstract class GrpcXdsClientImplTestBase { private static final String NODE_ID = "cool-node-id"; private static final Node NODE = Node.newBuilder().setId(NODE_ID).build(); private static final Any FAILING_ANY = MessageFactory.FAILING_ANY; - private static final ChannelCredentials CHANNEL_CREDENTIALS = InsecureChannelCredentials.create(); private static final XdsResourceType LDS = XdsListenerResource.getInstance(); private static final XdsResourceType CDS = XdsClusterResource.getInstance(); private static final XdsResourceType RDS = XdsRouteConfigureResource.getInstance(); @@ -361,7 +358,8 @@ public void setUp() throws IOException { channel = cleanupRule.register(InProcessChannelBuilder.forName(serverName).directExecutor().build()); - xdsServerInfo = ServerInfo.create(SERVER_URI, CHANNEL_CREDENTIALS, ignoreResourceDeletion(), + xdsServerInfo = ServerInfo.create( + SERVER_URI, ImmutableMap.of("type", "insecure"), ignoreResourceDeletion(), true, false); BootstrapInfo bootstrapInfo = Bootstrapper.BootstrapInfo.builder() @@ -372,12 +370,12 @@ public void setUp() throws IOException { AuthorityInfo.create( "xdstp://authority.xds.com/envoy.config.listener.v3.Listener/%s", ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_CUSTOM_AUTHORITY, CHANNEL_CREDENTIALS))), + SERVER_URI_CUSTOM_AUTHORITY, ImmutableMap.of("type", "insecure")))), "", AuthorityInfo.create( "xdstp:///envoy.config.listener.v3.Listener/%s", ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_EMPTY_AUTHORITY, CHANNEL_CREDENTIALS))))) + SERVER_URI_EMPTY_AUTHORITY, ImmutableMap.of("type", "insecure")))))) .certProviders(ImmutableMap.of("cert-instance-name", CertificateProviderInfo.create("file-watcher", ImmutableMap.of()))) .build(); @@ -3191,7 +3189,7 @@ public void flowControlAbsent() throws Exception { @Test public void resourceTimerIsTransientError_schedulesExtendedTimeout() { BootstrapperImpl.xdsDataErrorHandlingEnabled = true; - ServerInfo serverInfo = ServerInfo.create(SERVER_URI, CHANNEL_CREDENTIALS, + ServerInfo serverInfo = ServerInfo.create(SERVER_URI, ImmutableMap.of("type", "insecure"), false, true, true); BootstrapInfo bootstrapInfo = Bootstrapper.BootstrapInfo.builder() @@ -3202,7 +3200,7 @@ public void resourceTimerIsTransientError_schedulesExtendedTimeout() { AuthorityInfo.create( "xdstp:///envoy.config.listener.v3.Listener/%s", ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_EMPTY_AUTHORITY, CHANNEL_CREDENTIALS))))) + SERVER_URI_EMPTY_AUTHORITY, ImmutableMap.of("type", "insecure")))))) .certProviders(ImmutableMap.of()) .build(); xdsClient = new XdsClientImpl( @@ -3236,8 +3234,8 @@ public void resourceTimerIsTransientError_schedulesExtendedTimeout() { @Test public void resourceTimerIsTransientError_callsOnErrorUnavailable() { BootstrapperImpl.xdsDataErrorHandlingEnabled = true; - xdsServerInfo = ServerInfo.create(SERVER_URI, CHANNEL_CREDENTIALS, ignoreResourceDeletion(), - true, true); + xdsServerInfo = ServerInfo.create( + SERVER_URI, ImmutableMap.of("type", "insecure"), ignoreResourceDeletion(), true, true); BootstrapInfo bootstrapInfo = Bootstrapper.BootstrapInfo.builder() .servers(Collections.singletonList(xdsServerInfo)) @@ -3247,12 +3245,12 @@ public void resourceTimerIsTransientError_callsOnErrorUnavailable() { AuthorityInfo.create( "xdstp://authority.xds.com/envoy.config.listener.v3.Listener/%s", ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_CUSTOM_AUTHORITY, CHANNEL_CREDENTIALS))), + SERVER_URI_CUSTOM_AUTHORITY, ImmutableMap.of("type", "insecure")))), "", AuthorityInfo.create( "xdstp:///envoy.config.listener.v3.Listener/%s", ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_EMPTY_AUTHORITY, CHANNEL_CREDENTIALS))))) + SERVER_URI_EMPTY_AUTHORITY, ImmutableMap.of("type", "insecure")))))) .certProviders(ImmutableMap.of("cert-instance-name", CertificateProviderInfo.create("file-watcher", ImmutableMap.of()))) .build(); @@ -4354,7 +4352,7 @@ private XdsClientImpl createXdsClient(String serverUri) { private BootstrapInfo buildBootStrap(String serverUri) { - ServerInfo xdsServerInfo = ServerInfo.create(serverUri, CHANNEL_CREDENTIALS, + ServerInfo xdsServerInfo = ServerInfo.create(serverUri, ImmutableMap.of("type", "insecure"), ignoreResourceDeletion(), true, false); return Bootstrapper.BootstrapInfo.builder() @@ -4365,12 +4363,12 @@ private BootstrapInfo buildBootStrap(String serverUri) { AuthorityInfo.create( "xdstp://authority.xds.com/envoy.config.listener.v3.Listener/%s", ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_CUSTOM_AUTHORITY, CHANNEL_CREDENTIALS))), + SERVER_URI_CUSTOM_AUTHORITY, ImmutableMap.of("type", "insecure")))), "", AuthorityInfo.create( "xdstp:///envoy.config.listener.v3.Listener/%s", ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_EMPTY_AUTHORITY, CHANNEL_CREDENTIALS))))) + SERVER_URI_EMPTY_AUTHORITY, ImmutableMap.of("type", "insecure")))))) .certProviders(ImmutableMap.of("cert-instance-name", CertificateProviderInfo.create("file-watcher", ImmutableMap.of()))) .build(); diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java index 66e0d4b3198..92a9ff44ccf 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java @@ -18,13 +18,13 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.service.discovery.v3.AggregatedDiscoveryServiceGrpc; import io.envoyproxy.envoy.service.discovery.v3.DiscoveryRequest; import io.envoyproxy.envoy.service.discovery.v3.DiscoveryResponse; import io.grpc.BindableService; import io.grpc.Grpc; -import io.grpc.InsecureChannelCredentials; import io.grpc.InsecureServerCredentials; import io.grpc.MethodDescriptor; import io.grpc.Server; @@ -95,7 +95,7 @@ public void callApis() throws Exception { new GrpcXdsTransportFactory(null) .create( Bootstrapper.ServerInfo.create( - "localhost:" + server.getPort(), InsecureChannelCredentials.create())); + "localhost:" + server.getPort(), ImmutableMap.of("type", "insecure"))); MethodDescriptor methodDescriptor = AggregatedDiscoveryServiceGrpc.getStreamAggregatedResourcesMethod(); XdsTransportFactory.StreamingCall streamingCall = diff --git a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java index 24f1750d5a8..467a73c275e 100644 --- a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java @@ -26,10 +26,10 @@ import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.OAuth2Credentials; +import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.SettableFuture; import io.grpc.CallCredentials; import io.grpc.Grpc; -import io.grpc.InsecureChannelCredentials; import io.grpc.InsecureServerCredentials; import io.grpc.Metadata; import io.grpc.MetricRecorder; @@ -88,7 +88,7 @@ public void noServer() throws XdsInitializationException { @Test public void sharedXdsClientObjectPool() throws XdsInitializationException { - ServerInfo server = ServerInfo.create(SERVER_URI, InsecureChannelCredentials.create()); + ServerInfo server = ServerInfo.create(SERVER_URI, ImmutableMap.of("type", "insecure")); BootstrapInfo bootstrapInfo = BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); when(bootstrapper.bootstrap()).thenReturn(bootstrapInfo); @@ -105,7 +105,7 @@ public void sharedXdsClientObjectPool() throws XdsInitializationException { @Test public void refCountedXdsClientObjectPool_delayedCreation() { - ServerInfo server = ServerInfo.create(SERVER_URI, InsecureChannelCredentials.create()); + ServerInfo server = ServerInfo.create(SERVER_URI, ImmutableMap.of("type", "insecure")); BootstrapInfo bootstrapInfo = BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); SharedXdsClientPoolProvider provider = new SharedXdsClientPoolProvider(bootstrapper); @@ -119,7 +119,7 @@ public void refCountedXdsClientObjectPool_delayedCreation() { @Test public void refCountedXdsClientObjectPool_refCounted() { - ServerInfo server = ServerInfo.create(SERVER_URI, InsecureChannelCredentials.create()); + ServerInfo server = ServerInfo.create(SERVER_URI, ImmutableMap.of("type", "insecure")); BootstrapInfo bootstrapInfo = BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); SharedXdsClientPoolProvider provider = new SharedXdsClientPoolProvider(bootstrapper); @@ -140,7 +140,7 @@ public void refCountedXdsClientObjectPool_refCounted() { @Test public void refCountedXdsClientObjectPool_getObjectCreatesNewInstanceIfAlreadyShutdown() { - ServerInfo server = ServerInfo.create(SERVER_URI, InsecureChannelCredentials.create()); + ServerInfo server = ServerInfo.create(SERVER_URI, ImmutableMap.of("type", "insecure")); BootstrapInfo bootstrapInfo = BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); SharedXdsClientPoolProvider provider = new SharedXdsClientPoolProvider(bootstrapper); @@ -186,7 +186,7 @@ public void xdsClient_usesCallCredentials() throws Exception { String xdsServerUri = "localhost:" + xdsServer.getPort(); // Set up bootstrap & xDS client pool provider - ServerInfo server = ServerInfo.create(xdsServerUri, InsecureChannelCredentials.create()); + ServerInfo server = ServerInfo.create(xdsServerUri, ImmutableMap.of("type", "insecure")); BootstrapInfo bootstrapInfo = BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); when(bootstrapper.bootstrap()).thenReturn(bootstrapInfo); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java index 1e7ce6dc2a2..d0af40d3fcc 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java @@ -36,6 +36,7 @@ import io.grpc.Status; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.FakeClock; +import io.grpc.internal.JsonUtil; import io.grpc.internal.ObjectPool; import io.grpc.xds.client.Bootstrapper; import io.grpc.xds.client.CommonBootstrapperTestUtils; @@ -346,8 +347,15 @@ public void connect_then_mainServerDown_fallbackServerUp() throws Exception { XdsTransportFactory xdsTransportFactory = new XdsTransportFactory() { @Override public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { - ChannelCredentials channelCredentials = - (ChannelCredentials) serverInfo.implSpecificConfig(); + ImmutableMap implSpecificConfig = serverInfo.implSpecificConfig(); + String type = JsonUtil.getString(implSpecificConfig, "type"); + XdsCredentialsProvider provider = XdsCredentialsRegistry.getDefaultRegistry() + .getProvider(type); + Map config = JsonUtil.getObject(implSpecificConfig, "config"); + if (config == null) { + config = ImmutableMap.of(); + } + ChannelCredentials channelCredentials = provider.newChannelCredentials(config); return new GrpcXdsTransportFactory.GrpcXdsTransport( Grpc.newChannelBuilder(serverInfo.target(), channelCredentials) .executor(executor) diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index b50bdfce01f..325c298697e 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -52,7 +52,6 @@ import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; import io.grpc.Deadline; -import io.grpc.InsecureChannelCredentials; import io.grpc.InternalConfigSelector; import io.grpc.InternalConfigSelector.Result; import io.grpc.LoadBalancer.PickDetailsConsumer; @@ -176,7 +175,7 @@ public ConfigOrError parseServiceConfig(Map rawServiceConfig) { private final MetricRecorder metricRecorder = new MetricRecorder() {}; private BootstrapInfo bootstrapInfo = BootstrapInfo.builder() .servers(ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create()))) + "td.googleapis.com", ImmutableMap.of("type", "insecure")))) .node(Node.newBuilder().build()) .build(); private String expectedLdsResourceName = AUTHORITY; @@ -301,7 +300,7 @@ public void resolving_withTargetAuthorityNotFound() { public void resolving_noTargetAuthority_templateWithoutXdstp() { bootstrapInfo = BootstrapInfo.builder() .servers(ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create()))) + "td.googleapis.com", ImmutableMap.of("type", "insecure")))) .node(Node.newBuilder().build()) .clientDefaultListenerResourceNameTemplate("%s/id=1") .build(); @@ -319,7 +318,7 @@ public void resolving_noTargetAuthority_templateWithoutXdstp() { public void resolving_noTargetAuthority_templateWithXdstp() { bootstrapInfo = BootstrapInfo.builder() .servers(ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create()))) + "td.googleapis.com", ImmutableMap.of("type", "insecure")))) .node(Node.newBuilder().build()) .clientDefaultListenerResourceNameTemplate( "xdstp://xds.authority.com/envoy.config.listener.v3.Listener/%s?id=1") @@ -340,7 +339,7 @@ public void resolving_noTargetAuthority_templateWithXdstp() { public void resolving_noTargetAuthority_xdstpWithMultipleSlashes() { bootstrapInfo = BootstrapInfo.builder() .servers(ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create()))) + "td.googleapis.com", ImmutableMap.of("type", "insecure")))) .node(Node.newBuilder().build()) .clientDefaultListenerResourceNameTemplate( "xdstp://xds.authority.com/envoy.config.listener.v3.Listener/%s?id=1") @@ -368,13 +367,13 @@ public void resolving_targetAuthorityInAuthoritiesMap() { String serviceAuthority = "[::FFFF:129.144.52.38]:80"; bootstrapInfo = BootstrapInfo.builder() .servers(ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create(), true, true, false))) + "td.googleapis.com", ImmutableMap.of("type", "insecure"), true, true, false))) .node(Node.newBuilder().build()) .authorities( ImmutableMap.of(targetAuthority, AuthorityInfo.create( "xdstp://" + targetAuthority + "/envoy.config.listener.v3.Listener/%s?foo=1&bar=2", ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create(), true, true, false))))) + "td.googleapis.com", ImmutableMap.of("type", "insecure"), true, true, false))))) .build(); expectedLdsResourceName = "xdstp://xds.authority.com/envoy.config.listener.v3.Listener/" + "%5B::FFFF:129.144.52.38%5D:80?bar=2&foo=1"; // query param canonified @@ -407,7 +406,7 @@ public void resolving_ldsResourceUpdateRdsName() { ImmutableMap.of()); bootstrapInfo = BootstrapInfo.builder() .servers(ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create()))) + "td.googleapis.com", ImmutableMap.of("type", "insecure")))) .clientDefaultListenerResourceNameTemplate("test-%s") .node(Node.newBuilder().build()) .build(); diff --git a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java index b0472b4729d..d6cf54870af 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java @@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol; -import io.grpc.InsecureChannelCredentials; import io.grpc.MetricRecorder; import io.grpc.internal.ObjectPool; import io.grpc.xds.EnvoyServerProtoData.ConnectionSourceType; @@ -67,7 +66,7 @@ public class XdsServerTestHelper { Bootstrapper.BootstrapInfo.builder() .servers(Arrays.asList( Bootstrapper.ServerInfo.create( - SERVER_URI, InsecureChannelCredentials.create()))) + SERVER_URI, ImmutableMap.of("type", "insecure")))) .node(BOOTSTRAP_NODE) .serverListenerResourceNameTemplate("grpc/server?udpa.resource.listening_address=%s") .build(); diff --git a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java index ce1c6b94a35..f1a8ebc78d7 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java @@ -39,7 +39,6 @@ import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol; import io.grpc.Attributes; -import io.grpc.InsecureChannelCredentials; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Server; @@ -149,7 +148,7 @@ public void testBootstrap() throws Exception { Bootstrapper.BootstrapInfo b = Bootstrapper.BootstrapInfo.builder() .servers(Arrays.asList( - Bootstrapper.ServerInfo.create("uri", InsecureChannelCredentials.create()))) + Bootstrapper.ServerInfo.create("uri", ImmutableMap.of("type", "insecure")))) .node(EnvoyProtoData.Node.newBuilder().setId("id").build()) .serverListenerResourceNameTemplate("grpc/server?udpa.resource.listening_address=%s") .build(); @@ -180,7 +179,7 @@ public void testBootstrap_noTemplate() throws Exception { Bootstrapper.BootstrapInfo b = Bootstrapper.BootstrapInfo.builder() .servers(Arrays.asList( - Bootstrapper.ServerInfo.create("uri", InsecureChannelCredentials.create()))) + Bootstrapper.ServerInfo.create("uri", ImmutableMap.of("type", "insecure")))) .node(EnvoyProtoData.Node.newBuilder().setId("id").build()) .build(); verifyBootstrapFail(b); @@ -220,7 +219,7 @@ public void testBootstrap_templateWithXdstp() throws Exception { Bootstrapper.BootstrapInfo b = Bootstrapper.BootstrapInfo.builder() .servers(Arrays.asList( Bootstrapper.ServerInfo.create( - "uri", InsecureChannelCredentials.create()))) + "uri", ImmutableMap.of("type", "insecure")))) .node(EnvoyProtoData.Node.newBuilder().setId("id").build()) .serverListenerResourceNameTemplate( "xdstp://xds.authority.com/envoy.config.listener.v3.Listener/grpc/server/%s") diff --git a/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java b/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java index 754e903f8a9..8f175060547 100644 --- a/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java +++ b/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java @@ -18,8 +18,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import io.grpc.ChannelCredentials; -import io.grpc.InsecureChannelCredentials; import io.grpc.internal.BackoffPolicy; import io.grpc.internal.FakeClock; import io.grpc.internal.JsonParser; @@ -35,7 +33,6 @@ public class CommonBootstrapperTestUtils { public static final String SERVER_URI = "trafficdirector.googleapis.com"; - private static final ChannelCredentials CHANNEL_CREDENTIALS = InsecureChannelCredentials.create(); private static final String SERVER_URI_CUSTOM_AUTHORITY = "trafficdirector2.googleapis.com"; private static final String SERVER_URI_EMPTY_AUTHORITY = "trafficdirector3.googleapis.com"; public static final String LDS_RESOURCE = "listener.googleapis.com"; @@ -203,7 +200,8 @@ public static Bootstrapper.BootstrapInfo buildBootStrap(List serverUris) List serverInfos = new ArrayList<>(); for (String uri : serverUris) { - serverInfos.add(ServerInfo.create(uri, CHANNEL_CREDENTIALS, false, true, false)); + serverInfos.add( + ServerInfo.create(uri, ImmutableMap.of("type", "insecure"), false, true,false)); } EnvoyProtoData.Node node = EnvoyProtoData.Node.newBuilder().setId("node-id").build(); @@ -214,13 +212,17 @@ public static Bootstrapper.BootstrapInfo buildBootStrap(List serverUris) "authority.xds.com", Bootstrapper.AuthorityInfo.create( "xdstp://authority.xds.com/envoy.config.listener.v3.Listener/%s", - ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_CUSTOM_AUTHORITY, CHANNEL_CREDENTIALS))), + ImmutableList.of( + Bootstrapper.ServerInfo.create( + SERVER_URI_CUSTOM_AUTHORITY, + ImmutableMap.of("type", "insecure")))), "", Bootstrapper.AuthorityInfo.create( "xdstp:///envoy.config.listener.v3.Listener/%s", - ImmutableList.of(Bootstrapper.ServerInfo.create( - SERVER_URI_EMPTY_AUTHORITY, CHANNEL_CREDENTIALS))))) + ImmutableList.of( + Bootstrapper.ServerInfo.create( + SERVER_URI_EMPTY_AUTHORITY, + ImmutableMap.of("type", "insecure")))))) .certProviders(ImmutableMap.of("cert-instance-name", Bootstrapper.CertificateProviderInfo.create("file-watcher", ImmutableMap.of()))) .build(); From 7e9f6c45dafaf69113d678a50bdff0e7d3f69362 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Thu, 31 Jul 2025 11:13:07 +0000 Subject: [PATCH 03/10] Bootstrap file validation for TLS channel creds --- .../java/io/grpc/TlsChannelCredentials.java | 24 ++++++++ .../io/grpc/xds/client/BootstrapperImpl.java | 18 +++++- .../internal/TlsXdsCredentialsProvider.java | 40 ++++++++++++- .../io/grpc/xds/GrpcBootstrapperImplTest.java | 59 +++++++++++++++++++ .../TlsXdsCredentialsProviderTest.java | 54 ++++++++++++++++- 5 files changed, 192 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/io/grpc/TlsChannelCredentials.java b/api/src/main/java/io/grpc/TlsChannelCredentials.java index b58048be0b2..83dc36ff325 100644 --- a/api/src/main/java/io/grpc/TlsChannelCredentials.java +++ b/api/src/main/java/io/grpc/TlsChannelCredentials.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; +import java.util.Map; import java.util.Set; import javax.net.ssl.KeyManager; import javax.net.ssl.TrustManager; @@ -49,6 +50,7 @@ public static ChannelCredentials create() { private final List keyManagers; private final byte[] rootCertificates; private final List trustManagers; + private final Map customCertificatesConfig; TlsChannelCredentials(Builder builder) { fakeFeature = builder.fakeFeature; @@ -58,6 +60,7 @@ public static ChannelCredentials create() { keyManagers = builder.keyManagers; rootCertificates = builder.rootCertificates; trustManagers = builder.trustManagers; + customCertificatesConfig = builder.customCertificatesConfig; } /** @@ -121,6 +124,21 @@ public List getTrustManagers() { return trustManagers; } + /** + * Returns custom certificates config. It contains following entries: + * + *
    + *
  • {@code "ca_certificate_file"} key containing the path to the root certificate file
    • + *
  • {@code "certificate_file"} key containing the path to the identity certificate file
    • + *
  • {@code "private_key_file"} key containing the path to the private key
    • + *
  • {@code "refresh_interval"} key specifying the frequency of updates to the above + * files
    • + *
+ */ + public Map getCustomCertificatesConfig() { + return customCertificatesConfig; + } + /** * Returns an empty set if this credential can be adequately understood via * the features listed, otherwise returns a hint of features that are lacking @@ -228,6 +246,7 @@ public static final class Builder { private List keyManagers; private byte[] rootCertificates; private List trustManagers; + private Map customCertificatesConfig; private Builder() {} @@ -355,6 +374,11 @@ public Builder trustManager(TrustManager... trustManagers) { return this; } + public Builder customCertificatesConfig(Map customCertificatesConfig) { + this.customCertificatesConfig = customCertificatesConfig; + return this; + } + private void clearTrustManagers() { this.rootCertificates = null; this.trustManagers = null; diff --git a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java index ead67640a75..fca007485a0 100644 --- a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import io.grpc.Internal; import io.grpc.InternalLogId; +import io.grpc.TlsChannelCredentials; import io.grpc.internal.GrpcUtil; import io.grpc.internal.GrpcUtil.GrpcBuildVersion; import io.grpc.internal.JsonParser; @@ -170,9 +171,9 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map rawData) builder.node(nodeBuilder.build()); Map certProvidersBlob = JsonUtil.getObject(rawData, "certificate_providers"); + Map certProviders = new HashMap<>(); if (certProvidersBlob != null) { logger.log(XdsLogLevel.INFO, "Configured with {0} cert providers", certProvidersBlob.size()); - Map certProviders = new HashMap<>(certProvidersBlob.size()); for (String name : certProvidersBlob.keySet()) { Map valueMap = JsonUtil.getObject(certProvidersBlob, name); String pluginName = @@ -183,6 +184,21 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map rawData) CertificateProviderInfo.create(pluginName, config); certProviders.put(name, certificateProviderInfo); } + } + + for (ServerInfo serverInfo : servers) { + Object creds = serverInfo.implSpecificConfig(); + if (creds instanceof TlsChannelCredentials) { + Map config = ((TlsChannelCredentials)creds).getCustomCertificatesConfig(); + if (config != null) { + CertificateProviderInfo certificateProviderInfo = + CertificateProviderInfo.create("file_watcher", config); + certProviders.put("mtls_channel_creds_identity_certs", certificateProviderInfo); + } + } + } + + if (!certProviders.isEmpty()) { builder.certProviders(certProviders); } diff --git a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java index f4d26a83795..797495670e5 100644 --- a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java @@ -18,7 +18,10 @@ import io.grpc.ChannelCredentials; import io.grpc.TlsChannelCredentials; +import io.grpc.internal.JsonUtil; import io.grpc.xds.XdsCredentialsProvider; +import java.io.File; +import java.io.IOException; import java.util.Map; /** @@ -30,7 +33,42 @@ public final class TlsXdsCredentialsProvider extends XdsCredentialsProvider { @Override protected ChannelCredentials newChannelCredentials(Map jsonConfig) { - return TlsChannelCredentials.create(); + TlsChannelCredentials.Builder builder = TlsChannelCredentials.newBuilder(); + + if (jsonConfig == null) { + return builder.build(); + } + + // use trust certificate file path from bootstrap config if provided; else use system default + String rootCertPath = JsonUtil.getString(jsonConfig, "ca_certificate_file"); + if (rootCertPath != null) { + try { + builder.trustManager(new File(rootCertPath)); + } catch (IOException e) { + return null; + } + } + + // use certificate chain and private key file paths from bootstrap config if provided. Mind that + // both JSON values must be either set (mTLS case) or both unset (TLS case) + String certChainPath = JsonUtil.getString(jsonConfig, "certificate_file"); + String privateKeyPath = JsonUtil.getString(jsonConfig, "private_key_file"); + if (certChainPath != null && privateKeyPath != null) { + try { + builder.keyManager(new File(certChainPath), new File(privateKeyPath)); + } catch (IOException e) { + return null; + } + } else if (certChainPath != null || privateKeyPath != null) { + return null; + } + + // save json config when custom certificate paths were provided in a bootstrap + if (rootCertPath != null || certChainPath != null) { + builder.customCertificatesConfig(jsonConfig); + } + + return builder.build(); } @Override diff --git a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java index b8bcbf3c1b2..962b13188ae 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java @@ -17,6 +17,9 @@ package io.grpc.xds; import static com.google.common.truth.Truth.assertThat; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CA_PEM_FILE; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_KEY_FILE; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_PEM_FILE; import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; @@ -26,9 +29,11 @@ import com.google.common.collect.Iterables; import io.grpc.internal.GrpcUtil; import io.grpc.internal.GrpcUtil.GrpcBuildVersion; +import io.grpc.internal.testing.TestUtils; import io.grpc.xds.client.Bootstrapper; import io.grpc.xds.client.Bootstrapper.AuthorityInfo; import io.grpc.xds.client.Bootstrapper.BootstrapInfo; +import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo; import io.grpc.xds.client.Bootstrapper.ServerInfo; import io.grpc.xds.client.BootstrapperImpl; import io.grpc.xds.client.CommonBootstrapperTestUtils; @@ -896,6 +901,60 @@ public void badFederationConfig() { } } + @Test + public void parseTlsChannelCredentialsWithCustomCertificatesConfig() + throws XdsInitializationException, IOException { + String rootCertPath = TestUtils.loadCert(CA_PEM_FILE).getAbsolutePath(); + String certChainPath = TestUtils.loadCert(CLIENT_PEM_FILE).getAbsolutePath(); + String privateKeyPath = TestUtils.loadCert(CLIENT_KEY_FILE).getAbsolutePath(); + + String rawData = "{\n" + + " \"xds_servers\": [\n" + + " {\n" + + " \"server_uri\": \"" + SERVER_URI + "\",\n" + + " \"channel_creds\": [\n" + + " {\n" + + " \"type\": \"tls\"," + + " \"config\": {\n" + + " \"ca_certificate_file\": \"" + rootCertPath + "\",\n" + + " \"certificate_file\": \"" + certChainPath + "\",\n" + + " \"private_key_file\": \"" + privateKeyPath + "\"\n" + + " }\n" + + " }\n" + + " ]\n" + + " }\n" + + " ]\n" + + "}"; + + bootstrapper.setFileReader(createFileReader(BOOTSTRAP_FILE_PATH, rawData)); + BootstrapInfo info = bootstrapper.bootstrap(); + assertThat(info.servers()).hasSize(1); + ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); + assertThat(serverInfo.target()).isEqualTo(SERVER_URI); + assertThat(serverInfo.implSpecificConfig()).isEqualTo( + ImmutableMap.of( + "type", "tls", + "config", ImmutableMap.of( + "ca_certificate_file", rootCertPath, + "certificate_file", certChainPath, + "private_key_file", privateKeyPath))); + assertThat(info.node()).isEqualTo(getNodeBuilder().build()); + + assertThat(info.certProviders()).hasSize(1); + ImmutableMap certProviderInfo = info.certProviders(); + assertThat(certProviderInfo.keySet()).containsExactly("mtls_channel_creds_identity_certs"); + CertificateProviderInfo mtlsChannelCredCertProviderInfo = + certProviderInfo.get("mtls_channel_creds_identity_certs"); + assertThat(mtlsChannelCredCertProviderInfo.config().keySet()) + .containsExactly("ca_certificate_file", "certificate_file", "private_key_file"); + assertThat(mtlsChannelCredCertProviderInfo.config().get("ca_certificate_file")) + .isEqualTo(rootCertPath); + assertThat(mtlsChannelCredCertProviderInfo.config().get("certificate_file")) + .isEqualTo(certChainPath); + assertThat(mtlsChannelCredCertProviderInfo.config().get("private_key_file")) + .isEqualTo(privateKeyPath); + } + private static BootstrapperImpl.FileReader createFileReader( final String expectedPath, final String rawData) { return new BootstrapperImpl.FileReader() { diff --git a/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java index 3ba26bdb281..ef4322d71e2 100644 --- a/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java @@ -16,14 +16,21 @@ package io.grpc.xds.internal; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.google.common.collect.ImmutableMap; +import io.grpc.ChannelCredentials; import io.grpc.InternalServiceProviders; import io.grpc.TlsChannelCredentials; import io.grpc.xds.XdsCredentialsProvider; +import java.io.File; +import java.util.Map; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -32,6 +39,9 @@ public class TlsXdsCredentialsProviderTest { private TlsXdsCredentialsProvider provider = new TlsXdsCredentialsProvider(); + @Rule + public TemporaryFolder tempFolder = new TemporaryFolder(); + @Test public void provided() { for (XdsCredentialsProvider current @@ -50,8 +60,50 @@ public void isAvailable() { } @Test - public void channelCredentials() { + public void channelCredentialsWhenNullConfig() { assertSame(TlsChannelCredentials.class, provider.newChannelCredentials(null).getClass()); } + + @Test + public void channelCredentialsWhenNotExistingTrustFileConfig() { + Map jsonConfig = ImmutableMap.of( + "ca_certificate_file", "/tmp/not-exisiting-file.txt"); + assertNull(provider.newChannelCredentials(jsonConfig)); + } + + @Test + public void channelCredentialsWhenNotExistingCertificateFileConfig() { + Map jsonConfig = ImmutableMap.of( + "certificate_file", "/tmp/not-exisiting-file.txt", + "private_key_file", "/tmp/not-exisiting-file-2.txt"); + assertNull(provider.newChannelCredentials(jsonConfig)); + } + + @Test + public void channelCredentialsWhenInvalidConfig() throws Exception { + File certFile = tempFolder.newFile(new String("identity.cert")); + Map jsonConfig = ImmutableMap.of("certificate_file", certFile.toString()); + assertNull(provider.newChannelCredentials(jsonConfig)); + } + + @Test + public void channelCredentialsWhenValidConfig() throws Exception { + File trustFile = tempFolder.newFile(new String("root.cert")); + File certFile = tempFolder.newFile(new String("identity.cert")); + File keyFile = tempFolder.newFile(new String("private.key")); + + Map jsonConfig = ImmutableMap.of( + "ca_certificate_file", trustFile.toString(), + "certificate_file", certFile.toString(), + "private_key_file", keyFile.toString()); + + ChannelCredentials creds = provider.newChannelCredentials(jsonConfig); + assertSame(TlsChannelCredentials.class, creds.getClass()); + assertSame(((TlsChannelCredentials) creds).getCustomCertificatesConfig(), jsonConfig); + + trustFile.delete(); + certFile.delete(); + keyFile.delete(); + } } From 2556c97939058533a2b92ff3f9f8ffa3a06c79b5 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Thu, 31 Jul 2025 11:26:29 +0000 Subject: [PATCH 04/10] mTLS client-server test from bootstrap configuration --- .../grpc/xds/XdsSecurityClientServerTest.java | 48 +++++++++++++++++++ .../client/CommonBootstrapperTestUtils.java | 39 +++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java index 23068d665bf..dd3ba9c8a5f 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java @@ -51,8 +51,10 @@ import io.grpc.Status; import io.grpc.StatusOr; import io.grpc.StatusRuntimeException; +import io.grpc.TlsServerCredentials; import io.grpc.stub.StreamObserver; import io.grpc.testing.GrpcCleanupRule; +import io.grpc.testing.TlsTesting; import io.grpc.testing.protobuf.SimpleRequest; import io.grpc.testing.protobuf.SimpleResponse; import io.grpc.testing.protobuf.SimpleServiceGrpc; @@ -513,6 +515,36 @@ public void mtlsClientServer_changeServerContext_expectException() } } + @Test + public void mtlsClientServer_withClientAuthentication_withTlsChannelCredsFromBootstrap() + throws Exception { + final String mtlsCertProviderInstanceName = "mtls_channel_creds_identity_certs"; + + UpstreamTlsContext upstreamTlsContext = + setBootstrapInfoWithMTlsChannelCredsAndBuildUpstreamTlsContext( + mtlsCertProviderInstanceName, CLIENT_KEY_FILE, CLIENT_PEM_FILE, CA_PEM_FILE); + + DownstreamTlsContext downstreamTlsContext = + setBootstrapInfoWithMTlsChannelCredsAndBuildDownstreamTlsContext( + mtlsCertProviderInstanceName, SERVER_1_KEY_FILE, SERVER_1_PEM_FILE, CA_PEM_FILE); + + ServerCredentials serverCreds = TlsServerCredentials.newBuilder() + .keyManager(TlsTesting.loadCert(SERVER_1_PEM_FILE), TlsTesting.loadCert(SERVER_1_KEY_FILE)) + .trustManager(TlsTesting.loadCert(CA_PEM_FILE)) + .clientAuth(TlsServerCredentials.ClientAuth.REQUIRE) + .build(); + + buildServer( + XdsServerBuilder.forPort(0, serverCreds) + .xdsClientPoolFactory(fakePoolFactory) + .addService(new SimpleServiceImpl()), + downstreamTlsContext); + + SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub = + getBlockingStub(upstreamTlsContext, OVERRIDE_AUTHORITY); + assertThat(unaryRpc("buddy", blockingStub)).isEqualTo("Hello buddy"); + } + private void performMtlsTestAndGetListenerWatcher( UpstreamTlsContext upstreamTlsContext, String certInstanceName2, String privateKey2, String cert2, String trustCa2) @@ -573,6 +605,22 @@ private UpstreamTlsContext setBootstrapInfoAndBuildUpstreamTlsContextForUsingSys .build()); } + private UpstreamTlsContext setBootstrapInfoWithMTlsChannelCredsAndBuildUpstreamTlsContext( + String instanceName, String clientKeyFile, String clientPemFile, String caCertFile) { + bootstrapInfoForClient = CommonBootstrapperTestUtils + .buildBootstrapInfoForMTlsChannelCredentialServerInfo( + instanceName, clientKeyFile, clientPemFile, caCertFile); + return CommonTlsContextTestsUtil.buildUpstreamTlsContext(instanceName, true); + } + + private DownstreamTlsContext setBootstrapInfoWithMTlsChannelCredsAndBuildDownstreamTlsContext( + String instanceName, String serverKeyFile, String serverPemFile, String caCertFile) { + bootstrapInfoForServer = CommonBootstrapperTestUtils + .buildBootstrapInfoForMTlsChannelCredentialServerInfo( + instanceName, serverKeyFile, serverPemFile, caCertFile); + return CommonTlsContextTestsUtil.buildDownstreamTlsContext(instanceName, true, true); + } + private void buildServerWithTlsContext(DownstreamTlsContext downstreamTlsContext) throws Exception { buildServerWithTlsContext(downstreamTlsContext, InsecureServerCredentials.create()); diff --git a/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java b/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java index 8f175060547..acab6b2af21 100644 --- a/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java +++ b/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java @@ -18,12 +18,15 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import io.grpc.ChannelCredentials; +import io.grpc.TlsChannelCredentials; import io.grpc.internal.BackoffPolicy; import io.grpc.internal.FakeClock; import io.grpc.internal.JsonParser; import io.grpc.xds.client.Bootstrapper.ServerInfo; import io.grpc.xds.internal.security.CommonTlsContextTestsUtil; import io.grpc.xds.internal.security.TlsContextManagerImpl; +import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; @@ -157,6 +160,42 @@ public static Bootstrapper.BootstrapInfo buildBootstrapInfo( .build(); } + public static Bootstrapper.BootstrapInfo buildBootstrapInfoForMTlsChannelCredentialServerInfo( + String instanceName, String privateKey, String cert, String trustCa) { + try { + privateKey = CommonTlsContextTestsUtil.getTempFileNameForResourcesFile(privateKey); + cert = CommonTlsContextTestsUtil.getTempFileNameForResourcesFile(cert); + trustCa = CommonTlsContextTestsUtil.getTempFileNameForResourcesFile(trustCa); + } catch (IOException ioe) { + throw new RuntimeException(ioe); + } + + HashMap config = new HashMap<>(); + config.put("certificate_file", cert); + config.put("private_key_file", privateKey); + config.put("ca_certificate_file", trustCa); + + ChannelCredentials creds; + try { + creds = TlsChannelCredentials.newBuilder() + .customCertificatesConfig(config) + .keyManager(new File(cert), new File(privateKey)) + .trustManager(new File(trustCa)) + .build(); + } catch (IOException ioe) { + throw new RuntimeException(ioe); + } + + // config for tls channel credentials and for certificate provider are the same + return Bootstrapper.BootstrapInfo.builder() + .servers(ImmutableList.of(ServerInfo.create(SERVER_URI, creds))) + .node(EnvoyProtoData.Node.newBuilder().build()) + .certProviders(ImmutableMap.of( + instanceName, + Bootstrapper.CertificateProviderInfo.create("file_watcher", config))) + .build(); + } + public static boolean setEnableXdsFallback(boolean target) { boolean oldValue = BootstrapperImpl.enableXdsFallback; BootstrapperImpl.enableXdsFallback = target; From 4d51850729db23bc57ee0efe1883c78753a474ce Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Fri, 1 Aug 2025 10:36:57 +0000 Subject: [PATCH 05/10] Allowing file watcher empty identity certs --- .../FileWatcherCertificateProvider.java | 45 ++++++------ ...ileWatcherCertificateProviderProvider.java | 34 ++++------ ...atcherCertificateProviderProviderTest.java | 68 +++++++++++++++++-- .../FileWatcherCertificateProviderTest.java | 29 ++++++++ 4 files changed, 130 insertions(+), 46 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java index 304124cc7f2..747c76e260a 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java @@ -73,10 +73,12 @@ final class FileWatcherCertificateProvider extends CertificateProvider implement this.scheduledExecutorService = checkNotNull(scheduledExecutorService, "scheduledExecutorService"); this.timeProvider = checkNotNull(timeProvider, "timeProvider"); - this.certFile = Paths.get(checkNotNull(certFile, "certFile")); - this.keyFile = Paths.get(checkNotNull(keyFile, "keyFile")); - checkArgument((trustFile != null || spiffeTrustMapFile != null), - "either trustFile or spiffeTrustMapFile must be present"); + checkArgument(((certFile != null) == (keyFile != null)), + "certFile and keyFile must be both set or both unset"); + this.certFile = certFile == null ? null : Paths.get(certFile); + this.keyFile = keyFile == null ? null : Paths.get(keyFile); + checkArgument((trustFile != null || spiffeTrustMapFile != null || keyFile != null), + "must be watching either root or identity certificates"); if (spiffeTrustMapFile != null) { this.spiffeTrustMapFile = Paths.get(spiffeTrustMapFile); this.trustFile = null; @@ -113,23 +115,26 @@ private synchronized void scheduleNextRefreshCertificate(long delayInSeconds) { void checkAndReloadCertificates() { try { try { - FileTime currentCertTime = Files.getLastModifiedTime(certFile); - FileTime currentKeyTime = Files.getLastModifiedTime(keyFile); - if (!currentCertTime.equals(lastModifiedTimeCert) - && !currentKeyTime.equals(lastModifiedTimeKey)) { - byte[] certFileContents = Files.readAllBytes(certFile); - byte[] keyFileContents = Files.readAllBytes(keyFile); - FileTime currentCertTime2 = Files.getLastModifiedTime(certFile); - FileTime currentKeyTime2 = Files.getLastModifiedTime(keyFile); - if (currentCertTime2.equals(currentCertTime) && currentKeyTime2.equals(currentKeyTime)) { - try (ByteArrayInputStream certStream = new ByteArrayInputStream(certFileContents); - ByteArrayInputStream keyStream = new ByteArrayInputStream(keyFileContents)) { - PrivateKey privateKey = CertificateUtils.getPrivateKey(keyStream); - X509Certificate[] certs = CertificateUtils.toX509Certificates(certStream); - getWatcher().updateCertificate(privateKey, Arrays.asList(certs)); + if (certFile != null && keyFile != null) { + FileTime currentCertTime = Files.getLastModifiedTime(certFile); + FileTime currentKeyTime = Files.getLastModifiedTime(keyFile); + if (!currentCertTime.equals(lastModifiedTimeCert) + && !currentKeyTime.equals(lastModifiedTimeKey)) { + byte[] certFileContents = Files.readAllBytes(certFile); + byte[] keyFileContents = Files.readAllBytes(keyFile); + FileTime currentCertTime2 = Files.getLastModifiedTime(certFile); + FileTime currentKeyTime2 = Files.getLastModifiedTime(keyFile); + if (currentCertTime2.equals(currentCertTime) + && currentKeyTime2.equals(currentKeyTime)) { + try (ByteArrayInputStream certStream = new ByteArrayInputStream(certFileContents); + ByteArrayInputStream keyStream = new ByteArrayInputStream(keyFileContents)) { + PrivateKey privateKey = CertificateUtils.getPrivateKey(keyStream); + X509Certificate[] certs = CertificateUtils.toX509Certificates(certStream); + getWatcher().updateCertificate(privateKey, Arrays.asList(certs)); + } + lastModifiedTimeCert = currentCertTime; + lastModifiedTimeKey = currentKeyTime; } - lastModifiedTimeCert = currentCertTime; - lastModifiedTimeKey = currentKeyTime; } } } catch (Throwable t) { diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java index e4871dc4c84..b191aac023b 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java @@ -17,7 +17,6 @@ package io.grpc.xds.internal.security.certprovider; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -94,30 +93,27 @@ public CertificateProvider createCertificateProvider( timeProvider); } - private static String checkForNullAndGet(Map map, String key) { - return checkNotNull(JsonUtil.getString(map, key), "'" + key + "' is required in the config"); - } - private static Config validateAndTranslateConfig(Object config) { checkArgument(config instanceof Map, "Only Map supported for config"); @SuppressWarnings("unchecked") Map map = (Map)config; Config configObj = new Config(); - configObj.certFile = checkForNullAndGet(map, CERT_FILE_KEY); - configObj.keyFile = checkForNullAndGet(map, KEY_FILE_KEY); - if (enableSpiffe) { - if (!map.containsKey(ROOT_FILE_KEY) && !map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) { - throw new NullPointerException( - String.format("either '%s' or '%s' is required in the config", - ROOT_FILE_KEY, SPIFFE_TRUST_MAP_FILE_KEY)); - } - if (map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) { - configObj.spiffeTrustMapFile = JsonUtil.getString(map, SPIFFE_TRUST_MAP_FILE_KEY); - } else { - configObj.rootFile = JsonUtil.getString(map, ROOT_FILE_KEY); - } + configObj.certFile = JsonUtil.getString(map, CERT_FILE_KEY); + configObj.keyFile = JsonUtil.getString(map, KEY_FILE_KEY); + if ((configObj.certFile != null) != (configObj.keyFile != null)) { + throw new NullPointerException( + String.format("'%s' and '%s' must be both set or both unset", + CERT_FILE_KEY, KEY_FILE_KEY)); + } + if (!map.containsKey(ROOT_FILE_KEY) + && !map.containsKey(CERT_FILE_KEY) + && (!enableSpiffe || !map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY))) { + throw new NullPointerException("must be watching either root or identity certificates"); + } + if (enableSpiffe && map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) { + configObj.spiffeTrustMapFile = JsonUtil.getString(map, SPIFFE_TRUST_MAP_FILE_KEY); } else { - configObj.rootFile = checkForNullAndGet(map, ROOT_FILE_KEY); + configObj.rootFile = JsonUtil.getString(map, ROOT_FILE_KEY); } String refreshIntervalString = JsonUtil.getString(map, REFRESH_INTERVAL_KEY); if (refreshIntervalString != null) { diff --git a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProviderTest.java index 304a2dd5441..4b987b69a54 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProviderTest.java @@ -206,7 +206,9 @@ public void createProvider_missingCert_expectException() throws IOException { provider.createCertificateProvider(map, distWatcher, true); fail("exception expected"); } catch (NullPointerException npe) { - assertThat(npe).hasMessageThat().isEqualTo("'certificate_file' is required in the config"); + assertThat(npe) + .hasMessageThat() + .isEqualTo("'certificate_file' and 'private_key_file' must be both set or both unset"); } } @@ -220,24 +222,69 @@ public void createProvider_missingKey_expectException() throws IOException { provider.createCertificateProvider(map, distWatcher, true); fail("exception expected"); } catch (NullPointerException npe) { - assertThat(npe).hasMessageThat().isEqualTo("'private_key_file' is required in the config"); + assertThat(npe) + .hasMessageThat() + .isEqualTo("'certificate_file' and 'private_key_file' must be both set or both unset"); } } @Test - public void createProvider_missingRoot_expectException() throws IOException { - String expectedMessage = enableSpiffe ? "either 'ca_certificate_file' or " - + "'spiffe_trust_bundle_map_file' is required in the config" - : "'ca_certificate_file' is required in the config"; + public void createProvider_missingRootAndSpiffeConfig() throws IOException { CertificateProvider.DistributorWatcher distWatcher = new CertificateProvider.DistributorWatcher(); @SuppressWarnings("unchecked") Map map = (Map) JsonParser.parse(MISSING_ROOT_AND_SPIFFE_CONFIG); + ScheduledExecutorService mockService = mock(ScheduledExecutorService.class); + when(scheduledExecutorServiceFactory.create()).thenReturn(mockService); + provider.createCertificateProvider(map, distWatcher, true); + verify(fileWatcherCertificateProviderFactory, times(1)) + .create( + eq(distWatcher), + eq(true), + eq("/var/run/gke-spiffe/certs/certificates.pem"), + eq("/var/run/gke-spiffe/certs/private_key.pem"), + eq(null), + eq(null), + eq(600L), + eq(mockService), + eq(timeProvider)); + } + + @Test + public void createProvider_missingCertAndKeyConfig() throws IOException { + CertificateProvider.DistributorWatcher distWatcher = + new CertificateProvider.DistributorWatcher(); + @SuppressWarnings("unchecked") + Map map = (Map) JsonParser.parse(MISSING_CERT_AND_KEY_CONFIG); + ScheduledExecutorService mockService = mock(ScheduledExecutorService.class); + when(scheduledExecutorServiceFactory.create()).thenReturn(mockService); + provider.createCertificateProvider(map, distWatcher, true); + verify(fileWatcherCertificateProviderFactory, times(1)) + .create( + eq(distWatcher), + eq(true), + eq(null), + eq(null), + eq("/var/run/gke-spiffe/certs/ca_certificates.pem"), + eq(null), + eq(600L), + eq(mockService), + eq(timeProvider)); + } + + @Test + public void createProvider_emptyConfig_expectException() throws IOException { + CertificateProvider.DistributorWatcher distWatcher = + new CertificateProvider.DistributorWatcher(); + @SuppressWarnings("unchecked") + Map map = (Map) JsonParser.parse(EMPTY_CONFIG); try { provider.createCertificateProvider(map, distWatcher, true); fail("exception expected"); } catch (NullPointerException npe) { - assertThat(npe).hasMessageThat().isEqualTo(expectedMessage); + assertThat(npe) + .hasMessageThat() + .isEqualTo("must be watching either root or identity certificates"); } } @@ -292,6 +339,13 @@ public void createProvider_missingRoot_expectException() throws IOException { + " \"private_key_file\": \"/var/run/gke-spiffe/certs/private_key.pem\"" + " }"; + private static final String MISSING_CERT_AND_KEY_CONFIG = + "{\n" + + " \"ca_certificate_file\": \"/var/run/gke-spiffe/certs/ca_certificates.pem\"" + + " }"; + + private static final String EMPTY_CONFIG = "{\n}"; + private static final String ZERO_REFRESH_INTERVAL = "{\n" + " \"certificate_file\": \"/var/run/gke-spiffe/certs/certificates2.pem\"," diff --git a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderTest.java index 620ee0a7ff7..d566bee2f28 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderTest.java @@ -25,6 +25,8 @@ import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SPIFFE_TRUST_MAP_1_FILE; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; @@ -357,6 +359,33 @@ public void getCertificate_missingRootFile() throws IOException, InterruptedExce verifyWatcherErrorUpdates(Status.Code.UNKNOWN, NoSuchFileException.class, 1, 0, "root.pem"); } + @Test + public void illegalConstructorArguments_MissingPrivateKeyWhileCertChainPresent() + throws IllegalArgumentException { + Exception ex = assertThrows( + IllegalArgumentException.class, + () -> new FileWatcherCertificateProvider( + watcher, true, null, keyFile, rootFile, null, 600L, timeService, timeProvider)); + + String expectedMsg = "certFile and keyFile must be both set or both unset"; + String actualMsg = ex.getMessage(); + + assertEquals(expectedMsg, actualMsg); + } + + @Test + public void illegalConstructorArguments_NoFilesToWatch() throws IllegalArgumentException { + Exception ex = assertThrows( + IllegalArgumentException.class, + () -> new FileWatcherCertificateProvider( + watcher, true, null, null, null, null, 600L, timeService, timeProvider)); + + String expectedMsg = "must be watching either root or identity certificates"; + String actualMsg = ex.getMessage(); + + assertEquals(expectedMsg, actualMsg); + } + private void commonErrorTest( String certFile, String keyFile, From a7f01e44d4b18e455993c7f7bdddc5aea2bc865a Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 27 Aug 2025 13:26:23 +0000 Subject: [PATCH 06/10] Added logs for `TlsXdsCredentialsProvider` --- .../io/grpc/xds/internal/TlsXdsCredentialsProvider.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java index 797495670e5..ed6ae94cce6 100644 --- a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java @@ -23,12 +23,15 @@ import java.io.File; import java.io.IOException; import java.util.Map; +import java.util.logging.Level; +import java.util.logging.Logger; /** * A wrapper class that supports {@link TlsChannelCredentials} for Xds * by implementing {@link XdsCredentialsProvider}. */ public final class TlsXdsCredentialsProvider extends XdsCredentialsProvider { + private static final Logger logger = Logger.getLogger(TlsXdsCredentialsProvider.class.getName()); private static final String CREDS_NAME = "tls"; @Override @@ -45,6 +48,7 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { try { builder.trustManager(new File(rootCertPath)); } catch (IOException e) { + logger.log(Level.WARNING, "Unable to read root certificates", e); return null; } } @@ -57,9 +61,11 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { try { builder.keyManager(new File(certChainPath), new File(privateKeyPath)); } catch (IOException e) { + logger.log(Level.WARNING, "Unable to read certificate chain or private key", e); return null; } } else if (certChainPath != null || privateKeyPath != null) { + logger.log(Level.WARNING, "Certificate chain and private key must be both set or unset"); return null; } From f6a2379304d5fcb3e576c7fe3b6670f29874b841 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 27 Aug 2025 13:29:36 +0000 Subject: [PATCH 07/10] Reverted redundant test --- .../grpc/xds/XdsSecurityClientServerTest.java | 48 ------------------- .../client/CommonBootstrapperTestUtils.java | 39 --------------- 2 files changed, 87 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java index dd3ba9c8a5f..23068d665bf 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java @@ -51,10 +51,8 @@ import io.grpc.Status; import io.grpc.StatusOr; import io.grpc.StatusRuntimeException; -import io.grpc.TlsServerCredentials; import io.grpc.stub.StreamObserver; import io.grpc.testing.GrpcCleanupRule; -import io.grpc.testing.TlsTesting; import io.grpc.testing.protobuf.SimpleRequest; import io.grpc.testing.protobuf.SimpleResponse; import io.grpc.testing.protobuf.SimpleServiceGrpc; @@ -515,36 +513,6 @@ public void mtlsClientServer_changeServerContext_expectException() } } - @Test - public void mtlsClientServer_withClientAuthentication_withTlsChannelCredsFromBootstrap() - throws Exception { - final String mtlsCertProviderInstanceName = "mtls_channel_creds_identity_certs"; - - UpstreamTlsContext upstreamTlsContext = - setBootstrapInfoWithMTlsChannelCredsAndBuildUpstreamTlsContext( - mtlsCertProviderInstanceName, CLIENT_KEY_FILE, CLIENT_PEM_FILE, CA_PEM_FILE); - - DownstreamTlsContext downstreamTlsContext = - setBootstrapInfoWithMTlsChannelCredsAndBuildDownstreamTlsContext( - mtlsCertProviderInstanceName, SERVER_1_KEY_FILE, SERVER_1_PEM_FILE, CA_PEM_FILE); - - ServerCredentials serverCreds = TlsServerCredentials.newBuilder() - .keyManager(TlsTesting.loadCert(SERVER_1_PEM_FILE), TlsTesting.loadCert(SERVER_1_KEY_FILE)) - .trustManager(TlsTesting.loadCert(CA_PEM_FILE)) - .clientAuth(TlsServerCredentials.ClientAuth.REQUIRE) - .build(); - - buildServer( - XdsServerBuilder.forPort(0, serverCreds) - .xdsClientPoolFactory(fakePoolFactory) - .addService(new SimpleServiceImpl()), - downstreamTlsContext); - - SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub = - getBlockingStub(upstreamTlsContext, OVERRIDE_AUTHORITY); - assertThat(unaryRpc("buddy", blockingStub)).isEqualTo("Hello buddy"); - } - private void performMtlsTestAndGetListenerWatcher( UpstreamTlsContext upstreamTlsContext, String certInstanceName2, String privateKey2, String cert2, String trustCa2) @@ -605,22 +573,6 @@ private UpstreamTlsContext setBootstrapInfoAndBuildUpstreamTlsContextForUsingSys .build()); } - private UpstreamTlsContext setBootstrapInfoWithMTlsChannelCredsAndBuildUpstreamTlsContext( - String instanceName, String clientKeyFile, String clientPemFile, String caCertFile) { - bootstrapInfoForClient = CommonBootstrapperTestUtils - .buildBootstrapInfoForMTlsChannelCredentialServerInfo( - instanceName, clientKeyFile, clientPemFile, caCertFile); - return CommonTlsContextTestsUtil.buildUpstreamTlsContext(instanceName, true); - } - - private DownstreamTlsContext setBootstrapInfoWithMTlsChannelCredsAndBuildDownstreamTlsContext( - String instanceName, String serverKeyFile, String serverPemFile, String caCertFile) { - bootstrapInfoForServer = CommonBootstrapperTestUtils - .buildBootstrapInfoForMTlsChannelCredentialServerInfo( - instanceName, serverKeyFile, serverPemFile, caCertFile); - return CommonTlsContextTestsUtil.buildDownstreamTlsContext(instanceName, true, true); - } - private void buildServerWithTlsContext(DownstreamTlsContext downstreamTlsContext) throws Exception { buildServerWithTlsContext(downstreamTlsContext, InsecureServerCredentials.create()); diff --git a/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java b/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java index acab6b2af21..8f175060547 100644 --- a/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java +++ b/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java @@ -18,15 +18,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import io.grpc.ChannelCredentials; -import io.grpc.TlsChannelCredentials; import io.grpc.internal.BackoffPolicy; import io.grpc.internal.FakeClock; import io.grpc.internal.JsonParser; import io.grpc.xds.client.Bootstrapper.ServerInfo; import io.grpc.xds.internal.security.CommonTlsContextTestsUtil; import io.grpc.xds.internal.security.TlsContextManagerImpl; -import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; @@ -160,42 +157,6 @@ public static Bootstrapper.BootstrapInfo buildBootstrapInfo( .build(); } - public static Bootstrapper.BootstrapInfo buildBootstrapInfoForMTlsChannelCredentialServerInfo( - String instanceName, String privateKey, String cert, String trustCa) { - try { - privateKey = CommonTlsContextTestsUtil.getTempFileNameForResourcesFile(privateKey); - cert = CommonTlsContextTestsUtil.getTempFileNameForResourcesFile(cert); - trustCa = CommonTlsContextTestsUtil.getTempFileNameForResourcesFile(trustCa); - } catch (IOException ioe) { - throw new RuntimeException(ioe); - } - - HashMap config = new HashMap<>(); - config.put("certificate_file", cert); - config.put("private_key_file", privateKey); - config.put("ca_certificate_file", trustCa); - - ChannelCredentials creds; - try { - creds = TlsChannelCredentials.newBuilder() - .customCertificatesConfig(config) - .keyManager(new File(cert), new File(privateKey)) - .trustManager(new File(trustCa)) - .build(); - } catch (IOException ioe) { - throw new RuntimeException(ioe); - } - - // config for tls channel credentials and for certificate provider are the same - return Bootstrapper.BootstrapInfo.builder() - .servers(ImmutableList.of(ServerInfo.create(SERVER_URI, creds))) - .node(EnvoyProtoData.Node.newBuilder().build()) - .certProviders(ImmutableMap.of( - instanceName, - Bootstrapper.CertificateProviderInfo.create("file_watcher", config))) - .build(); - } - public static boolean setEnableXdsFallback(boolean target) { boolean oldValue = BootstrapperImpl.enableXdsFallback; BootstrapperImpl.enableXdsFallback = target; From 3ea57497967f52d208b428396199d051b2a69527 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 3 Sep 2025 11:07:48 +0000 Subject: [PATCH 08/10] Changed polling mechanism for certificate files --- .../java/io/grpc/TlsChannelCredentials.java | 24 ------- .../io/grpc/xds/client/BootstrapperImpl.java | 18 +---- .../internal/TlsXdsCredentialsProvider.java | 29 ++++---- .../FileWatcherCertificateProvider.java | 45 ++++++------ ...ileWatcherCertificateProviderProvider.java | 34 ++++++---- .../io/grpc/xds/GrpcBootstrapperImplTest.java | 16 ----- .../TlsXdsCredentialsProviderTest.java | 44 ++++++------ ...atcherCertificateProviderProviderTest.java | 68 ++----------------- .../FileWatcherCertificateProviderTest.java | 29 -------- 9 files changed, 88 insertions(+), 219 deletions(-) diff --git a/api/src/main/java/io/grpc/TlsChannelCredentials.java b/api/src/main/java/io/grpc/TlsChannelCredentials.java index 83dc36ff325..b58048be0b2 100644 --- a/api/src/main/java/io/grpc/TlsChannelCredentials.java +++ b/api/src/main/java/io/grpc/TlsChannelCredentials.java @@ -26,7 +26,6 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; -import java.util.Map; import java.util.Set; import javax.net.ssl.KeyManager; import javax.net.ssl.TrustManager; @@ -50,7 +49,6 @@ public static ChannelCredentials create() { private final List keyManagers; private final byte[] rootCertificates; private final List trustManagers; - private final Map customCertificatesConfig; TlsChannelCredentials(Builder builder) { fakeFeature = builder.fakeFeature; @@ -60,7 +58,6 @@ public static ChannelCredentials create() { keyManagers = builder.keyManagers; rootCertificates = builder.rootCertificates; trustManagers = builder.trustManagers; - customCertificatesConfig = builder.customCertificatesConfig; } /** @@ -124,21 +121,6 @@ public List getTrustManagers() { return trustManagers; } - /** - * Returns custom certificates config. It contains following entries: - * - *
    - *
  • {@code "ca_certificate_file"} key containing the path to the root certificate file
    • - *
  • {@code "certificate_file"} key containing the path to the identity certificate file
    • - *
  • {@code "private_key_file"} key containing the path to the private key
    • - *
  • {@code "refresh_interval"} key specifying the frequency of updates to the above - * files
    • - *
- */ - public Map getCustomCertificatesConfig() { - return customCertificatesConfig; - } - /** * Returns an empty set if this credential can be adequately understood via * the features listed, otherwise returns a hint of features that are lacking @@ -246,7 +228,6 @@ public static final class Builder { private List keyManagers; private byte[] rootCertificates; private List trustManagers; - private Map customCertificatesConfig; private Builder() {} @@ -374,11 +355,6 @@ public Builder trustManager(TrustManager... trustManagers) { return this; } - public Builder customCertificatesConfig(Map customCertificatesConfig) { - this.customCertificatesConfig = customCertificatesConfig; - return this; - } - private void clearTrustManagers() { this.rootCertificates = null; this.trustManagers = null; diff --git a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java index fca007485a0..ead67640a75 100644 --- a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableMap; import io.grpc.Internal; import io.grpc.InternalLogId; -import io.grpc.TlsChannelCredentials; import io.grpc.internal.GrpcUtil; import io.grpc.internal.GrpcUtil.GrpcBuildVersion; import io.grpc.internal.JsonParser; @@ -171,9 +170,9 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map rawData) builder.node(nodeBuilder.build()); Map certProvidersBlob = JsonUtil.getObject(rawData, "certificate_providers"); - Map certProviders = new HashMap<>(); if (certProvidersBlob != null) { logger.log(XdsLogLevel.INFO, "Configured with {0} cert providers", certProvidersBlob.size()); + Map certProviders = new HashMap<>(certProvidersBlob.size()); for (String name : certProvidersBlob.keySet()) { Map valueMap = JsonUtil.getObject(certProvidersBlob, name); String pluginName = @@ -184,21 +183,6 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map rawData) CertificateProviderInfo.create(pluginName, config); certProviders.put(name, certificateProviderInfo); } - } - - for (ServerInfo serverInfo : servers) { - Object creds = serverInfo.implSpecificConfig(); - if (creds instanceof TlsChannelCredentials) { - Map config = ((TlsChannelCredentials)creds).getCustomCertificatesConfig(); - if (config != null) { - CertificateProviderInfo certificateProviderInfo = - CertificateProviderInfo.create("file_watcher", config); - certProviders.put("mtls_channel_creds_identity_certs", certificateProviderInfo); - } - } - } - - if (!certProviders.isEmpty()) { builder.certProviders(certProviders); } diff --git a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java index ed6ae94cce6..f697af0e09d 100644 --- a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java @@ -19,9 +19,10 @@ import io.grpc.ChannelCredentials; import io.grpc.TlsChannelCredentials; import io.grpc.internal.JsonUtil; +import io.grpc.util.AdvancedTlsX509KeyManager; +import io.grpc.util.AdvancedTlsX509TrustManager; import io.grpc.xds.XdsCredentialsProvider; import java.io.File; -import java.io.IOException; import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -33,6 +34,9 @@ public final class TlsXdsCredentialsProvider extends XdsCredentialsProvider { private static final Logger logger = Logger.getLogger(TlsXdsCredentialsProvider.class.getName()); private static final String CREDS_NAME = "tls"; + private static final String CERT_FILE_KEY = "certificate_file"; + private static final String KEY_FILE_KEY = "private_key_file"; + private static final String ROOT_FILE_KEY = "ca_certificate_file"; @Override protected ChannelCredentials newChannelCredentials(Map jsonConfig) { @@ -43,11 +47,13 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { } // use trust certificate file path from bootstrap config if provided; else use system default - String rootCertPath = JsonUtil.getString(jsonConfig, "ca_certificate_file"); + String rootCertPath = JsonUtil.getString(jsonConfig, ROOT_FILE_KEY); if (rootCertPath != null) { try { - builder.trustManager(new File(rootCertPath)); - } catch (IOException e) { + AdvancedTlsX509TrustManager trustManager = AdvancedTlsX509TrustManager.newBuilder().build(); + trustManager.updateTrustCredentials(new File(rootCertPath)); + builder.trustManager(trustManager); + } catch (Exception e) { logger.log(Level.WARNING, "Unable to read root certificates", e); return null; } @@ -55,12 +61,14 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { // use certificate chain and private key file paths from bootstrap config if provided. Mind that // both JSON values must be either set (mTLS case) or both unset (TLS case) - String certChainPath = JsonUtil.getString(jsonConfig, "certificate_file"); - String privateKeyPath = JsonUtil.getString(jsonConfig, "private_key_file"); + String certChainPath = JsonUtil.getString(jsonConfig, CERT_FILE_KEY); + String privateKeyPath = JsonUtil.getString(jsonConfig, KEY_FILE_KEY); if (certChainPath != null && privateKeyPath != null) { try { - builder.keyManager(new File(certChainPath), new File(privateKeyPath)); - } catch (IOException e) { + AdvancedTlsX509KeyManager keyManager = new AdvancedTlsX509KeyManager(); + keyManager.updateIdentityCredentials(new File(certChainPath), new File(privateKeyPath)); + builder.keyManager(keyManager); + } catch (Exception e) { logger.log(Level.WARNING, "Unable to read certificate chain or private key", e); return null; } @@ -69,11 +77,6 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { return null; } - // save json config when custom certificate paths were provided in a bootstrap - if (rootCertPath != null || certChainPath != null) { - builder.customCertificatesConfig(jsonConfig); - } - return builder.build(); } diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java index 747c76e260a..304124cc7f2 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java @@ -73,12 +73,10 @@ final class FileWatcherCertificateProvider extends CertificateProvider implement this.scheduledExecutorService = checkNotNull(scheduledExecutorService, "scheduledExecutorService"); this.timeProvider = checkNotNull(timeProvider, "timeProvider"); - checkArgument(((certFile != null) == (keyFile != null)), - "certFile and keyFile must be both set or both unset"); - this.certFile = certFile == null ? null : Paths.get(certFile); - this.keyFile = keyFile == null ? null : Paths.get(keyFile); - checkArgument((trustFile != null || spiffeTrustMapFile != null || keyFile != null), - "must be watching either root or identity certificates"); + this.certFile = Paths.get(checkNotNull(certFile, "certFile")); + this.keyFile = Paths.get(checkNotNull(keyFile, "keyFile")); + checkArgument((trustFile != null || spiffeTrustMapFile != null), + "either trustFile or spiffeTrustMapFile must be present"); if (spiffeTrustMapFile != null) { this.spiffeTrustMapFile = Paths.get(spiffeTrustMapFile); this.trustFile = null; @@ -115,26 +113,23 @@ private synchronized void scheduleNextRefreshCertificate(long delayInSeconds) { void checkAndReloadCertificates() { try { try { - if (certFile != null && keyFile != null) { - FileTime currentCertTime = Files.getLastModifiedTime(certFile); - FileTime currentKeyTime = Files.getLastModifiedTime(keyFile); - if (!currentCertTime.equals(lastModifiedTimeCert) - && !currentKeyTime.equals(lastModifiedTimeKey)) { - byte[] certFileContents = Files.readAllBytes(certFile); - byte[] keyFileContents = Files.readAllBytes(keyFile); - FileTime currentCertTime2 = Files.getLastModifiedTime(certFile); - FileTime currentKeyTime2 = Files.getLastModifiedTime(keyFile); - if (currentCertTime2.equals(currentCertTime) - && currentKeyTime2.equals(currentKeyTime)) { - try (ByteArrayInputStream certStream = new ByteArrayInputStream(certFileContents); - ByteArrayInputStream keyStream = new ByteArrayInputStream(keyFileContents)) { - PrivateKey privateKey = CertificateUtils.getPrivateKey(keyStream); - X509Certificate[] certs = CertificateUtils.toX509Certificates(certStream); - getWatcher().updateCertificate(privateKey, Arrays.asList(certs)); - } - lastModifiedTimeCert = currentCertTime; - lastModifiedTimeKey = currentKeyTime; + FileTime currentCertTime = Files.getLastModifiedTime(certFile); + FileTime currentKeyTime = Files.getLastModifiedTime(keyFile); + if (!currentCertTime.equals(lastModifiedTimeCert) + && !currentKeyTime.equals(lastModifiedTimeKey)) { + byte[] certFileContents = Files.readAllBytes(certFile); + byte[] keyFileContents = Files.readAllBytes(keyFile); + FileTime currentCertTime2 = Files.getLastModifiedTime(certFile); + FileTime currentKeyTime2 = Files.getLastModifiedTime(keyFile); + if (currentCertTime2.equals(currentCertTime) && currentKeyTime2.equals(currentKeyTime)) { + try (ByteArrayInputStream certStream = new ByteArrayInputStream(certFileContents); + ByteArrayInputStream keyStream = new ByteArrayInputStream(keyFileContents)) { + PrivateKey privateKey = CertificateUtils.getPrivateKey(keyStream); + X509Certificate[] certs = CertificateUtils.toX509Certificates(certStream); + getWatcher().updateCertificate(privateKey, Arrays.asList(certs)); } + lastModifiedTimeCert = currentCertTime; + lastModifiedTimeKey = currentKeyTime; } } } catch (Throwable t) { diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java index b191aac023b..e4871dc4c84 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java @@ -17,6 +17,7 @@ package io.grpc.xds.internal.security.certprovider; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -93,27 +94,30 @@ public CertificateProvider createCertificateProvider( timeProvider); } + private static String checkForNullAndGet(Map map, String key) { + return checkNotNull(JsonUtil.getString(map, key), "'" + key + "' is required in the config"); + } + private static Config validateAndTranslateConfig(Object config) { checkArgument(config instanceof Map, "Only Map supported for config"); @SuppressWarnings("unchecked") Map map = (Map)config; Config configObj = new Config(); - configObj.certFile = JsonUtil.getString(map, CERT_FILE_KEY); - configObj.keyFile = JsonUtil.getString(map, KEY_FILE_KEY); - if ((configObj.certFile != null) != (configObj.keyFile != null)) { - throw new NullPointerException( - String.format("'%s' and '%s' must be both set or both unset", - CERT_FILE_KEY, KEY_FILE_KEY)); - } - if (!map.containsKey(ROOT_FILE_KEY) - && !map.containsKey(CERT_FILE_KEY) - && (!enableSpiffe || !map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY))) { - throw new NullPointerException("must be watching either root or identity certificates"); - } - if (enableSpiffe && map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) { - configObj.spiffeTrustMapFile = JsonUtil.getString(map, SPIFFE_TRUST_MAP_FILE_KEY); + configObj.certFile = checkForNullAndGet(map, CERT_FILE_KEY); + configObj.keyFile = checkForNullAndGet(map, KEY_FILE_KEY); + if (enableSpiffe) { + if (!map.containsKey(ROOT_FILE_KEY) && !map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) { + throw new NullPointerException( + String.format("either '%s' or '%s' is required in the config", + ROOT_FILE_KEY, SPIFFE_TRUST_MAP_FILE_KEY)); + } + if (map.containsKey(SPIFFE_TRUST_MAP_FILE_KEY)) { + configObj.spiffeTrustMapFile = JsonUtil.getString(map, SPIFFE_TRUST_MAP_FILE_KEY); + } else { + configObj.rootFile = JsonUtil.getString(map, ROOT_FILE_KEY); + } } else { - configObj.rootFile = JsonUtil.getString(map, ROOT_FILE_KEY); + configObj.rootFile = checkForNullAndGet(map, ROOT_FILE_KEY); } String refreshIntervalString = JsonUtil.getString(map, REFRESH_INTERVAL_KEY); if (refreshIntervalString != null) { diff --git a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java index 962b13188ae..0536e7c15b2 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java @@ -33,7 +33,6 @@ import io.grpc.xds.client.Bootstrapper; import io.grpc.xds.client.Bootstrapper.AuthorityInfo; import io.grpc.xds.client.Bootstrapper.BootstrapInfo; -import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo; import io.grpc.xds.client.Bootstrapper.ServerInfo; import io.grpc.xds.client.BootstrapperImpl; import io.grpc.xds.client.CommonBootstrapperTestUtils; @@ -938,21 +937,6 @@ public void parseTlsChannelCredentialsWithCustomCertificatesConfig() "ca_certificate_file", rootCertPath, "certificate_file", certChainPath, "private_key_file", privateKeyPath))); - assertThat(info.node()).isEqualTo(getNodeBuilder().build()); - - assertThat(info.certProviders()).hasSize(1); - ImmutableMap certProviderInfo = info.certProviders(); - assertThat(certProviderInfo.keySet()).containsExactly("mtls_channel_creds_identity_certs"); - CertificateProviderInfo mtlsChannelCredCertProviderInfo = - certProviderInfo.get("mtls_channel_creds_identity_certs"); - assertThat(mtlsChannelCredCertProviderInfo.config().keySet()) - .containsExactly("ca_certificate_file", "certificate_file", "private_key_file"); - assertThat(mtlsChannelCredCertProviderInfo.config().get("ca_certificate_file")) - .isEqualTo(rootCertPath); - assertThat(mtlsChannelCredCertProviderInfo.config().get("certificate_file")) - .isEqualTo(certChainPath); - assertThat(mtlsChannelCredCertProviderInfo.config().get("private_key_file")) - .isEqualTo(privateKeyPath); } private static BootstrapperImpl.FileReader createFileReader( diff --git a/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java index ef4322d71e2..85347202036 100644 --- a/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java @@ -16,21 +16,28 @@ package io.grpc.xds.internal; +import static com.google.common.truth.Truth.assertThat; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CA_PEM_FILE; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_KEY_FILE; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_PEM_FILE; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import io.grpc.ChannelCredentials; import io.grpc.InternalServiceProviders; import io.grpc.TlsChannelCredentials; +import io.grpc.internal.testing.TestUtils; +import io.grpc.util.AdvancedTlsX509KeyManager; +import io.grpc.util.AdvancedTlsX509TrustManager; import io.grpc.xds.XdsCredentialsProvider; -import java.io.File; import java.util.Map; -import org.junit.Rule; +import javax.net.ssl.KeyManager; +import javax.net.ssl.TrustManager; import org.junit.Test; -import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -39,9 +46,6 @@ public class TlsXdsCredentialsProviderTest { private TlsXdsCredentialsProvider provider = new TlsXdsCredentialsProvider(); - @Rule - public TemporaryFolder tempFolder = new TemporaryFolder(); - @Test public void provided() { for (XdsCredentialsProvider current @@ -82,28 +86,30 @@ public void channelCredentialsWhenNotExistingCertificateFileConfig() { @Test public void channelCredentialsWhenInvalidConfig() throws Exception { - File certFile = tempFolder.newFile(new String("identity.cert")); - Map jsonConfig = ImmutableMap.of("certificate_file", certFile.toString()); + String certChainPath = TestUtils.loadCert(CLIENT_PEM_FILE).getAbsolutePath(); + Map jsonConfig = ImmutableMap.of("certificate_file", certChainPath.toString()); assertNull(provider.newChannelCredentials(jsonConfig)); } @Test public void channelCredentialsWhenValidConfig() throws Exception { - File trustFile = tempFolder.newFile(new String("root.cert")); - File certFile = tempFolder.newFile(new String("identity.cert")); - File keyFile = tempFolder.newFile(new String("private.key")); + String rootCertPath = TestUtils.loadCert(CA_PEM_FILE).getAbsolutePath(); + String certChainPath = TestUtils.loadCert(CLIENT_PEM_FILE).getAbsolutePath(); + String privateKeyPath = TestUtils.loadCert(CLIENT_KEY_FILE).getAbsolutePath(); Map jsonConfig = ImmutableMap.of( - "ca_certificate_file", trustFile.toString(), - "certificate_file", certFile.toString(), - "private_key_file", keyFile.toString()); + "ca_certificate_file", rootCertPath, + "certificate_file", certChainPath, + "private_key_file", privateKeyPath); ChannelCredentials creds = provider.newChannelCredentials(jsonConfig); assertSame(TlsChannelCredentials.class, creds.getClass()); - assertSame(((TlsChannelCredentials) creds).getCustomCertificatesConfig(), jsonConfig); - - trustFile.delete(); - certFile.delete(); - keyFile.delete(); + TlsChannelCredentials tlsChannelCredentials = (TlsChannelCredentials) creds; + assertThat(tlsChannelCredentials.getKeyManagers()).hasSize(1); + KeyManager keyManager = Iterables.getOnlyElement(tlsChannelCredentials.getKeyManagers()); + assertThat(keyManager).isInstanceOf(AdvancedTlsX509KeyManager.class); + assertThat(tlsChannelCredentials.getTrustManagers()).hasSize(1); + TrustManager trustManager = Iterables.getOnlyElement(tlsChannelCredentials.getTrustManagers()); + assertThat(trustManager).isInstanceOf(AdvancedTlsX509TrustManager.class); } } diff --git a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProviderTest.java index 4b987b69a54..304a2dd5441 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProviderTest.java @@ -206,9 +206,7 @@ public void createProvider_missingCert_expectException() throws IOException { provider.createCertificateProvider(map, distWatcher, true); fail("exception expected"); } catch (NullPointerException npe) { - assertThat(npe) - .hasMessageThat() - .isEqualTo("'certificate_file' and 'private_key_file' must be both set or both unset"); + assertThat(npe).hasMessageThat().isEqualTo("'certificate_file' is required in the config"); } } @@ -222,69 +220,24 @@ public void createProvider_missingKey_expectException() throws IOException { provider.createCertificateProvider(map, distWatcher, true); fail("exception expected"); } catch (NullPointerException npe) { - assertThat(npe) - .hasMessageThat() - .isEqualTo("'certificate_file' and 'private_key_file' must be both set or both unset"); + assertThat(npe).hasMessageThat().isEqualTo("'private_key_file' is required in the config"); } } @Test - public void createProvider_missingRootAndSpiffeConfig() throws IOException { + public void createProvider_missingRoot_expectException() throws IOException { + String expectedMessage = enableSpiffe ? "either 'ca_certificate_file' or " + + "'spiffe_trust_bundle_map_file' is required in the config" + : "'ca_certificate_file' is required in the config"; CertificateProvider.DistributorWatcher distWatcher = new CertificateProvider.DistributorWatcher(); @SuppressWarnings("unchecked") Map map = (Map) JsonParser.parse(MISSING_ROOT_AND_SPIFFE_CONFIG); - ScheduledExecutorService mockService = mock(ScheduledExecutorService.class); - when(scheduledExecutorServiceFactory.create()).thenReturn(mockService); - provider.createCertificateProvider(map, distWatcher, true); - verify(fileWatcherCertificateProviderFactory, times(1)) - .create( - eq(distWatcher), - eq(true), - eq("/var/run/gke-spiffe/certs/certificates.pem"), - eq("/var/run/gke-spiffe/certs/private_key.pem"), - eq(null), - eq(null), - eq(600L), - eq(mockService), - eq(timeProvider)); - } - - @Test - public void createProvider_missingCertAndKeyConfig() throws IOException { - CertificateProvider.DistributorWatcher distWatcher = - new CertificateProvider.DistributorWatcher(); - @SuppressWarnings("unchecked") - Map map = (Map) JsonParser.parse(MISSING_CERT_AND_KEY_CONFIG); - ScheduledExecutorService mockService = mock(ScheduledExecutorService.class); - when(scheduledExecutorServiceFactory.create()).thenReturn(mockService); - provider.createCertificateProvider(map, distWatcher, true); - verify(fileWatcherCertificateProviderFactory, times(1)) - .create( - eq(distWatcher), - eq(true), - eq(null), - eq(null), - eq("/var/run/gke-spiffe/certs/ca_certificates.pem"), - eq(null), - eq(600L), - eq(mockService), - eq(timeProvider)); - } - - @Test - public void createProvider_emptyConfig_expectException() throws IOException { - CertificateProvider.DistributorWatcher distWatcher = - new CertificateProvider.DistributorWatcher(); - @SuppressWarnings("unchecked") - Map map = (Map) JsonParser.parse(EMPTY_CONFIG); try { provider.createCertificateProvider(map, distWatcher, true); fail("exception expected"); } catch (NullPointerException npe) { - assertThat(npe) - .hasMessageThat() - .isEqualTo("must be watching either root or identity certificates"); + assertThat(npe).hasMessageThat().isEqualTo(expectedMessage); } } @@ -339,13 +292,6 @@ public void createProvider_emptyConfig_expectException() throws IOException { + " \"private_key_file\": \"/var/run/gke-spiffe/certs/private_key.pem\"" + " }"; - private static final String MISSING_CERT_AND_KEY_CONFIG = - "{\n" - + " \"ca_certificate_file\": \"/var/run/gke-spiffe/certs/ca_certificates.pem\"" - + " }"; - - private static final String EMPTY_CONFIG = "{\n}"; - private static final String ZERO_REFRESH_INTERVAL = "{\n" + " \"certificate_file\": \"/var/run/gke-spiffe/certs/certificates2.pem\"," diff --git a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderTest.java index d566bee2f28..620ee0a7ff7 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderTest.java @@ -25,8 +25,6 @@ import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SPIFFE_TRUST_MAP_1_FILE; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; @@ -359,33 +357,6 @@ public void getCertificate_missingRootFile() throws IOException, InterruptedExce verifyWatcherErrorUpdates(Status.Code.UNKNOWN, NoSuchFileException.class, 1, 0, "root.pem"); } - @Test - public void illegalConstructorArguments_MissingPrivateKeyWhileCertChainPresent() - throws IllegalArgumentException { - Exception ex = assertThrows( - IllegalArgumentException.class, - () -> new FileWatcherCertificateProvider( - watcher, true, null, keyFile, rootFile, null, 600L, timeService, timeProvider)); - - String expectedMsg = "certFile and keyFile must be both set or both unset"; - String actualMsg = ex.getMessage(); - - assertEquals(expectedMsg, actualMsg); - } - - @Test - public void illegalConstructorArguments_NoFilesToWatch() throws IllegalArgumentException { - Exception ex = assertThrows( - IllegalArgumentException.class, - () -> new FileWatcherCertificateProvider( - watcher, true, null, null, null, null, 600L, timeService, timeProvider)); - - String expectedMsg = "must be watching either root or identity certificates"; - String actualMsg = ex.getMessage(); - - assertEquals(expectedMsg, actualMsg); - } - private void commonErrorTest( String certFile, String keyFile, From afbc96ee76a82b71ffeb976e7c258796cb79089e Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 3 Sep 2025 12:21:28 +0000 Subject: [PATCH 09/10] Added refresh interval support --- .../internal/TlsXdsCredentialsProvider.java | 51 ++++++++++++++++++- .../TlsXdsCredentialsProviderTest.java | 10 +++- 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java index f697af0e09d..72db592c430 100644 --- a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java @@ -16,14 +16,21 @@ package io.grpc.xds.internal; +import com.google.protobuf.Duration; +import com.google.protobuf.util.Durations; import io.grpc.ChannelCredentials; import io.grpc.TlsChannelCredentials; +import io.grpc.internal.GrpcUtil; import io.grpc.internal.JsonUtil; import io.grpc.util.AdvancedTlsX509KeyManager; import io.grpc.util.AdvancedTlsX509TrustManager; import io.grpc.xds.XdsCredentialsProvider; import java.io.File; +import java.text.ParseException; import java.util.Map; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -37,6 +44,10 @@ public final class TlsXdsCredentialsProvider extends XdsCredentialsProvider { private static final String CERT_FILE_KEY = "certificate_file"; private static final String KEY_FILE_KEY = "private_key_file"; private static final String ROOT_FILE_KEY = "ca_certificate_file"; + private static final String REFRESH_INTERVAL_KEY = "refresh_interval"; + private static final long REFRESH_INTERVAL_DEFAULT = 600L; + private static final ScheduledExecutorServiceFactory scheduledExecutorServiceFactory = + ScheduledExecutorServiceFactory.DEFAULT_INSTANCE; @Override protected ChannelCredentials newChannelCredentials(Map jsonConfig) { @@ -46,12 +57,29 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { return builder.build(); } + // use refresh interval from bootstrap config if provided; else defaults to 600s + long refreshIntervalSeconds = REFRESH_INTERVAL_DEFAULT; + String refreshIntervalFromConfig = JsonUtil.getString(jsonConfig, REFRESH_INTERVAL_KEY); + if (refreshIntervalFromConfig != null) { + try { + Duration duration = Durations.parse(refreshIntervalFromConfig); + refreshIntervalSeconds = Durations.toSeconds(duration); + } catch (ParseException e) { + logger.log(Level.WARNING, "Unable to parse refresh interval", e); + return null; + } + } + // use trust certificate file path from bootstrap config if provided; else use system default String rootCertPath = JsonUtil.getString(jsonConfig, ROOT_FILE_KEY); if (rootCertPath != null) { try { AdvancedTlsX509TrustManager trustManager = AdvancedTlsX509TrustManager.newBuilder().build(); - trustManager.updateTrustCredentials(new File(rootCertPath)); + trustManager.updateTrustCredentials( + new File(rootCertPath), + refreshIntervalSeconds, + TimeUnit.SECONDS, + scheduledExecutorServiceFactory.create()); builder.trustManager(trustManager); } catch (Exception e) { logger.log(Level.WARNING, "Unable to read root certificates", e); @@ -66,7 +94,12 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { if (certChainPath != null && privateKeyPath != null) { try { AdvancedTlsX509KeyManager keyManager = new AdvancedTlsX509KeyManager(); - keyManager.updateIdentityCredentials(new File(certChainPath), new File(privateKeyPath)); + keyManager.updateIdentityCredentials( + new File(certChainPath), + new File(privateKeyPath), + refreshIntervalSeconds, + TimeUnit.SECONDS, + scheduledExecutorServiceFactory.create()); builder.keyManager(keyManager); } catch (Exception e) { logger.log(Level.WARNING, "Unable to read certificate chain or private key", e); @@ -95,4 +128,18 @@ public int priority() { return 5; } + abstract static class ScheduledExecutorServiceFactory { + + private static final ScheduledExecutorServiceFactory DEFAULT_INSTANCE = + new ScheduledExecutorServiceFactory() { + + @Override + ScheduledExecutorService create() { + return Executors.newSingleThreadScheduledExecutor( + GrpcUtil.getThreadFactory("grpc-certificate-files-%d", true)); + } + }; + + abstract ScheduledExecutorService create(); + } } diff --git a/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java index 85347202036..7a480c0b81f 100644 --- a/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java @@ -69,6 +69,13 @@ public void channelCredentialsWhenNullConfig() { provider.newChannelCredentials(null).getClass()); } + @Test + public void channelCredentialsWhenInvalidRefreshInterval() { + Map jsonConfig = ImmutableMap.of( + "refresh_interval", "invalid-duration-format"); + assertNull(provider.newChannelCredentials(jsonConfig)); + } + @Test public void channelCredentialsWhenNotExistingTrustFileConfig() { Map jsonConfig = ImmutableMap.of( @@ -100,7 +107,8 @@ public void channelCredentialsWhenValidConfig() throws Exception { Map jsonConfig = ImmutableMap.of( "ca_certificate_file", rootCertPath, "certificate_file", certChainPath, - "private_key_file", privateKeyPath); + "private_key_file", privateKeyPath, + "refresh_interval", "440s"); ChannelCredentials creds = provider.newChannelCredentials(jsonConfig); assertSame(TlsChannelCredentials.class, creds.getClass()); From 434579a8b376c5046467e222bc3cce2a422bceb7 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 10 Sep 2025 14:03:21 +0000 Subject: [PATCH 10/10] Added support for allocated resources management --- .../internal/TlsXdsCredentialsProvider.java | 40 ++++++++++++++----- .../TlsXdsCredentialsProviderTest.java | 13 +++++- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java index 72db592c430..d5f62a24e1f 100644 --- a/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/TlsXdsCredentialsProvider.java @@ -16,15 +16,18 @@ package io.grpc.xds.internal; +import com.google.common.collect.ImmutableList; import com.google.protobuf.Duration; import com.google.protobuf.util.Durations; import io.grpc.ChannelCredentials; +import io.grpc.ResourceAllocatingChannelCredentials; import io.grpc.TlsChannelCredentials; import io.grpc.internal.GrpcUtil; import io.grpc.internal.JsonUtil; import io.grpc.util.AdvancedTlsX509KeyManager; import io.grpc.util.AdvancedTlsX509TrustManager; import io.grpc.xds.XdsCredentialsProvider; +import java.io.Closeable; import java.io.File; import java.text.ParseException; import java.util.Map; @@ -51,10 +54,10 @@ public final class TlsXdsCredentialsProvider extends XdsCredentialsProvider { @Override protected ChannelCredentials newChannelCredentials(Map jsonConfig) { - TlsChannelCredentials.Builder builder = TlsChannelCredentials.newBuilder(); + TlsChannelCredentials.Builder tlsChannelCredsBuilder = TlsChannelCredentials.newBuilder(); if (jsonConfig == null) { - return builder.build(); + return tlsChannelCredsBuilder.build(); } // use refresh interval from bootstrap config if provided; else defaults to 600s @@ -70,17 +73,22 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { } } + ImmutableList.Builder resourcesBuilder = ImmutableList.builder(); + ScheduledExecutorService scheduledExecutorService = null; + // use trust certificate file path from bootstrap config if provided; else use system default String rootCertPath = JsonUtil.getString(jsonConfig, ROOT_FILE_KEY); if (rootCertPath != null) { try { + scheduledExecutorService = scheduledExecutorServiceFactory.create(); AdvancedTlsX509TrustManager trustManager = AdvancedTlsX509TrustManager.newBuilder().build(); - trustManager.updateTrustCredentials( + Closeable trustManagerFuture = trustManager.updateTrustCredentials( new File(rootCertPath), refreshIntervalSeconds, TimeUnit.SECONDS, - scheduledExecutorServiceFactory.create()); - builder.trustManager(trustManager); + scheduledExecutorService); + resourcesBuilder.add(trustManagerFuture); + tlsChannelCredsBuilder.trustManager(trustManager); } catch (Exception e) { logger.log(Level.WARNING, "Unable to read root certificates", e); return null; @@ -93,14 +101,18 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { String privateKeyPath = JsonUtil.getString(jsonConfig, KEY_FILE_KEY); if (certChainPath != null && privateKeyPath != null) { try { + if (scheduledExecutorService == null) { + scheduledExecutorService = scheduledExecutorServiceFactory.create(); + } AdvancedTlsX509KeyManager keyManager = new AdvancedTlsX509KeyManager(); - keyManager.updateIdentityCredentials( + Closeable keyManagerFuture = keyManager.updateIdentityCredentials( new File(certChainPath), new File(privateKeyPath), refreshIntervalSeconds, TimeUnit.SECONDS, - scheduledExecutorServiceFactory.create()); - builder.keyManager(keyManager); + scheduledExecutorService); + resourcesBuilder.add(keyManagerFuture); + tlsChannelCredsBuilder.keyManager(keyManager); } catch (Exception e) { logger.log(Level.WARNING, "Unable to read certificate chain or private key", e); return null; @@ -110,7 +122,17 @@ protected ChannelCredentials newChannelCredentials(Map jsonConfig) { return null; } - return builder.build(); + // if executor was initialized, add it to allocated resource list + if (scheduledExecutorService != null) { + resourcesBuilder.add(asCloseable(scheduledExecutorService)); + } + + return ResourceAllocatingChannelCredentials.create( + tlsChannelCredsBuilder.build(), resourcesBuilder.build()); + } + + private static Closeable asCloseable(ScheduledExecutorService scheduledExecutorService) { + return () -> scheduledExecutorService.shutdownNow(); } @Override diff --git a/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java index 7a480c0b81f..553559490a4 100644 --- a/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/TlsXdsCredentialsProviderTest.java @@ -29,11 +29,13 @@ import com.google.common.collect.Iterables; import io.grpc.ChannelCredentials; import io.grpc.InternalServiceProviders; +import io.grpc.ResourceAllocatingChannelCredentials; import io.grpc.TlsChannelCredentials; import io.grpc.internal.testing.TestUtils; import io.grpc.util.AdvancedTlsX509KeyManager; import io.grpc.util.AdvancedTlsX509TrustManager; import io.grpc.xds.XdsCredentialsProvider; +import java.io.Closeable; import java.util.Map; import javax.net.ssl.KeyManager; import javax.net.ssl.TrustManager; @@ -111,13 +113,20 @@ public void channelCredentialsWhenValidConfig() throws Exception { "refresh_interval", "440s"); ChannelCredentials creds = provider.newChannelCredentials(jsonConfig); - assertSame(TlsChannelCredentials.class, creds.getClass()); - TlsChannelCredentials tlsChannelCredentials = (TlsChannelCredentials) creds; + assertSame(ResourceAllocatingChannelCredentials.class, creds.getClass()); + ResourceAllocatingChannelCredentials resourceAllocatingChannelCredentials = + (ResourceAllocatingChannelCredentials) creds; + TlsChannelCredentials tlsChannelCredentials = + (TlsChannelCredentials) resourceAllocatingChannelCredentials.getChannelCredentials(); assertThat(tlsChannelCredentials.getKeyManagers()).hasSize(1); KeyManager keyManager = Iterables.getOnlyElement(tlsChannelCredentials.getKeyManagers()); assertThat(keyManager).isInstanceOf(AdvancedTlsX509KeyManager.class); assertThat(tlsChannelCredentials.getTrustManagers()).hasSize(1); TrustManager trustManager = Iterables.getOnlyElement(tlsChannelCredentials.getTrustManagers()); assertThat(trustManager).isInstanceOf(AdvancedTlsX509TrustManager.class); + assertThat(resourceAllocatingChannelCredentials.getAllocatedResources()).hasSize(3); + for (Closeable resource : resourceAllocatingChannelCredentials.getAllocatedResources()) { + resource.close(); + } } }