skip credentials file if creds are present in envCfg, fixes: #2455

[sanketplus]

As mentioned in #2455 and at aws cli userguide, it should be safe to not load credentials file when creds are present in env vars.

Please let me know if it looks good, I shall change the status from Draft PR.

[jasdel]

Thanks for taking the time to create this PR @sanketplus. The AWS SDK for Go doesn't distinguish between the shared config file (/aws/config) and shared credentials file (/aws/credentials). To the SDK they are the same file.

With that said the SDK will not read the additional information such as region and assume role fields out of the file unless the Session's Shared Config mode is enabled.

If Session Shared Config support mode is not enabled, the SDK might be able to drop all entries in the cfgFiles slice, (from your PR change). We need to evaluate this to not be a breaking change, and add tests to ensure the behavior is maintained in the future.

[sanketplus]

Regardless of shared config enabled or not, the PR drops files that has name *credentials if both key id and secret keys are present in env vars. since credentials from env vars has higher precedence, it is assumed safe to ignore those files.

Am I missing something obvious here? Also for test case, I see one to be test for providing conf from both env vars and a file and assert that one from env takes precedence. Looking forward to your comments @jasdel

[jasdel]

Thanks for the update @sanketplus. I'm suggesting, that if the Session's Shared Config support is not enabled it maybe be safe for the SDK to remove all filenames from that slice. The name of the credentials file can be customized and specified via other means. In addition, from the SDK's perspective there is no difference between the config and credentials file. This is mostly true for the AWS CLI as well. The SDKs suggest credentials only exist within the credentials file, but the SDK and CLI all support the same behavior from the config file as well. The credentials file is just read first. This is why I suggest if Session Shared Config support is not enabled the full list of files to load might be safe to clear.

[jasdel]

Created PR #2731 to address this issue by allowing the configuration files to fail to load if credentials are available, and the file is not being used. We can use that PR to resolve this issue.

Thanks again for taking the time to create this initial PR and bring the issue to our attention.