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

Unexpected behavior of default environment (region/account) #59

Closed
mindstorms6 opened this issue Jun 7, 2018 · 12 comments · Fixed by #128
Closed

Unexpected behavior of default environment (region/account) #59

mindstorms6 opened this issue Jun 7, 2018 · 12 comments · Fixed by #128
Assignees

Comments

@mindstorms6
Copy link
Contributor

Hiya friends!

AWS IS SO SECURE 🔐

Tl;dr I have some creds in my env

> env | grep AWS
AWS_ACCESS_KEY_ID=nope
AWS_SECRET_ACCESS_KEY=nope
AWS_SESSION_TOKEN=nice try
┌[breland☮Brelands-MacBook-Pro-2.local]-(~/repos/aws-cdk/packages/swa-test)-[git://dev-brelandm ✗]-

And I'd love to use them:

┌[breland☮Brelands-MacBook-Pro-2.local]-(~/repos/aws-cdk/packages/swa-test)-[git://dev-brelandm ✗]-
└> ../aws-cdk-toolkit/bin/cdk -v bootstrap
Defaults: {  "app": "node index.js"}
Obtaining default region from AWS configuration
Setting "default-region" context to undefined
Looking up default account ID from STS
Setting "default-account" context to 532610000315
node index.js '{"type":"list","context":{"default-account":"532610000315"}}'
Stack name not specified, so defaulting to all available stacks: SWA
 ⏳  Bootstrapping environment 532610000315/us-west-1...
 ❌  Environment 532610000315/us-west-1 failed bootstrapping: Error: Need to perform AWS calls for account 532610000315, but no credentials found. Tried: default credentials.
Need to perform AWS calls for account 532610000315, but no credentials found. Tried: default credentials.
Error: Need to perform AWS calls for account 532610000315, but no credentials found. Tried: default credentials.
    at SDK.getCredentialProvider (/Users/breland/repos/aws-cdk/packages/aws-cdk-toolkit/lib/api/util/sdk.ts:124:15)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:160:7)
┌[breland☮Brelands-MacBook-Pro-2.local]-(~/repos/aws-cdk/packages/swa-test)-[git://dev-brelandm ✗]-

Is there some magic trick I need to use? The docs would indicate this should work... What am I missing?

@mindstorms6
Copy link
Contributor Author

I fixed it by doing a horrifying thing

class Test implements CredentialProviderSource {
    public readonly name: string = "CustomCreds";

    public isAvailable() {
        return new Promise<boolean>((_resolve, _reject) => {
            _resolve(true);
        });
    }

    public canProvideCredentials(_accountId: string): Promise<boolean> {
        return new Promise((resolve, _reject) => {
                resolve(true);
            });
    }

    getProvider(_accountId: string, _mode: Mode): Promise<CredentialProviderChain> {
        return new Promise<CredentialProviderChain>((resolve, _reject) => {
            resolve(new CredentialProviderChain());
        });
    }
}

PluginHost.instance.registerCredentialProviderSource(new Test());
const app = new App(process.argv);

@mindstorms6
Copy link
Contributor Author

Actually no, that didn't fix it :(

@mindstorms6
Copy link
Contributor Author

mindstorms6 commented Jun 8, 2018

I was able to aws configure with an IAM user and be ok. I cannot quite understand why env creds didn't work though :/

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 8, 2018

That's funky. I use it with environment variables all the time.

@mindstorms6
Copy link
Contributor Author

Hmmm. I'll clean some things up and try again but I tried valiantly and it was not happy

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 8, 2018

This error could also be consistent with credentials for one particular account but default-account setting for a different one?

@mindstorms6
Copy link
Contributor Author

Oh interesting - I thought i had those aligned - lemme check

@Doug-AWS
Copy link
Contributor

Doug-AWS commented Jun 8, 2018

When you guys get this working could you add a blurb in the readme on how to access an env var?

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 11, 2018

Nobody needs to access env vars. People might need to set them, but I think there should be a limit to what we need to explain.

Do we also tell people how to start a Terminal window?

@eladb
Copy link
Contributor

eladb commented Jun 11, 2018

@Doug-AWS using environment variables for credentials should be transparent. However there is an order of precedence which is definitely something we should document well

@fulghum
Copy link
Contributor

fulghum commented Jun 18, 2018

+1 for documenting credential loading precedence. Doug is right that it should be consistent with the other AWS development tools.

@aws aws deleted a comment from Doug-AWS Jun 19, 2018
@eladb
Copy link
Contributor

eladb commented Jun 19, 2018

Let's work backwards from the desired behavior (without toolkit credentials plugins installed):

  • If env is not specified when a stack is created, the account and region should be exactly like the aws cli would behave if --region was not specified, based on the default credentials chain as @Doug-AWS indicates.
  • If only env.region is specified (no account), then the account should derive from the default credentials chain but region should be based on the region.
  • If env.account is also specified, then region must also be defined (?) and then the toolkit should be defensive and fail if the default credentials chain didn't include credentials for that account.
  • If there are no credentials configured in the default credentials chain, the toolkit should fail with an error "missing credentials", and link to https://docs.aws.amazon.com/cli/latest/userguide/cli-chap-getting-started.html for instructions on how to configure credentials.

Now for some implementation-related issues. I think one of the reasons we keep seeing inconsistencies is because there were previously conflicting instructions on specifying the default-region and default-account contextual parameters in cdk.json (or ~/.cdk.json). These parameters superseded the normal behavior and things got messy.

Since we still need to relay the region/account in case they are not explicitly specified when a Stack is created, My recommendation is to deprecate default-account and default-region (error if they are specified) and use a different context argument to pass the default account/region from the toolkit. Perhaps something like cdk:toolkit:defaultenv=<ACCOUNT>/<REGION>. This argument should never be explicitly specified in cdk.json (maybe we can even add some defense that doesn't allow context that starts with "cdk:" to be specified by humans...).

@eladb eladb changed the title Env credentials don't work? Unexpected behavior of default environment (region/account) Jun 19, 2018
RomainMuller added a commit that referenced this issue Jun 19, 2018
* Use environment variables in a way that is consistent with what the
  various AWS SDKs and CLIs do:
  + `AWS_REGION` has precedence over `AWS_DEFAULT_REGION`
  + `AWS_SHARED_CREDENTIALS_FILE` can be used to override the location
    of the `~/.aws/credentials` file
  + `AWS_CONFIG_FILE` can be used to override the location of the
    `.aws/config` file
* Ensure the environment variable instructing the JS AWS SDK to load up
  data from the `~/.aws/config` file gets set by the toolkit before
  `aws-sdk` is loaded
* Create `~/.aws/credentials` if `~/.aws/config` exists, since the JS
  AWS SDK will start by loading configuration from there, and will fail
  if it does not exist
* Retire the `default-account` and `default-region` keys from the
  `context` map, and replace them with `aws:cdk:toolkit:` keys that
  cannot be user-provided

Fixes #59
eladb pushed a commit that referenced this issue Jun 19, 2018
* Adopt SDK-standard behavior when no environment is specified

* Use environment variables in a way that is consistent with what the
  various AWS SDKs and CLIs do:
  + `AWS_REGION` has precedence over `AWS_DEFAULT_REGION`
  + `AWS_SHARED_CREDENTIALS_FILE` can be used to override the location
    of the `~/.aws/credentials` file
  + `AWS_CONFIG_FILE` can be used to override the location of the
    `.aws/config` file
* Ensure the environment variable instructing the JS AWS SDK to load up
  data from the `~/.aws/config` file gets set by the toolkit before
  `aws-sdk` is loaded
* Create `~/.aws/credentials` if `~/.aws/config` exists, since the JS
  AWS SDK will start by loading configuration from there, and will fail
  if it does not exist
* Retire the `default-account` and `default-region` keys from the
  `context` map, and replace them with `aws:cdk:toolkit:` keys that
  cannot be user-provided

Fixes #59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants