diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java index b672f8e6790..de3536b219d 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java @@ -22,8 +22,8 @@ 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; @@ -31,14 +31,16 @@ 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; /** @@ -126,8 +128,9 @@ public static OpenTelemetrySdk create( /** * Parse the {@code configuration} YAML and return the {@link OpenTelemetryConfigurationModel}. * - *

Before parsing, environment variable substitution is performed as described in {@link - * EnvSubstitutionConstructor}. + *

During parsing, environment variable substitution is performed as defined in the + * OpenTelemetry Configuration Data Model specification. * * @throws DeclarativeConfigException if unable to parse */ @@ -149,7 +152,7 @@ static OpenTelemetryConfigurationModel parse( // Visible for testing static Object loadYaml(InputStream inputStream, Map 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); } @@ -241,89 +244,110 @@ static R createAndMaybeCleanup(Factory factory, SpiHelper spiHelper } } + private static final class EnvLoad extends Load { + + private final LoadSettings settings; + private final Map environmentVariables; + + public EnvLoad(LoadSettings settings, Map 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 + * OpenTelemetry Configuration Data Model specification. + * + *

This composer supports: * - *

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_]*}. + *

* - *

Environment variable substitution only takes place on scalar values of maps. References to - * environment variables in keys or sets are ignored. + *

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. * - *

If a referenced environment variable is not defined, it is replaced with {@code ""}. + *

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 environmentVariables; + private final ScalarResolver scalarResolver; - private EnvSubstitutionConstructor( - LoadSettings loadSettings, Map 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 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 constructMapping(MappingNode node) { - Map mapping = settings.getDefaultMap().apply(node.getValue().size()); - List 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 @@ -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( diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigurationParseTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigurationParseTest.java index 32a8ad878bc..08594158d9b 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigurationParseTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigurationParseTest.java @@ -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; @@ -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) { @@ -928,7 +950,7 @@ private static java.util.stream.Stream 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 @@ -938,13 +960,23 @@ private static java.util.stream.Stream 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}"))),