Don't mix env variable with configuration keys (#281)

Author: jderusse

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Change Log
 
+## 0.3.2
+
+### Fixed
+
+- `Configuration` don't mix anymore attributes injected by php array and env variables.
+
 ## 0.3.1
 
 ### Added

src/Configuration.php

@@ -39,17 +39,22 @@ class Configuration
         self::OPTION_ROLE_SESSION_NAME => true,
     ];
 
+    // Put fallback options into groups to avoid mixing of provided config and environment variables
     private const FALLBACK_OPTIONS = [
-        self::OPTION_REGION => ['AWS_REGION', 'AWS_DEFAULT_REGION'],
-        self::OPTION_PROFILE => ['AWS_PROFILE', 'AWS_DEFAULT_PROFILE'],
-        self::OPTION_ACCESS_KEY_ID => ['AWS_ACCESS_KEY_ID', 'AWS_ACCESS_KEY'],
-        self::OPTION_SECRET_ACCESS_KEY => ['AWS_SECRET_ACCESS_KEY', 'AWS_SECRET_KEY'],
-        self::OPTION_SESSION_TOKEN => 'AWS_SESSION_TOKEN',
-        self::OPTION_SHARED_CREDENTIALS_FILE => 'AWS_SHARED_CREDENTIALS_FILE',
-        self::OPTION_SHARED_CONFIG_FILE => 'AWS_CONFIG_FILE',
-        self::OPTION_ROLE_ARN => 'AWS_ROLE_ARN',
-        self::OPTION_WEB_IDENTITY_TOKEN_FILE => 'AWS_WEB_IDENTITY_TOKEN_FILE',
-        self::OPTION_ROLE_SESSION_NAME => 'AWS_ROLE_SESSION_NAME',
+        [self::OPTION_REGION => ['AWS_REGION', 'AWS_DEFAULT_REGION']],
+        [self::OPTION_PROFILE => ['AWS_PROFILE', 'AWS_DEFAULT_PROFILE']],
+        [
+            self::OPTION_ACCESS_KEY_ID => ['AWS_ACCESS_KEY_ID', 'AWS_ACCESS_KEY'],
+            self::OPTION_SECRET_ACCESS_KEY => ['AWS_SECRET_ACCESS_KEY', 'AWS_SECRET_KEY'],
+            self::OPTION_SESSION_TOKEN => 'AWS_SESSION_TOKEN',
+        ],
+        [self::OPTION_SHARED_CREDENTIALS_FILE => 'AWS_SHARED_CREDENTIALS_FILE'],
+        [self::OPTION_SHARED_CONFIG_FILE => 'AWS_CONFIG_FILE'],
+        [
+            self::OPTION_ROLE_ARN => 'AWS_ROLE_ARN',
+            self::OPTION_WEB_IDENTITY_TOKEN_FILE => 'AWS_WEB_IDENTITY_TOKEN_FILE',
+            self::OPTION_ROLE_SESSION_NAME => 'AWS_ROLE_SESSION_NAME',
+        ],
     ];
 
     private const DEFAULT_OPTIONS = [
@@ -69,26 +74,32 @@ public static function create(array $options)
             throw new InvalidArgument(\sprintf('Invalid option(s) "%s" passed to "%s::%s". ', \implode('", "', \array_keys($invalidOptions)), __CLASS__, __METHOD__));
         }
 
-        foreach (self::FALLBACK_OPTIONS as $option => $envVariableNames) {
-            if (isset($options[$option])) {
-                continue;
+        foreach (self::FALLBACK_OPTIONS as $fallbackGroup) {
+            // prevent mixing env variables with config keys
+            foreach ($fallbackGroup as $option => $envVariableNames) {
+                if (isset($options[$option])) {
+                    continue 2;
+                }
             }
 
-            foreach ((array) $envVariableNames as $envVariableName) {
-                if (false !== $value = \getenv($envVariableName)) {
-                    $options[$option] = $value;
+            foreach ($fallbackGroup as $option => $envVariableNames) {
+                $envVariableNames = (array) $envVariableNames;
+                foreach ($envVariableNames as $envVariableName) {
+                    if (false !== $value = \getenv($envVariableName)) {
+                        $options[$option] = $value;
 
-                    break;
+                        break;
+                    }
                 }
             }
         }
 
-        foreach (self::DEFAULT_OPTIONS as $option => $defaultValue) {
-            if (isset($options[$option])) {
+        foreach (self::DEFAULT_OPTIONS as $optionTrigger => $defaultValue) {
+            if (isset($options[$optionTrigger])) {
                 continue;
             }
 
-            $options[$option] = $defaultValue;
+            $options[$optionTrigger] = $defaultValue;
         }
 
         $configuration = new Configuration();

tests/Unit/ConfigurationTest.php

@@ -9,30 +9,41 @@
 
 class ConfigurationTest extends TestCase
 {
-    public function testDefaultValues()
+    /**
+     * @dataProvider provideConfiguration
+     */
+    public function testCreate($config, $env, $expected)
     {
-        $config = Configuration::create(['endpoint' => 'foo']);
-        self::assertEquals('foo', $config->get('endpoint'));
-
-        $config = Configuration::create(['endpoint' => '']);
-        self::assertEquals('', $config->get('endpoint'));
-
-        $config = Configuration::create([]);
-        self::assertEquals('https://%service%.%region%.amazonaws.com', $config->get('endpoint'));
-
-        // If a configuration value is set to null, we should use default value.
-        $config = Configuration::create(['endpoint' => null]);
-        self::assertEquals('https://%service%.%region%.amazonaws.com', $config->get('endpoint'));
-
-        $config = Configuration::create([]);
-        self::assertEquals('default', $config->get('profile'));
-
-        putenv('AWS_PROFILE=foo');
-        $config = Configuration::create([]);
-        self::assertEquals('foo', $config->get('profile'));
+        foreach ($env as $key => $value) {
+            putenv("$key=$value");
+        }
+
+        try {
+            $config = Configuration::create($config);
+            foreach ($expected as $key => $value) {
+                self::assertEquals($value, $config->get($key));
+            }
+        } finally {
+            foreach ($env as $key => $value) {
+                putenv($key);
+            }
+        }
+    }
 
-        putenv('AWS_PROFILE');
-        $config = Configuration::create([]);
-        self::assertEquals('default', $config->get('profile'));
+    public function provideConfiguration(): iterable
+    {
+        yield 'simple config' => [['endpoint' => 'foo'], [], ['endpoint' => 'foo']];
+        yield 'empty config' => [['endpoint' => ''], [], ['endpoint' => '']];
+        yield 'default' => [[], [], ['endpoint' => 'https://%service%.%region%.amazonaws.com']];
+        yield 'default when null' => [['endpoint' => null], [], ['endpoint' => 'https://%service%.%region%.amazonaws.com']];
+
+        yield 'default when env missing' => [[], [], ['profile' => 'default']];
+        yield 'fallback env' => [[], ['AWS_PROFILE' => 'foo'], ['profile' => 'foo']];
+        yield 'config priority on env' => [['profile' => 'bar'], ['AWS_PROFILE' => 'foo'], ['profile' => 'bar']];
+
+        yield 'config with env group' => [['accessKeyId' => 'key'], [], ['accessKeyId' => 'key', 'sessionToken' => null]];
+        yield 'config with env group 2' => [['accessKeySecret' => 'secret'], [], ['accessKeySecret' => 'secret', 'accessKeyId' => null, 'sessionToken' => null]];
+        yield 'mix config and env' => [['accessKeyId' => 'key'], ['AWS_SESSION_TOKEN' => 'token'], ['accessKeyId' => 'key', 'sessionToken' => null]];
+        yield 'null config with env group' => [['accessKeyId' => null], ['AWS_SESSION_TOKEN' => 'token'], ['accessKeyId' => null, 'sessionToken' => 'token']];
     }
 }