Add AWS secretsmanager as a backend

[jrhoward]

Hi All I added support for AWS SecretsManager Including some tests under integrations/secretsmanager Also added secretmanager client to the aws vendor libraries

It only only supports SecretString not SecretBinary

[jrhoward]

Hmm. ok Im relatively new to go. I will retry

[jrhoward]

Ok the nested and iterative options are a bit tricky for secretsmanager if you want to keep the algorithms efficient. I will close for now and open once I have worked it out

[okushchenko]

@jahoward thank you for opening a PR! I'll be able to review it by the end of this week, but it looks good so far.

[ozbillwang]

Nice feature. 👍

By the way, since AWS Secrets Manager enables you to store text up to 4096 bytes in length in the encrypted secret data portion of a secret. So how do you extend the length if required?

[ozbillwang]

@jrhoward

Cloud you please update the document quick start guide as well in this PR so we can easily understand how to use this new feature.

So I can test it from your feature branch.

[jrhoward]

Hi I've been away for a few days with no computer but will be back online later today. I will update the doco tomorrow.

[jrhoward]

Regarding extending the length of secrets I am only interfacing with the existing aws API so you would have to DYI which wouldn't be to hard if you scripted the creation of a secret by breaking it up into multiple secrets. On saying that I would look at a different solution if I needed to encrypt large amounts of data. maybe encrypt the data previously and store the key in secret manager

[ozbillwang]

For SecretBinary support, normally I convert the binary to BASE64 and save as SecretString in secrets manager and decode it later.

[ozbillwang]

Not sure if this is the issue in confd or new backend secretsmanager. Seems I have to manually set the environment variable AWS_REGION, otherwise, i got the error

I can confirm I have set region in both ~/.aws/credentials and ~/.aws/config

$ confd -onetime -backend secretsmanager
2018-04-27T05:49:40Z ubuntu-xenial confd[10791]: INFO Backend set to secretsmanager
2018-04-27T05:49:40Z ubuntu-xenial confd[10791]: INFO Starting confd
2018-04-27T05:49:40Z ubuntu-xenial confd[10791]: INFO Backend source(s) set to
2018-04-27T05:49:41Z ubuntu-xenial confd[10791]: INFO Target config /tmp/myconfig.conf out of sync
2018-04-27T05:49:41Z ubuntu-xenial confd[10791]: INFO Target config /tmp/myconfig.conf has been updated

$ AWS_REGION=ap-southeast-2 confd -onetime -backend secretsmanager
2018-04-27T05:54:42Z ubuntu-xenial confd[10993]: INFO Backend set to secretsmanager
2018-04-27T05:54:42Z ubuntu-xenial confd[10993]: INFO Starting confd
2018-04-27T05:54:42Z ubuntu-xenial confd[10993]: INFO Backend source(s) set to
2018-04-27T05:54:43Z ubuntu-xenial confd[10993]: INFO /tmp/myconfig.conf has md5sum fa2f0b3e6d5cb3195c1463833fd393c6 should be 8e76340b541b8ee29023c001a5e4da18
2018-04-27T05:54:43Z ubuntu-xenial confd[10993]: INFO Target config /tmp/myconfig.conf out of sync
2018-04-27T05:54:43Z ubuntu-xenial confd[10993]: INFO Target config /tmp/myconfig.conf has been updated

The rest are all fine, and waiting for it to be merged. 😃

[jrhoward]

great thanks. yes I noted that you have to explicitly set the region. I assume this is the behaviour of the AWS libs rather than the code I added

[ozbillwang]

@jrhoward

Can you check if your new backend secretsmanager has problem with this issue I reported https://github.com/kelseyhightower/confd/issues/704

[jrhoward]

Fix for issue #704

[ozbillwang]

@jrhoward

got problem when build with the latest codes.

$ cat /etc/confd/conf.d/myconfig.toml
[template]
src = "myconfig.conf.tmpl"
dest = "/tmp/myconfig.conf"
keys = [
    "/myapp/database/user",
]

$ cat /etc/confd/templates/myconfig.conf.tmpl
[myconfig]
database_url = {{getv "/myapp/database/url"}}
database_user = {{getv "/myapp/database/user"}}

$ AWS_REGION=ap-southeast-2 confd -onetime -backend secretsmanager
2018-04-29T13:57:41Z ubuntu-xenial confd[2950]: INFO Backend set to secretsmanager
2018-04-29T13:57:41Z ubuntu-xenial confd[2950]: INFO Starting confd
2018-04-29T13:57:41Z ubuntu-xenial confd[2950]: INFO Backend source(s) set to
2018-04-29T13:57:42Z ubuntu-xenial confd[2950]: ERROR template: myconfig.conf.tmpl:2:17: executing "myconfig.conf.tmpl" at <getv "/myapp/databas...>: error calling getv: key does not exist: /myapp/database/url
2018-04-29T13:57:42Z ubuntu-xenial confd[2950]: FATAL template: myconfig.conf.tmpl:2:17: executing "myconfig.conf.tmpl" at <getv "/myapp/databas...>: error calling getv: key does not exist: /myapp/database/url

[jrhoward]

@ozbillwang

I think that is as expected. The backend has only retrieved "/myapp/database/user" as expected and the templating engine is throwing an error because you are referencing a key that doesn't exist ie. {{getv "/myapp/database/url"}} if you run with -log-level DEBUG you can see it returns the correct value to the templating engine as defined in the TOML file.

.....
 bin/confd[6408]: DEBUG Got the following map from store: map[/myapp/database/user:rob]
 bin/confd[6408]: DEBUG Using source template /etc/confd/templates/myconfig.conf.tmpl
 bin/confd[6408]: DEBUG Compiling source template /etc/confd/templates/myconfig.conf.tmpl
 bin/confd[6408]: ERROR template: myconfig.conf.tmpl:2:17: executing "myconfig.conf.tmpl" at <getv "/myapp/databas...>: error calling getv: key does not exist: /myapp/database/url
 bin/confd[6408]: FATAL template: myconfig.conf.tmpl:2:17: executing "myconfig.conf.tmpl" at <getv "/myapp/databas...>: error calling getv: key does not exist: /myapp/database/url

If you change keys to

keys = [
    "/myapp/database",
]

It will retrieve all the keys successfully.

bin/confd[6655]: DEBUG Got the following map from store: map[/myapp/database/url:"localhost:8080" /myapp/database/user:rob]

Would the correct way to handle an error like that be to add a default value for a key if it does not exist in the template? eg.

[myconfig]
database_url = {{getv "/myapp/database/url" "key does not exist" }}
database_user = {{getv "/myapp/database/user" "key does not exist"}}

[ozbillwang]

ok, maybe I am new for this tool.

My purpose is, I need convert secrets for different environments.

$ cat /etc/confd/conf.d/myconfig-dev.toml
[template]
src = "myconfig.conf.tmpl"
dest = "/tmp/myconfig.conf"
keys = [
    "/myapp/database/dev/user",
]

$ cat /etc/confd/templates/myconfig.conf.tmpl
[dev]
database_user = {{getv "/myapp/database/dev/user"}}
[staging]
database_user = {{getv "/myapp/database/staging/user"}}

So each time, I only need convert several secrets for nominated environment, whatever all keys are marked by {{ getv ...}}.

Not sure how to do that.

[jrhoward]

ok with the caveat that I also have not used confd extensively

look at the following page https://github.com/kelseyhightower/confd/blob/master/docs/templates.md you could do this:

{{if exists "/key"}}
    value: {{getv "/key"}}
{{end}}

You could use an environment variable to control the logic by injecting a per environment value see:

https://github.com/kelseyhightower/confd/blob/master/docs/templates.md#getenv

Note also that is supports the full go templating syntax

https://golang.org/pkg/text/template/

[hubo1016]

When "/abc/def" is set in keys, the HasPrefix approach retrieves not only "/abc/def" and "/abc/def/ghi", but also keys like "/abc/def123". There are plenty of backends with this bug and have not been fixed, but should we fix them in new backends from the beginning?

[jrhoward]

seems fair I will validate if this pull request has that bug and fix if it does

[jrhoward]

Fix for partial match of keys: When "/abc/def" is set in keys, the HasPrefix approach retrieves not only "/abc/def" and "/abc/def/ghi", but also keys like "/abc/def123".

Secrets manager backend will not match /abc/def123".

[ozbillwang]

when can this PR be merged? So I needn't build it from fork.

[donkeysharp]

amazing, looking forward for this feature to be merged

[sbita26]

Hi @jrhoward

I have confd in my container in start.sh CMD and able to run it locally but when I try to run the same container on ECS it gives me following error

/confd[5]: INFO Backend set to secretsmanager /confd[5]: INFO Starting confd /confd[5]: INFO Backend source(s) set to /confd[5]: ERROR RequestError: send request failed caused by: Post https://secretsmanager.ap-southeast-2.amazonaws.com/: dial tcp xx.xxx.xxx.xx:443: i/o timeout

The AWS TaskRole for the container has ECS admin and aws secrets manager list/get/describe policies. Is there a possibility of confd not recognizing the ecs task roles

[jrhoward]

@luckygal26

dial tcp xx.xxx.xxx.xx:443: i/o timeout

Suggests a network issue not an authentication/authorisation issue which is what you would see if it were a Role based issue.

By chance is the IP address 169.254.169.254 ? If so this is the Instance metadata IP. I would suggest you pose this question in an ECS forum in the context of the ASW golang SDK to see if there are any known issues/limitations in ECS

[michaeljs1990]

Anything that needs to be done still to help get this merged in?

[jrhoward]

Not from my perspective. Just waiting :] Are we waiting for the implementation of the backend plugin ?

[okushchenko]

@jahoward yes, we're waiting for the #568 to be merged. Sorry for the delay, I've had not enough time to finish it this month, but hopefully it will change next week.

@micahhausler you can help by providing your feedback on #568. Reviewers welcome.

[ltatakis]

Is #568 blocking the merging of this PR? We have tested and confirmed that this branch would bring great benefit to your workflow, and wondering if we can expedite things. Is there anything we can do to move things along?

[okushchenko]

@ltatakis there was a decision made by Kelsey that no new backends will be merged into the core of confd. All new backends must be implemented as plugins. So yes, #568 is blocking this PR from getting merged. You can help by providing your feedback on #568.

[ozbillwang]

can we have something running parallelised?

I understand the idea to add new backends via plugin is good, but before it (#568) is ready, and block the current PR will be bad decision.

Can we have this backend merged more than hold by #568 a while? #568 doesn't look like a simple PR and will take long time to be merged.

[jmervine]

For those interested and patiently awaiting this PR, I'm using a work around with summon and it's summon-aws-secrets plugin.

summon -p summon-aws-secrets -f /etc/secrets.yaml --ignore-all \
    confd -confdir /etc/confd -onetime -backend=env

[fllaca]

Hi, following up on this. https://github.com/kelseyhightower/confd/pull/568 seems to be a bit staled, while this PR looks quite ready and will give value to many confd users. Can we merge this one? @okushchenko

[devopssenpai]

@okushchenko @kelseyhightower is this good to get merged? or has this project been abandoned? I haven't seen a commit go into master since July 2018

[nthienan]

@okushchenko @kelseyhightower can you guys merge this pr?