Skip to content
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.

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

Fix for multiple ENV variables sharing the same bref-ssm: parameters #9

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions src/Secrets.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = nu
return self::retrieveParametersFromSsm($ssmClient, array_values($ssmNames));
});

foreach ($parameters as $parameterName => $parameterValue) {
$envVar = array_search($parameterName, $ssmNames, true);
foreach ($envVarsToDecrypt as $envVar => $prefixedSsmRefName) {
$parameterName = substr($prefixedSsmRefName, strlen('bref-ssm:'));
$parameterValue = $parameters[$parameterName];
$_SERVER[$envVar] = $_ENV[$envVar] = $parameterValue;
putenv("$envVar=$parameterValue");
}
Expand Down
61 changes: 51 additions & 10 deletions tests/SecretsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@

class SecretsTest extends TestCase
{
private array $resultParams = [
'Name' => '/some/parameter',
'Value' => 'foobar',
];

public function setUp(): void
{
if (file_exists(sys_get_temp_dir() . '/bref-ssm-parameters.php')) {
Expand All @@ -27,25 +32,56 @@ public function test decrypts env variables(): void
$this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE'));
$this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE'));

Secrets::loadSecretEnvironmentVariables($this->mockSsmClient());
$ssmClient = $this->mockSsmClient([new Parameter($this->resultParams)]);
Secrets::loadSecretEnvironmentVariables($ssmClient);

$this->assertSame('foobar', getenv('SOME_VARIABLE'));
$this->assertSame('foobar', $_SERVER['SOME_VARIABLE']);
$this->assertSame('foobar', $_ENV['SOME_VARIABLE']);
// Check that the other variable was not modified
$this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE'));

// Cleanup
putenv('SOME_VARIABLE');
putenv('SOME_OTHER_VARIABLE');
}

public function test caches parameters to call SSM only once(): void
{
putenv('SOME_VARIABLE=bref-ssm:/some/parameter');

// Call twice, the mock will assert that SSM was only called once
$ssmClient = $this->mockSsmClient();
$ssmClient = $this->mockSsmClient([new Parameter($this->resultParams)]);
Secrets::loadSecretEnvironmentVariables($ssmClient);
Secrets::loadSecretEnvironmentVariables($ssmClient);

$this->assertSame('foobar', getenv('SOME_VARIABLE'));

// Cleanup
putenv('SOME_VARIABLE');
}

public function test same ssm value can be assigned more than once(): void
{
putenv('VAR1=bref-ssm:/some/parameter');
putenv('VAR2=bref-ssm:/some/parameter');

// Sanity checks
$this->assertSame('bref-ssm:/some/parameter', getenv('VAR1'));
$this->assertSame('bref-ssm:/some/parameter', getenv('VAR2'));

$ssmClient = $this->mockSsmClient([
new Parameter($this->resultParams),
new Parameter($this->resultParams),
]);
Secrets::loadSecretEnvironmentVariables($ssmClient);

$this->assertSame('foobar', getenv('VAR1'));
$this->assertSame('foobar', getenv('VAR2'));

// Cleanup
putenv('VAR1');
putenv('VAR2');
}

public function test throws a clear error message on missing permissions(): void
Expand All @@ -62,28 +98,33 @@ public function test throws a clear error message on missing permissions
$expected = preg_quote("Bref was not able to resolve secrets contained in environment variables from SSM because of a permissions issue with the SSM API. Did you add IAM permissions in serverless.yml to allow Lambda to access SSM? (docs: https://bref.sh/docs/environment/variables.html#at-deployment-time).\nFull exception message:", '/');
$this->expectExceptionMessageMatches("/$expected .+/");
Secrets::loadSecretEnvironmentVariables($ssmClient);

// Cleanup
putenv('SOME_VARIABLE');
}

private function mockSsmClient(): SsmClient
/**
* @param array<Parameter> $resultParameters
*/
private function mockSsmClient(array $resultParameters): SsmClient
{
$ssmClient = $this->getMockBuilder(SsmClient::class)
->disableOriginalConstructor()
->onlyMethods(['getParameters'])
->getMock();

$result = ResultMockFactory::create(GetParametersResult::class, [
'Parameters' => [
new Parameter([
'Name' => '/some/parameter',
'Value' => 'foobar',
]),
],
'Parameters' => $resultParameters,
]);

$expectedNames = [];
foreach ($resultParameters as $resultParameter) {
$expectedNames[] = $resultParameter->getName();
}
$ssmClient->expects($this->once())
->method('getParameters')
->with([
'Names' => ['/some/parameter'],
'Names' => $expectedNames,
'WithDecryption' => true,
])
->willReturn($result);
Expand Down