jcberquist / aws-cfml

Lucee/ColdFusion library for interacting with AWS API's
MIT License
72 stars 53 forks source link

Add ability to pass through host to SSM #85

Open burncitiesburn opened 1 year ago

burncitiesburn commented 1 year ago

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 commented 1 year ago

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?

burncitiesburn commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.