From 6b80efcfa87a61ad21f2bf71d926d5f5f3801971 Mon Sep 17 00:00:00 2001 From: sanjaypujare Date: Tue, 4 Oct 2022 12:41:17 -0700 Subject: [PATCH] xds: security code refactoring: delete unused code and rename misc things (#9583) --- .../security/SecurityProtocolNegotiators.java | 6 +- .../internal/security/SslContextProvider.java | 4 +- .../security/SslContextProviderSupplier.java | 4 +- .../java/io/grpc/xds/XdsClientTestHelper.java | 137 ------------------ .../security/CommonTlsContextTestsUtil.java | 2 +- .../SecurityProtocolNegotiatorsTest.java | 6 +- .../SslContextProviderSupplierTest.java | 4 +- 7 files changed, 13 insertions(+), 150 deletions(-) delete mode 100644 xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java diff --git a/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java b/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java index 0c1271d1dc..08f2e86fb6 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java @@ -208,10 +208,10 @@ public final class SecurityProtocolNegotiators { new SslContextProvider.Callback(ctx.executor()) { @Override - public void updateSecret(SslContext sslContext) { + public void updateSslContext(SslContext sslContext) { logger.log( Level.FINEST, - "ClientSdsHandler.updateSecret authority={0}, ctx.name={1}", + "ClientSdsHandler.updateSslContext authority={0}, ctx.name={1}", new Object[]{grpcHandler.getAuthority(), ctx.name()}); ChannelHandler handler = InternalProtocolNegotiators.tls(sslContext).newHandler(grpcHandler); @@ -347,7 +347,7 @@ public final class SecurityProtocolNegotiators { new SslContextProvider.Callback(ctx.executor()) { @Override - public void updateSecret(SslContext sslContext) { + public void updateSslContext(SslContext sslContext) { ChannelHandler handler = InternalProtocolNegotiators.serverTls(sslContext).newHandler(grpcHandler); diff --git a/xds/src/main/java/io/grpc/xds/internal/security/SslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/SslContextProvider.java index 7544f5d9fc..a0c4ed37df 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/SslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/SslContextProvider.java @@ -57,7 +57,7 @@ public abstract class SslContextProvider implements Closeable { } /** Informs callee of new/updated SslContext. */ - @VisibleForTesting public abstract void updateSecret(SslContext sslContext); + @VisibleForTesting public abstract void updateSslContext(SslContext sslContext); /** Informs callee of an exception that was generated. */ @VisibleForTesting protected abstract void onException(Throwable throwable); @@ -120,7 +120,7 @@ public abstract class SslContextProvider implements Closeable { public void run() { try { SslContext sslContext = sslContextGetter.get(); - callback.updateSecret(sslContext); + callback.updateSslContext(sslContext); } catch (Throwable e) { callback.onException(e); } diff --git a/xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java b/xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java index e429eff44a..5f62927317 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java @@ -66,8 +66,8 @@ public final class SslContextProviderSupplier implements Closeable { new SslContextProvider.Callback(callback.getExecutor()) { @Override - public void updateSecret(SslContext sslContext) { - callback.updateSecret(sslContext); + public void updateSslContext(SslContext sslContext) { + callback.updateSslContext(sslContext); releaseSslContextProvider(toRelease); } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java deleted file mode 100644 index 9ea9b1f8dc..0000000000 --- a/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java +++ /dev/null @@ -1,137 +0,0 @@ -/* - * Copyright 2019 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.protobuf.Any; -import io.envoyproxy.envoy.config.core.v3.Address; -import io.envoyproxy.envoy.config.listener.v3.ApiListener; -import io.envoyproxy.envoy.config.listener.v3.FilterChain; -import io.envoyproxy.envoy.config.listener.v3.Listener; -import io.envoyproxy.envoy.config.route.v3.Route; -import io.envoyproxy.envoy.config.route.v3.RouteAction; -import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; -import io.envoyproxy.envoy.config.route.v3.RouteMatch; -import io.envoyproxy.envoy.config.route.v3.VirtualHost; -import io.envoyproxy.envoy.service.discovery.v3.DiscoveryRequest; -import io.envoyproxy.envoy.service.discovery.v3.DiscoveryResponse; -import io.grpc.xds.EnvoyProtoData.Node; -import java.util.List; - -/** - * Helper methods for building protobuf messages with custom data for xDS protocols. - */ -// TODO(chengyuanzhang, sanjaypujare): delete this class, should not dump everything here. -class XdsClientTestHelper { - static DiscoveryResponse buildDiscoveryResponse(String versionInfo, - List resources, String typeUrl, String nonce) { - return - DiscoveryResponse.newBuilder() - .setVersionInfo(versionInfo) - .setTypeUrl(typeUrl) - .addAllResources(resources) - .setNonce(nonce) - .build(); - } - - static io.envoyproxy.envoy.api.v2.DiscoveryResponse buildDiscoveryResponseV2(String versionInfo, - List resources, String typeUrl, String nonce) { - return - io.envoyproxy.envoy.api.v2.DiscoveryResponse.newBuilder() - .setVersionInfo(versionInfo) - .setTypeUrl(typeUrl) - .addAllResources(resources) - .setNonce(nonce) - .build(); - } - - static DiscoveryRequest buildDiscoveryRequest(Node node, String versionInfo, - List resourceNames, String typeUrl, String nonce) { - return - DiscoveryRequest.newBuilder() - .setVersionInfo(versionInfo) - .setNode(node.toEnvoyProtoNode()) - .setTypeUrl(typeUrl) - .addAllResourceNames(resourceNames) - .setResponseNonce(nonce) - .build(); - } - - static Listener buildListener(String name, com.google.protobuf.Any apiListener) { - return - Listener.newBuilder() - .setName(name) - .setAddress(Address.getDefaultInstance()) - .addFilterChains(FilterChain.getDefaultInstance()) - .setApiListener(ApiListener.newBuilder().setApiListener(apiListener)) - .build(); - } - - static io.envoyproxy.envoy.api.v2.Listener buildListenerV2( - String name, com.google.protobuf.Any apiListener) { - return - io.envoyproxy.envoy.api.v2.Listener.newBuilder() - .setName(name) - .setAddress(io.envoyproxy.envoy.api.v2.core.Address.getDefaultInstance()) - .addFilterChains(io.envoyproxy.envoy.api.v2.listener.FilterChain.getDefaultInstance()) - .setApiListener(io.envoyproxy.envoy.config.listener.v2.ApiListener.newBuilder() - .setApiListener(apiListener)) - .build(); - } - - static RouteConfiguration buildRouteConfiguration(String name, - List virtualHosts) { - return - RouteConfiguration.newBuilder() - .setName(name) - .addAllVirtualHosts(virtualHosts) - .build(); - } - - static io.envoyproxy.envoy.api.v2.RouteConfiguration buildRouteConfigurationV2(String name, - List virtualHosts) { - return - io.envoyproxy.envoy.api.v2.RouteConfiguration.newBuilder() - .setName(name) - .addAllVirtualHosts(virtualHosts) - .build(); - } - - static VirtualHost buildVirtualHost(List domains, String clusterName) { - return VirtualHost.newBuilder() - .setName("virtualhost00.googleapis.com") // don't care - .addAllDomains(domains) - .addRoutes( - Route.newBuilder() - .setRoute(RouteAction.newBuilder().setCluster(clusterName)) - .setMatch(RouteMatch.newBuilder().setPrefix(""))) - .build(); - } - - static io.envoyproxy.envoy.api.v2.route.VirtualHost buildVirtualHostV2( - List domains, String clusterName) { - return io.envoyproxy.envoy.api.v2.route.VirtualHost.newBuilder() - .setName("virtualhost00.googleapis.com") // don't care - .addAllDomains(domains) - .addRoutes( - io.envoyproxy.envoy.api.v2.route.Route.newBuilder() - .setRoute( - io.envoyproxy.envoy.api.v2.route.RouteAction.newBuilder() - .setCluster(clusterName)) - .setMatch(io.envoyproxy.envoy.api.v2.route.RouteMatch.newBuilder().setPrefix(""))) - .build(); - } -} diff --git a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java index 81b267587f..728bb06efe 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java @@ -424,7 +424,7 @@ public class CommonTlsContextTestsUtil { } @Override - public void updateSecret(SslContext sslContext) { + public void updateSslContext(SslContext sslContext) { updatedSslContext = sslContext; } diff --git a/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java b/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java index 0531189f2a..863e4dcfec 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java @@ -168,7 +168,7 @@ public class SecurityProtocolNegotiatorsTest { sslContextProviderSupplier .updateSslContext(new SslContextProvider.Callback(MoreExecutors.directExecutor()) { @Override - public void updateSecret(SslContext sslContext) { + public void updateSslContext(SslContext sslContext) { future.set(sslContext); } @@ -245,7 +245,7 @@ public class SecurityProtocolNegotiatorsTest { sslContextProviderSupplier .updateSslContext(new SslContextProvider.Callback(MoreExecutors.directExecutor()) { @Override - public void updateSecret(SslContext sslContext) { + public void updateSslContext(SslContext sslContext) { future.set(sslContext); } @@ -381,7 +381,7 @@ public class SecurityProtocolNegotiatorsTest { sslContextProviderSupplier .updateSslContext(new SslContextProvider.Callback(MoreExecutors.directExecutor()) { @Override - public void updateSecret(SslContext sslContext) { + public void updateSslContext(SslContext sslContext) { future.set(sslContext); } diff --git a/xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java b/xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java index 35c1437d34..8030b458d2 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java @@ -85,8 +85,8 @@ public class SslContextProviderSupplierTest { SslContextProvider.Callback capturedCallback = callbackCaptor.getValue(); assertThat(capturedCallback).isNotNull(); SslContext mockSslContext = mock(SslContext.class); - capturedCallback.updateSecret(mockSslContext); - verify(mockCallback, times(1)).updateSecret(eq(mockSslContext)); + capturedCallback.updateSslContext(mockSslContext); + verify(mockCallback, times(1)).updateSslContext(eq(mockSslContext)); verify(mockTlsContextManager, times(1)) .releaseClientSslContextProvider(eq(mockSslContextProvider)); SslContextProvider.Callback mockCallback = mock(SslContextProvider.Callback.class);