Skip to content

Commit 81eb800

Browse files
mtoffl01paullegranddcPerfectSlayer
authored
🍒 9201 - Match Hands Off Config selectors on process_arguments value (#9223)
* feat: match stable config java properties on other operator than 'contains' (cherry picked from commit 2df85ac) * fix: lowercase match value (cherry picked from commit 314525a) * fix: extra parameter (cherry picked from commit b051555) * fix: remove duplicate values (cherry picked from commit c094176) * fix: lint (cherry picked from commit a58bd09) * fi: wrong variable (cherry picked from commit db6e5c7) * fix lint (cherry picked from commit 5865dc8) * fix: tests (cherry picked from commit 2f93fad) * fix: null matches with exist should work (cherry picked from commit ebf333b) * feat: add tests for equal operator on arguments (cherry picked from commit 75f6dae) * fix: use equal method instead of operator (cherry picked from commit cfb37db) * Update internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java Co-authored-by: Bruce Bujon <PerfectSlayer@users.noreply.github.com> (cherry picked from commit a8ff152) * apply PR suggestions (cherry picked from commit 6f1ea89) * fix typo (cherry picked from commit db73932) * Add missing locale (cherry picked from commit 12c673d) * add matches == null check back in to avoid NPE (cherry picked from commit 4f4c5bc) --------- Co-authored-by: paullegranddc <paul.legranddescloizeaux@datadoghq.com> Co-authored-by: Bruce Bujon <PerfectSlayer@users.noreply.github.com>
1 parent ce41a21 commit 81eb800

File tree

2 files changed

+72
-56
lines changed

2 files changed

+72
-56
lines changed

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

Lines changed: 36 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
import java.util.Collections;
1313
import java.util.LinkedHashMap;
1414
import java.util.List;
15+
import java.util.Locale;
1516
import java.util.Map;
16-
import java.util.function.BiPredicate;
1717
import org.slf4j.Logger;
1818
import org.slf4j.LoggerFactory;
1919

@@ -104,90 +104,70 @@ private static boolean doesRuleMatch(Rule rule) {
104104
return true; // Return true if all selectors match
105105
}
106106

107-
private static boolean validOperatorForLanguageOrigin(String operator) {
108-
operator = operator.toLowerCase();
109-
// "exists" is not valid
110-
switch (operator) {
111-
case "equals":
112-
case "starts_with":
113-
case "ends_with":
114-
case "contains":
115-
return true;
116-
default:
117-
return false;
107+
private static boolean matchOperator(String value, String operator, List<String> matches) {
108+
if (value == null || operator == null) {
109+
return false;
118110
}
119-
}
120-
121-
private static boolean checkEnvMatches(
122-
List<String> values, List<String> matches, BiPredicate<String, String> compareFunc) {
123-
// envValue shouldn't be null, but doing an extra check to avoid NullPointerException on
124-
// compareFunc.test
125-
if (values == null) {
111+
if ("exists".equals(operator)) {
112+
return true;
113+
}
114+
if (matches == null || matches.isEmpty()) {
126115
return false;
127116
}
117+
value = value.toLowerCase(Locale.ROOT);
128118
for (String match : matches) {
129119
if (match == null) {
130120
continue;
131121
}
132-
for (String value : values) {
133-
if (compareFunc.test(value, match.toLowerCase())) {
134-
return true;
135-
}
122+
match = match.toLowerCase(Locale.ROOT);
123+
switch (operator) {
124+
case "equals":
125+
return value.equals(match);
126+
case "starts_with":
127+
return value.startsWith(match);
128+
case "ends_with":
129+
return value.endsWith(match);
130+
case "contains":
131+
return value.contains(match);
132+
default:
133+
return false;
136134
}
137135
}
138136
return false;
139137
}
140138

141-
// We do all of the case insensitivity modifications in this function, because each selector will
139+
// We do all the case insensitivity modifications in this function, because each selector will
142140
// be viewed just once
143141
static boolean selectorMatch(String origin, List<String> matches, String operator, String key) {
144-
switch (origin.toLowerCase()) {
142+
if (operator == null) {
143+
return false;
144+
}
145+
operator = operator.toLowerCase(Locale.ROOT);
146+
switch (origin.toLowerCase(Locale.ROOT)) {
145147
case "language":
146-
if (!validOperatorForLanguageOrigin(operator)) {
148+
if ("exists".equals(operator)) {
147149
return false;
148150
}
149-
for (String entry : matches) {
150-
// loose match on any reference to "*java*"
151-
if (entry.toLowerCase().contains("java")) {
152-
return true;
153-
}
154-
}
151+
return matchOperator("java", operator, matches);
155152
case "environment_variables":
156153
if (key == null) {
157154
return false;
158155
}
159-
String envValue = System.getenv(key.toUpperCase());
160-
if (envValue == null) {
156+
String envValue = System.getenv(key.toUpperCase(Locale.ROOT));
157+
return matchOperator(envValue, operator, matches);
158+
case "process_arguments":
159+
if (key == null) {
161160
return false;
162161
}
163-
envValue = envValue.toLowerCase();
164-
switch (operator.toLowerCase()) {
165-
case "exists":
166-
// We don't care about the value
167-
return true;
168-
case "equals":
169-
return checkEnvMatches(
170-
Collections.singletonList(envValue), matches, String::equalsIgnoreCase);
171-
case "starts_with":
172-
return checkEnvMatches(
173-
Collections.singletonList(envValue), matches, String::startsWith);
174-
case "ends_with":
175-
return checkEnvMatches(Collections.singletonList(envValue), matches, String::endsWith);
176-
case "contains":
177-
return checkEnvMatches(Collections.singletonList(envValue), matches, String::contains);
178-
default:
179-
return false;
180-
}
181-
case "process_arguments":
182162
// TODO: flesh out the meaning of each operator for process_arguments
183163
if (!key.startsWith("-D")) {
184164
log.warn(
185165
"Ignoring unsupported process_arguments entry in selector match, '{}'. Only system properties specified with the '-D' prefix are supported.",
186166
key);
187167
return false;
188168
}
189-
// Cut the -D prefix
190-
return System.getProperty(key.substring(2)) != null;
169+
String argValue = System.getProperty(key.substring(2));
170+
return matchOperator(argValue, operator, matches);
191171
case "tags":
192172
// TODO: Support this down the line (Must define the source of "tags" first)
193173
return false;
@@ -249,7 +229,7 @@ private static String processTemplateVar(String templateVar) throws IOException
249229
if (envVar.isEmpty()) {
250230
throw new IOException("Empty environment variable name in template");
251231
}
252-
String value = System.getenv(envVar.toUpperCase());
232+
String value = System.getenv(envVar.toUpperCase(Locale.ROOT));
253233
if (value == null || value.isEmpty()) {
254234
return UNDEFINED_VALUE;
255235
}

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,40 @@ apm_configuration_rules:
6262
Files.delete(filePath)
6363
}
6464

65+
def "test parse and template"() {
66+
when:
67+
Path filePath = Files.createTempFile("testFile_", ".yaml")
68+
then:
69+
if (filePath == null) {
70+
throw new AssertionError("Failed to create: " + filePath)
71+
}
72+
73+
when:
74+
String yaml = """
75+
apm_configuration_rules:
76+
- selectors:
77+
- origin: process_arguments
78+
key: "-Dtest_parse_and_template"
79+
operator: exists
80+
configuration:
81+
DD_SERVICE: {{process_arguments['-Dtest_parse_and_template']}}
82+
"""
83+
System.setProperty("test_parse_and_template", "myservice")
84+
Files.write(filePath, yaml.getBytes())
85+
StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString())
86+
87+
then:
88+
cfg.get("DD_SERVICE") == "myservice"
89+
}
90+
6591
def "test selectorMatch"() {
6692
when:
6793
// Env vars
6894
injectEnvConfig("DD_PROFILING_ENABLED", "true")
6995
injectEnvConfig("DD_SERVICE", "mysvc")
7096
injectEnvConfig("DD_TAGS", "team:apm,component:web")
97+
System.setProperty("test_selectorMatch", "value1")
98+
7199
def match = StableConfigParser.selectorMatch(origin, matches, operator, key)
72100

73101
then:
@@ -83,6 +111,7 @@ apm_configuration_rules:
83111
"language" | ["java"] | "exists" | "" | false
84112
"language" | ["java"] | "something unexpected" | "" | false
85113
"environment_variables" | [] | "exists" | "DD_TAGS" | true
114+
"environment_variables" | null | "exists" | "DD_TAGS" | true
86115
"environment_variables" | ["team:apm"] | "contains" | "DD_TAGS" | true
87116
"ENVIRONMENT_VARIABLES" | ["TeAm:ApM"] | "CoNtAiNs" | "Dd_TaGs" | true // check case insensitivity
88117
"environment_variables" | ["team:apm"] | "equals" | "DD_TAGS" | false
@@ -96,6 +125,13 @@ apm_configuration_rules:
96125
"environment_variables" | ["svc"] | "contains" | "DD_SERVICE" | true
97126
"environment_variables" | ["other"] | "contains" | "DD_SERVICE" | false
98127
"environment_variables" | [null] | "contains" | "DD_SERVICE" | false
128+
"environment_variables" | [] | "equals" | null | false
129+
"environment_variables" | null | "equals" | "DD_SERVICE" | false
130+
"language" | ["java"] | null | "" | false
131+
"process_arguments" | null | "exists" | "-Dtest_selectorMatch" | true
132+
"process_arguments" | null | "exists" | "-Darg2" | false
133+
"process_arguments" | ["value1"] | "equals" | "-Dtest_selectorMatch" | true
134+
"process_arguments" | ["value2"] | "equals" | "-Dtest_selectorMatch" | false
99135
}
100136

101137
def "test duplicate entries not allowed"() {

0 commit comments

Comments
 (0)