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

Aws secrets #4

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Aws secrets #4

wants to merge 10 commits into from

Conversation

r-kok
Copy link

@r-kok r-kok commented Mar 20, 2017

This PR adds the option to load secrets from the AWS Parameter Store.
Use it by specifying {{ .AWS_Secrets.VARIABLE }} in a template.
Details are described in the README file.

@rprieto
Copy link

rprieto commented Apr 11, 2017

This would be very useful for applications deployed to ECS, as it's one of the simplest way to store secrets in AWS 👍

@melvyndekort
Copy link

melvyndekort commented Apr 13, 2017

We're already using this in production, works like a charm!

@arminc
Copy link

arminc commented May 21, 2017

What is needed to get this approved? So I don't need to have a separate build.

@cyberious
Copy link

cyberious commented Jun 23, 2017

love the idea of this, currently bouncing between this and https://github.com/awslabs/ecs-secrets

@les2
Copy link

les2 commented Aug 11, 2017

@markriggins what do you think about this?

**Dockerfy** can also load secrets stored in the AWS Systems Manager [Parameter Store](https://aws.amazon.com/ec2/systems-manager/parameter-store/).
If you specify an expression like `{{ .AWS_Secret.**VARNAME** }}` in a template then dockerfy will try to fetch the parameter from the AWS Parameter store.
If the parameter cannot be found (due to lack or permission or because it does not exist) dockerfy falls back to using the value of a corresponding ENVIRONMENT value.
**Dockerfy** retrieves the decoded values of at most 10 parameters.
Copy link
Contributor

@markriggins markriggins Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already have another mechanism for defaults . {{ default var "value"}} that could be used for defaults. Secrets should never be in the environment, so that doesn't make a good default.


To select a specific parameter that matches the prefix specify option `--aws-secret-prefix PROD_`.
You can use `{{ .AWS_Secret.**DB_PASSWORD** }}` in your template, thus without the prefix.

Copy link
Contributor

@markriggins markriggins Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting. is there any easy way to simply use a different parameter store?

secrets := fetchAWS_Secrets(svc,filtered)
return asMap(secrets)
}

Copy link
Contributor

@markriggins markriggins Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like mixing secrets and the environment.

@@ -128,6 +129,10 @@ Arguments:

dockerfy --start /bin/sleep 5 -- /bin/service
`)
println(` Retrieve parameters with a given prefix from AWS Parameter store and use them in a template:

dockerfy --aws-secret-prefix PRODUCTION /bin/echo '{{ AWS_Secret.password }}'
Copy link
Contributor

@markriggins markriggins Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use a different Parameter store for prod and testing? Wouldn't you want to keep your Prod secrets very secret?

Copy link

@rprieto rprieto Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree ideally test and prod secrets are stored separately - probably even run from different AWS accounts, however it can be a useful feature for simple setups. It works well with IAM where you create policies allow access to Test.* and Prod.*, and attach them to the corresponding ECS tasks.

Similarly, you can namespace your applications, e.g. have a ParameterStore entry called ApplicationFoo.DbPassword and ApplicationBar.ApiKey, and a corresponding policy to grant access ApplicationFoo.*.

Would it work if you specify a full path like {{ .Secrets.ApplicationFoo.DbPassword }}? Otherwise that's where the prefix is handy, so you can use {{ .Secrets.DbPassword }} and --aws-ssm-prefix ApplicationFoo (called it ssm to be more explicit 😃 ).

Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The templates support full GoLang functionality. See the README section "Advanced Templates" for examples.

It would be inappropriate to mix test and prod secrets. For test, defaults are usually all you need.

password = '{{ default .Secret.PASSWORD "test-password" }}'

Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I don't care for the --aws-secret-prefix option. There's nothing to keep you from programatically accessing them in that way though. ALL secrets (from multiple files and sources) get copied into the container into a combined file pointed to by $SECRETS_FILE. You need to put your settings into the secrets map so they too will be available at runtime.

import json
import os

with open(os.getenv('SECRETS_FILE') as secrets_file:    
    secrets = json.load(secrets_file)

Then you can decide which of these secrets to use like this:

password = secrets[prefix + '_PASSWORD']


Copy link

@rprieto rprieto Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for tests defaults are all you need is true in many cases, but can be restrictive for some systems. We often have many different test environments that all need different credentials, also loaded from Parameter Store. Test and prod have different AWS accounts / Parameter Stores, but all test environments often share the same account - so we use IAM roles to grant access to specific secrets.

Actually looking into it some more, AWS just released support for hierarchies: https://aws.amazon.com/about-aws/whats-new/2017/06/amazon-ec2-systems-manager-adds-hierarchy-tagging-and-notification-support-for-parameter-store/. This means we can retrieve all key/values for a particular hierarchy (see docs):

aws ssm get-parameters-by-path --path 'MyApp/UAT' --recursive --with-decryption

What about removing the prefix support (which was a custom setup), but adding support for hierarchies which is a native AWS concept?

Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can almost do this by dockerfy --run aws ssm get-parameters-by-path > secrets.json -- --secrets-files secrets.json .... . But --run commands and --start commands do not support file redirection.

Instead, create a file nameed secret-generator like this to run your command:

#!/usr/bin/env bash
cat << EOF > $1
{
	"password": "PASSWORD"
}
EOF

Then you can run it like this:

dockerfy \
--run ./secret-generator /tmp/secrets.json -- \
--secrets-files /tmp/secrets.json -- \
echo 'PW={{.Secret.password}}'

Giving you output PW=PASSWORD

Copy link

@rprieto rprieto Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be a good starting point. From a user's point of view, it's probably less ideal than built-in support as you had to write the glue code yourself, which might have edge cases (e.g. I think you need to make several API calls if retrieving > 10 secrets).

Are you thinking of supporting generic commands first, and then adding built-in SSM support? Do you think this PR is very far off adding acceptable built-in support?

Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think aws ssm is an ideal use-case for dockerfy that could use first-class support.

We could also update the README to show how to write a script to run any-command-that-outputs-JSON to load secrets, but built-in support for ssm would be awesome.

Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So lets just get the flags right, and load it into the secrets and env maps. Mocking would be the best way to test, but that's not that easy in GoLang. Another way would be to actually run the aws ecs commands to setup a temporary environment with hosts and param store, etc, use it, then tear it all down. That feels like a lot of work though

Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, I'm fine with first-class support for aws ssm, but don't have time to do all the testing. The script approach on the other hand, works as is

@@ -14,7 +14,7 @@ import (
)

type TemplateContext struct {
env, secrets map[string]string
env, secrets, aws_secrets map[string]string
}
Copy link
Contributor

@markriggins markriggins Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't we just load AWS secrets in to the secrets map? why two maps?

Copy link
Contributor

@markriggins markriggins Sep 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that there are 2 sources of secrets. but secrets are secrets

@markriggins
Copy link
Contributor

markriggins commented Sep 3, 2017

Can you write a unit test? See the unit test section of this Makefile for examples of how to use docker to setup the environment, files etc. https://github.com/SocialCodeInc/dockerfy/blob/master/test/Makefile

you could probably mock the service call to make the test easy

}
return secrets
}

Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our secrets global is already a map. lets use that one

return c.aws_secrets
}


Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a lot of this code will drop off if you just use the secrets map and defaults.

criteria := &ssm.DescribeParametersInput{
MaxResults: aws.Int64(45), // limited by API call GetParametersInput
}
resp, err := svc.DescribeParameters(criteria)
Copy link
Contributor

@markriggins markriggins Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this api has a limit. Any reason not to blow it up to 256 or even 1024? Going off host for an answer is expensive because of the round trip, returning a result under 10k should be fast.

@composer22
Copy link

composer22 commented Nov 3, 2017

No support for k/v versions I can see

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants