Add AWS SSM Parameter support #248

tyrken · 2018-01-30T10:07:21Z

Early version - still WIP, not sure whether func or datasource is best

tyrken · 2018-01-30T10:17:09Z

data/datasource.go

 	"github.com/blang/vfs"
+	gaws "github.com/hairyhenderson/gomplate/aws"


Obviously, not going to re-use code in your 'aws' package if we decide on the datasource path - just borrowing for now while playing with both

tyrken · 2018-01-31T14:49:56Z

OK, some questions/statements for you to comment on in case I'm wrong-headed.

aws Function or data source? Agreeing it might fit more under data source (though the config is a bit more complex), so continuing to expand tests for that and drop the aws Function.
Shall I make the data source implementation (assuming going that way) obey the AWS_TIMEOUT env var?
Will check for an AWS_REGION or AWS_DEFAULT_REGION env var, but if none use the instances' current region like aws.EC2Tag does.
Currently it makes single requests of the AWS API as asked via the framework one-at-a-time, whereas it's more efficient to use the GetParameters (note 's' at end) to request multiple at once. I'm assuming you have no plans to batch up all requests for a given data source & provide them in one hit - certainly not planning to propose a PR to do so myself.
AWS has two functions & permissions to get params, depending on whether getting a single value (GetParameter) or many (GetParameters). Currently using the former owing to the pattern of usage noted in prior question, but suspect users might enable one permission thinking it's using that & not the other, and so getting confused. Could defensively try both if getting a permission failure but that's a bit yucky - what do you think?

Hopefully dropping the aws function side of the code will clean up the codeclimate issue.

hairyhenderson · 2018-02-01T00:33:50Z

aws Function or data source?

Let's start with datasource for this PR. If a function is useful, it can come later.

Shall I make the data source implementation (assuming going that way) obey the AWS_TIMEOUT env var?

Yes please!

Will check for an AWS_REGION or AWS_DEFAULT_REGION env var, but if none use the instances' current region like aws.EC2Tag does.

That sounds reasonable. It's probably useful to make sure there's sensible error messages if no region is specified and gomplate is running outside of AWS.

Currently it makes single requests of the AWS API as asked via the framework one-at-a-time, whereas it's more efficient to use the GetParameters (note 's' at end) to request multiple at once. I'm assuming you have no plans to batch up all requests for a given data source & provide them in one hit - certainly not planning to propose a PR to do so myself.

It may actually be more "natural" to use GetParametersByPath and take the path from the URL given. So if there's 4 parameters like this:

abc: foo
/a/b: bar
/a/c: baz
/b/c: qux

Then it'd be handy to have access to all 4 params, or maybe just the params under /a, from the datasource. For example:

$ gomplate -d params=ssm+params:/// -i '{{ range (ds "params") }}{{ .Value }}, {{end}}'
foo, bar, baz, qux
$ gomplate -d params=ssm+params:///a -i '{{ range (ds "params") }}{{ .Value }}, {{end}}'
bar, baz

Does that make sense?

AWS has two functions & permissions to get params, depending on whether getting a single value (GetParameter) or many (GetParameters). Currently using the former owing to the pattern of usage noted in prior question, but suspect users might enable one permission thinking it's using that & not the other, and so getting confused. Could defensively try both if getting a permission failure but that's a bit yucky - what do you think?

I think if we document any permissions that are required, that's probably enough.

hairyhenderson · 2018-02-01T00:49:42Z

data/datasource.go

@@ -53,6 +58,7 @@ func init() {
 	addSourceReader("consul+http", readConsul)
 	addSourceReader("consul+https", readConsul)
 	addSourceReader("boltdb", readBoltDB)
+	addSourceReader("aws+ssm+param", readAWSSSMParam)


This schema feels a bit long, though I'm not sure I can can think much better - perhaps aws+smp (smp for Systems Manager Parameter - which is what they're called in the docs)?

tyrken · 2018-02-01T22:37:14Z

Agree with most of the above, but I had a play with GetParamtersByPath (see below) & it's not so good as you can't get a single result out of it (my common use case for passwords & similar). Hence whilst getting a list of params is useful, I think we need both forms.

If you see the most recent code in this PR, I've used a second argument that is == "recursive" to turn on recursion (which means scan all child 'folders' also rather than just the immediate 'folder' of params). Theoretically I could say no 2nd arg (or query param?) -> GetParameter (which would return a naked value & throw an Fatal error if the parameter couldn't be found in AWS), and a 2nd arg of "recursive" or "one-level" to do GetParametersByPath in recursive/non-recursive modes.

@HOLB ➜  gomplate rbenv:(2.3.3) git:(aws-ssm-param) ✗ aws ssm get-parameters-by-path --path /foo --recursive
{
    "Parameters": [
        {
            "Name": "/foo/second/p1",
            "Type": "String",
            "Value": "aaa",
            "Version": 1
        },
        {
            "Name": "/foo/second/p2",
            "Type": "String",
            "Value": "yyy",
            "Version": 1
        }
    ]
}
@HOLB ➜  gomplate rbenv:(2.3.3) git:(aws-ssm-param) ✗ aws ssm get-parameters-by-path --path /foo       
{
    "Parameters": []
}
@HOLB ➜  gomplate rbenv:(2.3.3) git:(aws-ssm-param) ✗ aws ssm get-parameters-by-path --path /foo/second
{
    "Parameters": [
        {
            "Name": "/foo/second/p1",
            "Type": "String",
            "Value": "aaa",
            "Version": 1
        },
        {
            "Name": "/foo/second/p2",
            "Type": "String",
            "Value": "yyy",
            "Version": 1
        }
    ]
}
@HOLB ➜  gomplate rbenv:(2.3.3) git:(aws-ssm-param) ✗ aws ssm get-parameters-by-path --path /foo/second/p1
{
    "Parameters": []
}

hairyhenderson · 2018-02-02T01:09:02Z

@tyrken 🤔 I see... Typical AWS API in my experience - seems straightforward at first, but the more you dig the muddier it gets 😉

I'm not a fan of a query parameter to control whether it's recursive or not.

I suppose we could simplify and treat it like the vault:// datasource - i.e:

$ gomplate -d v=vault:///path/to/secret -i '{{ (ds "v").value }}'
supersecret
$ gomplate -d v=vault:///path -i '{{ (ds "v" "to/secret").value }}'
supersecret

Now Vault only has a single API to read a secret, so a recursive get isn't an option there. In the AWS SMP case it'd be less efficient than it could be, but simply going back to GetParameter would make the UX and the code easier to deal with.

As a possible future enhancement, and taking a hint from #229, we could add some sort of ability to call the DescribeParameters with a filter to get a list of key names in the given path (or all key names).

Sorry about the back-and-forth - and thank you for sticking with it - I think copying the vault:// approach will get us there - with GetParameter.

Make sense?

hairyhenderson · 2018-02-02T01:10:17Z

data/datasource.go

+	p := source.URL.Path
+	recursive := false
+	if len(args) >= 1 {
+		// TODO: Note in docs that path.Join semantics are used (where are docs?)


Here: https://github.com/hairyhenderson/gomplate/blob/master/docs/content/functions/data.md 😉

tyrken · 2018-02-02T08:45:31Z

Thanks @hairyhenderson - In #217 @tallpauley suggests using a 3rd arg to trigger "list" behaviour as I currently do with "recursive", but you say to try a query param.
(Mistakenly in previous posts I say 2nd arg - which it is for the func but it's actually 3rd for the user.)

...but just now you say you're not a fan of query params? I think query params are better than positional args - less magical (have to read the docs to understand what's happening), more declarative.

Path including merged 2nd arg like readVault	Method	Description
somepath	GetParameter	Simple case, maybe 3rd arg to be a default if absent (otherwise error)
somepath?mode=one-level	GetParametersByPath (non-recursive)	Get a single hierarchy level's params, no 3rd arg
somepath?mode=recursive	GetParametersByPath (recursive)	Get a whole hierarchy's params, no 3rd arg

Alternatively, as dealing with just three choices we could use different protocols (like consul:// vs consol+https://).

Protocol	Method	Description
aws+smp://	GetParameter	Simple case, maybe 3rd arg to be a default if absent (otherwise error)
aws+smpl://	GetParametersByPath (non-recursive)	Get a single hierarchy level's params, no 3rd arg
aws+smplr://	GetParametersByPath (recursive)	Get a whole hierarchy's params, no 3rd arg

Not used GetParametersByPath's ParameterFilters option yet, but I suspect some new query param could do it, with a little parsing work.

tyrken · 2018-02-02T09:18:46Z

Also wondering what stricture to use for the output - for GetParameterByPath calls it seems correct to return a list of Parameters, so you need to use obj.value to get the text and can access .type, .name or even .version if you want.

Less clear on GetParameter - think stick to a Parameter object for consistency, and if allowing a textual default to manufacture one, perhaps with .version set to -1 or an extra '.default' boolean set to true.

hairyhenderson · 2018-02-04T02:00:04Z

using a 3rd arg to trigger "list" behaviour as I currently do with "recursive", but you say to try a query param.
(Mistakenly in previous posts I say 2nd arg - which it is for the func but it's actually 3rd for the user.)

...but just now you say you're not a fan of query params? I think query params are better than positional args - less magical (have to read the docs to understand what's happening), more declarative.

In that particular case, the list query param suggestion was because Vault's API already supports that, and I was wondering if it'd work already through gomplate without modification (which, I think, didn't).

For Vault, we already use positional args to complete the path, and IMO the usefulness outweighs the magic, since you can re-use the same datasource alias to refer to multiple keys in the same scope.

As for using multiple schemas - I don't think it's necessary.

Also wondering what stricture to use for the output [...]
Less clear on GetParameter - think stick to a Parameter object for consistency,

I think just using the Parameter object is fine - that's similar to what we do with vault too.

and if allowing a textual default to manufacture one, perhaps with .version set to -1 or an extra '.default' boolean set to true.

Let's not allow a default just yet (at least, not in this PR). The datasource function currently doesn't accept a default in any case. There was some discussion around defaults in #213.

hairyhenderson · 2018-02-04T02:05:42Z

data/datasource.go

+
+	results := make([]*ssm.Parameter, 0, 50)
+	// var results []*ssm.Parameter
+	err := source.ASMPG.GetParametersByPathPages(input,


I know this is still a WIP and you're still trying different options, but I think for an initial cut I'd really like to see GetParameter be used instead.

I think this function shouldn't look too much different from the readVault function above (ln383)

tyrken · 2018-02-06T10:23:42Z

@hairyhenderson - OK, please review code & docs now. Sorry but I don't see how to reasonably simplify parseAWSSMPArgs - can we ignore that?

hairyhenderson · 2018-02-07T01:51:02Z

@tyrken yeah, don't worry about the codeclimate bit - I can come back later and see about clearing things up.

I think the non-recursive functionality looks overall OK, but I'd really rather not put the "recursive mode" functionality in yet. The UX doesn't seem right to me.

I suppose I wasn't very clear in #248 (comment) - but can you simplify this PR to only use the GetParameter call? That is, drop the "recursive mode" functionality and the third argument for now.

Let's start with just the simple case ("one-level") in this PR. I'm not closing the door on the recursive case being added later.

Again, thank you for your work and persistence on this - we're almost there! 🙂

tyrken · 2018-02-08T01:56:55Z

OK, will do. BTW, 'one-level' is the mode using GetParametersByPath with Recursive=false to get everything directly under a hierarchy, I'll be removing that as I think it's what you actually want. I'll only keep the no-mode (and no 3rd arg) plain GetParameters one which gets a single value or throws a log.Fatalf panic. Not sure if you want this style of error handling as opposed to just returning an error...

Remove the json/yaml array mimetypes also as not needed any more (assume yes)? Will squash current state down into one commit to leave behind the GetParametersByPath code in a commit I/others can come back to if needed.

* Includes single value and multi-value results - will drop latter

tyrken · 2018-02-08T17:48:26Z

Ok @hairyhenderson - please do final review.

hairyhenderson · 2018-02-08T23:57:34Z

I'll be removing that as I think it's what you actually want.

Yes - thanks for the correction 😉

hairyhenderson

Fantastic! Thank you for this @tyrken 🙂

tyrken · 2018-02-10T10:07:37Z

Thanks - now to get on & use it...

fhenri · 2018-11-14T10:06:45Z

Hi @tyrken @hairyhenderson
Thanks for making the aws ssm datasource, this is really great -
has there been any other thoughts in using the recursive mode ?
we have use-case where we have multiple keys and we would like to use as presented above

$ gomplate -d params=ssm+params:///a -i '{{ range (ds "params") }}{{ .Value }}, {{end}}'
bar, baz

I like the approach proposed by @tyrken with the Path including merged 2nd arg like readVault

Let me know if thats something we can help with -

Thanks,
Fred

hairyhenderson · 2018-11-14T14:39:11Z

@fhenri can you open a new issue about recursive support and link to this?

tyrken mentioned this pull request Jan 30, 2018

Enable reading from AWS SSM Parameter Store? #245

Closed

tyrken commented Jan 30, 2018

View reviewed changes

hairyhenderson changed the title ~~Add AWS SSM Parameter support~~ [WIP] Add AWS SSM Parameter support Jan 31, 2018

hairyhenderson added the enhancement label Jan 31, 2018

hairyhenderson reviewed Feb 1, 2018

View reviewed changes

tyrken force-pushed the aws-ssm-param branch from a5d26c6 to 7b796e4 Compare February 1, 2018 22:38

hairyhenderson reviewed Feb 2, 2018

View reviewed changes

hairyhenderson reviewed Feb 4, 2018

View reviewed changes

tyrken changed the title ~~[WIP] Add AWS SSM Parameter support~~ Add AWS SSM Parameter support Feb 6, 2018

Datasource for reading from AWS Systems Manager Parameters

0137821

* Includes single value and multi-value results - will drop latter

tyrken force-pushed the aws-ssm-param branch from 1706ab2 to 0137821 Compare February 8, 2018 17:23

Back to just the single-value GetParameter call

94fcc43

hairyhenderson approved these changes Feb 9, 2018

View reviewed changes

hairyhenderson merged commit 816d482 into hairyhenderson:master Feb 9, 2018

tyrken deleted the aws-ssm-param branch February 10, 2018 10:07

fhenri mentioned this pull request Nov 14, 2018

Support for aws ssm in recursive mode #427

Closed

hairyhenderson mentioned this pull request Apr 15, 2023

Add azure.Meta function to query Azure IMDS endpoint #1713

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AWS SSM Parameter support #248

Add AWS SSM Parameter support #248

tyrken commented Jan 30, 2018

tyrken Jan 30, 2018

tyrken commented Jan 31, 2018

hairyhenderson commented Feb 1, 2018

hairyhenderson Feb 1, 2018

tyrken commented Feb 1, 2018

hairyhenderson commented Feb 2, 2018

hairyhenderson Feb 2, 2018

tyrken commented Feb 2, 2018

tyrken commented Feb 2, 2018

hairyhenderson commented Feb 4, 2018

hairyhenderson Feb 4, 2018

tyrken commented Feb 6, 2018

hairyhenderson commented Feb 7, 2018

tyrken commented Feb 8, 2018

tyrken commented Feb 8, 2018

hairyhenderson commented Feb 8, 2018

hairyhenderson left a comment

tyrken commented Feb 10, 2018

fhenri commented Nov 14, 2018

hairyhenderson commented Nov 14, 2018

		"github.com/blang/vfs"
		gaws "github.com/hairyhenderson/gomplate/aws"

Add AWS SSM Parameter support #248

Add AWS SSM Parameter support #248

Conversation

tyrken commented Jan 30, 2018

tyrken Jan 30, 2018

Choose a reason for hiding this comment

tyrken commented Jan 31, 2018

hairyhenderson commented Feb 1, 2018

hairyhenderson Feb 1, 2018

Choose a reason for hiding this comment

tyrken commented Feb 1, 2018

hairyhenderson commented Feb 2, 2018

hairyhenderson Feb 2, 2018

Choose a reason for hiding this comment

tyrken commented Feb 2, 2018

tyrken commented Feb 2, 2018

hairyhenderson commented Feb 4, 2018

hairyhenderson Feb 4, 2018

Choose a reason for hiding this comment

tyrken commented Feb 6, 2018

hairyhenderson commented Feb 7, 2018

tyrken commented Feb 8, 2018

tyrken commented Feb 8, 2018

hairyhenderson commented Feb 8, 2018

hairyhenderson left a comment

Choose a reason for hiding this comment

tyrken commented Feb 10, 2018

fhenri commented Nov 14, 2018

hairyhenderson commented Nov 14, 2018