Middy: Enhance ssm middleware to support getParametersByPath method

[hollg]

The ssm middleware currently fetches paramters from the ssm store using the getParameters method from aws-sdk, which takes an array of names of the parameters you want to access. aws-sdk also features a getParametersByPath method which in my view is much more useful if you're dealing with parameters in the ssm store in any serious quantities.

The way it works, basically, is that if you have two parameters in the store, '/dev/myApi/connString' and '/dev/myApi/password' you make a request for the path '/dev/myApi/', both those parameters will be returned. You can also nest namespaces further, and return parameters recursively rather than just returning the parameters at the top level of the path you specify.

https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/SSM.html#getParametersByPath-property

On a fork of this repo I've an updated the ssm middleware to offer a choice between these two methods. Where currently the middleware takes a params argument, which is a map of property names to be added to process.env (or context) and the names of the ssm parameters whose values should go inside those properties, my version will also take a path argument, which is simply a string.

First, would there be any interest in a PR to bring this into Middy? I really believe that getParametersByPath is generally speaking the best option because:

1. it encourages a logical hierarchy to your ssm parameters
2. it allows you to specify a single string in your code rather than every name individually
3. down the line you can simply add a parameter with the same namespacing and you'll have access to it in your code, rather than having to add a new parameter and update the list of parameters that you access

Secondly, there are some complexities with this approach that don't exist for the current implementation of the ssm middleware:

With the current means of specifying the parameters to be accessed, you're able to specify a name for each one (as detailed above). But if you only provide a path and retrieve all parameters from it, there's no obviously good way to specify what you expect to get back/ what name it should be given on an individual basis (and if we implement a way of doing this, I think we lose the benefit of (3) above).

At the moment my solution to this is for the middleware to also take a getParamNameFromPath function as a configuration option, which tells it how to figure out what to call the property on process.env based on the provided path. The default option currently looks like this:

// returns full parameter name sans the path as specified, with slashes replaced with underscores
// e.g. if path is '/dev/myApi/', the parameter '/dev/myApi/connString/default' will be returned with the name 'conString_default'
// see: https://docs.aws.amazon.com/systems-manager/latest/userguide/sysman-paramstore-su-organize.html
function getParamNameFromPathDefault (path, name) {
  return name
    .split(path)
    .join(``) // replace path
    .split(`/`)
    .splice(1) // remove starting slash
    .join(`_`) // replace remaining slashes with underscores
}

... but that's just based on our current use case. I'd be very happy to take advice on the most reasonable default. I'm also sure mine can probably be refactored into one splice() call or something!

If you're interested in a PR for this then I'd be very glad to make it, and/or if there's any discussion to be had first about it then I'm happy to answer any questions about what I've done so far!

[hollg]

I've worked a little more on this and the version of the ssm middleware we're using now also supports multiple paths.

[lmammino]

This sounds like a very good idea and I'd be very happy to have support both options. The goal would be to figure out a nice and flexible configuration syntax so that people can use it naturally without much cognitive effort.

Trying to break the problem down into smaller problems I came up with the following questions:

1. Should the 2 options (by path prefix and by explicit list) be mutually exclusive or does it make sense a scenario where you can specify both and expect all the matching parameters to be fetched? For instance, you might want to fetch some global parameter and a set of specific parameters for the current lambda.
   At the moment I'd be happy to support both options without making them mutually exclusive.
2. How to assign local names to the fetched parameters? We currently allow the user to specify a hashmap of LOCAL_NAME -> PARAMETER_PATH couples, but we cannot take the same approach to path based parameters.
   In this case, I'd be happy to introduce a breaking change and remove/deprecate the params hashmap options in favor of something called keys that will contain an array of keys. At this point, we will need a function that is able to compute the local name for a parameter and we can definitely find a sensible default, but I'll leave this to the next point.
3. How to automatically name local variables?
   Here I am happy with the solution @garyhollandxyz proposed, except that I wouldn't really remove the prefix so that we can easily use this function for both path prefix based parameters and explicit key-based parameters.

So, just to recap, my idea would look like the following:

Supported options:

• keys: an array of keys of parameters to fetch
• prefix: a single string indicating the path prefix of the keys to fetch
• getParamLocalName(key): function that calculates the local name of a parameter based on the full path of the parameter on SSM. By default it removes the trailing /, replaces all the other / with _ and uppercases the string (we might also need to sanitize the resulting var to make sure it can be a valid JS object property name).

Of course, I'd keep the current awsSdkOptions, setToContext and cache options, and deprecate the params option.

Pseudo-implementation

if (prefix) {
  paramsByPrefix = fetchByPrefix(prefix)
  for every key in paramsByPrefix:
    targetObject[getParamLocalName(key)] = paramsByPrefix[key]
}

if ( keys ) {
   paramsByKey = fetchByKeys(keys)
   for every key in paramsByKey:
     targetObject[getParamLocalName(key)] = paramsByKey[key]
}

Do you think this approach would be actually simpler and more intuitive to use?

I'd be curious to have @vladgolubev opinion since he implemented the original middleware. Also, having @dkatavic opinion would be super.

Once we formalize an API I'd be very happy to have a PR to support parameters by prefix.

[hollg]

Thanks for the detailed response!

Re your questions:

1. Should the 2 options (by path prefix and by explicit list) be mutually exclusive or does it make sense a scenario where you can specify both and expect all the matching parameters to be fetched?

We've been working under the assumption that fetching parameters by path and fetching names _would_ be mutually exclusive (I think the reason for this really boils down to the fact that for our use they could be, and it seemed to be simpler to implement).

The current implementation (changed somewhat since my original post -- many thanks to @tommy5dollar here) actually takes a single params object and a usePaths boolean flag. If usePaths is false, it uses names and works exactly as it always has done. If usePaths is true, then the params object is a map of PREFIX -> PARAMETER_PATH, and the name of the property that gets set on process.env will be the name as defined by the getParamLocalName(key) function, but prefixed with PREFIX_.

This goes some way to mitigating a possible issue where you have two parameters, /api1/access_token and /api2/access_token, and end up pulling them both in as access_token, with one overwriting the other. (This also begins to touch on the next point.)

As for whether they _should_ be mutually exclusive if fetching by path is supported by the library itself ... I don't know. Very happy to wait for other people to weigh in on this one.

1. How to assign local names to the fetched parameters? We currently allow the user to specify a hashmap of LOCAL_NAME -> PARAMETER_PATH couples, but we cannot take the same approach to path based parameters.

So, as described above, in a limited sense we _can_ specify a name for the parameters that we fetch by path. However, to do so we do seem to be making certain assumptions about how users will configure their SSM parameter names, and how they will (or should?) access them in their code.

1. How to automatically name local variables? Here I am happy with the solution @garyhollandxyz proposed, except that I wouldn't really remove the prefix so that we can easily use this function for both path prefix based parameters and explicit key-based parameters.

I agree in so far as if we are going to support both named parameters and paths, then reusing that function if possible seems like a no-brainer, but I do think there is a strong case for removing what you refer to here as the prefix: the AWS docs suggest starting paths with (something like) /dev/ (or whatever stage), in which case we definitely want to get rid of that first part, so that we only refer to one version of the parameter in our code, but get a different version in each environment.

keys:

• an array of keys of parameters to fetch
• prefix: a single string indicating the path prefix of the keys to fetch

This rules out multiple paths, right? If anything (again this is based just on my use of SSM and what has been useful for our team here), I think supporting multiple paths is more important than supporting both paths and names. E.g. we have multiple Lambda projects, some with parameters specific to them, and we also have a bunch of parameters that are not project-specific. If we support multiple paths then we can pull in some /common path _and_ the /project path.

I'll definitely make a PR once we've sorted the API. Looking forward to hearing what everyone else thinks!

[dkatavic]

After reading this thread, I definitely feel like getParametersByPath is the way to go. @garyhollandxyz have made some good points there. I am just not sure about the way of implementing it, because it may cause additional cognitive load for users

My way of solving this would be:
1.) Fetch the parameters by path
2.) Map them using the dictionary (dictionary per path)
3.) Apply dictionary as they were before

This could easily be backward compatible. Middleware would support passing an array of path&mappers for a parameter.

From my perspective, I should know what to expect in SSM during develop time, and I prefer to whitelist passed parameters because it is gonna make my life easier during the debugging phase.

I understand that you want flexibility and have a mapper function, so I am fine by it because a dictionary is just a limited implementation of mapper function. If I want I can still define a function that is gonna use the dictionary

Would it make sense to have an API to something like this?

handler.use(ssm({
  pathConfig: [
    {
        path: '/dev/',
        mapper: (path) => path.split(`/`).splice(1).join(`_`)
    },
         ...
  ]
}))

I am definitely not sold on the naming of the parameters :D . All the other parameters (like cache) could be the same. I am just throwing ideas, I am not sure does this makes sense

[hollg]

@dkatavic an array of objects to map paths to naming functions like that definitely makes sense to me. I think in a sense it's more intuitive than what we currently have with one object mapping prefixes to paths.

The only negative thing I can think of here, and it is probably specific to the fact that we're using the Serverless framework (though I'm sure similar situations could arise without it) is that with our current implementation, we're adding the SSM Parameter Store paths as environment variables via Serverless config files, e.g. ssm_path_auth0: /path/to/parameters and ssm_path_: /path/to/other/parameters, and we have a function that turns these strings into our map object, the upshot being that if the environment variable name has anything after ssm_path_, that something becomes the prefix for parameters retrieved from that path. It feels neat to be able to specify _everything_ in the config file (which we obviously couldn't (or at least shouldn't try to!) do with the mapper function).

However, I definitely also see the benefits of providing a mapper for each path, including that the mapper could in fact supersede a prefix. Really I'm just playing devil's advocate! In fact, as I've written this comment I've become more and more fond of your suggested option, but perhaps it's useful to see how we're currently using this.