[druid] branch 24.0.0 updated: fix bug in /status/properties filtering (#13045) (#13054)

[vo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vogievetsky pushed a commit to branch 24.0.0
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/24.0.0 by this push:
     new 3974a4153e fix bug in /status/properties filtering (#13045) (#13054)
3974a4153e is described below

commit 3974a4153ea37ab5bc17c468440363bd8abaad27
Author: Vadim Ogievetsky <va...@ogievetsky.com>
AuthorDate: Wed Sep 7 22:36:12 2022 -0700

    fix bug in /status/properties filtering (#13045) (#13054)
    
    * fix bug in /status/properties filtering
    
    * Refactor tests to use jackson for parsing druid.server.hiddenProperties instead of hacky string modifications
    
    * make javadoc more descriptive using example
    
    * add in a sanity assertion that raw properties keyset size is greater than filtered properties keyset size
    
    Co-authored-by: Lucas Capistrant <ca...@users.noreply.github.com>
---
 .../org/apache/druid/server/StatusResource.java    | 20 +++++++++++--------
 .../apache/druid/server/StatusResourceTest.java    | 23 +++++++++++++++-------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/server/src/main/java/org/apache/druid/server/StatusResource.java b/server/src/main/java/org/apache/druid/server/StatusResource.java
index 2476cf4238..e15ddaf43b 100644
--- a/server/src/main/java/org/apache/druid/server/StatusResource.java
+++ b/server/src/main/java/org/apache/druid/server/StatusResource.java
@@ -43,6 +43,7 @@ import javax.ws.rs.core.MediaType;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -84,6 +85,9 @@ public class StatusResource
   /**
    * filter out entries from allProperties with key containing elements in hiddenProperties (case insensitive)
    *
+   * for example, if hiddenProperties = ["pwd"] and allProperties = {"foopwd": "secret", "foo": "bar", "my.pwd": "secret"},
+   * this method will return {"foo":"bar"}
+   *
    * @return map of properties that are not filtered out.
    */
   @Nonnull
@@ -92,15 +96,15 @@ public class StatusResource
       Map<String, String> allProperties
   )
   {
-    return Maps.filterEntries(
-        allProperties,
-        (entry) -> hiddenProperties
-            .stream()
-            .anyMatch(
-                hiddenPropertyElement ->
-                    !StringUtils.toLowerCase(entry.getKey()).contains(StringUtils.toLowerCase(hiddenPropertyElement))
-            )
+    Map<String, String> propertyCopy = new HashMap<>(allProperties);
+    allProperties.keySet().forEach(
+        (key) -> {
+          if (hiddenProperties.stream().anyMatch((hiddenProperty) -> StringUtils.toLowerCase(key).contains(StringUtils.toLowerCase(hiddenProperty)))) {
+            propertyCopy.remove(key);
+          }
+        }
     );
+    return propertyCopy;
   }
 
   @GET
diff --git a/server/src/test/java/org/apache/druid/server/StatusResourceTest.java b/server/src/test/java/org/apache/druid/server/StatusResourceTest.java
index e72c34e64b..52fb0c80d5 100644
--- a/server/src/test/java/org/apache/druid/server/StatusResourceTest.java
+++ b/server/src/test/java/org/apache/druid/server/StatusResourceTest.java
@@ -19,7 +19,8 @@
 
 package org.apache.druid.server;
 
-import com.google.common.base.Splitter;
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableList;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
@@ -32,9 +33,9 @@ import org.junit.Test;
 
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -64,18 +65,18 @@ public class StatusResourceTest
   }
 
   @Test
-  public void testHiddenProperties()
+  public void testHiddenProperties() throws Exception
   {
     testHiddenPropertiesWithPropertyFileName("status.resource.test.runtime.properties");
   }
 
   @Test
-  public void testHiddenPropertiesContain()
+  public void testHiddenPropertiesContain() throws Exception
   {
     testHiddenPropertiesWithPropertyFileName("status.resource.test.runtime.hpc.properties");
   }
 
-  private void testHiddenPropertiesWithPropertyFileName(String fileName)
+  private void testHiddenPropertiesWithPropertyFileName(String fileName) throws Exception
   {
     Injector injector = Guice.createInjector(Collections.singletonList(new PropertiesModule(Collections.singletonList(
         fileName))));
@@ -84,8 +85,16 @@ public class StatusResourceTest
                                                            .stream()
                                                            .map(StringUtils::toLowerCase)
                                                            .collect(Collectors.toSet());
-    Set<String> hiddenProperties = new HashSet<>();
-    Splitter.on(",").split(returnedProperties.get("druid.server.hiddenProperties")).forEach(hiddenProperties::add);
+
+    Assert.assertTrue(
+        "The list of unfiltered Properties is not > the list of filtered Properties?!?",
+        injector.getInstance(Properties.class).stringPropertyNames().size() > returnedProperties.size()
+    );
+
+    Set<String> hiddenProperties = new ObjectMapper().readValue(
+        returnedProperties.get("druid.server.hiddenProperties"),
+        new TypeReference<Set<String>>() {});
+
     hiddenProperties.forEach(
         (property) -> {
           lowerCasePropertyNames.forEach(


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org