Skip to content

Conversation

@burncitiesburn
Copy link

This allows passing through a custom host through to ssm to allow testing with localstack and similar.

This matches how the S3.cfc works.

This also fixes an issue when the SSM service doesn't return json back from the API

@jcberquist
Copy link
Owner

Thanks for this. I do have a couple of questions.

I see you added the ability to specify the host in per request settings, but it doesn't look like that is then used anywhere?

What was the reason for specifying encodeUrl=true in the api.call() for ssm.cfc, were calls erroring without that?

Remove passing true for encodeURL.
Testing shows its not needed.
@burncitiesburn
Copy link
Author

Hey!

I double checked my testing, and it appears that encodeUrl=true isn't required. I must have had a case when it didn't work, or I was doing something odd.
I've updated my pull to remove that field.

The host in the settings is used when getHost is called as part of apiCall.

I'm just updating the getHost function to match the one here.
https://github.com/jcberquist/aws-cfml/blob/master/services/s3.cfc#L966

It's so that I can enable testing against localstack such as

<cfset constructorArgs = {}>
<cfset constructorArgs["ssm"] = { host:"10.0.2.0:4566", region:"ap-southeast-2">
<cfset aws = aws(constructorArgs=constructorArgs)>

<cfset stResponse = aws.ssm.getParameter(Name="/test/test", WithDecryption=true)>

Previously passing "host" in the constructorArgs would be ignored by the ssm service.

@jcberquist
Copy link
Owner

Thanks for the follow up. With regard to the settings for the host, I am referring specifically to the change made to com/api.cfc where you added host to the per request settings. The other change you made pertains to settings that are applied on component initialization, which will then be used for every method call/api request, and will not vary per request. Does that make sense?

@burncitiesburn
Copy link
Author

I understand, apologies. I'll double check why I updated api.cfc with host.
I see what you mean - host is passed from ssm -> api.call. Adding host to api settings doesn't make sense / doesn't pertain to that change.

I'll get back to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants