feat(cli): support SSO #19454
feat(cli): support SSO #19454
Conversation
|
|
comcalvi
added
the
pr-linter/exempt-test
| await forceSdkToReadConfigIfPresent(); | ||
| sources.push(() => new AWS.SsoCredentials({ profile: implicitProfile })); | ||
| } | ||
|
|
||
| if (await fs.pathExists(credentialsFileName())) { |
There was a problem hiding this comment.
Can you try it without this if, and putting the SsoCredentials inside the sources array below?
There was a problem hiding this comment.
@rix0rrr yep, this change works, with both an existing (but empty) credentials file and with no credentials file. Removing the config file breaks it, but that's expected. I also had to change one of the tests to use the 5th member of the chain instead of the 2nd (see diff), but this should be fine because that test seems to check for existence only.
| @@ -3241,6 +3241,16 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | |||
| OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | |||
| THE SOFTWARE. | |||
|
|
|||
| ---------------- | |||
There was a problem hiding this comment.
I want Eli to have a look at this.
There was a problem hiding this comment.
I'm not sure why, but looks like the aws-sdk upgrade in the CLI package broke the hoisting.
Currently on master we have:
<repoRoot>/node_modules/aws-sdk (version 2.1094.0)When I upgraded aws-sdk@^2.979.0 to aws-sdk@^2.1093.0 and ran yarn install, I got the following directory structure:
<repoRoot>/node_modules/aws-sdk (version 2.1094.0)
<repoRoot>/packages/aws-cdk/node_modules/aws-sdk (version 2.1096.0)I expected version 2.1096.0 to be hoisted and replace 2.1094.0 - but thats not what happens.
So in practice the CLI uses two different versions of aws-sdk. One coming from its own direct dependency, which isn't hoisted anymore (2.1096.0), and one coming transitively from cdk-assets, which is hoisted (2.1094.0).
We need to figure out why yarn isn't hoisting it. @rix0rrr Any ideas?
There was a problem hiding this comment.
No, feels like it should be able to. Wonder if there's a constraint somewhere that's preventing the hoist?
| // Force reading the `config` file if it exists by setting the appropriate | ||
| // environment variable. | ||
| await forceSdkToReadConfigIfPresent(); | ||
| sources.push(() => profileCredentials(implicitProfile)); |
There was a problem hiding this comment.
Seems like the list of sources is duplicated in two places in the code. Maybe find a way to reuse?
rix0rrr
added
pr-linter/exempt-integ-test
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
1 similar comment
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
|
||
| function iniFileCredentialFactories(theProfile: string) { | ||
| return [ | ||
| () => profileCredentials(theProfile), |
There was a problem hiding this comment.
Hello @comcalvi, is there a reason why credentials from the profile would take precedence here over the SSO? It's different than e.g. behavior of JS SDK v3 - https://github.com/aws/aws-sdk-js-v3/blob/31e72ac6063fdb3393c2d5042b8964238ce1b23a/packages/credential-provider-node/src/defaultProvider.ts#L52-L54
Or am I missing something here?
Adds support for SSO.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license