Skip to content

fix: Resolve environment variable substitution for mixed quotes #7433

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Resolve environment variable substitution for mixed quotes #7433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
174 changes: 96 additions & 78 deletions ...in/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,25 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.logging.Logger;
import java.util.regex.MatchResult;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import org.snakeyaml.engine.v2.api.Load;
import org.snakeyaml.engine.v2.api.LoadSettings;
import org.snakeyaml.engine.v2.api.YamlUnicodeReader;
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.composer.Composer;
import org.snakeyaml.engine.v2.nodes.MappingNode;
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.nodes.Tag;
import org.snakeyaml.engine.v2.parser.ParserImpl;
import org.snakeyaml.engine.v2.resolver.ScalarResolver;
import org.snakeyaml.engine.v2.scanner.StreamReader;
import org.snakeyaml.engine.v2.schema.CoreSchema;

/**
Expand Down Expand Up @@ -126,8 +128,9 @@ public static OpenTelemetrySdk create(
/**
* Parse the {@code configuration} YAML and return the {@link OpenTelemetryConfigurationModel}.
*
* <p>Before parsing, environment variable substitution is performed as described in {@link
* EnvSubstitutionConstructor}.
* <p>During parsing, environment variable substitution is performed as defined in the <a
* href="https://opentelemetry.io/docs/specs/otel/configuration/data-model/#environment-variable-substitution">
* OpenTelemetry Configuration Data Model specification</a>.
*
* @throws DeclarativeConfigException if unable to parse
*/
Expand All @@ -149,7 +152,7 @@ static OpenTelemetryConfigurationModel parse(
// Visible for testing
static Object loadYaml(InputStream inputStream, Map<String, String> environmentVariables) {
LoadSettings settings = LoadSettings.builder().setSchema(new CoreSchema()).build();
Load yaml = new Load(settings, new EnvSubstitutionConstructor(settings, environmentVariables));
Load yaml = new EnvLoad(settings, environmentVariables);
return yaml.loadFromInputStream(inputStream);
}

Expand Down Expand Up @@ -241,89 +244,110 @@ static <M, R> R createAndMaybeCleanup(Factory<M, R> factory, SpiHelper spiHelper
}
}

private static final class EnvLoad extends Load {

private final LoadSettings settings;
private final Map<String, String> environmentVariables;

public EnvLoad(LoadSettings settings, Map<String, String> environmentVariables) {
super(settings);
this.settings = settings;
this.environmentVariables = environmentVariables;
}

@Override
public Object loadFromInputStream(InputStream yamlStream) {
Objects.requireNonNull(yamlStream, "InputStream cannot be null");
return loadOne(
new EnvComposer(
settings,
new ParserImpl(
settings, new StreamReader(settings, new YamlUnicodeReader(yamlStream))),
environmentVariables));
}
}

/**
* {@link StandardConstructor} which substitutes environment variables.
* A YAML Composer that performs environment variable substitution according to the <a
* href="https://opentelemetry.io/docs/specs/otel/configuration/data-model/#environment-variable-substitution">
* OpenTelemetry Configuration Data Model specification</a>.
*
* <p>This composer supports:
*
* <p>Environment variables follow the syntax {@code ${VARIABLE}}, where {@code VARIABLE} is an
* environment variable matching the regular expression {@code [a-zA-Z_]+[a-zA-Z0-9_]*}.
* <ul>
* <li>Environment variable references: {@code ${ENV_VAR}} or {@code ${env:ENV_VAR}}
* <li>Default values: {@code ${ENV_VAR:-default_value}}
* <li>Escape sequences: {@code $$} is replaced with a single {@code $}
* </ul>
*
* <p>Environment variable substitution only takes place on scalar values of maps. References to
* environment variables in keys or sets are ignored.
* <p>Environment variable substitution only applies to scalar values. Mapping keys are not
* candidates for substitution. Referenced environment variables that are undefined, null, or
* empty are replaced with empty values unless a default value is provided.
*
* <p>If a referenced environment variable is not defined, it is replaced with {@code ""}.
* <p>The {@code $} character serves as an escape sequence where {@code $$} in the input is
* translated to a single {@code $} in the output. This prevents environment variable substitution
* for the escaped content.
*/
private static final class EnvSubstitutionConstructor extends StandardConstructor {
private static final class EnvComposer extends Composer {

// Load is not thread safe but this instance is always used on the same thread
private final Load load;
private final Map<String, String> environmentVariables;
private final ScalarResolver scalarResolver;

private EnvSubstitutionConstructor(
LoadSettings loadSettings, Map<String, String> environmentVariables) {
super(loadSettings);
load = new Load(loadSettings);
private static final String ESCAPE_SEQUENCE = "$$";
private static final int ESCAPE_SEQUENCE_LENGTH = ESCAPE_SEQUENCE.length();
private static final char ESCAPE_SEQUENCE_REPLACEMENT = '$';

public EnvComposer(
LoadSettings settings, ParserImpl parser, Map<String, String> environmentVariables) {
super(settings, parser);
this.load = new Load(settings);
this.environmentVariables = environmentVariables;
this.scalarResolver = settings.getSchema().getScalarResolver();
}

/**
* 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) {
Map<Object, Object> mapping = settings.getDefaultMap().apply(node.getValue().size());
List<NodeTuple> nodeValue = node.getValue();
for (NodeTuple tuple : nodeValue) {
Node keyNode = tuple.getKeyNode();
Object key = constructObject(keyNode);
if (key != null) {
try {
key.hashCode(); // check circular dependencies
} catch (Exception e) {
throw new ConstructorException(
"while constructing a mapping",
node.getStartMark(),
"found unacceptable key " + key,
tuple.getKeyNode().getStartMark(),
e);
}
}
Node valueNode = tuple.getValueNode();
Object value = constructValueObject(valueNode);
if (keyNode.isRecursive()) {
if (settings.getAllowRecursiveKeys()) {
postponeMapFilling(mapping, key, value);
} else {
throw new YamlEngineException(
"Recursive key for mapping is detected but it is not configured to be allowed.");
}
} else {
mapping.put(key, value);
}
protected Node composeValueNode(MappingNode node) {
Node itemValue = super.composeValueNode(node);
if (!(itemValue instanceof ScalarNode)) {
// Only apply environment variable substitution to ScalarNodes
return itemValue;
}
ScalarNode scalarNode = (ScalarNode) itemValue;
String envSubstitution = envSubstitution(scalarNode.getValue());

return mapping;
}
// If the environment variable substitution does not change the value, do not modify the node
if (envSubstitution.equals(scalarNode.getValue())) {
return itemValue;
}

private static final String ESCAPE_SEQUENCE = "$$";
private static final int ESCAPE_SEQUENCE_LENGTH = ESCAPE_SEQUENCE.length();
private static final char ESCAPE_SEQUENCE_REPLACEMENT = '$';
Object envSubstitutionObj = load.loadFromString(envSubstitution);
Tag tag = itemValue.getTag();
ScalarStyle scalarStyle = scalarNode.getScalarStyle();

private Object constructValueObject(Node node) {
Object value = constructObject(node);
if (!(node instanceof ScalarNode)) {
return value;
}
if (!(value instanceof String)) {
return value;
Tag resolvedTag =
envSubstitutionObj == null
? Tag.NULL
: scalarResolver.resolve(envSubstitutionObj.toString(), true);

// Only non-quoted substituted scalars can have their tag changed
if (!itemValue.getTag().equals(resolvedTag)
&& scalarStyle != ScalarStyle.SINGLE_QUOTED
&& scalarStyle != ScalarStyle.DOUBLE_QUOTED) {
tag = resolvedTag;
}

String val = (String) value;
ScalarStyle scalarStyle = ((ScalarNode) node).getScalarStyle();
boolean resolved = true;
return new ScalarNode(
tag,
resolved,
envSubstitution,
scalarStyle,
itemValue.getStartMark(),
itemValue.getEndMark());
}

private String envSubstitution(String val) {
// Iterate through val left to right, search for escape sequence "$$"
// For the substring of val between the last escape sequence and the next found, perform
// environment variable substitution
Expand All @@ -346,13 +370,7 @@ private Object constructValueObject(Node node) {
}
}

// 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.insert(0, "\"");
newVal.append("\"");
}
return load.loadFromString(newVal.toString());
return newVal.toString();
}

private StringBuilder envVarSubstitution(
Expand Down
36 changes: 34 additions & 2 deletions ...o/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigurationParseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand Down Expand Up @@ -855,6 +856,27 @@ void parse_nullBoxedPrimitivesParsedToNull() {
.withTraceIdRatioBased(new TraceIdRatioBasedSamplerModel()))));
}

@Test
void parse_quotedInput() {
String yaml =
"resource:\n"
+ " attributes:\n"
+ " - name: single_quote\n"
+ " value: '\"single\"'\n"
+ " - name: double_quote\n"
+ " value: \"\\\"double\\\"\"";

OpenTelemetryConfigurationModel model =
DeclarativeConfiguration.parse(
new ByteArrayInputStream(yaml.getBytes(StandardCharsets.UTF_8)));

Assertions.assertNotNull(model.getResource());
assertThat(model.getResource().getAttributes())
.containsExactly(
new AttributeNameValueModel().withName("single_quote").withValue("\"single\""),
new AttributeNameValueModel().withName("double_quote").withValue("\"double\""));
}

@ParameterizedTest
@MethodSource("coreSchemaValuesArgs")
void coreSchemaValues(String rawYaml, Object expectedYamlResult) {
Expand Down Expand Up @@ -928,7 +950,7 @@ private static java.util.stream.Stream<Arguments> envVarSubstitutionArgs() {
// Undefined / empty environment variable
Arguments.of("key1: ${EMPTY_STR}\n", mapOf(entry("key1", null))),
Arguments.of("key1: ${STR_3}\n", mapOf(entry("key1", null))),
Arguments.of("key1: ${STR_1} ${STR_3}\n", mapOf(entry("key1", "value1"))),
Arguments.of("key1: ${STR_1} ${STR_3}\n", mapOf(entry("key1", "value1 "))),
// Environment variable keys must match pattern: [a-zA-Z_]+[a-zA-Z0-9_]*
Arguments.of("key1: ${VAR&}\n", mapOf(entry("key1", "${VAR&}"))),
// Environment variable substitution only takes place in scalar values of maps
Expand All @@ -938,13 +960,23 @@ private static java.util.stream.Stream<Arguments> envVarSubstitutionArgs() {
mapOf(entry("key1", mapOf(entry("${STR_1}", "value1"))))),
Arguments.of(
"key1:\n - ${STR_1}\n", mapOf(entry("key1", Collections.singletonList("${STR_1}")))),
// Quoted environment variables
// Double-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: \"${EMPTY_STR}\"\n", mapOf(entry("key1", ""))),
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} ${BOOL} ${INT}\"\n", mapOf(entry("key1", "0xdeadbeef true 1"))),
// Single-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: '${EMPTY_STR}'\n", mapOf(entry("key1", ""))),
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} ${BOOL} ${INT}'\n", mapOf(entry("key1", "0xdeadbeef true 1"))),
// Escaped
Arguments.of("key1: ${FOO}\n", mapOf(entry("key1", "BAR"))),
Arguments.of("key1: $${FOO}\n", mapOf(entry("key1", "${FOO}"))),
Expand Down
Loading