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

SSM StringParameter should query multiple paths #7259

Closed
2 tasks
brettswift opened this issue Apr 8, 2020 · 8 comments
Closed
2 tasks

SSM StringParameter should query multiple paths #7259

brettswift opened this issue Apr 8, 2020 · 8 comments
Assignees
Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@brettswift
Copy link

The ability to look up multiple SSM paths to satisfy a construct.

Use Case

In our applications we have a ../common/.. pathing and ../<namespace>/.. pathing in ssm values, where <namespace> comes from a cdk context variable -c namespace=bswift.

The pattern is pretty common, grab all the defaults for that account and if I want something special on my deploy - then just set that one value.

This doesn't work with CDK as ValidationErrors are thrown.

Proposed Solution

const value = ssm.StringParameter.valueFromLookup(this, 
     [
        '/service/common/value', 
        '/service/bswift/value'
    ]
  )

Or something to that effect.

Currently neither valueFromLookup or fromStringParameterName allow me to do my own null checking so I'm not sure if there's a workaround internal to CDK. The only thing I can think of is to do it outside and use a context parameter. It would be nice if I could do this within the Construct.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@brettswift brettswift added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 8, 2020
@SomayaB SomayaB added the @aws-cdk/aws-ssm Related to AWS Systems Manager label Apr 9, 2020
@MrArnoldPalmer MrArnoldPalmer added effort/medium Medium work item – several days of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 10, 2020
@MrArnoldPalmer
Copy link
Contributor

@brettswift to help understand the use case better. You want the construct to accept a list of names and it looks for a parameter using each name in order until it finds one that is defined?

Do you use this pattern in cloudformation or other IAC tools currently? Would love to see some examples if so.

@brettswift
Copy link
Author

Hey,
Your rephrase of my question is correct, however there might be a lower effort higher value solution.

We do use this lookup strategy in our service API's so far. I used to use puppet, where there was a hierarchy tool called hiera. It's way too in depth to go into here, but pretty powerful. Something similar to our puppet hierarchies was default behaviour based on linux vs windows. In AWS that might be account level defaults for ASG size. In our dev account that would be 1. But if I want to do some testing on rolling updates, I might update my instance config that way. It's a different mechanism than context parameters or hard coding it. With context parameters it's hard to control all of our services at a global level - we'd have to go change defaults in every service.

I could write a library to do this but think it'd be nice to have in CDK. The tough part is we'd need to use the SDK, which is async. So CDK helps us synchronize calls to that by giving us a .stringValue option. (Actually I'm not sure.. maybe CDK uses the API? I haven't dug that deep yet).

Anyways what would be the first level of implementation to bring value, might be just allowing a lookup and return undefined instead of throwing an error if the string isn't there. That way I could come up with my own hierarchical lookup code.

For the software, Microsoft has a config library for dotnet core that does this automatically, and has an SSM resolver.

We leverage this and would like the same for infrastructure.

Does that help? We're off until Tuesday in Canada but I might be able to dig something more concrete up.

The short version is the errors being thrown are getting in my way of working around this as exceptions aren't propagated to my construct it seems.. so I can't swallow them.

@MrArnoldPalmer
Copy link
Contributor

This does make sense. I have been thinking about higher level constructs for SSM and this use case feels like it could fit into that. Custom resources might have to play a role here to get around some of the peculiarities of SSM support in cloudformation.

I'd argue this probably wouldn't go into @aws-cdk/aws-ssm and should probably go into something like @aws-cdk/aws-ssm-patterns, similar to the ecs-patterns library. The L2 constructs are intentionally barebones/generic so they are appropriate to use at the base of these higher level constructs.

An RFC probably should be made detailing the design of this library and the team could discuss whether it fits into the cdk codebase or should be a separate library.

@brettswift
Copy link
Author

I agree it makes sense to be out of the main ssm lib.

When logic and configuration come together, it can be confusing if this is deployed by someone else as far as answering the question: What values would be resolved if I were to deploy this?

Transparency here is important - and determining how this hierarchy is flattened / resolved before deploy time would be helpful.

Be that a cdk synth or cdk <resolve-ssm-string-values-command>.

A custom resource might make this a bit more difficult.

@MrArnoldPalmer
Copy link
Contributor

I agree, though resolving values with the sdk locally could complicate the synth/deploy lifecycle.

Resolving these values at synth time and outputting them to the template could potentially expose sensitive values that we shouldn't be putting in the template. If we just check that these resources exist and output a reference to them during synth, then if the user is synthesizing beforehand and using the cdk.out artifact to deploy with, the parameters could have been deleted or otherwise changed in the meantime.

In my mind a custom resource could perform more complex logic like checking for the existence of parameters and moving through the hierarchy, as well as creating default values and such. It can this all at deploy time to make value resolution as lazy as possible.

I think there are use cases for being able to access configuration values at various points in the application lifecycle. We should detail these and figure out if we can design a module to handle these with relative transparency. It also feels like we could create nice interface for managing the creation and access of these parameters with support for hierarchical namespacing etc.

I guess I'm saying this feature could fit into a more holistic evaluation of high level constructs for configuration in CDK applications leveraging SSM, secrets-manager, and cfn parameters/outputs. Transparency is definitely something to consider here and it might be useful to give users access to the building blocks used so they can easily compose their own resolution logic based on whatever conditions they need (null values, pattern matching on name/value etc). This likely can be accomplished using Cfn intrinsic functions.

I think some experimentation in a separate module is warranted here.

@brettswift
Copy link
Author

brettswift commented Apr 11, 2020 via email

@MrArnoldPalmer
Copy link
Contributor

@brettswift feel free to go as deep as you want. An RFC or new separate module are both good places to start. Module probably would be my instinct since it still feels like we need to flesh out the patterns a bit before we can cover all this in an RFC. Reach out whenever if you want feedback or to collaborate.

@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Apr 12, 2021
mergify bot pushed a commit that referenced this issue Sep 13, 2024
…arameter (#31415)

Updates StringParameter.valueFromLookup with an optional "defaultValue" When specified this value will be used:
 - in place of the standard dummyValue
 - to signal that an Error should not be raised during synthesis if the parameter is not found in the account.

Test are updated to prove that this works

### Issue # (if applicable)

Resolves #7051 

There are some closed issues which also benefit from this change:
- #22064 
- #7259

### Reason for this change

We have a library which has a fixed set of SSM parameters on which it depends.  The values from those parameters are made available as attributes of a custom Stack.  We have many users in many different AWS accounts, and not all of the parameters are guaranteed to exist.  This is okay.  In general, teams would simply not use those values and be happy with that outcome.  Unfortunately, CDK crashes when you look up an SSM parameter that does not exist in the account.  This is unacceptable.

### Description of changes

To address the issue described above, I implemented an optional parameter on the `valueFromLookup` method: `defaultValue`.  The idea is that if this value is specified, and we fail to look up a parameter in the account, we will return this value and suppress the Error that is currently raised when a parameter is not found.

To implement that functionality, I added a field to the `GetContextValueOptions` interface which is used to flag that we're not going to raise the error.  Then, in `valueFromLookup`, I set that flag to `true` if the `dummyValue` is specified.  `valueFromLookup` then calls `ContextProvider.getValue` passing along those values.

`ContextProvider.getValue` is modified so that when it calls `stack.reportMissingContextKey` it passes a modified set of `props` which include the `defaultValue` and the `ignoreErrorOnMissingContext` flag.

These finally land in the `aws-cdk` context provider for `ssm-parameter`.  That code has been updated so that if the value is not found in SSM, and we're told to suppress the error, then we'll simply return the `defaultValue` that was passed in. 

### Description of how you validated changes


I added a unit tests which covers when the default value is set.  I also updated the original unit test as the `props` now contain some additional field.

I added an integration test which calls `valueFromLookup` with a `defaultValue` set and then confirms that no exception is raised and that `valueFromLookup` returned the `defaultValue`

**NOTE**
I considered that the changes made _might_ need to be a part of the `cloud-assembly-schema` but chose to work around that for now.  I'm open to incorporating them there if that's a more correct path.

**NOTE 2**
I'm unsure about how to update API documentation for this change.  This does alter the public API for `valueFromLookup` and the function doesn't appear to have a proper `TSDoc` header on it.  Please let me know if there's a proper way for me to update the documentation.


### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ssm Related to AWS Systems Manager closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

3 participants