-
Notifications
You must be signed in to change notification settings - Fork 151
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
Allow assuming a role via AWS_PROFILE or configuration files #580
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
/bin/ | ||
/node_modules/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
name: create-user-and-role | ||
runtime: nodejs | ||
description: Demonstrate use of AWS profiles for role switching |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import * as aws from "@pulumi/aws"; | ||
import * as pulumi from "@pulumi/pulumi"; | ||
|
||
const config = new pulumi.Config(); | ||
const unprivilegedUsername = config.require("unprivilegedUsername"); | ||
|
||
const unprivilegedUser = new aws.iam.User("unprivileged-user", { | ||
name: unprivilegedUsername, | ||
}); | ||
|
||
const unprivilegedUserCreds = new aws.iam.AccessKey("unprivileged-user-key", { | ||
user: unprivilegedUser.name, | ||
}); | ||
|
||
const allowS3ManagementRole = new aws.iam.Role("allow-s3-management", { | ||
description: "Allow management of S3 buckets", | ||
assumeRolePolicy: unprivilegedUser.arn.apply(arn => { | ||
return aws.iam.assumeRolePolicyForPrincipal({AWS: arn}) | ||
}), | ||
}); | ||
|
||
new aws.iam.RolePolicy("allow-s3-management-policy", { | ||
role: allowS3ManagementRole, | ||
policy: { | ||
Version: "2012-10-17", | ||
Statement: [{ | ||
Sid: "AllowS3Management", | ||
Effect: "Allow", | ||
Resource: "*", | ||
Action: "s3:*", | ||
}] | ||
} | ||
}, {parent: allowS3ManagementRole}); | ||
|
||
export const roleArn = allowS3ManagementRole.arn; | ||
export const accessKeyId = unprivilegedUserCreds.id; | ||
export const secretAccessKey = unprivilegedUserCreds.secret; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"name": "typescript", | ||
"devDependencies": { | ||
"@types/node": "latest" | ||
}, | ||
"dependencies": { | ||
"@pulumi/aws": "^0.18.6", | ||
"@pulumi/pulumi": "latest" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
{ | ||
"compilerOptions": { | ||
"outDir": "bin", | ||
"target": "es6", | ||
"lib": [ | ||
"es6" | ||
], | ||
"module": "commonjs", | ||
"moduleResolution": "node", | ||
"sourceMap": true, | ||
"experimentalDecorators": true, | ||
"pretty": true, | ||
"noFallthroughCasesInSwitch": true, | ||
"noImplicitAny": true, | ||
"noImplicitReturns": true, | ||
"forceConsistentCasingInFileNames": true, | ||
"strictNullChecks": true | ||
}, | ||
"files": [ | ||
"index.ts" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
/bin/ | ||
/node_modules/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
name: use-role | ||
runtime: nodejs | ||
description: Use the created role |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import * as aws from "@pulumi/aws"; | ||
|
||
const bucket = new aws.s3.Bucket("created-with-role"); | ||
|
||
export const bucketArn = bucket.arn; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"name": "typescript", | ||
"devDependencies": { | ||
"@types/node": "latest" | ||
}, | ||
"dependencies": { | ||
"@pulumi/aws": "^0.18.6", | ||
"@pulumi/pulumi": "latest" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
{ | ||
"compilerOptions": { | ||
"outDir": "bin", | ||
"target": "es6", | ||
"lib": [ | ||
"es6" | ||
], | ||
"module": "commonjs", | ||
"moduleResolution": "node", | ||
"sourceMap": true, | ||
"experimentalDecorators": true, | ||
"pretty": true, | ||
"noFallthroughCasesInSwitch": true, | ||
"noImplicitAny": true, | ||
"noImplicitReturns": true, | ||
"forceConsistentCasingInFileNames": true, | ||
"strictNullChecks": true | ||
}, | ||
"files": [ | ||
"index.ts" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
// Copyright 2016-2018, Pulumi Corporation.( | ||
// Copyright 2016-2018, Pulumi Corporation. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
|
@@ -191,17 +191,7 @@ func preConfigureCallback(vars resource.PropertyMap, c *terraform.ResourceConfig | |
} | ||
config.CredsFilename = credsPath | ||
|
||
// TODO[pulumi/pulumi-terraform#48] We should also be setting `config.AssumeRole*` here, but we are currently | ||
// blocked on not being able to read out list-valued provider config. | ||
|
||
creds, err := awsbase.GetCredentials(config) | ||
if err != nil { | ||
return errors.New("unable to discover AWS AccessKeyID and/or SecretAccessKey " + | ||
"- see https://pulumi.io/install/aws.html for details on configuration") | ||
} | ||
|
||
_, err = creds.Get() | ||
if err != nil { | ||
if _, err := awsbase.GetCredentials(config); err != nil { | ||
return errors.New("unable to discover AWS AccessKeyID and/or SecretAccessKey " + | ||
"- see https://pulumi.io/install/aws.html for details on configuration") | ||
} | ||
|
@@ -230,6 +220,11 @@ func Provider() tfbridge.ProviderInfo { | |
EnvVars: []string{"AWS_REGION", "AWS_DEFAULT_REGION"}, | ||
}, | ||
}, | ||
"profile": { | ||
Default: &tfbridge.DefaultInfo{ | ||
EnvVars: []string{"AWS_PROFILE"}, | ||
}, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are quite a few more config vars that can be populated from https://www.terraform.io/docs/providers/aws/index.html#environment-variables There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should - I'll follow up on this with a new PR soon. |
||
}, | ||
PreConfigureCallback: preConfigureCallback, | ||
Resources: map[string]*tfbridge.ResourceInfo{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow why this is the right fix.
All of the code here is intended to catch and report failures to authenticate prior to the Terraform Provider
Configure
call reporting back failures. But the goal is to do the same checks the Terraform Provider will do.Per https://github.com/hashicorp/aws-sdk-go-base/blob/8fac28668e59a2e5afa6b2d034614c648ddc418e/session.go#L31, it appears the call to
creds.Get()
will be made there.Is the problem instead that
Profile: stringValue(vars, "profile"),
isn't correctly reading the profile here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason using
AWS_PROFILE
was failing is because we weren't reading the profile correctly before - whether through a profile configured in~/.aws/config
or~/.aws/credentials
.I don't think removing the call to
Get
on the returnedCredentials
struct is strictly necessary to fix the bug, but I also don't think it buys us anything to do it.GetCredentials
itself callsGet
here (after constructing a valid credentials chain):https://github.com/hashicorp/aws-sdk-go-base/blob/master/awsauth.go#L239
It also tests that a role can be assumed, if appropriate:
https://github.com/hashicorp/aws-sdk-go-base/blob/master/awsauth.go#L283
If we hit either of these cases, the call to
GetCredentials
would have failed before anyway, soGet
would not be called. If it worked the first time (inGetCredentials
), calling it a second time should not yield different behaviour without configuration changes.It's worth noting that the
aws-sdk-go-base
library is newer than the Pulumi AWS provider, and that the checks done by Terraform have changed as a result of using this library.Finally, the error message we produce in response to any error from configuration complains about
AccessKeyID
andSecretAccessKey
. In future, we should change the error to reflect the actual source of the problem. We could duplicate the entire logic ofGetCredentials
in the pre-configure callback to give better error messages here, but it like we'd be better off doing string detection on the error values to avoid doing so.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it - makes sense.
Totally agreed.