From d88a3cae64774020ecb48405374804a97cc47823 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 24 Jan 2025 10:47:56 +1100 Subject: [PATCH 1/3] add support for env: prefix in yaml config --- .../Internal/EnvSubstitutionNormalization.php | 2 +- .../ConfigurationFactoryTest.php | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Config/SDK/Configuration/Internal/EnvSubstitutionNormalization.php b/src/Config/SDK/Configuration/Internal/EnvSubstitutionNormalization.php index cfe0bf979..c7e85557d 100644 --- a/src/Config/SDK/Configuration/Internal/EnvSubstitutionNormalization.php +++ b/src/Config/SDK/Configuration/Internal/EnvSubstitutionNormalization.php @@ -65,7 +65,7 @@ private function doApply(NodeDefinition $node): void private function replaceEnvVariables(string $value, int $filter = FILTER_DEFAULT): mixed { $replaced = preg_replace_callback( - '/\$\{(?[a-zA-Z_]\w*)(?::-(?[^\n]*))?}/', + '/\$\{(?:env:)?(?[a-zA-Z_]\w*)(?::-(?[^\n]*))?}/', fn (array $matches): string => $this->envReader->read($matches['ENV_NAME']) ?? $matches['DEFAULT_VALUE'] ?? '', $value, -1, diff --git a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php index 5f797ec05..50d92613d 100644 --- a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php +++ b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php @@ -76,6 +76,7 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil ->scalarNode('string_key_with_default')->end() ->variableNode('undefined_key')->end() ->variableNode('${STRING_VALUE}')->end() + ->scalarNode('env_key')->end() ->end() ; @@ -110,6 +111,7 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil string_key_with_default: ${UNDEFINED_KEY:-fallback} # UNDEFINED_KEY is not defined but a default value is included undefined_key: ${UNDEFINED_KEY} # Invalid reference, UNDEFINED_KEY is not defined and is replaced with "" ${STRING_VALUE}: value # Invalid reference, substitution is not valid in mapping keys and reference is ignored + env_key: ${env:STRING_VALUE} # Valid reference to STRING_VALUE, using `env:` prefix YAML), ]); @@ -129,6 +131,7 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil # undefined_key removed as null is treated as unset undefined_key: # Interpreted as type null, tag URI tag:yaml.org,2002:null ${STRING_VALUE}: value # Interpreted as type string, tag URI tag:yaml.org,2002:str + env_key: value # Interpreted as type string, tag URI tag:yaml.org,2002:str YAML), self::getPropertiesFromPlugin($parsed), ); @@ -152,6 +155,24 @@ public function test_env_substitution_string(): void $this->assertSame('example-service', self::getPropertiesFromPlugin($parsed)['resource']['attributes']['service.name']); } + #[BackupGlobals(true)] + #[CoversNothing] + public function test_env_substitution_with_env_prefix(): void + { + $_SERVER['OTEL_SERVICE_NAME'] = 'example-service'; + $parsed = self::factory()->process([[ + 'file_format' => '0.1', + 'resource' => [ + 'attributes' => [ + 'service.name' => '${env:OTEL_SERVICE_NAME}', + ], + ], + ]]); + + $this->assertInstanceOf(ComponentPlugin::class, $parsed); + $this->assertSame('example-service', self::getPropertiesFromPlugin($parsed)['resource']['attributes']['service.name']); + } + #[BackupGlobals(true)] public function test_env_substitution_non_string(): void { From 853c8f0500e54039231943534cd961054d5dac26 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 24 Jan 2025 10:51:44 +1100 Subject: [PATCH 2/3] align test to example in spec --- .../Config/SDK/Configuration/ConfigurationFactoryTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php index 50d92613d..c993304c2 100644 --- a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php +++ b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php @@ -64,6 +64,7 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil $node ->children() ->scalarNode('string_key')->end() + ->scalarNode('env_string_key')->end() ->scalarNode('other_string_key')->end() ->scalarNode('another_string_key')->end() ->scalarNode('string_key_with_quoted_hex_value')->end() @@ -76,7 +77,6 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil ->scalarNode('string_key_with_default')->end() ->variableNode('undefined_key')->end() ->variableNode('${STRING_VALUE}')->end() - ->scalarNode('env_key')->end() ->end() ; @@ -99,6 +99,7 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil $parsed = $factory->process([ Yaml::parse(<<<'YAML' string_key: ${STRING_VALUE} # Valid reference to STRING_VALUE + env_string_key: ${env:STRING_VALUE} # Valid reference to STRING_VALUE other_string_key: "${STRING_VALUE}" # Valid reference to STRING_VALUE inside double quotes another_string_key: "${BOOl_VALUE}" # Valid reference to BOOl_VALUE inside double quotes string_key_with_quoted_hex_value: "${HEX_VALUE}" # Valid reference to HEX_VALUE inside double quotes @@ -111,13 +112,13 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil string_key_with_default: ${UNDEFINED_KEY:-fallback} # UNDEFINED_KEY is not defined but a default value is included undefined_key: ${UNDEFINED_KEY} # Invalid reference, UNDEFINED_KEY is not defined and is replaced with "" ${STRING_VALUE}: value # Invalid reference, substitution is not valid in mapping keys and reference is ignored - env_key: ${env:STRING_VALUE} # Valid reference to STRING_VALUE, using `env:` prefix YAML), ]); $this->assertSame( Yaml::parse(<<<'YAML' string_key: value # Interpreted as type string, tag URI tag:yaml.org,2002:str + env_string_key: value # Interpreted as type string, tag URI tag:yaml.org,2002:str other_string_key: "value" # Interpreted as type string, tag URI tag:yaml.org,2002:str another_string_key: "true" # Interpreted as type string, tag URI tag:yaml.org,2002:str string_key_with_quoted_hex_value: "0xdeadbeef" # Interpreted as type string, tag URI tag:yaml.org,2002:str @@ -131,7 +132,6 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil # undefined_key removed as null is treated as unset undefined_key: # Interpreted as type null, tag URI tag:yaml.org,2002:null ${STRING_VALUE}: value # Interpreted as type string, tag URI tag:yaml.org,2002:str - env_key: value # Interpreted as type string, tag URI tag:yaml.org,2002:str YAML), self::getPropertiesFromPlugin($parsed), ); From 162f22e00f7d2f4038cb64ee6ff45ade1c62dec0 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 24 Jan 2025 11:28:40 +1100 Subject: [PATCH 3/3] do not recursively replace env vars - fix env substitution normalization so that it doesn't apply 2 replacement normalizers for string nodes --- .../Internal/EnvSubstitutionNormalization.php | 3 +- .../ConfigurationFactoryTest.php | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/Config/SDK/Configuration/Internal/EnvSubstitutionNormalization.php b/src/Config/SDK/Configuration/Internal/EnvSubstitutionNormalization.php index c7e85557d..72008b87f 100644 --- a/src/Config/SDK/Configuration/Internal/EnvSubstitutionNormalization.php +++ b/src/Config/SDK/Configuration/Internal/EnvSubstitutionNormalization.php @@ -50,8 +50,7 @@ private function doApply(NodeDefinition $node): void default => FILTER_DEFAULT, }; $node->beforeNormalization()->ifString()->then(fn (string $v) => $this->replaceEnvVariables($v, $filter))->end(); - } - if ($node instanceof VariableNodeDefinition) { + } elseif ($node instanceof VariableNodeDefinition) { $node->beforeNormalization()->always($this->replaceEnvVariablesRecursive(...))->end(); } diff --git a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php index c993304c2..ac433c233 100644 --- a/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php +++ b/tests/Unit/Config/SDK/Configuration/ConfigurationFactoryTest.php @@ -77,6 +77,7 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil ->scalarNode('string_key_with_default')->end() ->variableNode('undefined_key')->end() ->variableNode('${STRING_VALUE}')->end() + ->scalarNode('recursive_key')->end() ->end() ; @@ -91,6 +92,8 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil 'FLOAT_VALUE' => '1.1', 'HEX_VALUE' => '0xdeadbeef', 'INVALID_MAP_VALUE' => "value\nkey:value", + 'DO_NOT_REPLACE_ME' => 'Never use this value', // An unused environment variable + 'REPLACE_ME' => '${DO_NOT_REPLACE_ME}', // A valid replacement text, used verbatim, not replaced with "Never use this value" ]), ]), ); @@ -112,6 +115,7 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil string_key_with_default: ${UNDEFINED_KEY:-fallback} # UNDEFINED_KEY is not defined but a default value is included undefined_key: ${UNDEFINED_KEY} # Invalid reference, UNDEFINED_KEY is not defined and is replaced with "" ${STRING_VALUE}: value # Invalid reference, substitution is not valid in mapping keys and reference is ignored + recursive_key: ${REPLACE_ME} # Valid reference to REPLACE_ME YAML), ]); @@ -132,6 +136,7 @@ public function getConfig(ComponentProviderRegistry $registry, NodeBuilder $buil # undefined_key removed as null is treated as unset undefined_key: # Interpreted as type null, tag URI tag:yaml.org,2002:null ${STRING_VALUE}: value # Interpreted as type string, tag URI tag:yaml.org,2002:str + recursive_key: ${DO_NOT_REPLACE_ME} # Interpreted as type string, tag URI tag:yaml.org,2002:str YAML), self::getPropertiesFromPlugin($parsed), ); @@ -188,6 +193,29 @@ public function test_env_substitution_non_string(): void $this->assertSame(2048, self::getPropertiesFromPlugin($parsed)['attribute_limits']['attribute_value_length_limit']); } + /** + * It MUST NOT be possible to inject environment variable by environment variables. + * For example, see references to DO_NOT_REPLACE_ME environment variable + */ + #[BackupGlobals(true)] + #[CoversNothing] + public function test_env_substitution_recursive_does_not_inject_environment_variables(): void + { + $_SERVER['DO_NOT_REPLACE_ME'] = 'Never use this value'; + $_SERVER['REPLACE_ME'] = '${DO_NOT_REPLACE_ME}'; + $parsed = self::factory()->process([[ + 'file_format' => '0.1', + 'resource' => [ + 'attributes' => [ + 'service.name' => '${REPLACE_ME}', + ], + ], + ]]); + + $this->assertInstanceOf(ComponentPlugin::class, $parsed); + $this->assertSame('${DO_NOT_REPLACE_ME}', self::getPropertiesFromPlugin($parsed)['resource']['attributes']['service.name']); + } + /** * If a property has a default value defined (i.e. is _not_ required) and is * missing or present but null, Create MUST ensure the SDK component is configured