fix: gracefully handle absence fo the ~/.aws/credentials file
#541
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The JS SDK assumes the file exists when it is passed as an argument, so this change makes the CDK Toolkit check for file existence before passing down to the SDK layer. Additionally, the SDK "ini file" re-implementation failed to check for file existece before attempting to load file contents. Fixes #540
eladb
changed the title
~/.aws/credentials file~/.aws/credentials file
eladb
reviewed
Aug 12, 2018
eladb
approved these changes
Aug 12, 2018
|
Please fix your commit title and message to adhere to conventional commits |
rix0rrr
reviewed
Aug 13, 2018
| this.parsedContents = (AWS as any).util.ini.parse( | ||
| (AWS as any).util.readFileSync(this.filename) | ||
| ); | ||
| this.parsedContents = fs.pathExistsSync(this.filename) |
There was a problem hiding this comment.
Are you sure this is necessary? I'm positive I tested this WITHOUT an ~/.aws/config file and no crash.
There was a problem hiding this comment.
My observations show that a missing ~/.aws/credentials would blow up right this spot.
rix0rrr
approved these changes
Aug 13, 2018
costleya
approved these changes
Aug 14, 2018
| (AWS as any).util.readFileSync(this.filename) | ||
| ); | ||
| this.parsedContents = fs.pathExistsSync(this.filename) | ||
| ? (AWS as any).util.ini.parse((AWS as any).util.readFileSync(this.filename)) |
There was a problem hiding this comment.
Can we save of (AWS as any).util as a variable to make this conditional a bit less verbose?
eladb
approved these changes
Aug 14, 2018
|
@eladb I though my issue title & message are following conventional commits format already. |
|
@RomainMuller it was "Gracefully handle absence fo the |
RomainMuller
changed the title
~/.aws/credentials file~/.aws/credentials file
RomainMuller
changed the title
~/.aws/credentials file~/.aws/credentials file
rix0rrr
added a commit
that referenced
this pull request
Aug 15, 2018
### Features * __@aws-cdk/cdk__: Tokens can now be transparently embedded into strings and encoded into JSON without losing their semantics. This makes it possible to treat late-bound (deploy-time) values as if they were regular strings ([@rix0rrr] in [#518](#518)). * __@aws-cdk/aws-s3__: add support for bucket notifications to Lambda, SNS, and SQS targets ([@eladb] in [#201](#201), [#560](#560), [#561](#561), [#564](#564)) * __@aws-cdk/cdk__: non-alphanumeric characters can now be used as construct identifiers ([@eladb] in [#556](#556)) * __@aws-cdk/aws-iam__: add support for `maxSessionDuration` for Roles ([@eladb] in [#545](#545)). ### Changes * __@aws-cdk/aws-lambda__ (_**BREAKING**_): most classes renamed to be shorter and more in line with official service naming (`Lambda` renamed to `Function` or ommitted) ([@eladb] in [#550](#550)) * __@aws-cdk/aws-codepipeline__ (_**BREAKING**_): move all CodePipeline actions from `@aws-cdk/aws-xxx-codepipeline` packages into the regular `@aws-cdk/aws-xxx` service packages ([@skinny85] in [#459](#459)). * __@aws-cdk/aws-custom-resources__ (_**BREAKING**_): package was removed, and the Custom Resource construct added to the __@aws-cdk/aws-cloudformation__ package ([@rix0rrr] in [#513](#513)) ### Fixes * __@aws-cdk/aws-lambda__: Lambdas that are triggered by CloudWatch Events now show up in the console, and can only be triggered the indicated Event Rule. _**BREAKING**_ for middleware writers (as this introduces an API change), but transparent to regular consumers ([@eladb] in [#558](#558)) * __@aws-cdk/aws-codecommit__: fix a bug where `pollForSourceChanges` could not be set to `false` ([@maciejwalkowiak] in [#534](#534)) * __aws-cdk__: don't fail if the `~/.aws/credentials` file is missing ([@RomainMuller] in [#541](#541)) * __@aws-cdk/aws-cloudformation__: fix a bug in the CodePipeline actions to correctly support TemplateConfiguration ([@mindstorms6] in [#571](#571)). * __@aws-cdk/aws-cloudformation__: fix a bug in the CodePipeline actions to correctly support ParameterOverrides ([@mindstorms6] in [#574](#574)). ### Known Issues * `cdk init` will try to init a `git` repository and fail if no global `user.name` and `user.email` have been configured.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The JS SDK assumes the file exists when it is passed as an argument, so
this change makes the CDK Toolkit check for file existence before
passing down to the SDK layer. Additionally, the SDK "ini file"
re-implementation failed to check for file existece before attempting to
load file contents.
Fixes #540