-
Notifications
You must be signed in to change notification settings - Fork 55
Add missing functions for SSM Parameter Store #91
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
base: master
Are you sure you want to change the base?
Conversation
| * @Recursive If true, retrieves parameters from the entire hierarchy under the specified path. | ||
| * @withDecryption If true, retrieves decrypted values for secure string parameters. | ||
| */ | ||
| public array function getAllParametersByPath( |
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 am not sure about adding this method. I don't think there are any other places in the code base where there is a method that calls the API multiple times (potentially) before returning. I know other libraries do add iterators, so maybe there is a place for this, but it is not something that has been done before. I would have expected the iteration to be implemented in your own calling code.
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.
Its your decision. Remove or not? I typically create a helper function to handle common repetitive tasks, especially when reusing the same library across multiple projects. SSM's 10 parameter limit is quite low by pagination standards.
My use case is that I store application configurations by path in the parameter store and always end up with more than 10 i need to retrieve.
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.
Yeah, I understand the low parameter limit is annoying, but for now I'd like to leave helper functions like this, that don't actually call the API themselves, out of the library.
| } | ||
| if ( !isNull( arguments.NextToken ) ) { | ||
| payload[ 'NextToken' ] = arguments.NextToken; | ||
| } |
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.
Can the four null checks above be combined into a for loop? e.g.:
for (
var key in [
'Recursive',
'WithDecryption',
'MaxResults',
'NextToken'
]
) {
if ( structKeyExists( arguments, key ) ) {
payload[ key ] = arguments[ key ];
}
}
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.
Sure. I just code in the same style and patterns I see in the library. Line 168 of personalize.cfc or Line 235 in connect.cfc for instance. Can i blame Eric Peterson? :)
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.
Fair point 😁 I haven't been consistent about asking for this. But I do prefer the style above.
| var requestSettings = api.resolveRequestSettings( argumentCollection = arguments ); | ||
| var payload = { | ||
| 'Name': arguments.Name, | ||
| 'Value': (len(arguments.Value) ? arguments.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.
What is the idea here with the overwrite of the value argument when it is an empty string?
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.
SSM doesn't allow empty values. They must have a length greater than or equal to 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.
I'd rather leave this choice up to the calling code - whatever that passes in as the value should be passed on to API. I don't want to change the value argument like this.
Added missing functions for SSM parameter store putParameter, deleteParameter, deleteParameters and getParametersByPath. Additionally added the helper function getAllParametersByPath which retrieves all parameters by path by calling getParametersByPath with NextToken until no results are left.