Skip to content

Commit e2c1189

Browse files
jeanbisuttijeanbisutti
committed
Fix connection string env var precedence over ${file:...} config lookup
1 parent 171189a commit e2c1189

13 files changed

Lines changed: 287 additions & 18 deletions

File tree

‎agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/configuration/ConfigurationBuilder.java‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.Locale;
4141
import java.util.Map;
4242
import java.util.function.Function;
43+
import java.util.function.Supplier;
4344
import javax.annotation.Nullable;
4445
import org.apache.commons.text.StringSubstitutor;
4546
import org.apache.commons.text.lookup.StringLookup;
@@ -715,16 +716,9 @@ static void overlayFromEnv(
715716
Function<String, String> envVarsFunction,
716717
Function<String, String> systemPropertiesFunction)
717718
throws IOException {
718-
// load connection string from a file if connection string is in the format of
719-
// "${file:mounted_connection_string_file.txt}"
720-
Map<String, StringLookup> stringLookupMap =
721-
Collections.singletonMap(StringLookupFactory.KEY_FILE, new FileStringLookup(baseDir));
722-
StringLookup stringLookup =
723-
StringLookupFactory.INSTANCE.interpolatorStringLookup(stringLookupMap, null, false);
724-
StringSubstitutor stringSubstitutor = new StringSubstitutor(stringLookup);
725719
config.connectionString =
726720
overlayConnectionStringFromEnv(
727-
stringSubstitutor.replace(config.connectionString),
721+
() -> resolveFileLookup(config.connectionString, baseDir),
728722
envVarsFunction,
729723
systemPropertiesFunction);
730724

@@ -828,27 +822,35 @@ public static void overlayFromEnv(
828822
Function<String, String> systemPropertiesFunction) {
829823
config.connectionString =
830824
overlayConnectionStringFromEnv(
831-
config.connectionString, envVarsFunction, systemPropertiesFunction);
825+
() -> config.connectionString, envVarsFunction, systemPropertiesFunction);
832826
config.sampling.percentage =
833827
overlayWithEnvVar(
834828
APPLICATIONINSIGHTS_SAMPLING_PERCENTAGE, config.sampling.percentage, envVarsFunction);
835829
}
836830

831+
private static String resolveFileLookup(String value, Path baseDir) {
832+
Map<String, StringLookup> stringLookupMap =
833+
Collections.singletonMap(StringLookupFactory.KEY_FILE, new FileStringLookup(baseDir));
834+
StringLookup stringLookup =
835+
StringLookupFactory.INSTANCE.interpolatorStringLookup(stringLookupMap, null, false);
836+
StringSubstitutor stringSubstitutor = new StringSubstitutor(stringLookup);
837+
return stringSubstitutor.replace(value);
838+
}
839+
837840
@Nullable
838841
private static String overlayConnectionStringFromEnv(
839-
String connectionString,
842+
Supplier<String> connectionStringSupplier,
840843
Function<String, String> envVarsFunction,
841844
Function<String, String> systemPropertiesFunction) {
842-
String value =
845+
String connectionString =
843846
overlayWithSysPropEnvVar(
844847
APPLICATIONINSIGHTS_CONNECTION_STRING_SYS,
845848
APPLICATIONINSIGHTS_CONNECTION_STRING_ENV,
846-
connectionString,
849+
connectionStringSupplier,
847850
envVarsFunction,
848851
systemPropertiesFunction);
849-
850-
if (value != null) {
851-
return value;
852+
if (connectionString != null) {
853+
return connectionString;
852854
}
853855

854856
// this is for backwards compatibility only
@@ -971,12 +973,31 @@ public static String overlayWithSysPropEnvVar(
971973
String defaultValue,
972974
Function<String, String> envVarsFunction,
973975
Function<String, String> systemPropertiesFunction) {
976+
return overlayWithSysPropEnvVar(
977+
systemPropertyName,
978+
envVarName,
979+
() -> defaultValue,
980+
envVarsFunction,
981+
systemPropertiesFunction);
982+
}
983+
984+
@Nullable
985+
static String overlayWithSysPropEnvVar(
986+
String systemPropertyName,
987+
String envVarName,
988+
Supplier<String> defaultValueSupplier,
989+
Function<String, String> envVarsFunction,
990+
Function<String, String> systemPropertiesFunction) {
974991
String value = getSystemProperty(systemPropertyName, systemPropertiesFunction);
975992
if (value != null) {
976993
configurationLogger.debug("using system property: {}", systemPropertyName);
977994
return value;
978995
}
979-
return overlayWithEnvVar(envVarName, defaultValue, envVarsFunction);
996+
String envVarValue = getEnvVar(envVarName, envVarsFunction);
997+
if (envVarValue != null) {
998+
return envVarValue;
999+
}
1000+
return defaultValueSupplier.get();
9801001
}
9811002

9821003
public static String overlayWithEnvVar(

‎settings.gradle.kts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ hideFromDependabot(":smoke-tests:apps:ClassicSdkLogbackInterop2x")
7070
hideFromDependabot(":smoke-tests:apps:ClassicSdkWebInterop2x")
7171
hideFromDependabot(":smoke-tests:apps:ClassicSdkWebInterop3x")
7272
hideFromDependabot(":smoke-tests:apps:ClassicSdkWebInterop3xUsingOld3xAgent")
73+
hideFromDependabot(":smoke-tests:apps:ConnectionStringFileNotFound")
74+
hideFromDependabot(":smoke-tests:apps:ConnectionStringFromFile")
7375
hideFromDependabot(":smoke-tests:apps:ConnectionStringOverrides")
7476
hideFromDependabot(":smoke-tests:apps:CoreAndFilter2x")
7577
hideFromDependabot(":smoke-tests:apps:CoreAndFilter3x")

‎smoke-tests/apps/ConnectionStringFileNotFound/build.gradle.kts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
plugins {
2+
id("ai.smoke-test-war")
3+
}

‎smoke-tests/apps/ConnectionStringFileNotFound/src/main/java/com/microsoft/applicationinsights/smoketestapp/ConnectionStringFileNotFoundServlet.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.microsoft.applicationinsights.smoketestapp;
5+
6+
import java.io.IOException;
7+
import javax.servlet.annotation.WebServlet;
8+
import javax.servlet.http.HttpServlet;
9+
import javax.servlet.http.HttpServletRequest;
10+
import javax.servlet.http.HttpServletResponse;
11+
12+
@WebServlet("/*")
13+
public class ConnectionStringFileNotFoundServlet extends HttpServlet {
14+
15+
@Override
16+
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
17+
resp.getWriter().println("ok");
18+
}
19+
}

‎smoke-tests/apps/ConnectionStringFileNotFound/src/smokeTest/java/com/microsoft/applicationinsights/smoketest/ConnectionStringFileNotFoundTest.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.microsoft.applicationinsights.smoketest;
5+
6+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11;
7+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11_OPENJ9;
8+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17;
9+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17_OPENJ9;
10+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_21;
11+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_21_OPENJ9;
12+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_25;
13+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_25_OPENJ9;
14+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8;
15+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8_OPENJ9;
16+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.WILDFLY_13_JAVA_8;
17+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.WILDFLY_13_JAVA_8_OPENJ9;
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.data.MapEntry.entry;
20+
21+
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.api.extension.RegisterExtension;
23+
24+
// Reproduces https://github.com/microsoft/ApplicationInsights-Java/issues/4214
25+
// When applicationinsights.json contains a ${file:...} reference to a nonexistent file,
26+
// the APPLICATIONINSIGHTS_CONNECTION_STRING env var should still take effect.
27+
@UseAgent
28+
abstract class ConnectionStringFileNotFoundTest {
29+
30+
@RegisterExtension static final SmokeTestExtension testing = SmokeTestExtension.create();
31+
32+
@Test
33+
@TargetUri("/test")
34+
void shouldUseEnvVarWhenFileNotFound() throws Exception {
35+
Telemetry telemetry = testing.getTelemetry(0);
36+
37+
assertThat(telemetry.rd.getName()).isEqualTo("GET /ConnectionStringFileNotFound/*");
38+
assertThat(telemetry.rd.getResponseCode()).isEqualTo("200");
39+
assertThat(telemetry.rd.getSuccess()).isTrue();
40+
assertThat(telemetry.rd.getProperties())
41+
.containsExactly(entry("_MS.ProcessedByMetricExtractors", "True"));
42+
assertThat(telemetry.rd.getMeasurements()).isEmpty();
43+
}
44+
45+
@Environment(TOMCAT_8_JAVA_8)
46+
static class Tomcat8Java8Test extends ConnectionStringFileNotFoundTest {}
47+
48+
@Environment(TOMCAT_8_JAVA_8_OPENJ9)
49+
static class Tomcat8Java8OpenJ9Test extends ConnectionStringFileNotFoundTest {}
50+
51+
@Environment(TOMCAT_8_JAVA_11)
52+
static class Tomcat8Java11Test extends ConnectionStringFileNotFoundTest {}
53+
54+
@Environment(TOMCAT_8_JAVA_11_OPENJ9)
55+
static class Tomcat8Java11OpenJ9Test extends ConnectionStringFileNotFoundTest {}
56+
57+
@Environment(TOMCAT_8_JAVA_17)
58+
static class Tomcat8Java17Test extends ConnectionStringFileNotFoundTest {}
59+
60+
@Environment(TOMCAT_8_JAVA_17_OPENJ9)
61+
static class Tomcat8Java17OpenJ9Test extends ConnectionStringFileNotFoundTest {}
62+
63+
@Environment(TOMCAT_8_JAVA_21)
64+
static class Tomcat8Java21Test extends ConnectionStringFileNotFoundTest {}
65+
66+
@Environment(TOMCAT_8_JAVA_21_OPENJ9)
67+
static class Tomcat8Java21OpenJ9Test extends ConnectionStringFileNotFoundTest {}
68+
69+
@Environment(TOMCAT_8_JAVA_25)
70+
static class Tomcat8Java23Test extends ConnectionStringFileNotFoundTest {}
71+
72+
@Environment(TOMCAT_8_JAVA_25_OPENJ9)
73+
static class Tomcat8Java23OpenJ9Test extends ConnectionStringFileNotFoundTest {}
74+
75+
@Environment(WILDFLY_13_JAVA_8)
76+
static class Wildfly13Java8Test extends ConnectionStringFileNotFoundTest {}
77+
78+
@Environment(WILDFLY_13_JAVA_8_OPENJ9)
79+
static class Wildfly13Java8OpenJ9Test extends ConnectionStringFileNotFoundTest {}
80+
}

‎smoke-tests/apps/ConnectionStringFileNotFound/src/smokeTest/resources/applicationinsights.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"connectionString": "${file:/app/secrets/applicationinsights_connection_string}",
3+
"role": {
4+
"name": "testrolename",
5+
"instance": "testroleinstance"
6+
}
7+
}

‎smoke-tests/apps/ConnectionStringFromFile/build.gradle.kts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
plugins {
2+
id("ai.smoke-test-war")
3+
}

‎smoke-tests/apps/ConnectionStringFromFile/src/main/java/com/microsoft/applicationinsights/smoketestapp/ConnectionStringFromFileServlet.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.microsoft.applicationinsights.smoketestapp;
5+
6+
import java.io.IOException;
7+
import javax.servlet.annotation.WebServlet;
8+
import javax.servlet.http.HttpServlet;
9+
import javax.servlet.http.HttpServletRequest;
10+
import javax.servlet.http.HttpServletResponse;
11+
12+
@WebServlet("/*")
13+
public class ConnectionStringFromFileServlet extends HttpServlet {
14+
15+
@Override
16+
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
17+
resp.getWriter().println("ok");
18+
}
19+
}

‎smoke-tests/apps/ConnectionStringFromFile/src/smokeTest/java/com/microsoft/applicationinsights/smoketest/ConnectionStringFromFileTest.java‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.microsoft.applicationinsights.smoketest;
5+
6+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11;
7+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11_OPENJ9;
8+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17;
9+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17_OPENJ9;
10+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_21;
11+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_21_OPENJ9;
12+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_25;
13+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_25_OPENJ9;
14+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8;
15+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8_OPENJ9;
16+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.WILDFLY_13_JAVA_8;
17+
import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.WILDFLY_13_JAVA_8_OPENJ9;
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.data.MapEntry.entry;
20+
21+
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.api.extension.RegisterExtension;
23+
24+
// Verifies that ${file:...} connection string lookup works correctly when
25+
// the APPLICATIONINSIGHTS_CONNECTION_STRING env var is NOT set.
26+
@UseAgent
27+
abstract class ConnectionStringFromFileTest {
28+
29+
@RegisterExtension
30+
static final SmokeTestExtension testing =
31+
SmokeTestExtension.builder()
32+
.doNotSetConnectionString()
33+
.addFile("connection_string.txt", "/app/secrets/connection_string.txt")
34+
.build();
35+
36+
@Test
37+
@TargetUri("/test")
38+
void shouldReadConnectionStringFromFile() throws Exception {
39+
Telemetry telemetry = testing.getTelemetry(0);
40+
41+
assertThat(telemetry.rd.getName()).isEqualTo("GET /ConnectionStringFromFile/*");
42+
assertThat(telemetry.rd.getResponseCode()).isEqualTo("200");
43+
assertThat(telemetry.rd.getSuccess()).isTrue();
44+
assertThat(telemetry.rd.getProperties())
45+
.containsExactly(entry("_MS.ProcessedByMetricExtractors", "True"));
46+
assertThat(telemetry.rd.getMeasurements()).isEmpty();
47+
}
48+
49+
@Environment(TOMCAT_8_JAVA_8)
50+
static class Tomcat8Java8Test extends ConnectionStringFromFileTest {}
51+
52+
@Environment(TOMCAT_8_JAVA_8_OPENJ9)
53+
static class Tomcat8Java8OpenJ9Test extends ConnectionStringFromFileTest {}
54+
55+
@Environment(TOMCAT_8_JAVA_11)
56+
static class Tomcat8Java11Test extends ConnectionStringFromFileTest {}
57+
58+
@Environment(TOMCAT_8_JAVA_11_OPENJ9)
59+
static class Tomcat8Java11OpenJ9Test extends ConnectionStringFromFileTest {}
60+
61+
@Environment(TOMCAT_8_JAVA_17)
62+
static class Tomcat8Java17Test extends ConnectionStringFromFileTest {}
63+
64+
@Environment(TOMCAT_8_JAVA_17_OPENJ9)
65+
static class Tomcat8Java17OpenJ9Test extends ConnectionStringFromFileTest {}
66+
67+
@Environment(TOMCAT_8_JAVA_21)
68+
static class Tomcat8Java21Test extends ConnectionStringFromFileTest {}
69+
70+
@Environment(TOMCAT_8_JAVA_21_OPENJ9)
71+
static class Tomcat8Java21OpenJ9Test extends ConnectionStringFromFileTest {}
72+
73+
@Environment(TOMCAT_8_JAVA_25)
74+
static class Tomcat8Java23Test extends ConnectionStringFromFileTest {}
75+
76+
@Environment(TOMCAT_8_JAVA_25_OPENJ9)
77+
static class Tomcat8Java23OpenJ9Test extends ConnectionStringFromFileTest {}
78+
79+
@Environment(WILDFLY_13_JAVA_8)
80+
static class Wildfly13Java8Test extends ConnectionStringFromFileTest {}
81+
82+
@Environment(WILDFLY_13_JAVA_8_OPENJ9)
83+
static class Wildfly13Java8OpenJ9Test extends ConnectionStringFromFileTest {}
84+
}

‎smoke-tests/apps/ConnectionStringFromFile/src/smokeTest/resources/applicationinsights.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"connectionString": "${file:/app/secrets/connection_string.txt}",
3+
"role": {
4+
"name": "testrolename",
5+
"instance": "testroleinstance"
6+
}
7+
}

0 commit comments

Comments
 (0)