tls config requirement

[smithc14]

I was using version 1.0.1 to retrieve secrets successfully but then when I built a new image with 1.1.0 it started to fail with a timeout trying to reach the secretsmanager endpoint over http:

failed to connect to 'http://secretsmanager.us-west-2.amazonaws.com:80'

I had not changed any configuration at all. I went looking in your unit tests and saw the extra change:

config.tls = true

If this is required now you might want to update your public examples to reflect this requirement.

Thanks for the great library.

[windmgc]

@StarlightIbuki @Tieske any idea whether we should make config.tls default to be true? (I think for me the answer is yes)

[StarlightIbuki]

@StarlightIbuki @Tieske any idea whether we should make config.tls default to be true? (I think for me the answer is yes)

It should be inferred that the client is trying to access non-tls from scheme and port. The tls setting seems irrelevant.

I think this should be a bug introduced in 1.1.0, which is fixed by 1.1.1: #39

[smithc14]

I have tested with 1.1.1 and it still requires:
config.tls = true
Without specifying the above line it will return the error:
SecretsManager:getSecretValue() failed to connect to 'http://secretsmanager.us-west-2.amazonaws.com:80': timeout

[StarlightIbuki]

I have tested with 1.1.1 and it still requires: config.tls = true Without specifying the above line it will return the error: SecretsManager:getSecretValue() failed to connect to 'http://secretsmanager.us-west-2.amazonaws.com:80': timeout

Not sure about the reason. Is it sending tls requests to port 80? (if that is the case it should be explicitly set to true, and we need to notify people about this breaking change). I will investigate it but I don't have enough bandwidth this week.

[Tieske]

Had a look at the tests;

lua-resty-aws/spec/04-services/01-secret_manager.lua

Lines 14 to 20 in ae4a939

	local AWS = require("resty.aws")
	local AWS_global_config = require("resty.aws.config").global
	local config = AWS_global_config
	config.tls = true
	local aws = AWS(config)

This is wrong, since it mixes 2 concepts; CLI config, and API config.

The module resty.aws.config will collect the CLI configuration, by reading environment variables, configuration files, profiles, and what not. This is separated out because it MUST be read on start in an Nginx phase where the variables are still accessible. As such it is also used by the credential classes since many of them need access to this information.

The API configuration is what is used when initializing the AWS object and the individual services. This is however NOT the same object as the CLI configuration.

So when constructing the AWS instance, or services, we should be copying the info from the global object as we need it (many properties will also have different names).

So this would be more appropriate:

local AWS = require("resty.aws") 
local AWS_cli_config = require("resty.aws.config").global 
  
  
local aws = AWS {
    region = AWS_cli_config.region,  -- copy values over instead of using the entire object
    tls = true,
}

Looks like we need to update the docs, since this distinction is not clear from the docs (I might have made the same mistake in some of the examples).

[Tieske]

@smithc14 a guess, but this line;

lua-resty-aws/src/resty/aws/request/build.lua

Line 84 in ffa4345

config.tls = scheme == "https"

Assumes the scheme is always set.

If you change it to:

config.tls = scheme ~= "http" 

(inverting the logic, and default to tls == true in absence of a scheme)
does that work?

[StarlightIbuki]

This is a bug that fails to detect the TLS setting when no scheme is provided. It now defaults the scheme to HTTPS, and the TLS setting thus defaults to true if no scheme is provided.