Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error message not good enough in case only region is not defined #131

Closed
eladb opened this issue Jun 19, 2018 · 14 comments
Closed

Error message not good enough in case only region is not defined #131

eladb opened this issue Jun 19, 2018 · 14 comments
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@eladb
Copy link
Contributor

eladb commented Jun 19, 2018

Repro:

  1. Clean up ~/.aws/config, ~/.aws/credentials and any environment variables.
  2. Run aws configure, supply access key and secret, but leave "region" to None.

Then, try to run cdk delpoy. The error message is:

Stack hello-cdk does not define an environment, and AWS credentails could not be obtained from standard locations.

Also, there's a typo "credentails" 😉

@RomainMuller
Copy link
Contributor

Alright the typo is an easy fix. As per the error message, I see the problem... I'm afraid this gets into rather verbose messaging though, what I'm inclined to use now is:

Stack ${stack.name} does not define an environment, and AWS credentials could not be obtained from standard locations or no region was configured.

It feels both verbose and barely actionable, which makes me unhappy. I'd appreciate any suggestion for a better wording. I was for one thing thinking about pointing the user to the documentation page of the AWS SDK/CLI on how to configure credentials/region, too.

RomainMuller added a commit that referenced this issue Jun 19, 2018
No environment can be inferred if we are unable to determine any of:
1. The account ID (no AWS credentials can be obtained via standard ways)
2. The region to be used

The error message covered 1 but not 2. New message covers both and corrects
a typo, but still is not as actionale as we'd like it to be.

See #131
@eladb
Copy link
Contributor Author

eladb commented Jun 20, 2018

I have no problem with verbosity in error messages. If it will help users identify what went wrong and how to fix it, it should be in the message.

@Doug-AWS
Copy link
Contributor

Point them to the "Getting Started" topic. It has a link to the CLI command and reinforces them to read the d@mn docs!

@rclark
Copy link

rclark commented Jun 21, 2018

👋 I'm taking my first baby steps beta testing the CDK today and I'm a bit stuck here. When running cdk deploy I get the error message that's under discussion above.

I have the following AWS credentials and configurations in environment variables:

$ printenv | grep AWS_
AWS_CREDENTIAL_EXPIRATION=2018-06-21T23:38:44.000Z
AWS_SESSION_TOKEN=xxx
AWS_SECRET_ACCESS_KEY=xxx
AWS_ACCESS_KEY_ID=xxx
AWS_ACCOUNT_ID=123456789012
AWS_DEFAULT_REGION=us-east-1

My organization sticks to environment variables, and uses temporary credentials at all times -- the awscli configure command is not a good fit for our workflows. Is there something missing in my environment that the CDK needs in order to get a deploy to work?

@RomainMuller
Copy link
Contributor

Hey @rclark - according to the AWS SDK for JS documentation, AWS_DEFAULT_REGION is not considered, only AWS_REGIONis. Can you try to set AWS_REGION=us-east-1 and see if this helps?

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 22, 2018

Did we remove support for AWS_DEFAULT_REGION? Because I definitely implemented that at the toolkit level at some point, taking boto3 and AWS CLI as a reference point.

I feel that since this is what the AWS CLI expects, and people will have written tooling to generate those environment variables, we should support that.

@RomainMuller
Copy link
Contributor

I guess it was dropped when we adopted "pure SDK" behavior. We can re-introduce, obviously, however I would rather get the JS SDK fixed so it behaves like the other SDKs...

@RomainMuller
Copy link
Contributor

I found aws/aws-sdk-js#373 on this particular topic. It would appear the CLI (and boto) are outliers compared to the other SDKs. What I propose is to amend our own code so that:

  • If AWS_DEFAULT_PROFILE is set and AWS_PROFILE is not, set AWS_PROFILE to what AWS_DEFAULT_PROFILE is.
  • If AWS_DEFAUL_REGION is set and AWS_REGION is not, set AWS_REGION to what AWS_DEFAULT_REGION is.

Then continue using the JS SDK for determining region, account, ....

@Doug-AWS
Copy link
Contributor

We need to document:

  • The environment values we recognize
  • Our credentials/config (region) lookup order

@rclark
Copy link

rclark commented Jun 22, 2018

Can you try to set AWS_REGION=us-east-1 and see if this helps?

Yes, this worked. Thanks!

@eladb
Copy link
Contributor Author

eladb commented Jun 22, 2018

I think we should align our behavior to the CLI, with all it's beautiful outlying quirks because the toolkit is a command-line-tool and user expectation would be for it to behave similarly to the CLI

@eladb
Copy link
Contributor Author

eladb commented Jun 25, 2018

@RomainMuller I think your proposal makes sense.

RomainMuller added a commit that referenced this issue Jun 25, 2018
The AWS CLI considers `AWS_DEFAULT_PROFILE` and `AWS_DEFAULT_REGION`
when creating default clients, however the AWS SDK for JS only considers
`AWS_PROFILE` and `AWS_REGION`. This aims to align the behavior of both
ways by automatically setting `AWS_PROFILE` and `AWS_REGION` from the
matching `AWS_DEFAULT_` variable (unless the variables were already set)

See #131
@RomainMuller
Copy link
Contributor

Submitted #175 to implement that suggestion.

@eladb
Copy link
Contributor Author

eladb commented Jul 9, 2018

#175 resolves this

@eladb eladb closed this as completed Jul 9, 2018
@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

6 participants