🍒 9201 - Match Hands Off Config selectors on process_arguments value

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java

@@ -12,8 +12,8 @@
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
-import java.util.function.BiPredicate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -104,90 +104,70 @@ private static boolean doesRuleMatch(Rule rule) {
     return true; // Return true if all selectors match
   }
 
-  private static boolean validOperatorForLanguageOrigin(String operator) {
-    operator = operator.toLowerCase();
-    // "exists" is not valid
-    switch (operator) {
-      case "equals":
-      case "starts_with":
-      case "ends_with":
-      case "contains":
-        return true;
-      default:
-        return false;
+  private static boolean matchOperator(String value, String operator, List<String> matches) {
+    if (value == null || operator == null) {
+      return false;
     }
-  }
-
-  private static boolean checkEnvMatches(
-      List<String> values, List<String> matches, BiPredicate<String, String> compareFunc) {
-    // envValue shouldn't be null, but doing an extra check to avoid NullPointerException on
-    // compareFunc.test
-    if (values == null) {
+    if ("exists".equals(operator)) {
+      return true;
+    }
+    if (matches == null || matches.isEmpty()) {
       return false;
     }
+    value = value.toLowerCase(Locale.ROOT);
     for (String match : matches) {
       if (match == null) {
         continue;
       }
-      for (String value : values) {
-        if (compareFunc.test(value, match.toLowerCase())) {
-          return true;
-        }
+      match = match.toLowerCase(Locale.ROOT);
+      switch (operator) {
+        case "equals":
+          return value.equals(match);
+        case "starts_with":
+          return value.startsWith(match);
+        case "ends_with":
+          return value.endsWith(match);
+        case "contains":
+          return value.contains(match);
+        default:
+          return false;
       }
     }
     return false;
   }
 
-  // We do all of the case insensitivity modifications in this function, because each selector will
+  // We do all the case insensitivity modifications in this function, because each selector will
   // be viewed just once
   static boolean selectorMatch(String origin, List<String> matches, String operator, String key) {
-    switch (origin.toLowerCase()) {
+    if (operator == null) {
+      return false;
+    }
+    operator = operator.toLowerCase(Locale.ROOT);
+    switch (origin.toLowerCase(Locale.ROOT)) {
       case "language":
-        if (!validOperatorForLanguageOrigin(operator)) {
+        if ("exists".equals(operator)) {
           return false;
         }
-        for (String entry : matches) {
-          // loose match on any reference to "*java*"
-          if (entry.toLowerCase().contains("java")) {
-            return true;
-          }
-        }
+        return matchOperator("java", operator, matches);
       case "environment_variables":
         if (key == null) {
           return false;
         }
-        String envValue = System.getenv(key.toUpperCase());
-        if (envValue == null) {
+        String envValue = System.getenv(key.toUpperCase(Locale.ROOT));
+        return matchOperator(envValue, operator, matches);
+      case "process_arguments":
+        if (key == null) {
           return false;
         }
-        envValue = envValue.toLowerCase();
-        switch (operator.toLowerCase()) {
-          case "exists":
-            // We don't care about the value
-            return true;
-          case "equals":
-            return checkEnvMatches(
-                Collections.singletonList(envValue), matches, String::equalsIgnoreCase);
-          case "starts_with":
-            return checkEnvMatches(
-                Collections.singletonList(envValue), matches, String::startsWith);
-          case "ends_with":
-            return checkEnvMatches(Collections.singletonList(envValue), matches, String::endsWith);
-          case "contains":
-            return checkEnvMatches(Collections.singletonList(envValue), matches, String::contains);
-          default:
-            return false;
-        }
-      case "process_arguments":
         // TODO: flesh out the meaning of each operator for process_arguments
         if (!key.startsWith("-D")) {
           log.warn(
               "Ignoring unsupported process_arguments entry in selector match, '{}'. Only system properties specified with the '-D' prefix are supported.",
               key);
           return false;
         }
-        // Cut the -D prefix
-        return System.getProperty(key.substring(2)) != null;
+        String argValue = System.getProperty(key.substring(2));
+        return matchOperator(argValue, operator, matches);
       case "tags":
         // TODO: Support this down the line (Must define the source of "tags" first)
         return false;
@@ -249,7 +229,7 @@ private static String processTemplateVar(String templateVar) throws IOException
       if (envVar.isEmpty()) {
         throw new IOException("Empty environment variable name in template");
       }
-      String value = System.getenv(envVar.toUpperCase());
+      String value = System.getenv(envVar.toUpperCase(Locale.ROOT));
       if (value == null || value.isEmpty()) {
         return UNDEFINED_VALUE;
       }

internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy

@@ -62,12 +62,40 @@ apm_configuration_rules:
     Files.delete(filePath)
   }
 
+  def "test parse and template"() {
+    when:
+    Path filePath = Files.createTempFile("testFile_", ".yaml")
+    then:
+    if (filePath == null) {
+      throw new AssertionError("Failed to create: " + filePath)
+    }
+
+    when:
+    String yaml = """
+    apm_configuration_rules:
+      - selectors:
+        - origin: process_arguments
+          key: "-Dtest_parse_and_template"
+          operator: exists
+        configuration:
+          DD_SERVICE: {{process_arguments['-Dtest_parse_and_template']}}
+"""
+    System.setProperty("test_parse_and_template", "myservice")
+    Files.write(filePath, yaml.getBytes())
+    StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString())
+
+    then:
+    cfg.get("DD_SERVICE") == "myservice"
+  }
+
   def "test selectorMatch"() {
     when:
     // Env vars
     injectEnvConfig("DD_PROFILING_ENABLED", "true")
     injectEnvConfig("DD_SERVICE", "mysvc")
     injectEnvConfig("DD_TAGS", "team:apm,component:web")
+    System.setProperty("test_selectorMatch", "value1")
+
     def match = StableConfigParser.selectorMatch(origin, matches, operator, key)
 
     then:
@@ -83,6 +111,7 @@ apm_configuration_rules:
     "language"              | ["java"]              | "exists"               | ""                     | false
     "language"              | ["java"]              | "something unexpected" | ""                     | false
     "environment_variables" | []                    | "exists"               | "DD_TAGS"              | true
+    "environment_variables" | null                  | "exists"               | "DD_TAGS"              | true
     "environment_variables" | ["team:apm"]          | "contains"             | "DD_TAGS"              | true
     "ENVIRONMENT_VARIABLES" | ["TeAm:ApM"]          | "CoNtAiNs"             | "Dd_TaGs"              | true // check case insensitivity
     "environment_variables" | ["team:apm"]          | "equals"               | "DD_TAGS"              | false
@@ -96,6 +125,13 @@ apm_configuration_rules:
     "environment_variables" | ["svc"]               | "contains"             | "DD_SERVICE"           | true
     "environment_variables" | ["other"]             | "contains"             | "DD_SERVICE"           | false
     "environment_variables" | [null]                | "contains"             | "DD_SERVICE"           | false
+    "environment_variables" | []                    | "equals"               | null                   | false
+    "environment_variables" | null                  | "equals"               | "DD_SERVICE"           | false
+    "language"              | ["java"]              | null                   | ""                     | false
+    "process_arguments"     | null                  | "exists"               | "-Dtest_selectorMatch" | true
+    "process_arguments"     | null                  | "exists"               | "-Darg2"               | false
+    "process_arguments"     | ["value1"]            | "equals"               | "-Dtest_selectorMatch" | true
+    "process_arguments"     | ["value2"]            | "equals"               | "-Dtest_selectorMatch" | false
   }
 
   def "test duplicate entries not allowed"() {