fix: The configfile interface cannot correctly obtain configuration t…

…hrough grayscale labels
apolloconfig · Aug 14, 2024 · 0ee0e88 · 0ee0e88
1 parent c28eb49
commit 0ee0e88
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 22 deletions.
diff --git a/.../src/main/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolder.java b/.../src/main/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolder.java
@@ -152,15 +152,36 @@ public Long findReleaseIdFromGrayReleaseRule(String clientAppId, String clientIp
   }
 
   /**
-   * Check whether there are gray release rules for the clientAppId, clientIp, namespace
+   * Check whether there are gray release rules for the clientAppId, clusterName, clientIp, clientLabel, namespace
    * combination. Please note that even there are gray release rules, it doesn't mean it will always
    * load gray releases. Because gray release rules actually apply to one more dimension - cluster.
    */
-  public boolean hasGrayReleaseRule(String clientAppId, String clientIp, String namespaceName) {
-    return reversedGrayReleaseRuleCache.containsKey(assembleReversedGrayReleaseRuleKey(clientAppId,
-        namespaceName, clientIp)) || reversedGrayReleaseRuleCache.containsKey
-        (assembleReversedGrayReleaseRuleKey(clientAppId, namespaceName, GrayReleaseRuleItemDTO
-            .ALL_IP));
+  public boolean hasGrayReleaseRule(String clientAppId, String clusterName, String clientIp, String clientLabel, String namespaceName) {
+    // check ip gray rule
+    if (reversedGrayReleaseRuleCache.containsKey(assembleReversedGrayReleaseRuleKey(clientAppId,
+            namespaceName, clientIp)) || reversedGrayReleaseRuleCache.containsKey
+            (assembleReversedGrayReleaseRuleKey(clientAppId, namespaceName, GrayReleaseRuleItemDTO
+                    .ALL_IP))) {
+      return true;
+    }
+
+    // check label gray rule
+    String key = assembleGrayReleaseRuleKey(clientAppId, clusterName, namespaceName);
+    if (!grayReleaseRuleCache.containsKey(key)) {
+      return false;
+    }
+    List<GrayReleaseRuleCache> rules = Lists.newArrayList(grayReleaseRuleCache.get(key));
+    for (GrayReleaseRuleCache rule : rules) {
+      //check branch status
+      if (rule.getBranchStatus() != NamespaceBranchStatus.ACTIVE) {
+        continue;
+      }
+      if (rule.matches(clientAppId, clientIp, clientLabel)) {
+        return true;
+      }
+    };
+
+    return false;
   }
 
   private void scanGrayReleaseRules() {

diff --git a/.../test/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolderTest.java b/.../test/java/com/ctrip/framework/apollo/biz/grayReleaseRule/GrayReleaseRulesHolderTest.java
@@ -109,18 +109,18 @@ public void testScanGrayReleaseRules() throws Exception {
     assertNull(grayReleaseRulesHolder.findReleaseIdFromGrayReleaseRule(anotherClientAppId,
         anotherClientIp, anotherClientLabel, someAppId, someClusterName, someNamespaceName));
 
-    assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClientIp,
+    assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, someClientIp, someClientLabel,
         someNamespaceName));
-    assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId.toUpperCase(), someClientIp,
+    assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId.toUpperCase(), someClusterName, someClientIp, someClientLabel,
         someNamespaceName.toUpperCase()));
-    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, anotherClientIp,
+    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, anotherClientIp, someClientLabel,
         someNamespaceName));
-    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClientIp,
+    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, someClientIp, someClientLabel,
         anotherNamespaceName));
 
-    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, anotherClientIp,
+    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, anotherClientIp, someClientLabel,
         someNamespaceName));
-    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, anotherClientIp,
+    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, anotherClientIp, someClientLabel,
         anotherNamespaceName));
 
     GrayReleaseRule anotherRule = assembleGrayReleaseRule(someAppId, someClusterName,
@@ -144,16 +144,16 @@ public void testScanGrayReleaseRules() throws Exception {
         (anotherClientAppId, anotherClientIp, anotherClientLabel, someAppId, someClusterName, someNamespaceName));
 
 
-    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClientIp,
+    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, someClientIp, someClientLabel,
         someNamespaceName));
-    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClientIp,
+    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(someClientAppId, someClusterName, someClientIp, someClientLabel,
         anotherNamespaceName));
 
-    assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, anotherClientIp,
+    assertTrue(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, anotherClientIp, someClientLabel,
         someNamespaceName));
-    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClientIp,
+    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, someClientIp, someClientLabel,
         someNamespaceName));
-    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, anotherClientIp,
+    assertFalse(grayReleaseRulesHolder.hasGrayReleaseRule(anotherClientAppId, someClusterName, anotherClientIp, someClientLabel,
         anotherNamespaceName));
   }
 

diff --git a/...c/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java b/...c/main/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileController.java
@@ -149,6 +149,7 @@ public ResponseEntity<String> queryConfigAsJson(@PathVariable String appId,
                                                   HttpServletRequest request,
                                                   HttpServletResponse response) throws IOException {
 
+    System.out.println("requeset1: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel);
     String result =
         queryConfig(ConfigFileOutputFormat.JSON, appId, clusterName, namespace, dataCenter,
             clientIp, clientLabel, request, response);
@@ -174,14 +175,14 @@ String queryConfig(ConfigFileOutputFormat outputFormat, String appId, String clu
     }
 
     //1. check whether this client has gray release rules
-    boolean hasGrayReleaseRule = grayReleaseRulesHolder.hasGrayReleaseRule(appId, clientIp,
-        namespace);
+    boolean hasGrayReleaseRule = grayReleaseRulesHolder.hasGrayReleaseRule(appId, clusterName, clientIp, clientLabel, namespace);
 
     String cacheKey = assembleCacheKey(outputFormat, appId, clusterName, namespace, dataCenter);
 
     //2. try to load gray release and return
     if (hasGrayReleaseRule) {
       Tracer.logEvent("ConfigFile.Cache.GrayRelease", cacheKey);
+      System.out.println("requeset2: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel);
       return loadConfig(outputFormat, appId, clusterName, namespace, dataCenter, clientIp, clientLabel,
           request, response);
     }
@@ -192,6 +193,7 @@ String queryConfig(ConfigFileOutputFormat outputFormat, String appId, String clu
     //4. if not exists, load from ConfigController
     if (Strings.isNullOrEmpty(result)) {
       Tracer.logEvent("ConfigFile.Cache.Miss", cacheKey);
+      System.out.println("requeset3: " + appId + " " + clusterName + " " + namespace + " " + dataCenter + " " + clientIp + " " + clientLabel);
       result = loadConfig(outputFormat, appId, clusterName, namespace, dataCenter, clientIp, clientLabel,
           request, response);
 
@@ -200,7 +202,7 @@ String queryConfig(ConfigFileOutputFormat outputFormat, String appId, String clu
       }
       //5. Double check if this client needs to load gray release, if yes, load from db again
       //This step is mainly to avoid cache pollution
-      if (grayReleaseRulesHolder.hasGrayReleaseRule(appId, clientIp, namespace)) {
+      if (grayReleaseRulesHolder.hasGrayReleaseRule(appId, clusterName, clientIp, clientLabel, namespace)) {
         Tracer.logEvent("ConfigFile.Cache.GrayReleaseConflict", cacheKey);
         return loadConfig(outputFormat, appId, clusterName, namespace, dataCenter, clientIp, clientLabel,
             request, response);

diff --git a/...st/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileControllerTest.java b/...st/java/com/ctrip/framework/apollo/configservice/controller/ConfigFileControllerTest.java
@@ -95,7 +95,7 @@ public void setUp() throws Exception {
 
     when(namespaceUtil.filterNamespaceName(someNamespace)).thenReturn(someNamespace);
     when(namespaceUtil.normalizeNamespace(someAppId, someNamespace)).thenReturn(someNamespace);
-    when(grayReleaseRulesHolder.hasGrayReleaseRule(anyString(), anyString(), anyString()))
+    when(grayReleaseRulesHolder.hasGrayReleaseRule(anyString(), anyString(), anyString(), anyString(), anyString()))
         .thenReturn(false);
 
     watchedKeys2CacheKey =
@@ -199,7 +199,7 @@ public void testQueryConfigWithGrayRelease() throws Exception {
     Map<String, String> configurations =
         ImmutableMap.of(someKey, someValue);
 
-    when(grayReleaseRulesHolder.hasGrayReleaseRule(someAppId, someClientIp, someNamespace))
+    when(grayReleaseRulesHolder.hasGrayReleaseRule(someAppId, someClusterName, someClientIp, someClientLabel, someNamespace))
         .thenReturn(true);
 
     ApolloConfig someApolloConfig = mock(ApolloConfig.class);