Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load file config YAML using core schema, ensure that env var substiut… #6436

Merged
merged 3 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,15 @@
import java.util.regex.Pattern;
import org.snakeyaml.engine.v2.api.Load;
import org.snakeyaml.engine.v2.api.LoadSettings;
import org.snakeyaml.engine.v2.common.ScalarStyle;
import org.snakeyaml.engine.v2.constructor.StandardConstructor;
import org.snakeyaml.engine.v2.exceptions.ConstructorException;
import org.snakeyaml.engine.v2.exceptions.YamlEngineException;
import org.snakeyaml.engine.v2.nodes.MappingNode;
import org.yaml.snakeyaml.Yaml;
import org.snakeyaml.engine.v2.nodes.Node;
import org.snakeyaml.engine.v2.nodes.NodeTuple;
import org.snakeyaml.engine.v2.nodes.ScalarNode;
import org.snakeyaml.engine.v2.schema.CoreSchema;

/**
* Configure {@link OpenTelemetrySdk} from YAML configuration files conforming to the schema in <a
Expand Down Expand Up @@ -127,7 +133,7 @@

// Visible for testing
static Object loadYaml(InputStream inputStream, Map<String, String> environmentVariables) {
LoadSettings settings = LoadSettings.builder().build();
LoadSettings settings = LoadSettings.builder().setSchema(new CoreSchema()).build();
Load yaml = new Load(settings, new EnvSubstitutionConstructor(settings, environmentVariables));
return yaml.loadFromInputStream(inputStream);
}
Expand All @@ -146,51 +152,93 @@
private static final class EnvSubstitutionConstructor extends StandardConstructor {

// Yaml is not thread safe but this instance is always used on the same thread
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
private final Yaml yaml = new Yaml();
private final Load load;
private final Map<String, String> environmentVariables;

private EnvSubstitutionConstructor(
LoadSettings loadSettings, Map<String, String> environmentVariables) {
super(loadSettings);
load = new Load(loadSettings);
this.environmentVariables = environmentVariables;
}

/**
* Implementation is same as {@link
* org.snakeyaml.engine.v2.constructor.BaseConstructor#constructMapping(MappingNode)} except we
* override the resolution of values with our custom {@link #constructValueObject(Node)}, which
* performs environment variable substitution.
*/
@Override
@SuppressWarnings({"ReturnValueIgnored", "CatchingUnchecked"})
protected Map<Object, Object> constructMapping(MappingNode node) {
// First call the super to construct mapping from MappingNode as usual
Map<Object, Object> result = super.constructMapping(node);

// Iterate through the map entries, and:
// 1. Identify entries which are scalar strings eligible for environment variable substitution
// 2. Apply environment variable substitution
// 3. Re-parse substituted value so it has correct type (i.e. yaml.load(newVal))
for (Map.Entry<Object, Object> entry : result.entrySet()) {
Object value = entry.getValue();
if (!(value instanceof String)) {
continue;
Map<Object, Object> mapping = settings.getDefaultMap().apply(node.getValue().size());
List<NodeTuple> nodeValue = node.getValue();
for (NodeTuple tuple : nodeValue) {
Node keyNode = tuple.getKeyNode();
Node valueNode = tuple.getValueNode();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move the valueNode down closer to where it's used.

Object key = constructObject(keyNode);
if (key != null) {
try {
key.hashCode(); // check circular dependencies
} catch (Exception e) {
throw new ConstructorException(

Check warning on line 184 in sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java#L183-L184

Added lines #L183 - L184 were not covered by tests
"while constructing a mapping",
node.getStartMark(),

Check warning on line 186 in sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java#L186

Added line #L186 was not covered by tests
"found unacceptable key " + key,
tuple.getKeyNode().getStartMark(),

Check warning on line 188 in sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java#L188

Added line #L188 was not covered by tests
e);
}
}

String val = (String) value;
Matcher matcher = ENV_VARIABLE_REFERENCE.matcher(val);
if (!matcher.find()) {
continue;
Object value = constructValueObject(valueNode);
if (keyNode.isRecursive()) {
if (settings.getAllowRecursiveKeys()) {
postponeMapFilling(mapping, key, value);

Check warning on line 195 in sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java#L195

Added line #L195 was not covered by tests
} else {
throw new YamlEngineException(

Check warning on line 197 in sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java#L197

Added line #L197 was not covered by tests
"Recursive key for mapping is detected but it is not configured to be allowed.");
}
} else {
mapping.put(key, value);
}
}

int offset = 0;
StringBuilder newVal = new StringBuilder();
do {
MatchResult matchResult = matcher.toMatchResult();
String replacement = environmentVariables.getOrDefault(matcher.group(1), "");
newVal.append(val, offset, matchResult.start()).append(replacement);
offset = matchResult.end();
} while (matcher.find());
if (offset != val.length()) {
newVal.append(val, offset, val.length());
}
entry.setValue(yaml.load(newVal.toString()));
return mapping;
}

private Object constructValueObject(Node node) {
if (!(node instanceof ScalarNode)) {
return super.constructObject(node);
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
}
Object value = super.constructObject(node);
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
if (!(value instanceof String)) {
return value;
}

String val = (String) value;
Matcher matcher = ENV_VARIABLE_REFERENCE.matcher(val);
if (!matcher.find()) {
return value;
}

return result;
int offset = 0;
StringBuilder newVal = new StringBuilder();
ScalarStyle scalarStyle = ((ScalarNode) node).getScalarStyle();
do {
MatchResult matchResult = matcher.toMatchResult();
String replacement = environmentVariables.getOrDefault(matcher.group(1), "");
newVal.append(val, offset, matchResult.start()).append(replacement);
offset = matchResult.end();
} while (matcher.find());
if (offset != val.length()) {
newVal.append(val, offset, val.length());
}
// If the value was double quoted, retain the double quotes so we don't change a value
// intended to be a string to a different type after environment variable substitution
if (scalarStyle == ScalarStyle.DOUBLE_QUOTED && newVal.length() != 0) {
newVal.insert(0, "\"");
newVal.append("\"");
}
return load.loadFromString(newVal.toString());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,25 @@ void parse_nullBoxedPrimitivesParsedToNull() {
new Sampler().withTraceIdRatioBased(new TraceIdRatioBased()))));
}

@ParameterizedTest
@MethodSource("coreSchemaValuesArgs")
void coreSchemaValues(String rawYaml, Object expectedYamlResult) {
Object yaml =
FileConfiguration.loadYaml(
new ByteArrayInputStream(rawYaml.getBytes(StandardCharsets.UTF_8)),
Collections.emptyMap());
assertThat(yaml).isEqualTo(expectedYamlResult);
}

@SuppressWarnings("unchecked")
private static java.util.stream.Stream<Arguments> coreSchemaValuesArgs() {
return java.util.stream.Stream.of(
Arguments.of("key1: 0o123\n", mapOf(entry("key1", 83))),
Arguments.of("key1: 0123\n", mapOf(entry("key1", 123))),
Arguments.of("key1: 0xdeadbeef\n", mapOf(entry("key1", 3735928559L))),
Arguments.of("key1: \"0xdeadbeef\"\n", mapOf(entry("key1", "0xdeadbeef"))));
}

@ParameterizedTest
@MethodSource("envVarSubstitutionArgs")
void envSubstituteAndLoadYaml(String rawYaml, Object expectedYamlResult) {
Expand All @@ -396,6 +415,7 @@ void envSubstituteAndLoadYaml(String rawYaml, Object expectedYamlResult) {
environmentVariables.put("BOOL", "true");
environmentVariables.put("INT", "1");
environmentVariables.put("FLOAT", "1.1");
environmentVariables.put("HEX", "0xdeadbeef");

Object yaml =
FileConfiguration.loadYaml(
Expand All @@ -412,6 +432,7 @@ private static java.util.stream.Stream<Arguments> envVarSubstitutionArgs() {
Arguments.of("key1: ${BOOL}\n", mapOf(entry("key1", true))),
Arguments.of("key1: ${INT}\n", mapOf(entry("key1", 1))),
Arguments.of("key1: ${FLOAT}\n", mapOf(entry("key1", 1.1))),
Arguments.of("key1: ${HEX}\n", mapOf(entry("key1", 3735928559L))),
Arguments.of(
"key1: ${STR_1}\n" + "key2: value2\n",
mapOf(entry("key1", "value1"), entry("key2", "value2"))),
Expand All @@ -432,7 +453,13 @@ private static java.util.stream.Stream<Arguments> envVarSubstitutionArgs() {
"key1:\n ${STR_1}: value1\n",
mapOf(entry("key1", mapOf(entry("${STR_1}", "value1"))))),
Arguments.of(
"key1:\n - ${STR_1}\n", mapOf(entry("key1", Collections.singletonList("${STR_1}")))));
"key1:\n - ${STR_1}\n", mapOf(entry("key1", Collections.singletonList("${STR_1}")))),
// Quoted environment variables
Arguments.of("key1: \"${HEX}\"\n", mapOf(entry("key1", "0xdeadbeef"))),
Arguments.of("key1: \"${STR_1}\"\n", mapOf(entry("key1", "value1"))),
Arguments.of("key1: \"${BOOL}\"\n", mapOf(entry("key1", "true"))),
Arguments.of("key1: \"${INT}\"\n", mapOf(entry("key1", "1"))),
Arguments.of("key1: \"${FLOAT}\"\n", mapOf(entry("key1", "1.1"))));
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
}

private static <K, V> Map.Entry<K, V> entry(K key, @Nullable V value) {
Expand Down
Loading