Environment variables set to empty cause silent failing of auth

[pgrzesik]

Hello,

recently, we stumbled upon an interesting 'bug' that took us some time to resolve - we had set AWS_ACCESS_KEY_ID env variable set to empty, which resulted of having it in os.environ as empty string. If it's there as an empty string, it causes config resolver to use EnvProvider - even if it's clearly invalid. While i realize that this issue is caused by us, I would like to propose a small improvement to EnvProvider - which will not only check if env variables are set - but also if they're valid and log/error out when that's not the case, allowing users for quicker debugging.

If I can help with the implementation, please let me know, I would gladly take a stab at it.

Regards,
Piotr

[joguSD]

Hm, I think this sounds reasonable but it's also arguably a breaking change / not a forwards compatible change. It might be difficult to actually define what a 'valid' credential is in all contexts. For example, the actual value set may not matter in a testing context (if the responses are stubbed, etc.). Let me run this by some of the other members and see what they think.

[pgrzesik]

Hello @joguSD, thanks for checking it my issue. Yeah, I can imagine how that can be a problem, so maybe the solution would be to just check if the actual values in os.environ are truthy ? that would prevent issues with empty strings

[jeffcasavant]

I hit this as well. We deploy applications to Kubernetes using Terraform. For testing, we deploy on Minikube locally, which requires us to set the auth variables in Terraform. HCL still doesn't have conditionals, so they're always set - even when we use the same module to deploy to a k8s cluster in AWS which is intending to use IAM. As a result auth fails in the real deployment (since the env vars are set but are empty).

I have a modification written & deployed right now that solves this. It does so conservatively by special-casing empty string values rather than checking truthiness. It does this for AWS_SECRET_ACCESS_KEY, AWS_ACCESS_KEY_ID, AWS_SECURITY_TOKEN, and AWS_ACCESS_TOKEN. I've got some tests in place for the empty string case.

I'm running the test suite right now, I should have the PR open this afternoon (EST). Obviously merging it will be dependent not only on code quality but also on the concerns you've voiced above @joguSD .

[jamesls]

Looks like the other AWS SDKs have the behavior of moving on to the next credential provider if the env var values are empty. I'm inclined to accept #1687.