From 09367030ae80b2f5e59e1ab7d4a47642f966d974 Mon Sep 17 00:00:00 2001 From: ZHANG Dapeng Date: Fri, 28 Aug 2020 13:00:44 -0700 Subject: [PATCH] all: fix lint --- compiler/src/java_plugin/cpp/java_generator.h | 4 +- .../ServiceConfigErrorHandlingTest.java | 2 - .../helloworldtls/HelloWorldServerTls.java | 1 - .../InternalProtocolNegotiationEvent.java | 2 + .../main/java/io/grpc/xds/EnvoyProtoData.java | 48 ++++---- .../java/io/grpc/xds/LoadReportClient.java | 8 +- .../java/io/grpc/xds/LoadStatsStoreImpl.java | 4 +- .../io/grpc/xds/PriorityLoadBalancer.java | 1 - .../CertProviderSslContextProvider.java | 2 +- .../rbac/engine/AuthorizationDecision.java | 10 +- .../rbac/engine/AuthorizationEngine.java | 8 +- .../internal/rbac/engine/EvaluateArgs.java | 4 +- .../sds/DynamicSslContextProvider.java | 4 +- .../java/io/grpc/xds/XdsClientImplTest.java | 8 +- .../java/io/grpc/xds/XdsClientImplTestV2.java | 8 +- .../java/io/grpc/xds/XdsClientTestHelper.java | 38 +++--- .../xds/XdsClientWrapperForServerSdsTest.java | 2 +- ...tProviderServerSslContextProviderTest.java | 2 +- ...MeshCaCertificateProviderProviderTest.java | 14 +-- .../engine/AuthzEngineEvaluationTest.java | 108 +++++++++--------- .../internal/rbac/engine/AuthzEngineTest.java | 3 +- .../rbac/engine/EvaluateArgsTest.java | 2 +- .../sds/CommonTlsContextTestsUtil.java | 12 +- 23 files changed, 144 insertions(+), 151 deletions(-) diff --git a/compiler/src/java_plugin/cpp/java_generator.h b/compiler/src/java_plugin/cpp/java_generator.h index 2ecc884efd..b499c49443 100644 --- a/compiler/src/java_plugin/cpp/java_generator.h +++ b/compiler/src/java_plugin/cpp/java_generator.h @@ -38,13 +38,13 @@ class LogHelper { } }; -// Abort the program after logging the mesage if the given condition is not +// Abort the program after logging the message if the given condition is not // true. Otherwise, do nothing. #define GRPC_CODEGEN_CHECK(x) !(x) && LogHelper(&std::cerr).get_os() \ << "CHECK FAILED: " << __FILE__ << ":" \ << __LINE__ << ": " -// Abort the program after logging the mesage. +// Abort the program after logging the message. #define GRPC_CODEGEN_FAIL GRPC_CODEGEN_CHECK(false) namespace java_grpc_generator { diff --git a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java index b8790ad34d..de6098e5bd 100644 --- a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java +++ b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java @@ -61,7 +61,6 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; @@ -103,7 +102,6 @@ public class ServiceConfigErrorHandlingTest { private final InternalChannelz channelz = new InternalChannelz(); - @Rule public final ExpectedException thrown = ExpectedException.none(); @Rule public final MockitoRule mocks = MockitoJUnit.rule(); private ManagedChannelImpl channel; diff --git a/examples/example-tls/src/main/java/io/grpc/examples/helloworldtls/HelloWorldServerTls.java b/examples/example-tls/src/main/java/io/grpc/examples/helloworldtls/HelloWorldServerTls.java index 8f5338b038..4f8a59abc0 100644 --- a/examples/example-tls/src/main/java/io/grpc/examples/helloworldtls/HelloWorldServerTls.java +++ b/examples/example-tls/src/main/java/io/grpc/examples/helloworldtls/HelloWorldServerTls.java @@ -25,7 +25,6 @@ import io.grpc.netty.NettyServerBuilder; import io.grpc.stub.StreamObserver; import io.netty.handler.ssl.ClientAuth; import io.netty.handler.ssl.SslContextBuilder; -import io.netty.handler.ssl.SslProvider; import java.io.File; import java.io.IOException; diff --git a/netty/src/main/java/io/grpc/netty/InternalProtocolNegotiationEvent.java b/netty/src/main/java/io/grpc/netty/InternalProtocolNegotiationEvent.java index 88cc0ba60a..3ddeff98b2 100644 --- a/netty/src/main/java/io/grpc/netty/InternalProtocolNegotiationEvent.java +++ b/netty/src/main/java/io/grpc/netty/InternalProtocolNegotiationEvent.java @@ -17,12 +17,14 @@ package io.grpc.netty; import io.grpc.Attributes; +import io.grpc.Internal; import io.grpc.InternalChannelz.Security; import javax.annotation.Nullable; /** * Internal accessor for {@link ProtocolNegotiationEvent}. */ +@Internal public final class InternalProtocolNegotiationEvent { private InternalProtocolNegotiationEvent() {} diff --git a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java index dc96aa0f69..ad78a6967c 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyProtoData.java @@ -1292,8 +1292,8 @@ final class EnvoyProtoData { io.envoyproxy.envoy.config.endpoint.v3.ClusterStats toEnvoyProtoClusterStats() { io.envoyproxy.envoy.config.endpoint.v3.ClusterStats.Builder builder = - io.envoyproxy.envoy.config.endpoint.v3.ClusterStats.newBuilder(); - builder.setClusterName(clusterName); + io.envoyproxy.envoy.config.endpoint.v3.ClusterStats.newBuilder() + .setClusterName(clusterName); if (clusterServiceName != null) { builder.setClusterServiceName(clusterServiceName); } @@ -1303,15 +1303,16 @@ final class EnvoyProtoData { for (DroppedRequests droppedRequests : droppedRequestsList) { builder.addDroppedRequests(droppedRequests.toEnvoyProtoDroppedRequests()); } - builder.setTotalDroppedRequests(totalDroppedRequests); - builder.setLoadReportInterval(Durations.fromNanos(loadReportIntervalNanos)); - return builder.build(); + return builder + .setTotalDroppedRequests(totalDroppedRequests) + .setLoadReportInterval(Durations.fromNanos(loadReportIntervalNanos)) + .build(); } io.envoyproxy.envoy.api.v2.endpoint.ClusterStats toEnvoyProtoClusterStatsV2() { io.envoyproxy.envoy.api.v2.endpoint.ClusterStats.Builder builder = - io.envoyproxy.envoy.api.v2.endpoint.ClusterStats.newBuilder(); - builder.setClusterName(clusterName); + io.envoyproxy.envoy.api.v2.endpoint.ClusterStats.newBuilder() + .setClusterName(clusterName); for (UpstreamLocalityStats upstreamLocalityStats : upstreamLocalityStatsList) { builder.addUpstreamLocalityStats( upstreamLocalityStats.toEnvoyProtoUpstreamLocalityStatsV2()); @@ -1319,9 +1320,10 @@ final class EnvoyProtoData { for (DroppedRequests droppedRequests : droppedRequestsList) { builder.addDroppedRequests(droppedRequests.toEnvoyProtoDroppedRequestsV2()); } - builder.setTotalDroppedRequests(totalDroppedRequests); - builder.setLoadReportInterval(Durations.fromNanos(loadReportIntervalNanos)); - return builder.build(); + return builder + .setTotalDroppedRequests(totalDroppedRequests) + .setLoadReportInterval(Durations.fromNanos(loadReportIntervalNanos)) + .build(); } @VisibleForTesting @@ -1534,13 +1536,12 @@ final class EnvoyProtoData { private io.envoyproxy.envoy.config.endpoint.v3.UpstreamLocalityStats toEnvoyProtoUpstreamLocalityStats() { io.envoyproxy.envoy.config.endpoint.v3.UpstreamLocalityStats.Builder builder - = io.envoyproxy.envoy.config.endpoint.v3.UpstreamLocalityStats.newBuilder(); - builder - .setLocality(locality.toEnvoyProtoLocality()) - .setTotalSuccessfulRequests(totalSuccessfulRequests) - .setTotalErrorRequests(totalErrorRequests) - .setTotalRequestsInProgress(totalRequestsInProgress) - .setTotalIssuedRequests(totalIssuedRequests); + = io.envoyproxy.envoy.config.endpoint.v3.UpstreamLocalityStats.newBuilder() + .setLocality(locality.toEnvoyProtoLocality()) + .setTotalSuccessfulRequests(totalSuccessfulRequests) + .setTotalErrorRequests(totalErrorRequests) + .setTotalRequestsInProgress(totalRequestsInProgress) + .setTotalIssuedRequests(totalIssuedRequests); for (EndpointLoadMetricStats endpointLoadMetricStats : loadMetricStatsList) { builder.addLoadMetricStats(endpointLoadMetricStats.toEnvoyProtoEndpointLoadMetricStats()); } @@ -1550,13 +1551,12 @@ final class EnvoyProtoData { private io.envoyproxy.envoy.api.v2.endpoint.UpstreamLocalityStats toEnvoyProtoUpstreamLocalityStatsV2() { io.envoyproxy.envoy.api.v2.endpoint.UpstreamLocalityStats.Builder builder - = io.envoyproxy.envoy.api.v2.endpoint.UpstreamLocalityStats.newBuilder(); - builder - .setLocality(locality.toEnvoyProtoLocalityV2()) - .setTotalSuccessfulRequests(totalSuccessfulRequests) - .setTotalErrorRequests(totalErrorRequests) - .setTotalRequestsInProgress(totalRequestsInProgress) - .setTotalIssuedRequests(totalIssuedRequests); + = io.envoyproxy.envoy.api.v2.endpoint.UpstreamLocalityStats.newBuilder() + .setLocality(locality.toEnvoyProtoLocalityV2()) + .setTotalSuccessfulRequests(totalSuccessfulRequests) + .setTotalErrorRequests(totalErrorRequests) + .setTotalRequestsInProgress(totalRequestsInProgress) + .setTotalIssuedRequests(totalIssuedRequests); for (EndpointLoadMetricStats endpointLoadMetricStats : loadMetricStatsList) { builder.addLoadMetricStats(endpointLoadMetricStats.toEnvoyProtoEndpointLoadMetricStatsV2()); } diff --git a/xds/src/main/java/io/grpc/xds/LoadReportClient.java b/xds/src/main/java/io/grpc/xds/LoadReportClient.java index c9cff2db6c..119ee4dfdc 100644 --- a/xds/src/main/java/io/grpc/xds/LoadReportClient.java +++ b/xds/src/main/java/io/grpc/xds/LoadReportClient.java @@ -416,8 +416,8 @@ final class LoadReportClient { io.envoyproxy.envoy.service.load_stats.v2.LoadStatsRequest toEnvoyProtoV2() { io.envoyproxy.envoy.service.load_stats.v2.LoadStatsRequest.Builder builder - = io.envoyproxy.envoy.service.load_stats.v2.LoadStatsRequest.newBuilder(); - builder.setNode(node.toEnvoyProtoNodeV2()); + = io.envoyproxy.envoy.service.load_stats.v2.LoadStatsRequest.newBuilder() + .setNode(node.toEnvoyProtoNodeV2()); if (clusterStatsList != null) { for (ClusterStats stats : clusterStatsList) { builder.addClusterStats(stats.toEnvoyProtoClusterStatsV2()); @@ -427,8 +427,8 @@ final class LoadReportClient { } LoadStatsRequest toEnvoyProtoV3() { - LoadStatsRequest.Builder builder = LoadStatsRequest.newBuilder(); - builder.setNode(node.toEnvoyProtoNode()); + LoadStatsRequest.Builder builder = LoadStatsRequest.newBuilder() + .setNode(node.toEnvoyProtoNode()); if (clusterStatsList != null) { for (ClusterStats stats : clusterStatsList) { builder.addClusterStats(stats.toEnvoyProtoClusterStats()); diff --git a/xds/src/main/java/io/grpc/xds/LoadStatsStoreImpl.java b/xds/src/main/java/io/grpc/xds/LoadStatsStoreImpl.java index afb91c3ecc..d465498725 100644 --- a/xds/src/main/java/io/grpc/xds/LoadStatsStoreImpl.java +++ b/xds/src/main/java/io/grpc/xds/LoadStatsStoreImpl.java @@ -17,6 +17,7 @@ package io.grpc.xds; import static com.google.common.base.Preconditions.checkNotNull; +import static java.util.concurrent.TimeUnit.NANOSECONDS; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; @@ -33,7 +34,6 @@ import io.grpc.xds.LoadStatsManager.LoadStatsStoreFactory; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import javax.annotation.Nullable; import javax.annotation.concurrent.NotThreadSafe; @@ -113,7 +113,7 @@ final class LoadStatsStoreImpl implements LoadStatsStore { statsBuilder.addDroppedRequests(new DroppedRequests(entry.getKey(),drops)); } statsBuilder.setTotalDroppedRequests(totalDrops); - statsBuilder.setLoadReportIntervalNanos(stopwatch.elapsed(TimeUnit.NANOSECONDS)); + statsBuilder.setLoadReportIntervalNanos(stopwatch.elapsed(NANOSECONDS)); stopwatch.reset().start(); return statsBuilder.build(); } diff --git a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java index f53ac28b95..d5876fd743 100644 --- a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java @@ -260,7 +260,6 @@ final class PriorityLoadBalancer extends LoadBalancer { policy = newPolicy; lb.switchTo(lbProvider); } - // TODO(zdapeng): Implement address filtering. lb.handleResolvedAddresses( addresses .toBuilder() diff --git a/xds/src/main/java/io/grpc/xds/internal/certprovider/CertProviderSslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/certprovider/CertProviderSslContextProvider.java index 5f03e3becc..eef5ee551e 100644 --- a/xds/src/main/java/io/grpc/xds/internal/certprovider/CertProviderSslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/certprovider/CertProviderSslContextProvider.java @@ -83,7 +83,7 @@ abstract class CertProviderSslContextProvider extends DynamicSslContextProvider } } - private CertificateProviderInfo getCertProviderConfig( + private static CertificateProviderInfo getCertProviderConfig( Map certProviders, String pluginInstanceName) { return certProviders.get(pluginInstanceName); } diff --git a/xds/src/main/java/io/grpc/xds/internal/rbac/engine/AuthorizationDecision.java b/xds/src/main/java/io/grpc/xds/internal/rbac/engine/AuthorizationDecision.java index 474ff7c180..43f69dc6bf 100644 --- a/xds/src/main/java/io/grpc/xds/internal/rbac/engine/AuthorizationDecision.java +++ b/xds/src/main/java/io/grpc/xds/internal/rbac/engine/AuthorizationDecision.java @@ -71,20 +71,20 @@ public class AuthorizationDecision { public String toString() { StringBuilder authzStr = new StringBuilder(); switch (this.decision) { - case ALLOW: + case ALLOW: authzStr.append("Authorization Decision: ALLOW. \n"); break; - case DENY: + case DENY: authzStr.append("Authorization Decision: DENY. \n"); break; - case UNKNOWN: + case UNKNOWN: authzStr.append("Authorization Decision: UNKNOWN. \n"); break; - default: + default: break; } for (String policyName : this.policyNames) { - authzStr.append(policyName + "; \n"); + authzStr.append(policyName).append("; \n"); } return authzStr.toString(); } diff --git a/xds/src/main/java/io/grpc/xds/internal/rbac/engine/AuthorizationEngine.java b/xds/src/main/java/io/grpc/xds/internal/rbac/engine/AuthorizationEngine.java index bb3005883f..a7e09b0f3f 100644 --- a/xds/src/main/java/io/grpc/xds/internal/rbac/engine/AuthorizationEngine.java +++ b/xds/src/main/java/io/grpc/xds/internal/rbac/engine/AuthorizationEngine.java @@ -102,7 +102,7 @@ public class AuthorizationEngine { * @param allowPolicy input Envoy RBAC policy with ALLOW action. * @throws IllegalArgumentException if the user inputs an invalid RBAC list. */ - public AuthorizationEngine(RBAC denyPolicy, RBAC allowPolicy) throws IllegalArgumentException { + public AuthorizationEngine(RBAC denyPolicy, RBAC allowPolicy) { checkArgument( denyPolicy.getAction() == Action.DENY && allowPolicy.getAction() == Action.ALLOW, "Invalid RBAC list, " @@ -140,7 +140,7 @@ public class AuthorizationEngine { if (authzDecision != null) { return authzDecision; } - if (unknownPolicyNames.size() > 0) { + if (!unknownPolicyNames.isEmpty()) { return new AuthorizationDecision( AuthorizationDecision.Output.UNKNOWN, unknownPolicyNames); } @@ -154,7 +154,7 @@ public class AuthorizationEngine { if (authzDecision != null) { return authzDecision; } - if (unknownPolicyNames.size() > 0) { + if (!unknownPolicyNames.isEmpty()) { return new AuthorizationDecision( AuthorizationDecision.Output.UNKNOWN, unknownPolicyNames); } @@ -198,7 +198,7 @@ public class AuthorizationEngine { try { Object result = interpretable.eval(activation); if (result instanceof Boolean) { - return Boolean.valueOf(result.toString()); + return Boolean.parseBoolean(result.toString()); } // Throw an InterpreterException if there are missing Envoy Attributes. if (result instanceof IncompleteData) { diff --git a/xds/src/main/java/io/grpc/xds/internal/rbac/engine/EvaluateArgs.java b/xds/src/main/java/io/grpc/xds/internal/rbac/engine/EvaluateArgs.java index e8ed1828a2..35945a2102 100644 --- a/xds/src/main/java/io/grpc/xds/internal/rbac/engine/EvaluateArgs.java +++ b/xds/src/main/java/io/grpc/xds/internal/rbac/engine/EvaluateArgs.java @@ -23,8 +23,8 @@ import io.grpc.ServerCall; /** The EvaluateArgs class holds evaluate arguments used in CEL-based Authorization Engine. */ public class EvaluateArgs { - private Metadata headers; - private ServerCall call; + private final Metadata headers; + private final ServerCall call; /** * Creates a new EvaluateArgs using the input {@code headers} for resolving headers diff --git a/xds/src/main/java/io/grpc/xds/internal/sds/DynamicSslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/sds/DynamicSslContextProvider.java index 7f40b822f0..c75347c1f5 100644 --- a/xds/src/main/java/io/grpc/xds/internal/sds/DynamicSslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/sds/DynamicSslContextProvider.java @@ -75,8 +75,8 @@ public abstract class DynamicSslContextProvider extends SslContextProvider { alpnList); sslContextBuilder.applicationProtocolConfig(apn); } - List pendingCallbacksCopy = null; - SslContext sslContextCopy = null; + List pendingCallbacksCopy; + SslContext sslContextCopy; synchronized (pendingCallbacks) { sslContext = sslContextBuilder.build(); sslContextCopy = sslContext; diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java index c06a6cc5fc..b55a649741 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java @@ -749,7 +749,7 @@ public class XdsClientImplTest { new EnvoyProtoData.Route( // path match with cluster route new io.grpc.xds.RouteMatch( - /* pathPrefixMatch= */ null,/* pathExactMatch= */ "/service1/method1"), + /* pathPrefixMatch= */ null, /* pathExactMatch= */ "/service1/method1"), new EnvoyProtoData.RouteAction( TimeUnit.SECONDS.toNanos(15L), "cl1.googleapis.com", null))); assertThat(routes.get(1)) @@ -757,7 +757,7 @@ public class XdsClientImplTest { new EnvoyProtoData.Route( // path match with weighted cluster route new io.grpc.xds.RouteMatch( - /* pathPrefixMatch= */ null,/* pathExactMatch= */ "/service2/method2"), + /* pathPrefixMatch= */ null, /* pathExactMatch= */ "/service2/method2"), new EnvoyProtoData.RouteAction( TimeUnit.SECONDS.toNanos(15L), null, @@ -769,7 +769,7 @@ public class XdsClientImplTest { new EnvoyProtoData.Route( // prefix match with cluster route new io.grpc.xds.RouteMatch( - /* pathPrefixMatch= */ "/service1/",/* pathExactMatch= */ null), + /* pathPrefixMatch= */ "/service1/", /* pathExactMatch= */ null), new EnvoyProtoData.RouteAction( TimeUnit.SECONDS.toNanos(15L), "cl1.googleapis.com", null))); assertThat(routes.get(3)) @@ -777,7 +777,7 @@ public class XdsClientImplTest { new EnvoyProtoData.Route( // default match with cluster route new io.grpc.xds.RouteMatch( - /* pathPrefixMatch= */ "",/* pathExactMatch= */ null), + /* pathPrefixMatch= */ "", /* pathExactMatch= */ null), new EnvoyProtoData.RouteAction( TimeUnit.SECONDS.toNanos(15L), "cluster.googleapis.com", null))); } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTestV2.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTestV2.java index 64da95d25b..d3691a9a47 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplTestV2.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTestV2.java @@ -751,7 +751,7 @@ public class XdsClientImplTestV2 { new EnvoyProtoData.Route( // path match with cluster route new io.grpc.xds.RouteMatch( - /* pathPrefixMatch= */ null,/* pathExactMatch= */ "/service1/method1"), + /* pathPrefixMatch= */ null, /* pathExactMatch= */ "/service1/method1"), new EnvoyProtoData.RouteAction( TimeUnit.SECONDS.toNanos(15L), "cl1.googleapis.com", null))); assertThat(routes.get(1)) @@ -759,7 +759,7 @@ public class XdsClientImplTestV2 { new EnvoyProtoData.Route( // path match with weighted cluster route new io.grpc.xds.RouteMatch( - /* pathPrefixMatch= */ null,/* pathExactMatch= */ "/service2/method2"), + /* pathPrefixMatch= */ null, /* pathExactMatch= */ "/service2/method2"), new EnvoyProtoData.RouteAction( TimeUnit.SECONDS.toNanos(15L), null, @@ -771,7 +771,7 @@ public class XdsClientImplTestV2 { new EnvoyProtoData.Route( // prefix match with cluster route new io.grpc.xds.RouteMatch( - /* pathPrefixMatch= */ "/service1/",/* pathExactMatch= */ null), + /* pathPrefixMatch= */ "/service1/", /* pathExactMatch= */ null), new EnvoyProtoData.RouteAction( TimeUnit.SECONDS.toNanos(15L), "cl1.googleapis.com", null))); assertThat(routes.get(3)) @@ -779,7 +779,7 @@ public class XdsClientImplTestV2 { new EnvoyProtoData.Route( // default match with cluster route new io.grpc.xds.RouteMatch( - /* pathPrefixMatch= */ "",/* pathExactMatch= */ null), + /* pathPrefixMatch= */ "", /* pathExactMatch= */ null), new EnvoyProtoData.RouteAction( TimeUnit.SECONDS.toNanos(15L), "cluster.googleapis.com", null))); } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java index 19bbed263b..3220a77e52 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientTestHelper.java @@ -22,6 +22,7 @@ import com.google.protobuf.UInt32Value; import io.envoyproxy.envoy.config.cluster.v3.Cluster; import io.envoyproxy.envoy.config.cluster.v3.Cluster.DiscoveryType; import io.envoyproxy.envoy.config.cluster.v3.Cluster.EdsClusterConfig; +import io.envoyproxy.envoy.config.cluster.v3.Cluster.LbPolicy; import io.envoyproxy.envoy.config.core.v3.Address; import io.envoyproxy.envoy.config.core.v3.AggregatedConfigSource; import io.envoyproxy.envoy.config.core.v3.ApiConfigSource; @@ -34,6 +35,8 @@ import io.envoyproxy.envoy.config.core.v3.SelfConfigSource; import io.envoyproxy.envoy.config.core.v3.SocketAddress; import io.envoyproxy.envoy.config.core.v3.TransportSocket; import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; +import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment.Policy; +import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment.Policy.DropOverload; import io.envoyproxy.envoy.config.endpoint.v3.Endpoint; import io.envoyproxy.envoy.config.endpoint.v3.LbEndpoint; import io.envoyproxy.envoy.config.endpoint.v3.LocalityLbEndpoints; @@ -206,7 +209,7 @@ class XdsClientTestHelper { edsClusterConfigBuilder.setServiceName(edsServiceName); } clusterBuilder.setEdsClusterConfig(edsClusterConfigBuilder); - clusterBuilder.setLbPolicy(Cluster.LbPolicy.ROUND_ROBIN); + clusterBuilder.setLbPolicy(LbPolicy.ROUND_ROBIN); if (enableLrs) { clusterBuilder.setLrsServer( ConfigSource.newBuilder() @@ -223,19 +226,20 @@ class XdsClientTestHelper { String clusterName, @Nullable String edsServiceName, boolean enableLrs, @Nullable io.envoyproxy.envoy.api.v2.auth.UpstreamTlsContext upstreamTlsContext) { io.envoyproxy.envoy.api.v2.Cluster.Builder clusterBuilder = - io.envoyproxy.envoy.api.v2.Cluster.newBuilder(); - clusterBuilder.setName(clusterName); - clusterBuilder.setType(io.envoyproxy.envoy.api.v2.Cluster.DiscoveryType.EDS); + io.envoyproxy.envoy.api.v2.Cluster.newBuilder() + .setName(clusterName) + .setType(io.envoyproxy.envoy.api.v2.Cluster.DiscoveryType.EDS); io.envoyproxy.envoy.api.v2.Cluster.EdsClusterConfig.Builder edsClusterConfigBuilder = - io.envoyproxy.envoy.api.v2.Cluster.EdsClusterConfig.newBuilder(); - edsClusterConfigBuilder.setEdsConfig( - io.envoyproxy.envoy.api.v2.core.ConfigSource.newBuilder() - .setAds(io.envoyproxy.envoy.api.v2.core.AggregatedConfigSource.getDefaultInstance())); + io.envoyproxy.envoy.api.v2.Cluster.EdsClusterConfig.newBuilder() + .setEdsConfig( + io.envoyproxy.envoy.api.v2.core.ConfigSource.newBuilder().setAds( + io.envoyproxy.envoy.api.v2.core.AggregatedConfigSource.getDefaultInstance())); if (edsServiceName != null) { edsClusterConfigBuilder.setServiceName(edsServiceName); } - clusterBuilder.setEdsClusterConfig(edsClusterConfigBuilder); - clusterBuilder.setLbPolicy(io.envoyproxy.envoy.api.v2.Cluster.LbPolicy.ROUND_ROBIN); + clusterBuilder + .setEdsClusterConfig(edsClusterConfigBuilder) + .setLbPolicy(io.envoyproxy.envoy.api.v2.Cluster.LbPolicy.ROUND_ROBIN); if (enableLrs) { clusterBuilder.setLrsServer( io.envoyproxy.envoy.api.v2.core.ConfigSource.newBuilder() @@ -250,19 +254,16 @@ class XdsClientTestHelper { } static ClusterLoadAssignment buildClusterLoadAssignment(String clusterName, - List localityLbEndpoints, - List dropOverloads) { + List localityLbEndpoints, List dropOverloads) { return ClusterLoadAssignment.newBuilder() .setClusterName(clusterName) .addAllEndpoints(localityLbEndpoints) - .setPolicy( - ClusterLoadAssignment.Policy.newBuilder() - .addAllDropOverloads(dropOverloads)) + .setPolicy(Policy.newBuilder().addAllDropOverloads(dropOverloads)) .build(); } - @SuppressWarnings("deprecation") + @SuppressWarnings("deprecation") // disableOverprovisioning is deprecated by needed for v2 static io.envoyproxy.envoy.api.v2.ClusterLoadAssignment buildClusterLoadAssignmentV2( String clusterName, List localityLbEndpoints, @@ -278,10 +279,9 @@ class XdsClientTestHelper { .build(); } - static ClusterLoadAssignment.Policy.DropOverload buildDropOverload( - String category, int dropPerMillion) { + static DropOverload buildDropOverload(String category, int dropPerMillion) { return - ClusterLoadAssignment.Policy.DropOverload.newBuilder() + DropOverload.newBuilder() .setCategory(category) .setDropPercentage( FractionalPercent.newBuilder() diff --git a/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTest.java b/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTest.java index 7965eeec36..7e099de6c3 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTest.java @@ -109,7 +109,7 @@ public class XdsClientWrapperForServerSdsTest { "exact IP over IPANY match, expect filter2" }, { - PORT,// matches dest port but no address match + PORT, // matches dest port but no address match "168.20.20.2", "10.1.2.4", "192.168.10.1", diff --git a/xds/src/test/java/io/grpc/xds/internal/certprovider/CertProviderServerSslContextProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/certprovider/CertProviderServerSslContextProviderTest.java index 3475c6e151..9de2081ed7 100644 --- a/xds/src/test/java/io/grpc/xds/internal/certprovider/CertProviderServerSslContextProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/certprovider/CertProviderServerSslContextProviderTest.java @@ -17,7 +17,6 @@ package io.grpc.xds.internal.certprovider; import static com.google.common.truth.Truth.assertThat; -import static io.grpc.xds.internal.certprovider.CertProviderClientSslContextProviderTest.QueuedExecutor; import static io.grpc.xds.internal.certprovider.CommonCertProviderTestUtils.getCertFromResourceName; import static io.grpc.xds.internal.sds.CommonTlsContextTestsUtil.CA_PEM_FILE; import static io.grpc.xds.internal.sds.CommonTlsContextTestsUtil.CLIENT_PEM_FILE; @@ -34,6 +33,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.EnvoyServerProtoData; +import io.grpc.xds.internal.certprovider.CertProviderClientSslContextProviderTest.QueuedExecutor; import io.grpc.xds.internal.sds.CommonTlsContextTestsUtil; import io.grpc.xds.internal.sds.CommonTlsContextTestsUtil.TestCallback; import org.junit.Before; diff --git a/xds/src/test/java/io/grpc/xds/internal/certprovider/MeshCaCertificateProviderProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/certprovider/MeshCaCertificateProviderProviderTest.java index 5da5b34d03..123db14a05 100644 --- a/xds/src/test/java/io/grpc/xds/internal/certprovider/MeshCaCertificateProviderProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/certprovider/MeshCaCertificateProviderProviderTest.java @@ -226,32 +226,32 @@ public class MeshCaCertificateProviderProviderTest { eq(TimeUnit.SECONDS.toMillis(RPC_TIMEOUT_SECONDS))); } - private Map buildFullConfig() throws IOException { + private static Map buildFullConfig() throws IOException { return getCertProviderConfig(CommonCertProviderTestUtils.getNonDefaultTestBootstrapInfo()); } - private Map buildMinimalConfig() throws IOException { + private static Map buildMinimalConfig() throws IOException { return getCertProviderConfig(CommonCertProviderTestUtils.getMinimalBootstrapInfo()); } - private Map buildBadClusterUrlConfig() throws IOException { + private static Map buildBadClusterUrlConfig() throws IOException { return getCertProviderConfig( CommonCertProviderTestUtils.getMinimalAndBadClusterUrlBootstrapInfo()); } - private Map buildMissingSaJwtLocationConfig() throws IOException { + private static Map buildMissingSaJwtLocationConfig() throws IOException { return getCertProviderConfig(CommonCertProviderTestUtils.getMissingSaJwtLocation()); } - private Map buildMissingGkeClusterUrlConfig() throws IOException { + private static Map buildMissingGkeClusterUrlConfig() throws IOException { return getCertProviderConfig(CommonCertProviderTestUtils.getMissingGkeClusterUrl()); } - private Map buildBadChannelCredsConfig() throws IOException { + private static Map buildBadChannelCredsConfig() throws IOException { return getCertProviderConfig(CommonCertProviderTestUtils.getBadChannelCredsConfig()); } - private Map getCertProviderConfig(Bootstrapper.BootstrapInfo bootstrapInfo) { + private static Map getCertProviderConfig(Bootstrapper.BootstrapInfo bootstrapInfo) { Map certProviders = bootstrapInfo.getCertProviders(); Bootstrapper.CertificateProviderInfo gcpIdInfo = diff --git a/xds/src/test/java/io/grpc/xds/internal/rbac/engine/AuthzEngineEvaluationTest.java b/xds/src/test/java/io/grpc/xds/internal/rbac/engine/AuthzEngineEvaluationTest.java index 61172d576c..bd8d35744b 100644 --- a/xds/src/test/java/io/grpc/xds/internal/rbac/engine/AuthzEngineEvaluationTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/rbac/engine/AuthzEngineEvaluationTest.java @@ -16,8 +16,8 @@ package io.grpc.xds.internal.rbac.engine; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; @@ -31,7 +31,6 @@ import io.envoyproxy.envoy.config.rbac.v2.RBAC; import io.envoyproxy.envoy.config.rbac.v2.RBAC.Action; import io.grpc.xds.internal.rbac.engine.cel.Activation; import io.grpc.xds.internal.rbac.engine.cel.InterpreterException; -import java.lang.StringBuilder; import java.util.Map; import org.junit.Before; import org.junit.Rule; @@ -51,14 +50,9 @@ public class AuthzEngineEvaluationTest { @Mock private EvaluateArgs args; - - @Mock - private Activation activation; - @Mock private Map attributes; - private AuthorizationEngine engine; private AuthorizationEngine spyEngine; private AuthorizationDecision evaluateResult; @@ -130,7 +124,7 @@ public class AuthzEngineEvaluationTest { @Before public void setupEngineSingleRbacAllow() { buildRbac(); - engine = new AuthorizationEngine(rbacAllow); + AuthorizationEngine engine = new AuthorizationEngine(rbacAllow); spyEngine = Mockito.spy(engine); doReturn(ImmutableMap.copyOf(attributes)).when(args).generateEnvoyAttributes(); } @@ -139,7 +133,7 @@ public class AuthzEngineEvaluationTest { @Before public void setupEngineSingleRbacDeny() { buildRbac(); - engine = new AuthorizationEngine(rbacDeny); + AuthorizationEngine engine = new AuthorizationEngine(rbacDeny); spyEngine = Mockito.spy(engine); doReturn(ImmutableMap.copyOf(attributes)).when(args).generateEnvoyAttributes(); } @@ -148,7 +142,7 @@ public class AuthzEngineEvaluationTest { @Before public void setupEngineRbacPair() { buildRbac(); - engine = new AuthorizationEngine(rbacDeny, rbacAllow); + AuthorizationEngine engine = new AuthorizationEngine(rbacDeny, rbacAllow); spyEngine = Mockito.spy(engine); doReturn(ImmutableMap.copyOf(attributes)).when(args).generateEnvoyAttributes(); } @@ -166,9 +160,9 @@ public class AuthzEngineEvaluationTest { doReturn(true).when(spyEngine).matches(eq(condition2), any(Activation.class)); doReturn(true).when(spyEngine).matches(eq(condition3), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.ALLOW); - assertEquals(evaluateResult.getPolicyNames().size(), 1); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 1")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.ALLOW); + assertThat(evaluateResult.getPolicyNames()).hasSize(1); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 1"); } /** @@ -184,9 +178,9 @@ public class AuthzEngineEvaluationTest { doReturn(false).when(spyEngine).matches(eq(condition2), any(Activation.class)); doReturn(false).when(spyEngine).matches(eq(condition3), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.DENY); - assertEquals(evaluateResult.getPolicyNames().size(), 0); - assertEquals(evaluateResult.toString(), + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.DENY); + assertThat(evaluateResult.getPolicyNames()).isEmpty(); + assertThat(evaluateResult.toString()).isEqualTo( new StringBuilder("Authorization Decision: DENY. \n").toString()); } @@ -207,7 +201,7 @@ public class AuthzEngineEvaluationTest { evaluateResult = spyEngine.evaluate(args); assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.ALLOW); assertEquals(evaluateResult.getPolicyNames().size(), 1); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 2")); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 2"); } /** @@ -226,11 +220,11 @@ public class AuthzEngineEvaluationTest { doThrow(new InterpreterException.Builder("Unknown result").build()) .when(spyEngine).matches(eq(condition3), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.UNKNOWN); - assertEquals(evaluateResult.getPolicyNames().size(), 2); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 2")); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 3")); - assertEquals(evaluateResult.toString(), + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.UNKNOWN); + assertThat(evaluateResult.getPolicyNames()).hasSize(2); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 2"); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 3"); + assertThat(evaluateResult.toString()).isEqualTo( new StringBuilder("Authorization Decision: UNKNOWN. \n" + "Policy 2; \n" + "Policy 3; \n").toString()); } @@ -250,10 +244,10 @@ public class AuthzEngineEvaluationTest { doThrow(new InterpreterException.Builder("Unknown result").build()) .when(spyEngine).matches(eq(condition3), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.ALLOW); - assertEquals(evaluateResult.getPolicyNames().size(), 1); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 2")); - assertEquals(evaluateResult.toString(), + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.ALLOW); + assertThat(evaluateResult.getPolicyNames()).hasSize(1); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 2"); + assertThat(evaluateResult.toString()).isEqualTo( new StringBuilder("Authorization Decision: ALLOW. \n" + "Policy 2; \n").toString()); } @@ -270,9 +264,9 @@ public class AuthzEngineEvaluationTest { doReturn(true).when(spyEngine).matches(eq(condition5), any(Activation.class)); doReturn(true).when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.DENY); - assertEquals(evaluateResult.getPolicyNames().size(), 1); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 4")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.DENY); + assertThat(evaluateResult.getPolicyNames()).hasSize(1); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 4"); } /** @@ -288,8 +282,8 @@ public class AuthzEngineEvaluationTest { doReturn(false).when(spyEngine).matches(eq(condition5), any(Activation.class)); doReturn(false).when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.ALLOW); - assertEquals(evaluateResult.getPolicyNames().size(), 0); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.ALLOW); + assertThat(evaluateResult.getPolicyNames()).isEmpty(); } /** @@ -307,9 +301,9 @@ public class AuthzEngineEvaluationTest { doReturn(true).when(spyEngine).matches(eq(condition5), any(Activation.class)); doReturn(true).when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.DENY); - assertEquals(evaluateResult.getPolicyNames().size(), 1); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 5")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.DENY); + assertThat(evaluateResult.getPolicyNames()).hasSize(1); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 5"); } /** @@ -328,10 +322,10 @@ public class AuthzEngineEvaluationTest { doThrow(new InterpreterException.Builder("Unknown result").build()) .when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.UNKNOWN); - assertEquals(evaluateResult.getPolicyNames().size(), 2); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 5")); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 6")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.UNKNOWN); + assertThat(evaluateResult.getPolicyNames()).hasSize(2); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 5"); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 6"); } /** @@ -349,9 +343,9 @@ public class AuthzEngineEvaluationTest { doThrow(new InterpreterException.Builder("Unknown result").build()) .when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.DENY); - assertEquals(evaluateResult.getPolicyNames().size(), 1); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 5")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.DENY); + assertThat(evaluateResult.getPolicyNames()).hasSize(1); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 5"); } /** @@ -371,9 +365,9 @@ public class AuthzEngineEvaluationTest { doReturn(true).when(spyEngine).matches(eq(condition5), any(Activation.class)); doReturn(true).when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.DENY); - assertEquals(evaluateResult.getPolicyNames().size(), 1); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 4")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.DENY); + assertThat(evaluateResult.getPolicyNames()).hasSize(1); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 4"); } /** @@ -396,9 +390,9 @@ public class AuthzEngineEvaluationTest { doThrow(new InterpreterException.Builder("Unknown result").build()) .when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.DENY); - assertEquals(evaluateResult.getPolicyNames().size(), 1); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 5")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.DENY); + assertThat(evaluateResult.getPolicyNames()).hasSize(1); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 5"); } /** @@ -419,10 +413,10 @@ public class AuthzEngineEvaluationTest { doThrow(new InterpreterException.Builder("Unknown result").build()) .when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.UNKNOWN); - assertEquals(evaluateResult.getPolicyNames().size(), 2); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 5")); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 6")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.UNKNOWN); + assertThat(evaluateResult.getPolicyNames()).hasSize(2); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 5"); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 6"); } /** @@ -446,10 +440,10 @@ public class AuthzEngineEvaluationTest { doReturn(false).when(spyEngine).matches(eq(condition5), any(Activation.class)); doReturn(false).when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.UNKNOWN); - assertEquals(evaluateResult.getPolicyNames().size(), 2); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 2")); - assertTrue(evaluateResult.getPolicyNames().contains("Policy 3")); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.UNKNOWN); + assertThat(evaluateResult.getPolicyNames()).hasSize(2); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 2"); + assertThat(evaluateResult.getPolicyNames()).contains("Policy 3"); } /** @@ -469,7 +463,7 @@ public class AuthzEngineEvaluationTest { doReturn(false).when(spyEngine).matches(eq(condition5), any(Activation.class)); doReturn(false).when(spyEngine).matches(eq(condition6), any(Activation.class)); evaluateResult = spyEngine.evaluate(args); - assertEquals(evaluateResult.getDecision(), AuthorizationDecision.Output.DENY); - assertEquals(evaluateResult.getPolicyNames().size(), 0); + assertThat(evaluateResult.getDecision()).isEqualTo(AuthorizationDecision.Output.DENY); + assertThat(evaluateResult.getPolicyNames()).isEmpty(); } } diff --git a/xds/src/test/java/io/grpc/xds/internal/rbac/engine/AuthzEngineTest.java b/xds/src/test/java/io/grpc/xds/internal/rbac/engine/AuthzEngineTest.java index 17407d3ce1..9716260af2 100644 --- a/xds/src/test/java/io/grpc/xds/internal/rbac/engine/AuthzEngineTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/rbac/engine/AuthzEngineTest.java @@ -68,7 +68,6 @@ public class AuthzEngineTest { private AuthorizationEngine engine; private RBAC rbacDeny; private RBAC rbacAllow; - private Expr expr; private Object result; @Before @@ -130,7 +129,7 @@ public class AuthzEngineTest { public void testCelInterface() throws InterpreterException { engine = new AuthorizationEngine(rbacAllow); when(interpretable.eval(any(Activation.class))).thenReturn(true); - expr = Expr.newBuilder().build(); + Expr expr = Expr.getDefaultInstance(); result = engine.matches(expr, activation); assertThat(messageProvider).isNotNull(); assertThat(dispatcher).isNotNull(); diff --git a/xds/src/test/java/io/grpc/xds/internal/rbac/engine/EvaluateArgsTest.java b/xds/src/test/java/io/grpc/xds/internal/rbac/engine/EvaluateArgsTest.java index 3fa09bda45..8f908f9188 100644 --- a/xds/src/test/java/io/grpc/xds/internal/rbac/engine/EvaluateArgsTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/rbac/engine/EvaluateArgsTest.java @@ -106,7 +106,7 @@ public class EvaluateArgsTest { verify(spyArgs, times(1)).getConnectionUriSanPeerCertificate(); verify(spyArgs, times(1)).getSourcePrincipal(); } - + @Test public void testEvaluateArgsAccessorFunctions() { // Set up args and call. diff --git a/xds/src/test/java/io/grpc/xds/internal/sds/CommonTlsContextTestsUtil.java b/xds/src/test/java/io/grpc/xds/internal/sds/CommonTlsContextTestsUtil.java index 8c150d0ccd..dad594f278 100644 --- a/xds/src/test/java/io/grpc/xds/internal/sds/CommonTlsContextTestsUtil.java +++ b/xds/src/test/java/io/grpc/xds/internal/sds/CommonTlsContextTestsUtil.java @@ -33,6 +33,7 @@ import io.envoyproxy.envoy.config.core.v3.DataSource; import io.envoyproxy.envoy.config.core.v3.GrpcService; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext; +import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CertificateProviderInstance; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CombinedCertificateValidationContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.SdsSecretConfig; @@ -497,19 +498,20 @@ public class CommonTlsContextTestsUtil { String rootCertName, CertificateValidationContext staticCertValidationContext) { if (rootInstanceName != null) { - CommonTlsContext.CertificateProviderInstance.Builder providerInstanceBuilder = - CommonTlsContext.CertificateProviderInstance.newBuilder() + CertificateProviderInstance providerInstance = + CertificateProviderInstance.newBuilder() .setInstanceName(rootInstanceName) - .setCertificateName(rootCertName); + .setCertificateName(rootCertName) + .build(); if (staticCertValidationContext != null) { CombinedCertificateValidationContext combined = CombinedCertificateValidationContext.newBuilder() .setDefaultValidationContext(staticCertValidationContext) - .setValidationContextCertificateProviderInstance(providerInstanceBuilder) + .setValidationContextCertificateProviderInstance(providerInstance) .build(); return builder.setCombinedValidationContext(combined); } - builder = builder.setValidationContextCertificateProviderInstance(providerInstanceBuilder); + builder = builder.setValidationContextCertificateProviderInstance(providerInstance); } return builder; }