From 9cd9a1613fae5f5f2a95656fdc98a2845641972a Mon Sep 17 00:00:00 2001 From: Ron Rademaker Date: Tue, 26 Apr 2016 18:10:39 +0200 Subject: [PATCH 1/6] Extracted generation method to a separate method --- src/Task/YamlConfigurationTask.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Task/YamlConfigurationTask.php b/src/Task/YamlConfigurationTask.php index 4366dc8..89daa6b 100644 --- a/src/Task/YamlConfigurationTask.php +++ b/src/Task/YamlConfigurationTask.php @@ -172,7 +172,7 @@ private function findKeyAndGenerateValue(array &$configuration, array $parameter $configuration[$key] = $value; } elseif (is_scalar($value)) { - $configuration[$key] = sha1(uniqid()); + $configuration[$key] = $this->generateValue(); } } } @@ -195,4 +195,14 @@ private function addEnvironmentVariables($value) return $value; } + + /** + * Generate a sha key + * + * @return string + */ + private function generateValue() + { + return sha1(uniqid()); + } } From 068c3f829c83f2583b9e26f2378cc505e796bda1 Mon Sep 17 00:00:00 2001 From: Ron Rademaker Date: Tue, 26 Apr 2016 18:11:06 +0200 Subject: [PATCH 2/6] Added test that reproduces issue #134 --- tests/Task/YamlConfigurationTaskTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/Task/YamlConfigurationTaskTest.php b/tests/Task/YamlConfigurationTaskTest.php index 4546f3b..e64d2f3 100644 --- a/tests/Task/YamlConfigurationTaskTest.php +++ b/tests/Task/YamlConfigurationTaskTest.php @@ -7,6 +7,7 @@ use Accompli\EventDispatcher\Event\InstallReleaseEvent; use Accompli\Task\YamlConfigurationTask; use PHPUnit_Framework_TestCase; +use ReflectionClass; /** * YamlConfigurationTaskTest. @@ -276,4 +277,20 @@ public function testOnInstallReleaseCreateOrUpdateConfigurationFailsCreatingANew $task = new YamlConfigurationTask('/parameters.yml', array('foo' => 'bar', 'baz' => '', 'bar' => array('baz' => '')), array('baz', 'bar.baz')); $task->onInstallReleaseCreateOrUpdateConfiguration($event, AccompliEvents::INSTALL_RELEASE, $eventDispatcherMock); } + + /** + * Tests if generated values are not regenerated within the same process. + */ + public function testGeneratedValuesAreNotRegenerated() + { + $task = new YamlConfigurationTask('foobar'); + $reflectionClass = new ReflectionClass('Accompli\Task\YamlConfigurationTask'); + $method = $reflectionClass->getMethod('generateValue'); + $method->setAccessible(true); + + $generated = $method->invoke($task, 'foobar'); + + $this->assertNotEmpty($generated); + $this->assertEquals($generated, $method->invoke($task, 'foobar')); + } } From 9918ea803262726585380149f818e720927d2bad Mon Sep 17 00:00:00 2001 From: Ron Rademaker Date: Tue, 26 Apr 2016 18:15:34 +0200 Subject: [PATCH 3/6] Keep generated values per key in a static variable --- src/Task/YamlConfigurationTask.php | 21 +++++++++++++++++---- tests/Task/YamlConfigurationTaskTest.php | 5 +++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/Task/YamlConfigurationTask.php b/src/Task/YamlConfigurationTask.php index 89daa6b..9c86d51 100644 --- a/src/Task/YamlConfigurationTask.php +++ b/src/Task/YamlConfigurationTask.php @@ -46,6 +46,13 @@ class YamlConfigurationTask extends AbstractConnectedTask */ private $environmentVariables; + /** + * Generated values to prevent different secrets on different nodes. + * + * @var array + */ + private static $generatedValues = array(); + /** * {@inheritdoc} */ @@ -172,7 +179,7 @@ private function findKeyAndGenerateValue(array &$configuration, array $parameter $configuration[$key] = $value; } elseif (is_scalar($value)) { - $configuration[$key] = $this->generateValue(); + $configuration[$key] = $this->generateValue($key); } } } @@ -197,12 +204,18 @@ private function addEnvironmentVariables($value) } /** - * Generate a sha key + * Generate a sha key for $key. + * + * @param string $key * * @return string */ - private function generateValue() + private function generateValue($key) { - return sha1(uniqid()); + if (!array_key_exists($key, self::$generatedValues)) { + self::$generatedValues[$key] = sha1(uniqid()); + } + + return self::$generatedValues[$key]; } } diff --git a/tests/Task/YamlConfigurationTaskTest.php b/tests/Task/YamlConfigurationTaskTest.php index e64d2f3..b385ccc 100644 --- a/tests/Task/YamlConfigurationTaskTest.php +++ b/tests/Task/YamlConfigurationTaskTest.php @@ -288,9 +288,10 @@ public function testGeneratedValuesAreNotRegenerated() $method = $reflectionClass->getMethod('generateValue'); $method->setAccessible(true); - $generated = $method->invoke($task, 'foobar'); + $generated = $method->invokeArgs($task, array('foobar')); $this->assertNotEmpty($generated); - $this->assertEquals($generated, $method->invoke($task, 'foobar')); + $this->assertEquals($generated, $method->invokeArgs($task, array('foobar'))); + $this->assertNotEquals($generated, $method->invokeArgs($task, array('baz'))); } } From a43ecc55649bc3b41a416bea4b77dd474573bbbe Mon Sep 17 00:00:00 2001 From: Ron Rademaker Date: Thu, 28 Apr 2016 18:48:37 +0200 Subject: [PATCH 4/6] Extracted value generation to a class --- src/Utility/SecretGenerator.php | 30 +++++++++++++++++++++++++ src/Utility/ValueGeneratorInterface.php | 20 +++++++++++++++++ tests/Utility/SecretGeneratorTest.php | 28 +++++++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 src/Utility/SecretGenerator.php create mode 100644 src/Utility/ValueGeneratorInterface.php create mode 100644 tests/Utility/SecretGeneratorTest.php diff --git a/src/Utility/SecretGenerator.php b/src/Utility/SecretGenerator.php new file mode 100644 index 0000000..52a27a7 --- /dev/null +++ b/src/Utility/SecretGenerator.php @@ -0,0 +1,30 @@ +generatedValues)) { + $this->generatedValues[$key] = sha1(uniqid()); + } + + return $this->generatedValues[$key]; + } +} diff --git a/src/Utility/ValueGeneratorInterface.php b/src/Utility/ValueGeneratorInterface.php new file mode 100644 index 0000000..cf5f593 --- /dev/null +++ b/src/Utility/ValueGeneratorInterface.php @@ -0,0 +1,20 @@ +generate('foobar'); + + $this->assertNotEmpty($generated); + $this->assertEquals($generated, $generator->generate('foobar')); + $this->assertNotEquals($generated, $generator->generate('baz')); + } +} From 5f9b160005dc8ac92b3e78565e91260c03ea7fbe Mon Sep 17 00:00:00 2001 From: Ron Rademaker Date: Thu, 28 Apr 2016 18:55:41 +0200 Subject: [PATCH 5/6] Let the yaml task use the secret generator by default, allow optional injection through a setter --- src/Task/YamlConfigurationTask.php | 51 ++++++++++++++---------- tests/Task/YamlConfigurationTaskTest.php | 18 --------- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/src/Task/YamlConfigurationTask.php b/src/Task/YamlConfigurationTask.php index 9c86d51..dbccdd2 100644 --- a/src/Task/YamlConfigurationTask.php +++ b/src/Task/YamlConfigurationTask.php @@ -8,6 +8,8 @@ use Accompli\EventDispatcher\Event\InstallReleaseEvent; use Accompli\EventDispatcher\Event\LogEvent; use Accompli\EventDispatcher\EventDispatcherInterface; +use Accompli\Utility\SecretGenerator; +use Accompli\Utility\ValueGeneratorInterface; use Psr\Log\LogLevel; use Symfony\Component\Yaml\Yaml; @@ -47,11 +49,11 @@ class YamlConfigurationTask extends AbstractConnectedTask private $environmentVariables; /** - * Generated values to prevent different secrets on different nodes. + * Generator to generate values. * - * @var array + * @var ValueGeneratorInterface */ - private static $generatedValues = array(); + private $valueGenerator; /** * {@inheritdoc} @@ -120,6 +122,31 @@ public function onInstallReleaseCreateOrUpdateConfiguration(InstallReleaseEvent } } + /** + * Sets a value generator. + * + * @param ValueGeneratorInterface + */ + public function setValueGenerator(ValueGeneratorInterface $valueGenerator) + { + $this->valueGenerator = $valueGenerator; + } + + /** + * Gets the value generator + * Defaults to SecretGenerator. + * + * @return ValueGeneratorInterface + */ + private function getValueGenerator() + { + if (!$this->valueGenerator instanceof ValueGeneratorInterface) { + $this->valueGenerator = new SecretGenerator(); + } + + return $this->valueGenerator; + } + /** * Gathers environment variables to use in the YAML configuration. * @@ -179,7 +206,7 @@ private function findKeyAndGenerateValue(array &$configuration, array $parameter $configuration[$key] = $value; } elseif (is_scalar($value)) { - $configuration[$key] = $this->generateValue($key); + $configuration[$key] = $this->getValueGenerator()->generate($key); } } } @@ -202,20 +229,4 @@ private function addEnvironmentVariables($value) return $value; } - - /** - * Generate a sha key for $key. - * - * @param string $key - * - * @return string - */ - private function generateValue($key) - { - if (!array_key_exists($key, self::$generatedValues)) { - self::$generatedValues[$key] = sha1(uniqid()); - } - - return self::$generatedValues[$key]; - } } diff --git a/tests/Task/YamlConfigurationTaskTest.php b/tests/Task/YamlConfigurationTaskTest.php index b385ccc..4546f3b 100644 --- a/tests/Task/YamlConfigurationTaskTest.php +++ b/tests/Task/YamlConfigurationTaskTest.php @@ -7,7 +7,6 @@ use Accompli\EventDispatcher\Event\InstallReleaseEvent; use Accompli\Task\YamlConfigurationTask; use PHPUnit_Framework_TestCase; -use ReflectionClass; /** * YamlConfigurationTaskTest. @@ -277,21 +276,4 @@ public function testOnInstallReleaseCreateOrUpdateConfigurationFailsCreatingANew $task = new YamlConfigurationTask('/parameters.yml', array('foo' => 'bar', 'baz' => '', 'bar' => array('baz' => '')), array('baz', 'bar.baz')); $task->onInstallReleaseCreateOrUpdateConfiguration($event, AccompliEvents::INSTALL_RELEASE, $eventDispatcherMock); } - - /** - * Tests if generated values are not regenerated within the same process. - */ - public function testGeneratedValuesAreNotRegenerated() - { - $task = new YamlConfigurationTask('foobar'); - $reflectionClass = new ReflectionClass('Accompli\Task\YamlConfigurationTask'); - $method = $reflectionClass->getMethod('generateValue'); - $method->setAccessible(true); - - $generated = $method->invokeArgs($task, array('foobar')); - - $this->assertNotEmpty($generated); - $this->assertEquals($generated, $method->invokeArgs($task, array('foobar'))); - $this->assertNotEquals($generated, $method->invokeArgs($task, array('baz'))); - } } From 50b9e1c68bac790e7420a2cc841eee9264272f74 Mon Sep 17 00:00:00 2001 From: Ron Rademaker Date: Thu, 28 Apr 2016 19:07:20 +0200 Subject: [PATCH 6/6] Added test for injecting a value generator --- tests/Task/YamlConfigurationTaskTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/Task/YamlConfigurationTaskTest.php b/tests/Task/YamlConfigurationTaskTest.php index 4546f3b..f055512 100644 --- a/tests/Task/YamlConfigurationTaskTest.php +++ b/tests/Task/YamlConfigurationTaskTest.php @@ -6,6 +6,7 @@ use Accompli\Deployment\Host; use Accompli\EventDispatcher\Event\InstallReleaseEvent; use Accompli\Task\YamlConfigurationTask; +use Accompli\Utility\SecretGenerator; use PHPUnit_Framework_TestCase; /** @@ -276,4 +277,16 @@ public function testOnInstallReleaseCreateOrUpdateConfigurationFailsCreatingANew $task = new YamlConfigurationTask('/parameters.yml', array('foo' => 'bar', 'baz' => '', 'bar' => array('baz' => '')), array('baz', 'bar.baz')); $task->onInstallReleaseCreateOrUpdateConfiguration($event, AccompliEvents::INSTALL_RELEASE, $eventDispatcherMock); } + + /** + * Test setting a value generator. + */ + public function testValueGeneratorSetter() + { + $task = new YamlConfigurationTask('/parameters.yml'); + $generator = new SecretGenerator(); + $task->setValueGenerator($generator); + + $this->assertAttributeEquals($generator, 'valueGenerator', $task); + } }