From 54d5db38f5b8d97fada0a85ca2d89fa027b0025e Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Thu, 3 Apr 2025 06:45:17 +0000 Subject: [PATCH 01/10] Float LRU Cache across interceptors --- .../io/grpc/xds/GcpAuthenticationFilter.java | 13 ++--- .../grpc/xds/GcpAuthenticationFilterTest.java | 57 +++++++++++++++---- 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java index b5568efe400..55b76b54b31 100644 --- a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java +++ b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java @@ -59,15 +59,17 @@ final class GcpAuthenticationFilter implements Filter { static final String TYPE_URL = "type.googleapis.com/envoy.extensions.filters.http.gcp_authn.v3.GcpAuthnFilterConfig"; - + private final LruCache callCredentialsCache; final String filterInstanceName; - GcpAuthenticationFilter(String name) { + GcpAuthenticationFilter(String name, int cacheSize) { filterInstanceName = checkNotNull(name, "name"); + this.callCredentialsCache = new LruCache<>(cacheSize); } - static final class Provider implements Filter.Provider { + long cacheSize = 10; + @Override public String[] typeUrls() { return new String[]{TYPE_URL}; @@ -80,7 +82,7 @@ public boolean isClientFilter() { @Override public GcpAuthenticationFilter newInstance(String name) { - return new GcpAuthenticationFilter(name); + return new GcpAuthenticationFilter(name, (int) cacheSize); } @Override @@ -97,7 +99,6 @@ public ConfigOrError parseFilterConfig(Message rawProto return ConfigOrError.fromError("Invalid proto: " + e); } - long cacheSize = 10; // Validate cache_config if (gcpAuthnProto.hasCacheConfig()) { TokenCacheConfig cacheConfig = gcpAuthnProto.getCacheConfig(); @@ -127,8 +128,6 @@ public ClientInterceptor buildClientInterceptor(FilterConfig config, @Nullable FilterConfig overrideConfig, ScheduledExecutorService scheduler) { ComputeEngineCredentials credentials = ComputeEngineCredentials.create(); - LruCache callCredentialsCache = - new LruCache<>(((GcpAuthenticationConfig) config).getCacheSize()); return new ClientInterceptor() { @Override public ClientCall interceptCall( diff --git a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java index a5e142b4094..88084d02c73 100644 --- a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java @@ -33,6 +33,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import com.google.common.collect.ImmutableList; @@ -89,8 +90,8 @@ public class GcpAuthenticationFilterTest { @Test public void testNewFilterInstancesPerFilterName() { - assertThat(new GcpAuthenticationFilter("FILTER_INSTANCE_NAME1")) - .isNotEqualTo(new GcpAuthenticationFilter("FILTER_INSTANCE_NAME1")); + assertThat(new GcpAuthenticationFilter("FILTER_INSTANCE_NAME1", 10)) + .isNotEqualTo(new GcpAuthenticationFilter("FILTER_INSTANCE_NAME1", 10)); } @Test @@ -152,7 +153,7 @@ public void testClientInterceptor_success() throws IOException, ResourceInvalidE .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); GcpAuthenticationConfig config = new GcpAuthenticationConfig(10); - GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME"); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 10); ClientInterceptor interceptor = filter.buildClientInterceptor(config, null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); Channel mockChannel = Mockito.mock(Channel.class); @@ -181,7 +182,7 @@ public void testClientInterceptor_createsAndReusesCachedCredentials() .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); GcpAuthenticationConfig config = new GcpAuthenticationConfig(10); - GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME"); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 10); ClientInterceptor interceptor = filter.buildClientInterceptor(config, null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); Channel mockChannel = Mockito.mock(Channel.class); @@ -190,7 +191,7 @@ public void testClientInterceptor_createsAndReusesCachedCredentials() interceptor.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel); interceptor.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel); - verify(mockChannel, Mockito.times(2)) + verify(mockChannel, times(2)) .newCall(eq(methodDescriptor), callOptionsCaptor.capture()); CallOptions firstCapturedOptions = callOptionsCaptor.getAllValues().get(0); CallOptions secondCapturedOptions = callOptionsCaptor.getAllValues().get(1); @@ -202,7 +203,7 @@ public void testClientInterceptor_createsAndReusesCachedCredentials() @Test public void testClientInterceptor_withoutClusterSelectionKey() throws Exception { GcpAuthenticationConfig config = new GcpAuthenticationConfig(10); - GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME"); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 10); ClientInterceptor interceptor = filter.buildClientInterceptor(config, null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); Channel mockChannel = mock(Channel.class); @@ -233,7 +234,7 @@ public void testClientInterceptor_clusterSelectionKeyWithoutPrefix() throws Exce Channel mockChannel = mock(Channel.class); GcpAuthenticationConfig config = new GcpAuthenticationConfig(10); - GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME"); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 10); ClientInterceptor interceptor = filter.buildClientInterceptor(config, null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); interceptor.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel); @@ -244,7 +245,7 @@ public void testClientInterceptor_clusterSelectionKeyWithoutPrefix() throws Exce @Test public void testClientInterceptor_xdsConfigDoesNotExist() throws Exception { GcpAuthenticationConfig config = new GcpAuthenticationConfig(10); - GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME"); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 10); ClientInterceptor interceptor = filter.buildClientInterceptor(config, null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); Channel mockChannel = mock(Channel.class); @@ -274,7 +275,7 @@ public void testClientInterceptor_incorrectClusterName() throws Exception { .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster") .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); GcpAuthenticationConfig config = new GcpAuthenticationConfig(10); - GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME"); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 10); ClientInterceptor interceptor = filter.buildClientInterceptor(config, null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); Channel mockChannel = mock(Channel.class); @@ -300,7 +301,7 @@ public void testClientInterceptor_statusOrError() throws Exception { .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); GcpAuthenticationConfig config = new GcpAuthenticationConfig(10); - GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME"); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 10); ClientInterceptor interceptor = filter.buildClientInterceptor(config, null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); Channel mockChannel = mock(Channel.class); @@ -329,7 +330,7 @@ public void testClientInterceptor_notAudienceWrapper() .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); GcpAuthenticationConfig config = new GcpAuthenticationConfig(10); - GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME"); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 10); ClientInterceptor interceptor = filter.buildClientInterceptor(config, null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); Channel mockChannel = Mockito.mock(Channel.class); @@ -342,6 +343,40 @@ public void testClientInterceptor_notAudienceWrapper() assertThat(clientCall.error.getDescription()).contains("GCP Authn found wrong type"); } + @Test + public void testLruCacheAcrossInterceptors() throws IOException, ResourceInvalidException { + XdsConfig.XdsClusterConfig clusterConfig = new XdsConfig.XdsClusterConfig( + CLUSTER_NAME, cdsUpdate, new EndpointConfig(StatusOr.fromValue(edsUpdate))); + XdsConfig defaultXdsConfig = new XdsConfig.XdsConfigBuilder() + .setListener(ldsUpdate) + .setRoute(rdsUpdate) + .setVirtualHost(rdsUpdate.virtualHosts.get(0)) + .addCluster(CLUSTER_NAME, StatusOr.fromValue(clusterConfig)).build(); + CallOptions callOptionsWithXds = CallOptions.DEFAULT + .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") + .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 2); + ClientInterceptor interceptor1 + = filter.buildClientInterceptor(new GcpAuthenticationConfig(2), null, null); + ClientInterceptor interceptor2 + = filter.buildClientInterceptor(new GcpAuthenticationConfig(2), null, null); + MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); + Channel mockChannel = Mockito.mock(Channel.class); + ArgumentCaptor callOptionsCaptor = ArgumentCaptor.forClass(CallOptions.class); + + interceptor1.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel); + verify(mockChannel).newCall(eq(methodDescriptor), callOptionsCaptor.capture()); + CallOptions capturedOptions1 = callOptionsCaptor.getAllValues().get(0); + assertNotNull(capturedOptions1.getCredentials()); + interceptor2.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel); + verify(mockChannel, times(2)) + .newCall(eq(methodDescriptor), callOptionsCaptor.capture()); + CallOptions capturedOptions2 = callOptionsCaptor.getAllValues().get(1); + assertNotNull(capturedOptions2.getCredentials()); + + assertSame(capturedOptions1.getCredentials(), capturedOptions2.getCredentials()); + } + private static LdsUpdate getLdsUpdate() { Filter.NamedFilterConfig routerFilterConfig = new Filter.NamedFilterConfig( serverName, RouterFilter.ROUTER_CONFIG); From 408a85e1afff0802faae0c861a72b4041a20555d Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Fri, 4 Apr 2025 20:19:22 +0000 Subject: [PATCH 02/10] respect new cache_size --- .../io/grpc/xds/GcpAuthenticationFilter.java | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java index 55b76b54b31..0f04d8c1d10 100644 --- a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java +++ b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java @@ -45,8 +45,10 @@ import io.grpc.xds.MetadataRegistry.MetadataValueParser; import io.grpc.xds.XdsConfig.XdsClusterConfig; import io.grpc.xds.client.XdsResourceType.ResourceInvalidException; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.ScheduledExecutorService; import java.util.function.Function; import javax.annotation.Nullable; @@ -102,11 +104,14 @@ public ConfigOrError parseFilterConfig(Message rawProto // Validate cache_config if (gcpAuthnProto.hasCacheConfig()) { TokenCacheConfig cacheConfig = gcpAuthnProto.getCacheConfig(); - cacheSize = cacheConfig.getCacheSize().getValue(); - if (cacheSize == 0) { - return ConfigOrError.fromError( - "cache_config.cache_size must be greater than zero"); + if (cacheConfig.hasCacheSize()) { + cacheSize = cacheConfig.getCacheSize().getValue(); + if (cacheSize == 0) { + return ConfigOrError.fromError( + "cache_config.cache_size must be greater than zero"); + } } + // LruCache's size is an int and briefly exceeds its maximum size before evicting entries cacheSize = UnsignedLongs.min(cacheSize, Integer.MAX_VALUE - 1); } @@ -128,6 +133,7 @@ public ClientInterceptor buildClientInterceptor(FilterConfig config, @Nullable FilterConfig overrideConfig, ScheduledExecutorService scheduler) { ComputeEngineCredentials credentials = ComputeEngineCredentials.create(); + callCredentialsCache.resizeCache(((GcpAuthenticationConfig) config).getCacheSize()); return new ClientInterceptor() { @Override public ClientCall interceptCall( @@ -254,8 +260,10 @@ public void sendMessage(ReqT message) {} private static final class LruCache { private final Map cache; + private int maxSize; LruCache(int maxSize) { + this.maxSize = maxSize; this.cache = new LinkedHashMap( maxSize, 0.75f, @@ -270,6 +278,19 @@ protected boolean removeEldestEntry(Map.Entry eldest) { V getOrInsert(K key, Function create) { return cache.computeIfAbsent(key, create); } + + private void resizeCache(int newSize) { + while (cache.size() > newSize) { + Iterator> iterator = cache.entrySet().iterator(); + if (iterator.hasNext()) { + iterator.next(); + iterator.remove(); + } else { + break; + } + } + maxSize = newSize; + } } static class AudienceMetadataParser implements MetadataValueParser { From 661ddfa605e12dcb19d775d335a2ef2c40165860 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Sat, 5 Apr 2025 13:08:11 +0000 Subject: [PATCH 03/10] respect new cache_size --- .../io/grpc/xds/GcpAuthenticationFilter.java | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java index 0f04d8c1d10..8162fa515f7 100644 --- a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java +++ b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java @@ -45,8 +45,9 @@ import io.grpc.xds.MetadataRegistry.MetadataValueParser; import io.grpc.xds.XdsConfig.XdsClusterConfig; import io.grpc.xds.client.XdsResourceType.ResourceInvalidException; -import java.util.Iterator; +import java.util.ArrayList; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.ScheduledExecutorService; @@ -133,7 +134,9 @@ public ClientInterceptor buildClientInterceptor(FilterConfig config, @Nullable FilterConfig overrideConfig, ScheduledExecutorService scheduler) { ComputeEngineCredentials credentials = ComputeEngineCredentials.create(); - callCredentialsCache.resizeCache(((GcpAuthenticationConfig) config).getCacheSize()); + synchronized (callCredentialsCache) { + callCredentialsCache.resizeCache(((GcpAuthenticationConfig) config).getCacheSize()); + } return new ClientInterceptor() { @Override public ClientCall interceptCall( @@ -259,7 +262,7 @@ public void sendMessage(ReqT message) {} private static final class LruCache { - private final Map cache; + private Map cache; private int maxSize; LruCache(int maxSize) { @@ -270,7 +273,7 @@ private static final class LruCache { true) { @Override protected boolean removeEldestEntry(Map.Entry eldest) { - return size() > maxSize; + return size() > LruCache.this.maxSize; } }; } @@ -280,15 +283,19 @@ V getOrInsert(K key, Function create) { } private void resizeCache(int newSize) { - while (cache.size() > newSize) { - Iterator> iterator = cache.entrySet().iterator(); - if (iterator.hasNext()) { - iterator.next(); - iterator.remove(); - } else { - break; - } + if (newSize >= maxSize) { + maxSize = newSize; + return; + } + LinkedHashMap newCache = new LinkedHashMap<>(newSize, 0.75f, true); + // Copy the MRU entries (which are at the end of access-order map) + List> entries = new ArrayList<>(cache.entrySet()); + int start = Math.max(0, entries.size() - newSize); + for (int i = entries.size() - 1; i >= start; i--) { + Map.Entry entry = entries.get(i); + newCache.put(entry.getKey(), entry.getValue()); } + cache = newCache; maxSize = newSize; } } From 6e0abb111f6aac7f0c8ce28248fea372bcfefb50 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Sat, 5 Apr 2025 13:49:47 +0000 Subject: [PATCH 04/10] respect new cache_size --- xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java index 88084d02c73..3ee92a2a6e9 100644 --- a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java @@ -357,7 +357,7 @@ public void testLruCacheAcrossInterceptors() throws IOException, ResourceInvalid .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 2); ClientInterceptor interceptor1 - = filter.buildClientInterceptor(new GcpAuthenticationConfig(2), null, null); + = filter.buildClientInterceptor(new GcpAuthenticationConfig(4), null, null); ClientInterceptor interceptor2 = filter.buildClientInterceptor(new GcpAuthenticationConfig(2), null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); From 244ac4e31a7e5807544e423bd0ead281772b561d Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Mon, 7 Apr 2025 18:17:35 +0000 Subject: [PATCH 05/10] respect new cache_size --- .../io/grpc/xds/GcpAuthenticationFilter.java | 33 ++++++++----------- .../grpc/xds/GcpAuthenticationFilterTest.java | 4 +-- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java index 8162fa515f7..353b3c5c7e9 100644 --- a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java +++ b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java @@ -45,11 +45,8 @@ import io.grpc.xds.MetadataRegistry.MetadataValueParser; import io.grpc.xds.XdsConfig.XdsClusterConfig; import io.grpc.xds.client.XdsResourceType.ResourceInvalidException; -import java.util.ArrayList; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.concurrent.ScheduledExecutorService; import java.util.function.Function; import javax.annotation.Nullable; @@ -267,15 +264,7 @@ private static final class LruCache { LruCache(int maxSize) { this.maxSize = maxSize; - this.cache = new LinkedHashMap( - maxSize, - 0.75f, - true) { - @Override - protected boolean removeEldestEntry(Map.Entry eldest) { - return size() > LruCache.this.maxSize; - } - }; + this.cache = createEvictingMap(maxSize); } V getOrInsert(K key, Function create) { @@ -287,17 +276,21 @@ private void resizeCache(int newSize) { maxSize = newSize; return; } - LinkedHashMap newCache = new LinkedHashMap<>(newSize, 0.75f, true); - // Copy the MRU entries (which are at the end of access-order map) - List> entries = new ArrayList<>(cache.entrySet()); - int start = Math.max(0, entries.size() - newSize); - for (int i = entries.size() - 1; i >= start; i--) { - Map.Entry entry = entries.get(i); - newCache.put(entry.getKey(), entry.getValue()); - } + LinkedHashMap newCache = createEvictingMap(newSize); + newCache.putAll(cache); cache = newCache; maxSize = newSize; } + + @SuppressWarnings("NonApiType") + private LinkedHashMap createEvictingMap(int size) { + return new LinkedHashMap(size, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > LruCache.this.maxSize; + } + }; + } } static class AudienceMetadataParser implements MetadataValueParser { diff --git a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java index 3ee92a2a6e9..c56421891f7 100644 --- a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java @@ -357,8 +357,6 @@ public void testLruCacheAcrossInterceptors() throws IOException, ResourceInvalid .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 2); ClientInterceptor interceptor1 - = filter.buildClientInterceptor(new GcpAuthenticationConfig(4), null, null); - ClientInterceptor interceptor2 = filter.buildClientInterceptor(new GcpAuthenticationConfig(2), null, null); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); Channel mockChannel = Mockito.mock(Channel.class); @@ -368,6 +366,8 @@ public void testLruCacheAcrossInterceptors() throws IOException, ResourceInvalid verify(mockChannel).newCall(eq(methodDescriptor), callOptionsCaptor.capture()); CallOptions capturedOptions1 = callOptionsCaptor.getAllValues().get(0); assertNotNull(capturedOptions1.getCredentials()); + ClientInterceptor interceptor2 + = filter.buildClientInterceptor(new GcpAuthenticationConfig(1), null, null); interceptor2.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel); verify(mockChannel, times(2)) .newCall(eq(methodDescriptor), callOptionsCaptor.capture()); From 954023a12b6249d64f3e9343236dcee1c06a9333 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Tue, 8 Apr 2025 02:27:23 +0000 Subject: [PATCH 06/10] respect new cache_size --- xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java index 353b3c5c7e9..9570904397c 100644 --- a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java +++ b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java @@ -276,18 +276,17 @@ private void resizeCache(int newSize) { maxSize = newSize; return; } - LinkedHashMap newCache = createEvictingMap(newSize); + LinkedHashMap newCache = (LinkedHashMap) createEvictingMap(newSize); newCache.putAll(cache); cache = newCache; maxSize = newSize; } - @SuppressWarnings("NonApiType") - private LinkedHashMap createEvictingMap(int size) { + private Map createEvictingMap(int size) { return new LinkedHashMap(size, 0.75f, true) { @Override protected boolean removeEldestEntry(Map.Entry eldest) { - return size() > LruCache.this.maxSize; + return size() > size; } }; } From 34558dd6f74a87f7f012588448674de3008df350 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Tue, 8 Apr 2025 04:47:41 +0000 Subject: [PATCH 07/10] respect new cache_size --- xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java index 9570904397c..6f8627feef6 100644 --- a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java +++ b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java @@ -276,17 +276,17 @@ private void resizeCache(int newSize) { maxSize = newSize; return; } - LinkedHashMap newCache = (LinkedHashMap) createEvictingMap(newSize); + Map newCache = createEvictingMap(newSize); + maxSize = newSize; newCache.putAll(cache); cache = newCache; - maxSize = newSize; } private Map createEvictingMap(int size) { return new LinkedHashMap(size, 0.75f, true) { @Override protected boolean removeEldestEntry(Map.Entry eldest) { - return size() > size; + return size() > LruCache.this.maxSize; } }; } From b0632fce80d54e9f81cf636cc9e09f95ec8ea404 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Tue, 8 Apr 2025 17:00:38 +0000 Subject: [PATCH 08/10] Address comment --- xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java index 6f8627feef6..8ec02f4f809 100644 --- a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java +++ b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java @@ -68,7 +68,7 @@ final class GcpAuthenticationFilter implements Filter { } static final class Provider implements Filter.Provider { - long cacheSize = 10; + private final int cacheSize = 10; @Override public String[] typeUrls() { @@ -82,7 +82,7 @@ public boolean isClientFilter() { @Override public GcpAuthenticationFilter newInstance(String name) { - return new GcpAuthenticationFilter(name, (int) cacheSize); + return new GcpAuthenticationFilter(name, cacheSize); } @Override @@ -99,6 +99,7 @@ public ConfigOrError parseFilterConfig(Message rawProto return ConfigOrError.fromError("Invalid proto: " + e); } + long cacheSize = 10; // Validate cache_config if (gcpAuthnProto.hasCacheConfig()) { TokenCacheConfig cacheConfig = gcpAuthnProto.getCacheConfig(); From 2f8532881e127a26092ad778d28b345cee80b146 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Wed, 16 Apr 2025 04:38:05 +0000 Subject: [PATCH 09/10] Address comment --- .../grpc/xds/GcpAuthenticationFilterTest.java | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java index c56421891f7..cd48fa7bdf0 100644 --- a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java @@ -28,6 +28,7 @@ import static io.grpc.xds.XdsTestUtils.getWrrLbConfigAsMap; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -377,6 +378,76 @@ public void testLruCacheAcrossInterceptors() throws IOException, ResourceInvalid assertSame(capturedOptions1.getCredentials(), capturedOptions2.getCredentials()); } + @Test + public void testLruCacheEvictionOnResize() throws IOException, ResourceInvalidException { + XdsConfig.XdsClusterConfig clusterConfig = new XdsConfig.XdsClusterConfig( + CLUSTER_NAME, cdsUpdate, new EndpointConfig(StatusOr.fromValue(edsUpdate))); + XdsConfig defaultXdsConfig = new XdsConfig.XdsConfigBuilder() + .setListener(ldsUpdate) + .setRoute(rdsUpdate) + .setVirtualHost(rdsUpdate.virtualHosts.get(0)) + .addCluster(CLUSTER_NAME, StatusOr.fromValue(clusterConfig)).build(); + CallOptions callOptionsWithXds = CallOptions.DEFAULT + .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") + .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); + GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 2); + MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); + ClientInterceptor interceptor1 = + filter.buildClientInterceptor(new GcpAuthenticationConfig(2), null, null); + Channel mockChannel1 = Mockito.mock(Channel.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(CallOptions.class); + interceptor1.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel1); + verify(mockChannel1).newCall(eq(methodDescriptor), captor.capture()); + CallOptions options1 = captor.getValue(); + assertNotNull(options1.getCredentials()); + ClientInterceptor interceptor2 = + filter.buildClientInterceptor(new GcpAuthenticationConfig(1), null, null); + Channel mockChannel2 = Mockito.mock(Channel.class); + interceptor2.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel2); + verify(mockChannel2).newCall(eq(methodDescriptor), captor.capture()); + CallOptions options2 = captor.getValue(); + assertNotNull(options2.getCredentials()); + clusterConfig = new XdsConfig.XdsClusterConfig( + CLUSTER_NAME, getCdsUpdate2(), new EndpointConfig(StatusOr.fromValue(edsUpdate))); + defaultXdsConfig = new XdsConfig.XdsConfigBuilder() + .setListener(ldsUpdate) + .setRoute(rdsUpdate) + .setVirtualHost(rdsUpdate.virtualHosts.get(0)) + .addCluster(CLUSTER_NAME, StatusOr.fromValue(clusterConfig)).build(); + callOptionsWithXds = CallOptions.DEFAULT + .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") + .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); + ClientInterceptor interceptor3 = + filter.buildClientInterceptor(new GcpAuthenticationConfig(1), null, null); + Channel mockChannel3 = Mockito.mock(Channel.class); + interceptor3.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel3); + verify(mockChannel3).newCall(eq(methodDescriptor), captor.capture()); + CallOptions options3 = captor.getValue(); + assertNotNull(options3.getCredentials()); + clusterConfig = new XdsConfig.XdsClusterConfig( + CLUSTER_NAME, cdsUpdate, new EndpointConfig(StatusOr.fromValue(edsUpdate))); + defaultXdsConfig = new XdsConfig.XdsConfigBuilder() + .setListener(ldsUpdate) + .setRoute(rdsUpdate) + .setVirtualHost(rdsUpdate.virtualHosts.get(0)) + .addCluster(CLUSTER_NAME, StatusOr.fromValue(clusterConfig)).build(); + callOptionsWithXds = CallOptions.DEFAULT + .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") + .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); + + ClientInterceptor interceptor4 = + filter.buildClientInterceptor(new GcpAuthenticationConfig(1), null, null); + Channel mockChannel4 = Mockito.mock(Channel.class); + interceptor4.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel4); + verify(mockChannel4).newCall(eq(methodDescriptor), captor.capture()); + CallOptions options4 = captor.getValue(); + assertNotNull(options4.getCredentials()); + + assertSame(options1.getCredentials(), options2.getCredentials()); + assertNotSame(options1.getCredentials(), options3.getCredentials()); + assertNotSame(options1.getCredentials(), options4.getCredentials()); + } + private static LdsUpdate getLdsUpdate() { Filter.NamedFilterConfig routerFilterConfig = new Filter.NamedFilterConfig( serverName, RouterFilter.ROUTER_CONFIG); @@ -419,6 +490,19 @@ private static CdsUpdate getCdsUpdate() { } } + private static CdsUpdate getCdsUpdate2() { + ImmutableMap.Builder parsedMetadata = ImmutableMap.builder(); + parsedMetadata.put("FILTER_INSTANCE_NAME", new AudienceWrapper("NEW_TEST_AUDIENCE")); + try { + CdsUpdate.Builder cdsUpdate = CdsUpdate.forEds( + CLUSTER_NAME, EDS_NAME, null, null, null, null, false) + .lbPolicyConfig(getWrrLbConfigAsMap()); + return cdsUpdate.parsedMetadata(parsedMetadata.build()).build(); + } catch (IOException ex) { + return null; + } + } + private static CdsUpdate getCdsUpdateWithIncorrectAudienceWrapper() throws IOException { ImmutableMap.Builder parsedMetadata = ImmutableMap.builder(); parsedMetadata.put("FILTER_INSTANCE_NAME", "TEST_AUDIENCE"); From a013542eed0fb541e5d0272e11db06522b561f17 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Wed, 16 Apr 2025 08:34:58 +0000 Subject: [PATCH 10/10] address suggestions --- .../grpc/xds/GcpAuthenticationFilterTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java index cd48fa7bdf0..d84d8c9d768 100644 --- a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java @@ -392,6 +392,7 @@ public void testLruCacheEvictionOnResize() throws IOException, ResourceInvalidEx .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); GcpAuthenticationFilter filter = new GcpAuthenticationFilter("FILTER_INSTANCE_NAME", 2); MethodDescriptor methodDescriptor = TestMethodDescriptors.voidMethod(); + ClientInterceptor interceptor1 = filter.buildClientInterceptor(new GcpAuthenticationConfig(2), null, null); Channel mockChannel1 = Mockito.mock(Channel.class); @@ -399,14 +400,16 @@ public void testLruCacheEvictionOnResize() throws IOException, ResourceInvalidEx interceptor1.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel1); verify(mockChannel1).newCall(eq(methodDescriptor), captor.capture()); CallOptions options1 = captor.getValue(); - assertNotNull(options1.getCredentials()); + // This will recreate the cache with max size of 1 and copy the credential for audience1. ClientInterceptor interceptor2 = filter.buildClientInterceptor(new GcpAuthenticationConfig(1), null, null); Channel mockChannel2 = Mockito.mock(Channel.class); interceptor2.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel2); verify(mockChannel2).newCall(eq(methodDescriptor), captor.capture()); CallOptions options2 = captor.getValue(); - assertNotNull(options2.getCredentials()); + + assertSame(options1.getCredentials(), options2.getCredentials()); + clusterConfig = new XdsConfig.XdsClusterConfig( CLUSTER_NAME, getCdsUpdate2(), new EndpointConfig(StatusOr.fromValue(edsUpdate))); defaultXdsConfig = new XdsConfig.XdsConfigBuilder() @@ -417,13 +420,17 @@ public void testLruCacheEvictionOnResize() throws IOException, ResourceInvalidEx callOptionsWithXds = CallOptions.DEFAULT .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); + + // This will evict the credential for audience1 and add new credential for audience2 ClientInterceptor interceptor3 = filter.buildClientInterceptor(new GcpAuthenticationConfig(1), null, null); Channel mockChannel3 = Mockito.mock(Channel.class); interceptor3.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel3); verify(mockChannel3).newCall(eq(methodDescriptor), captor.capture()); CallOptions options3 = captor.getValue(); - assertNotNull(options3.getCredentials()); + + assertNotSame(options1.getCredentials(), options3.getCredentials()); + clusterConfig = new XdsConfig.XdsClusterConfig( CLUSTER_NAME, cdsUpdate, new EndpointConfig(StatusOr.fromValue(edsUpdate))); defaultXdsConfig = new XdsConfig.XdsConfigBuilder() @@ -435,16 +442,14 @@ public void testLruCacheEvictionOnResize() throws IOException, ResourceInvalidEx .withOption(CLUSTER_SELECTION_KEY, "cluster:cluster0") .withOption(XDS_CONFIG_CALL_OPTION_KEY, defaultXdsConfig); + // This will create new credential for audience1 because it has been evicted ClientInterceptor interceptor4 = filter.buildClientInterceptor(new GcpAuthenticationConfig(1), null, null); Channel mockChannel4 = Mockito.mock(Channel.class); interceptor4.interceptCall(methodDescriptor, callOptionsWithXds, mockChannel4); verify(mockChannel4).newCall(eq(methodDescriptor), captor.capture()); CallOptions options4 = captor.getValue(); - assertNotNull(options4.getCredentials()); - assertSame(options1.getCredentials(), options2.getCredentials()); - assertNotSame(options1.getCredentials(), options3.getCredentials()); assertNotSame(options1.getCredentials(), options4.getCredentials()); }