fix(amplify-provider-awscloudformation): Ensure the right profile is used with SSO and with credential_process

[johnf]

Description of changes

When using credential_process in the AWS profile (to get SSO to work for example) the ProcessCredentials Provider is used.
When using the default chain from the aws-sdk no options are passed to it and it default to using AWS_PROFILE.

With this change, we detect credential_process and specifically pass the profile name into the credential chain to ensure it gets used.

The problem this solves for me is when using multiple environments.
When adding an environment with aws env add even though a profile is picked, if it is using credential_process it won't be used.

In my patrticular use case the environments are in multiple accounts.
I suspect most people are using environments in the same account and have an AWS_PROFILE set so the default fallback works.

I haven't added any tests but could attempt it given some guidance.

Issue

#4488 is an example where the workaround suggest setting AWS_PROFILE, which should be unnecessary.

Description of how you validated changes

# Configure two  AWS SSO profiles using separate accounts 
# Add a wrapped entries that uses credential_process with something like aws2wrap

unset AWS_PROFILE # Make sure we don't have any creds
# unset AWS_* # All the other standard export
amplify init 
# Choose the first profile

amplify env add
# Choose the other profil

Both environments are created without any auth errors

Checklist

• [x ] PR description included
• [ x] yarn test passes
• Tests are changed or added

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[johnf]

Note: The way the current fix works it assumes that if credential_process exists in the profile that this is the way you want to authenticate.
It only adds the process provider to the chain.
We could put the others in there as well in case AWS_ variables are also set

[johnf]

@sachscode Anything we can do to help get this merged? It's preventing me from moving to 8.x

[sachscode]

The Change looks good. Could you please add a unit test for the update.

[johnf]

@sachscode If you can help point me in the right direction I can try, but not really sure where to start

[sachscode]

Hi @johnf
Sure thing!
You could find referencing this test helpful:
https://github.com/aws-amplify/amplify-cli/blob/master/packages/amplify-provider-awscloudformation/src/__tests__/system-config-manager.test.ts#L28-L36
You can add your unit-test in this file.

1. The unit-tests use the Jest framework. https://jestjs.io/
2. The goal of the unit-test is to define a contract in the newly added code describing how it interacts with rest of the code in that file. A unit test consists of two important artifacts , mocking and the expect statement.
   a. Mocking is used to describe the expected input shape and representative data for the interfaces in the newly added code:
   e.g profileConfig.credential_process (boolean, and must be mocked to true for the code is executed ),
   What’s the interface returned from aws.CredentialProviderChain.
   What is the interface returned from aws.ProcessCredentials.
   2. Expect is used to assert the expected output shape and data for the given input data.
   you could add an expect clause to assert the final output available in the received awsConfigInfo.

If this newly added code changes in the future or any new code branches are added, then the unit-test hopefully breaks and future devs can update the unit-test accordingly. Please let me know if you have any other questions.

[johnf]

@sachscode thanks for the pointers.

I've added some extra assertions.
I'm checking we have a CredentialProviderChain, previously this was empty.
I wasn't sure of a good way to check aws.ProcessCredentials given it is inside an anonymous function inside an array.

We know it's the last one so I could mock ProcessCredentials and call the last entry in the array and ensure it's called. BUt that felt a bit brittle.

[sachscode]

LGTM

[sachscode]

Thanks @johnf for the contribution, the change would be merged after another approval. @akshbhu is taking a look.