Skip to content

Commit

Permalink
xds: bootstrapper fixes: remove extra readBootstrap & avoid parseConf…
Browse files Browse the repository at this point in the history
…ig (grpc#7436)
  • Loading branch information
sanjaypujare authored and dfawley committed Jan 15, 2021
1 parent bb01c4b commit 82549cb
Show file tree
Hide file tree
Showing 12 changed files with 269 additions and 357 deletions.
2 changes: 1 addition & 1 deletion xds/src/main/java/io/grpc/xds/Bootstrapper.java
Expand Up @@ -84,7 +84,7 @@ public static Bootstrapper getInstance() {
/** Parses a raw string into {@link BootstrapInfo}. */
@VisibleForTesting
@SuppressWarnings("unchecked")
public static BootstrapInfo parseConfig(String rawData) throws XdsInitializationException {
static BootstrapInfo parseConfig(String rawData) throws XdsInitializationException {
XdsLogger logger = XdsLogger.withPrefix(LOG_PREFIX);
logger.log(XdsLogLevel.INFO, "Reading bootstrap information");
Map<String, ?> rawBootstrap;
Expand Down
Expand Up @@ -34,8 +34,8 @@ final class ClientSslContextProviderFactory
private final CertProviderClientSslContextProvider.Factory
certProviderClientSslContextProviderFactory;

ClientSslContextProviderFactory() {
this(Bootstrapper.getInstance(), CertProviderClientSslContextProvider.Factory.getInstance());
ClientSslContextProviderFactory(Bootstrapper bootstrapper) {
this(bootstrapper, CertProviderClientSslContextProvider.Factory.getInstance());
}

ClientSslContextProviderFactory(
Expand All @@ -58,7 +58,7 @@ public SslContextProvider create(UpstreamTlsContext upstreamTlsContext) {
try {
return SdsClientSslContextProvider.getProvider(
upstreamTlsContext,
Bootstrapper.getInstance().readBootstrap().getNode().toEnvoyProtoNodeV2(),
bootstrapper.readBootstrap().getNode().toEnvoyProtoNodeV2(),
Executors.newSingleThreadExecutor(new ThreadFactoryBuilder()
.setNameFormat("client-sds-sslcontext-provider-%d")
.setDaemon(true)
Expand All @@ -70,10 +70,11 @@ public SslContextProvider create(UpstreamTlsContext upstreamTlsContext) {
} else if (CommonTlsContextUtil.hasCertProviderInstance(
upstreamTlsContext.getCommonTlsContext())) {
try {
Bootstrapper.BootstrapInfo bootstrapInfo = bootstrapper.readBootstrap();
return certProviderClientSslContextProviderFactory.getProvider(
upstreamTlsContext,
bootstrapper.readBootstrap().getNode().toEnvoyProtoNode(),
bootstrapper.readBootstrap().getCertProviders());
bootstrapInfo.getNode().toEnvoyProtoNode(),
bootstrapInfo.getCertProviders());
} catch (XdsInitializationException e) {
throw new RuntimeException(e);
}
Expand Down
Expand Up @@ -34,8 +34,8 @@ final class ServerSslContextProviderFactory
private final CertProviderServerSslContextProvider.Factory
certProviderServerSslContextProviderFactory;

ServerSslContextProviderFactory() {
this(Bootstrapper.getInstance(), CertProviderServerSslContextProvider.Factory.getInstance());
ServerSslContextProviderFactory(Bootstrapper bootstrapper) {
this(bootstrapper, CertProviderServerSslContextProvider.Factory.getInstance());
}

ServerSslContextProviderFactory(
Expand All @@ -60,7 +60,7 @@ public SslContextProvider create(
try {
return SdsServerSslContextProvider.getProvider(
downstreamTlsContext,
Bootstrapper.getInstance().readBootstrap().getNode().toEnvoyProtoNodeV2(),
bootstrapper.readBootstrap().getNode().toEnvoyProtoNodeV2(),
Executors.newSingleThreadExecutor(new ThreadFactoryBuilder()
.setNameFormat("server-sds-sslcontext-provider-%d")
.setDaemon(true)
Expand All @@ -72,10 +72,11 @@ public SslContextProvider create(
} else if (CommonTlsContextUtil.hasCertProviderInstance(
downstreamTlsContext.getCommonTlsContext())) {
try {
Bootstrapper.BootstrapInfo bootstrapInfo = bootstrapper.readBootstrap();
return certProviderServerSslContextProviderFactory.getProvider(
downstreamTlsContext,
bootstrapper.readBootstrap().getNode().toEnvoyProtoNode(),
bootstrapper.readBootstrap().getCertProviders());
bootstrapInfo.getNode().toEnvoyProtoNode(),
bootstrapInfo.getCertProviders());
} catch (XdsInitializationException e) {
throw new RuntimeException(e);
}
Expand Down
Expand Up @@ -19,6 +19,7 @@
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.VisibleForTesting;
import io.grpc.xds.Bootstrapper;
import io.grpc.xds.EnvoyServerProtoData.DownstreamTlsContext;
import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
import io.grpc.xds.internal.sds.ReferenceCountingMap.ValueFactory;
Expand All @@ -36,8 +37,11 @@ public final class TlsContextManagerImpl implements TlsContextManager {
private final ReferenceCountingMap<UpstreamTlsContext, SslContextProvider> mapForClients;
private final ReferenceCountingMap<DownstreamTlsContext, SslContextProvider> mapForServers;

private TlsContextManagerImpl() {
this(new ClientSslContextProviderFactory(), new ServerSslContextProviderFactory());
/** Create a TlsContextManagerImpl instance using the passed in {@link Bootstrapper}. */
@VisibleForTesting public TlsContextManagerImpl(Bootstrapper bootstrapper) {
this(
new ClientSslContextProviderFactory(bootstrapper),
new ServerSslContextProviderFactory(bootstrapper));
}

@VisibleForTesting
Expand All @@ -53,7 +57,7 @@ private TlsContextManagerImpl() {
/** Gets the TlsContextManagerImpl singleton. */
public static synchronized TlsContextManagerImpl getInstance() {
if (instance == null) {
instance = new TlsContextManagerImpl();
instance = new TlsContextManagerImpl(Bootstrapper.getInstance());
}
return instance;
}
Expand Down
74 changes: 74 additions & 0 deletions xds/src/test/java/io/grpc/xds/CommonBootstrapperTestUtils.java
@@ -0,0 +1,74 @@
/*
* Copyright 2020 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.xds;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import io.grpc.internal.JsonParser;
import java.io.IOException;
import java.util.Map;

public class CommonBootstrapperTestUtils {
private static final String FILE_WATCHER_CONFIG = "{\"path\": \"/etc/secret/certs\"}";
private static final String MESHCA_CONFIG =
"{\n"
+ " \"server\": {\n"
+ " \"api_type\": \"GRPC\",\n"
+ " \"grpc_services\": [{\n"
+ " \"google_grpc\": {\n"
+ " \"target_uri\": \"meshca.com\",\n"
+ " \"channel_credentials\": {\"google_default\": {}},\n"
+ " \"call_credentials\": [{\n"
+ " \"sts_service\": {\n"
+ " \"token_exchange_service\": \"securetoken.googleapis.com\",\n"
+ " \"subject_token_path\": \"/etc/secret/sajwt.token\"\n"
+ " }\n"
+ " }]\n" // end call_credentials
+ " },\n" // end google_grpc
+ " \"time_out\": {\"seconds\": 10}\n"
+ " }]\n" // end grpc_services
+ " },\n" // end server
+ " \"certificate_lifetime\": {\"seconds\": 86400},\n"
+ " \"renewal_grace_period\": {\"seconds\": 3600},\n"
+ " \"key_type\": \"RSA\",\n"
+ " \"key_size\": 2048,\n"
+ " \"location\": \"https://container.googleapis.com/v1/project/test-project1/locations/test-zone2/clusters/test-cluster3\"\n"
+ " }";

/** Creates a test bootstrap info object. */
@SuppressWarnings("unchecked")
public static Bootstrapper.BootstrapInfo getTestBootstrapInfo() {
try {
Bootstrapper.CertificateProviderInfo gcpId =
new Bootstrapper.CertificateProviderInfo(
"testca", (Map<String, ?>) JsonParser.parse(MESHCA_CONFIG));
Bootstrapper.CertificateProviderInfo fileProvider =
new Bootstrapper.CertificateProviderInfo(
"file_watcher", (Map<String, ?>) JsonParser.parse(FILE_WATCHER_CONFIG));
Map<String, Bootstrapper.CertificateProviderInfo> certProviders =
ImmutableMap.of("gcp_id", gcpId, "file_provider", fileProvider);
Bootstrapper.BootstrapInfo bootstrapInfo =
new Bootstrapper.BootstrapInfo(
ImmutableList.<Bootstrapper.ServerInfo>of(),
EnvoyProtoData.Node.newBuilder().build(),
certProviders);
return bootstrapInfo;
} catch (IOException e) {
throw new AssertionError(e);
}
}
}
4 changes: 3 additions & 1 deletion xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java
Expand Up @@ -80,10 +80,12 @@ public class XdsSdsClientServerTest {
@Rule public final GrpcCleanupRule cleanupRule = new GrpcCleanupRule();
private int port;
private FakeNameResolverFactory fakeNameResolverFactory;
private Bootstrapper mockBootstrapper;

@Before
public void setUp() throws IOException {
port = XdsServerTestHelper.findFreePort();
mockBootstrapper = mock(Bootstrapper.class);
}

@After
Expand Down Expand Up @@ -367,7 +369,7 @@ private SimpleServiceGrpc.SimpleServiceBlockingStub getBlockingStub(
? Attributes.newBuilder()
.set(XdsAttributes.ATTR_SSL_CONTEXT_PROVIDER_SUPPLIER,
new SslContextProviderSupplier(
upstreamTlsContext, TlsContextManagerImpl.getInstance()))
upstreamTlsContext, new TlsContextManagerImpl(mockBootstrapper)))
.build()
: Attributes.EMPTY;
fakeNameResolverFactory.setServers(
Expand Down
Expand Up @@ -34,6 +34,7 @@
import io.envoyproxy.envoy.config.core.v3.DataSource;
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
import io.grpc.xds.Bootstrapper;
import io.grpc.xds.CommonBootstrapperTestUtils;
import io.grpc.xds.EnvoyServerProtoData;
import io.grpc.xds.internal.sds.CommonTlsContextTestsUtil;
import io.grpc.xds.internal.sds.CommonTlsContextTestsUtil.TestCallback;
Expand Down Expand Up @@ -96,7 +97,7 @@ public void testProviderForClient_mtls() throws Exception {
getSslContextProvider(
"gcp_id",
"gcp_id",
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null);

Expand Down Expand Up @@ -159,7 +160,7 @@ public void testProviderForClient_queueExecutor() throws Exception {
getSslContextProvider(
"gcp_id",
"gcp_id",
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null);
QueuedExecutor queuedExecutor = new QueuedExecutor();
Expand Down Expand Up @@ -192,7 +193,7 @@ public void testProviderForClient_tls() throws Exception {
getSslContextProvider(
/* certInstanceName= */ null,
"gcp_id",
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null);

Expand Down Expand Up @@ -229,7 +230,7 @@ public void testProviderForClient_sslContextException_onError() throws Exception
getSslContextProvider(
/* certInstanceName= */ null,
"gcp_id",
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */null,
staticCertValidationContext);

Expand Down Expand Up @@ -260,7 +261,7 @@ public void testProviderForClient_rootInstanceNull_expectError() throws Exceptio
getSslContextProvider(
/* certInstanceName= */ null,
/* rootInstanceName= */ null,
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null);
fail("exception expected");
Expand Down
Expand Up @@ -32,6 +32,7 @@
import io.envoyproxy.envoy.config.core.v3.DataSource;
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
import io.grpc.xds.Bootstrapper;
import io.grpc.xds.CommonBootstrapperTestUtils;
import io.grpc.xds.EnvoyServerProtoData;
import io.grpc.xds.internal.certprovider.CertProviderClientSslContextProviderTest.QueuedExecutor;
import io.grpc.xds.internal.sds.CommonTlsContextTestsUtil;
Expand Down Expand Up @@ -90,7 +91,7 @@ public void testProviderForServer_mtls() throws Exception {
getSslContextProvider(
"gcp_id",
"gcp_id",
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null,
/* requireClientCert= */ true);
Expand Down Expand Up @@ -154,7 +155,7 @@ public void testProviderForServer_queueExecutor() throws Exception {
getSslContextProvider(
"gcp_id",
"gcp_id",
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null,
/* requireClientCert= */ true);
Expand Down Expand Up @@ -188,7 +189,7 @@ public void testProviderForServer_tls() throws Exception {
getSslContextProvider(
"gcp_id",
/* rootInstanceName= */ null,
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null,
/* requireClientCert= */ false);
Expand Down Expand Up @@ -229,7 +230,7 @@ public void testProviderForServer_sslContextException_onError() throws Exception
getSslContextProvider(
/* certInstanceName= */ "gcp_id",
/* rootInstanceName= */ "gcp_id",
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */null,
staticCertValidationContext,
/* requireClientCert= */ true);
Expand Down Expand Up @@ -266,7 +267,7 @@ public void testProviderForServer_certInstanceNull_expectError() throws Exceptio
getSslContextProvider(
/* certInstanceName= */ null,
/* rootInstanceName= */ null,
CommonCertProviderTestUtils.getTestBootstrapInfo(),
CommonBootstrapperTestUtils.getTestBootstrapInfo(),
/* alpnProtocols= */ null,
/* staticCertValidationContext= */ null,
/* requireClientCert= */ false);
Expand Down

0 comments on commit 82549cb

Please sign in to comment.