parameter_store parameter resolver. by andrewjhumphrey · Pull Request #211 · envato/stack_master

andrewjhumphrey · 2018-02-21T14:57:00Z

Resolves parameters from the AWS SSM parameter store service as alternative
to the dotgpg malarky. (which is great for large teams but is overkill for single
person/small team use)

This is the inital cut, just uses default AWS managed encryption keys.

Format is pretty simple
parameter.yml:

foo:
  parameter_store: ssm_name

Fetches the SecureString parameter ssm_name from the AWS SSM store using the prevailing
aws credentials.

DONE:

Add ability to specify KMS key_id to use to decrypt
(to make it useful for storing things like database passwords) (Can't get_parameter doesn't let you specify the key to use, it's stored with the parameter at encryption time)
Add ability to specify version to use to decrypt (Can't retrieve a particular version so of limited utility so not going to do)
Make it work with parameter arrays (Not required, 1 parameter 1 value and it can be a comma separated list if required)

TODO:
Nothing

ejoubaud

From what little understanding I have of stack_master, this looks good to me. And I ❤️ the idea!

patrobinson

Looks good so far

patrobinson · 2018-02-21T21:46:41Z

+  describe '#resolve' do
+
+    let(:config) { double(base_dir: '/base') }
+    let(:stack_definition) { double(stack_name: 'mystack', region: 'us-east-1') }


this should probably be an instance_double, which validates any methods called on it actually exist on an instance of the StackDefinition class

nod Probably but dunno it's worth rejigging everything in this PR, it's done this way in:

parameter_resolvers/latest_ami_by_tags_spec

parameter_resolvers/latest_ami_spec

stack_master/parameter_resolvers/env_spec
...etc
and I'd prefer to address this in a separate PR if we want to change the approach across a bunch of resolvers...

patrobinson · 2018-02-21T21:49:36Z

+      end
+      it 'should raise and error' do
+        expect { resolver.resolve(unknown_parameter_name) }
+            .to raise_error(Aws::SSM::Errors::ParameterNotFound, "Parameter #{unknown_parameter_name} not found")


I think we should catch and re-raise this exception with a different exception class we define. That's at least what all the other parameter resolvers do.

Addressed in 8e729dd following pattern found in parameter_resolvers/secret

patrobinson · 2018-02-21T21:49:51Z

@@ -0,0 +1,15 @@
+Given(/^(?:a|the) SSM parameter(?: named)? "([^"]*)" with value "([^"]*)" in region "([^"]*)"$/) do |parameter_name, parameter_value, parameter_region|
+  File.open("/tmp/humpy-log", 'a') { |file| file.write("In Here name: #{parameter_name} value: #{parameter_value} \n") }


I'm assuming this was in here for debugging? Can it be removed?

Yep, has been 17501ef Was debugging why everything appeared to be stubbed, before I discovered lib/stack_master/testing.rb

petervandoros

Looking very nice! I added a question around the StringList parameter store type.

I'm guessing that you're still planning on adding documentation for this parameter resolver?

petervandoros · 2018-02-21T21:34:08Z

+          name: value,
+          with_decryption: true
+        )
+        resp.parameter.value


Do you know how array parameter values are handled by StackMaster? Does a StringList parameter type need to be transformed into an Array or will it be handled just fine by StackMaster?

No idea, I'll add that to the todo list to figure out.

I don't actually thing it's an issue here, there is only ever one parameter with the same name, so you can only get one value, so iterating and fetching isn't sensible. (which is how the other array_parameters work)
That said if you need to you could store "1,2,3,4,5" as the value for the parameter and it will just work as all stackmaster does is concatenate with comma separators (at least according to my reading of https://github.com/envato/stack_master/blob/master/lib/stack_master/resolver_array.rb#L9-L13)

Yeah, that's my understanding also.

stevehodgkiss

Useful feature. Looks good so far!

stevehodgkiss · 2018-02-21T22:08:36Z

+      vpc_cidr:
+        parameter_store: CUCUMBER_TEST_VPC_CIDR
+      """
+    And a SSM parameter named "CUCUMBER_TEST_VPC_CIDR" with value "10.0.0.0/16" in region "us-east-2"


Parameters wouldn't usually be upper case. Maybe change this to something like /cucumber-test-vpc-cidr or /myapp/prod/something-api-key?

Shall do, the uppercase was a hangover where I was actually creating the parameter in SSM parameter store (before I realised that all aws calls are stubbed in test mode) and I didn't want to trip over real parameters.

Done: acd7f6f

patrobinson · 2018-02-22T09:34:50Z

+        begin
+          resp = ssm.get_parameter(
+            name: value,
+            with_decryption: true


Does this fail if the parameter is not encrypted?

Nope, it's ignored for anything except a SecureString

patrobinson · 2018-02-22T09:35:48Z

+An alternative to the secrets store, uses the AWS SSM Parameter store to protect
+secrets.   Expects a parameter of either `String` or `SecureString` type to be present in the
+same region as the stack. You can store the parameter using a command like this
+`aws-vault exec <whatever> -- aws ssm put-parameter --region <region> --name <parameter name> --value <secret> --type (String|SecureString)`


I don't think we should reference aws-vault here, as many people use different authentication methods.

This needs some more white space to avoid being included on the same line

patrobinson · 2018-02-22T09:37:09Z

+  module ParameterResolvers
+    class ParameterStore < Resolver
+
+      SecretNotFound = Class.new(StandardError)


I think ParameterNotFound would be more appropriate here

SecretNotFound was already used, but sure naming is hard :-)

Updated in 4d9ae15

…github.com:envato/stack_master into andrewjhumphrey-parameter-store-parameter-resolver

patrobinson

andrewjhumphrey added 4 commits February 21, 2018 21:30

Test for presence and absence of parameter

70b5376

First cut of using SSM parameter store for SM parameters

a9e9ce5

Wire it in

24f980f

Add integration test for parameter store parameters

bb1edb3

ejoubaud approved these changes Feb 21, 2018

View reviewed changes

patrobinson reviewed Feb 21, 2018

View reviewed changes

petervandoros reviewed Feb 21, 2018

View reviewed changes

stevehodgkiss reviewed Feb 21, 2018

View reviewed changes

andrewjhumphrey added 4 commits February 22, 2018 06:46

Remove debuging WTF code

17501ef

Use custom exception type instead of bubbling up the weird AWS one

8e729dd

Rename parameter to something more usual

acd7f6f

Add basic usage docs

78f8ed8

andrewjhumphrey changed the title ~~WIP: parameter_store parameter resolver.~~ parameter_store parameter resolver. Feb 22, 2018

patrobinson reviewed Feb 22, 2018

View reviewed changes

andrewjhumphrey added 3 commits February 22, 2018 18:32

Remove aws-vault reference + formatting.

a0688ab

Rename Exception

4d9ae15

Merge branch 'andrewjhumphrey-parameter-store-parameter-resolver' of …

e1f8f29

…github.com:envato/stack_master into andrewjhumphrey-parameter-store-parameter-resolver

patrobinson approved these changes Feb 22, 2018

View reviewed changes

petervandoros approved these changes Feb 22, 2018

View reviewed changes

andrewjhumphrey merged commit f9b2d0c into master Feb 23, 2018

andrewjhumphrey deleted the andrewjhumphrey-parameter-store-parameter-resolver branch February 23, 2018 01:50

		@@ -0,0 +1,15 @@
		Given(/^(?:a\|the) SSM parameter(?: named)? "([^"])" with value "([^"])" in region "([^"]*)"$/) do \|parameter_name, parameter_value, parameter_region\|
		File.open("/tmp/humpy-log", 'a') { \|file\| file.write("In Here name: #{parameter_name} value: #{parameter_value} \n") }

Conversation

andrewjhumphrey commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ejoubaud left a comment

Choose a reason for hiding this comment

Uh oh!

patrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petervandoros left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevehodgkiss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andrewjhumphrey commented Feb 21, 2018 •

edited

Loading