-
Notifications
You must be signed in to change notification settings - Fork 564
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
Added SSM integration #569
Conversation
|
||
func Run() { | ||
logger = helmexec.NewLogger(os.Stdout, "debug") | ||
ssmPath, exists := os.LookupEnv("SSM_PATH") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we could support multiple paths. Perhaps use ;
delimiter like other PATH
vars
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely think we should have the ability to support multiple paths.
Would you rather define each path in the environment? Or just directly in the helmfile? I think it makes more sense to just put right in the helmfile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lanmalkieri I'm looking at updating the helmfile.yml spec to include ssm:
at the root level of the YAML.
ssm:
region: us-west-2
prefix: /path/in/ssm
The idea is then the prefix is optional when you start using ssm directly
{{ ssm "postgres_pass" }}
instead of
{{ ssm "/path/in/ssm/postgres_pass" }}
But of course you can do something like..
{{ ssm "/path/one/postgres_pass" }}
{{ ssm "/path/two/redis_pass" }}
length = 0 | ||
} | ||
|
||
key := strings.Replace(strings.Trim(name[length:], "/"), "/", "_", -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For greater interoperability with chamber
which lowercases all parameters, it would be nice if there was an option to uppercase them for environment variables.
|
||
func setParameter(ssmPath string, parameter *ssm.Parameter) { | ||
if parameter == nil { | ||
logger.Error("SSM parameter is nil") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is more of a warning or debug.
key := strings.Replace(strings.Trim(name[length:], "/"), "/", "_", -1) | ||
value = strings.Replace(value, "\n", "\\n", -1) | ||
|
||
os.Setenv(key, value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe warn if overwriting an existing variable
Hi guys, could you explain what is SSM, and what does this have in common with helmfile and this PR brings. |
@sgandon Ah, yes, will do! What is AWS SSM AWS SSM: https://docs.aws.amazon.com/systems-manager/latest/userguide/what-is-systems-manager.html Specifically, we use Parameter Store within SSM: https://docs.aws.amazon.com/systems-manager/latest/userguide/systems-manager-parameter-store.html At a high level, AWS SSM Parameter Store is a free KV store provided by AWS. I've used it across various projects & companies. @osterman has https://cloudposse.com/ where his teams use it across multiple projects & companies as well. So it's really popular. What's the intersection between SSM & Helmfile? Helmfile is very powerful for many reasons! One of the awesome features is that we can What we do currently is use tools like https://github.com/segmentio/chamber and https://github.com/Droplr/aws-env to extract SSM variables and add to our Env before running This PR was aimed at solving that issue of the extra logic/dependencies. Next steps There are a few different approaches to take:
Honestly, the last approach would probably be the most robust. What do you think? |
Interesting. Just piggybacking to say I have a use-case for something similar, except using |
@rms1000watt thanks a lot for the very clear explanation. |
@sgandon I could mock up something similar to how serverless does it: But I think it would be more specific to the implementation like: ssm:
region: us-west-2
prefix: /path/within/ssm
# credentials for connecting to AWS would be omitted here and follow standard Golang SDK for connecting to AWS
repositories:
- name: "my-repo"
url: "s3://my-private-helm-chart-repo"
releases:
- name: hello-world
chart: my-repo/hello-world
values:
- hello-world:
url: http://www.fake-url.com
postgres_password: {{ ssm "postgres_password" }} If and only if (Add And to @aegershman 's point, the same structure could happen with credhub (although i've never used it and am not sure if this is makes sense with credhub). But for other KV stores (consul, etcd) this might make sense. It's sort of like dependency injection at the YAML level. What do you guys think? |
That is a great proposal !. |
@@ -569,6 +571,7 @@ Do you really want to delete? | |||
}, | |||
} | |||
|
|||
ssm.Run() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've brought me a very good idea making environment variables as the interface between helmfile and kv-backend like ssm. Good job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can merge this as an experimental feature and start implementing more like this once you wrap behind a feature flag. Perhaps something like if _, exists := os.LookupEnv("SSM_PATH"); exists { ssm.Run() }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this would be nice to have the ssm.Run
not triggerd everytime helmfile is launched and you don't care about ssm at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But also, I don’t mind just working on that more flexible solution proposed above. Like, we can keep this PR open (but on hold) just in case I hit a snag with that code implementation.
I really just wanted to throw something together here to start the conversation so we could structure something awesome together.
@sgandon @mumoshu @osterman Here is a newer proposal I'm going to start working on, after some discussions with @lanmalkieri. This incorporates some of their requests to handle multiple paths. (This assumes they're all from the same AWS account. Future iterations could specify an assume role or something? But that's down the road for SSM.) ssm:
- region: us-west-2
prefix: /global/path/to/ssm
name: global
- region: us-east-1
prefix: /us-east-1/path/to/ssm
name: east1
repositories:
- name: "my-repo"
url: "s3://my-private-helm-chart-repo"
releases:
- name: hello-world
chart: my-repo/hello-world
values:
- hello-world:
url: http://www.fake-url.com
postgres_password: {{ ssm "global:postgres_password" }}
redis_password: {{ ssm "east1:redis_password" }} |
Also, just throwing this out there... Gomplate supports lots of data sources. For an interface, it could be helpful to reference. https://github.com/hairyhenderson/gomplate/blob/master/docs/content/datasources.md related |
@osterman the only reason I'm not crazy about this solution: #392 (comment) is because the But yeah, I like that datasources:
ssmGlobal: aws+smp:///global/path/to/ssm
ssmEast1: aws+smp:///us-east-1/path/to/ssm
myvault: vault://vault.example.com:8200///secret-engine/my-application
repositories:
- name: "my-repo"
url: "s3://my-private-helm-chart-repo"
releases:
- name: hello-world
chart: my-repo/hello-world
values:
- hello-world:
url: http://www.fake-url.com
postgres_password: {{ (ds "ssmGlobal" "postgres_password").Value }}
redis_password: {{ (ds "ssmEast1" "redis_password").Value }}
es_password: {{ (ds "myvault" "es_password").Value }} Somethingggg like that.. would have to play around with it |
@osterman Gomplate might not be flexible enough in the long run for SSM. I might run with it for now though. I can imagine cases where we're pulling SSM values from different accounts and/or regions, and I'm not seeing anything in Gomplate to specify that: https://github.com/hairyhenderson/gomplate/blob/master/docs/content/datasources.md#using-awssmp-datasources |
@osterman Doing this with native gomplate syntax in the helmfile.yml feels weird: es_password: {{ gomplate `ds=aws+smp:///dev/us-west-2/ssm` `{{ (ds "ds" "es_password").Value }}` }} with code: cfg := &gomplate.Config{
DataSources: []string{ds},
Input: key,
OutputFiles: []string{tmpfile.Name()},
}
if err = gomplate.RunTemplates(cfg); err != nil {
return
} I have it working with the code above. But to make it more convenient for helmfile users.. I'd want to to create a translation layer so it's really simple. But at that point, then there's no real benefit to the end user if I use gomplate internally or not. So I'm actually going to look back at this syntax to make it easier for end users: ssm:
- region: us-west-2
prefix: /global/path/to/ssm
name: global
- region: us-east-1
prefix: /us-east-1/path/to/ssm
name: east1
releases:
- name: hello-world
chart: stable/hello-world
values:
- hello-world:
url: http://www.fake-url.com
postgres_password: {{ ssm "global:postgres_password" }}
redis_password: {{ ssm "east1:redis_password" }} |
Also, gomplate is weird to try and integrate into your code base (unless I'm totally just looking at it the wrong way). The repo doesn't expose that many public functions from their packages: https://godoc.org/github.com/hairyhenderson/gomplate So I had to do things like write to a temporary file for each variable, instead of keeping them in memory. (gomplate seems like it was designed for just CLI use only? I'm open to learning more if anyone else has a different opinion.) |
Closing in favor of: #573 |
I don't know about gomplate but I think this is really promizing. Thanks a lot for having a detailed look at it. You are pointing out many limitation of the library but do you think we could contribute to it to make more easily "integratable" ? cause it is nice promise to avoid duplicating some code already working and test that provide the feature we want. |
There's been some interest for
helmfile
integration with SSM. Here is an example of what it can look like.For our current workflows, we have Bash scripts that export Env Vars via
aws-env
then runhelmfile
. It would be way cleaner to have this integrated