rls: allow maxAge to exceed 5m if staleAge is set (grpc#11931)

Author: shivaspeaks

rls/src/main/java/io/grpc/rls/RlsProtoConverters.java

@@ -152,10 +152,15 @@ protected RouteLookupConfig doForward(Map<String, ?> json) {
         checkArgument(staleAge == null, "to specify staleAge, must have maxAge");
         maxAge = MAX_AGE_NANOS;
       }
-      if (staleAge == null) {
+      // If staleAge is not set, clamp maxAge to <= 5.
+      if (staleAge == null && maxAge > MAX_AGE_NANOS) {
+        maxAge = MAX_AGE_NANOS;
+      }
+      // Clamp staleAge to <= 5
+      if (staleAge == null || staleAge > MAX_AGE_NANOS) {
         staleAge = MAX_AGE_NANOS;
       }
-      maxAge = Math.min(maxAge, MAX_AGE_NANOS);
+      // Ignore staleAge if greater than maxAge.
       staleAge = Math.min(staleAge, maxAge);
       long cacheSize = orDefault(JsonUtil.getNumberAsLong(json, "cacheSizeBytes"), MAX_CACHE_SIZE);
       checkArgument(cacheSize > 0, "cacheSize must be positive");

rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java

@@ -469,6 +469,65 @@ public void convert_jsonRlsConfig_staleAgeGivenWithoutMaxAge() throws IOExceptio
     }
   }
 
+  @Test
+  public void convert_jsonRlsConfig_doNotClampMaxAgeIfStaleAgeIsSet() throws IOException {
+    String jsonStr = "{\n"
+        + "  \"grpcKeybuilders\": [\n"
+        + "    {\n"
+        + "      \"names\": [\n"
+        + "        {\n"
+        + "          \"service\": \"service1\",\n"
+        + "          \"method\": \"create\"\n"
+        + "        }\n"
+        + "      ],\n"
+        + "      \"headers\": [\n"
+        + "        {\n"
+        + "          \"key\": \"user\","
+        + "          \"names\": [\"User\", \"Parent\"],\n"
+        + "          \"optional\": true\n"
+        + "        },\n"
+        + "        {\n"
+        + "          \"key\": \"id\","
+        + "          \"names\": [\"X-Google-Id\"],\n"
+        + "          \"optional\": true\n"
+        + "        }\n"
+        + "      ]\n"
+        + "    }\n"
+        + "  ],\n"
+        + "  \"lookupService\": \"service1\",\n"
+        + "  \"lookupServiceTimeout\": \"2s\",\n"
+        + "  \"maxAge\": \"350s\",\n"
+        + "  \"staleAge\": \"310s\",\n"
+        + "  \"validTargets\": [\"a valid target\"],"
+        + "  \"cacheSizeBytes\": \"1000\",\n"
+        + "  \"defaultTarget\": \"us_east_1.cloudbigtable.googleapis.com\"\n"
+        + "}";
+
+    RouteLookupConfig expectedConfig =
+        RouteLookupConfig.builder()
+            .grpcKeybuilders(ImmutableList.of(
+                GrpcKeyBuilder.create(
+                    ImmutableList.of(Name.create("service1", "create")),
+                    ImmutableList.of(
+                        NameMatcher.create("user", ImmutableList.of("User", "Parent")),
+                        NameMatcher.create("id", ImmutableList.of("X-Google-Id"))),
+                    ExtraKeys.DEFAULT,
+                    ImmutableMap.<String, String>of())))
+            .lookupService("service1")
+            .lookupServiceTimeoutInNanos(TimeUnit.SECONDS.toNanos(2))
+            .maxAgeInNanos(TimeUnit.SECONDS.toNanos(350)) // Should not be clamped
+            .staleAgeInNanos(TimeUnit.SECONDS.toNanos(300)) // Should be clamped to max 300s
+            .cacheSizeBytes(1000)
+            .defaultTarget("us_east_1.cloudbigtable.googleapis.com")
+            .build();
+
+    RouteLookupConfigConverter converter = new RouteLookupConfigConverter();
+    @SuppressWarnings("unchecked")
+    Map<String, ?> parsedJson = (Map<String, ?>) JsonParser.parse(jsonStr);
+    RouteLookupConfig converted = converter.convert(parsedJson);
+    assertThat(converted).isEqualTo(expectedConfig);
+  }
+
   @Test
   public void convert_jsonRlsConfig_keyBuilderWithoutName() throws IOException {
     String jsonStr = "{\n"