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.

Sign up for GitHub

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

Jump to bottom

aws_secret_backend_role: support role_arns argument #407

Merged
merged 6 commits into from
Jun 3, 2019
Merged

aws_secret_backend_role: support role_arns argument #407

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7207d43
aws_secret_backend_role: add role_arns argument
anatolebeuzon May 3, 2019
0ca3c86
aws_secret_backend_role: use role_arns in tests
anatolebeuzon May 3, 2019
5176afb
aws_secret_backend_role: add role_arns to docs
anatolebeuzon May 3, 2019
c26f014
Merge pull request #1 from DataDog/anatole/add-role-arns
oxlay May 3, 2019
88ff545
Remove ConflictsWith clause for policy_document
anatolebeuzon May 8, 2019
dc957f6
Remove ConflictsWith clause with policy_document for role_arns
anatolebeuzon May 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions vault/resource_aws_secret_backend_role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func awsSecretBackendRoleResource() *schema.Resource {
"policy_arns": {
Type: schema.TypeList,
Optional: true,
ConflictsWith: []string{"policy", "policy_arn"},
ConflictsWith: []string{"policy", "policy_arn", "role_arns"},
Description: "ARN for an existing IAM policy the role should use.",
Elem: &schema.Schema{
Type: schema.TypeString,
Expand All @@ -46,21 +46,21 @@ func awsSecretBackendRoleResource() *schema.Resource {
"policy_arn": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"policy_document", "policy", "policy_arns"},
ConflictsWith: []string{"policy_document", "policy", "policy_arns", "role_arns"},
Description: "ARN for an existing IAM policy the role should use.",
Deprecated: `Use "policy_arns".`,
},
"policy_document": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"policy_arn", "policy"},
ConflictsWith: []string{"policy_arn", "policy", "role_arns"},
Description: "IAM policy the role should use in JSON format.",
DiffSuppressFunc: util.JsonDiffSuppress,
},
"policy": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"policy_arns", "policy_arn", "policy_document"},
ConflictsWith: []string{"policy_arns", "policy_arn", "policy_document", "role_arns"},
Description: "IAM policy the role should use in JSON format.",
DiffSuppressFunc: util.JsonDiffSuppress,
Deprecated: `Use "policy_document".`,
Expand All @@ -70,6 +70,16 @@ func awsSecretBackendRoleResource() *schema.Resource {
Required: true,
Description: "Role credential type.",
},
"role_arns": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do role_arns conflict with other fields in practice? The documentation doesn't say it does. Should we remove it from the conflicts with clauses?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • policy_document apparently does not conflict with role_arns (see test using both), so I've removed the ConflictsWith clause for this one
  • policy_arns and policy_arn are only valid when credential_type is iam_user, while role_arns is only valid when credential_type is assumed_role and prohibited otherwise (per the doc), so in practice, they are mutually exclusive
  • for policy, the doc says "cannot be mixed with the parameters listed above", hence the ConflictsWith clause

Type: schema.TypeList,
Elem: &schema.Schema{
Type: schema.TypeString,
},
Optional: true,
ForceNew: true,
ConflictsWith: []string{"policy", "policy_arn", "policy_arns", "policy_document"},
Description: "ARNs of AWS roles allowed to be assumed. Only valid when credential_type is 'assumed_role'",
},
},
}
}
Expand Down Expand Up @@ -97,8 +107,14 @@ func awsSecretBackendRoleWrite(d *schema.ResourceData, meta interface{}) error {
policy = d.Get("policy")
}

if policy == "" && len(policyARNs) == 0 {
return fmt.Errorf("either policy or policy_arn must be set.")
var roleARNs []string
roleARNsIfc := d.Get("role_arns")
for _, roleIfc := range roleARNsIfc.([]interface{}) {
roleARNs = append(roleARNs, roleIfc.(string))
}

if policy == "" && len(policyARNs) == 0 && len(roleARNs) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this check is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you talking about the whole if { } or just the && len(roleARNs) == 0 part?

return fmt.Errorf("either policy, policy_arns, or role_arns must be set")
}

data := map[string]interface{}{
Expand All @@ -110,6 +126,9 @@ func awsSecretBackendRoleWrite(d *schema.ResourceData, meta interface{}) error {
if len(policyARNs) != 0 {
data["policy_arns"] = policyARNs
}
if len(roleARNs) != 0 {
data["role_arns"] = roleARNs
}

log.Printf("[DEBUG] Creating role %q on AWS backend %q", name, backend)
_, err := client.Logical().Write(backend+"/roles/"+name, data)
Expand Down Expand Up @@ -160,6 +179,7 @@ func awsSecretBackendRoleRead(d *schema.ResourceData, meta interface{}) error {
}

d.Set("credential_type", secret.Data["credential_type"])
d.Set("role_arns", secret.Data["role_arns"])
d.Set("backend", strings.Join(pathPieces[:len(pathPieces)-2], "/"))
d.Set("name", pathPieces[len(pathPieces)-1])
return nil
Expand Down
40 changes: 38 additions & 2 deletions vault/resource_aws_secret_backend_role_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const testAccAWSSecretBackendRolePolicyInline_basic = `{"Version": "2012-10-17",
const testAccAWSSecretBackendRolePolicyInline_updated = `{"Version": "2012-10-17","Statement": [{"Effect": "Allow","Action": "ec2:*","Resource": "*"}]}`
const testAccAWSSecretBackendRolePolicyArn_basic = "arn:aws:iam::123456789123:policy/foo"
const testAccAWSSecretBackendRolePolicyArn_updated = "arn:aws:iam::123456789123:policy/bar"
const testAccAWSSecretBackendRoleRoleArn_basic = "arn:aws:iam::123456789123:role/foo"
const testAccAWSSecretBackendRoleRoleArn_updated = "arn:aws:iam::123456789123:role/bar"

func TestAccAWSSecretBackendRole_basic(t *testing.T) {
backend := acctest.RandomWithPrefix("tf-test-aws")
Expand All @@ -38,6 +40,9 @@ func TestAccAWSSecretBackendRole_basic(t *testing.T) {
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "backend", backend),
util.TestCheckResourceAttrJSON("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_document", testAccAWSSecretBackendRolePolicyInline_basic),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_arns.0", testAccAWSSecretBackendRolePolicyArn_basic),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "name", fmt.Sprintf("%s-role-arns", name)),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "backend", backend),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "role_arns.0", testAccAWSSecretBackendRoleRoleArn_basic),
),
},
{
Expand All @@ -53,6 +58,9 @@ func TestAccAWSSecretBackendRole_basic(t *testing.T) {
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "backend", backend),
util.TestCheckResourceAttrJSON("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_document", testAccAWSSecretBackendRolePolicyInline_updated),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_arns.0", testAccAWSSecretBackendRolePolicyArn_updated),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "name", fmt.Sprintf("%s-role-arns", name)),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "backend", backend),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "role_arns.0", testAccAWSSecretBackendRoleRoleArn_updated),
),
},
},
Expand Down Expand Up @@ -81,6 +89,9 @@ func TestAccAWSSecretBackendRole_import(t *testing.T) {
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "backend", backend),
util.TestCheckResourceAttrJSON("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_document", testAccAWSSecretBackendRolePolicyInline_basic),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_arns.0", testAccAWSSecretBackendRolePolicyArn_basic),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "name", fmt.Sprintf("%s-role-arns", name)),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "backend", backend),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "role_arns.0", testAccAWSSecretBackendRoleRoleArn_basic),
),
},
{
Expand All @@ -98,6 +109,11 @@ func TestAccAWSSecretBackendRole_import(t *testing.T) {
ImportState: true,
ImportStateVerify: true,
},
{
ResourceName: "vault_aws_secret_backend_role.test_role_arns",
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Expand All @@ -124,6 +140,9 @@ func TestAccAWSSecretBackendRole_nested(t *testing.T) {
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "backend", backend),
util.TestCheckResourceAttrJSON("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_document", testAccAWSSecretBackendRolePolicyInline_basic),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_arns.0", testAccAWSSecretBackendRolePolicyArn_basic),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "name", fmt.Sprintf("%s-role-arns", name)),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "backend", backend),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "role_arns.0", testAccAWSSecretBackendRoleRoleArn_basic),
),
},
{
Expand All @@ -139,6 +158,9 @@ func TestAccAWSSecretBackendRole_nested(t *testing.T) {
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "backend", backend),
util.TestCheckResourceAttrJSON("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_document", testAccAWSSecretBackendRolePolicyInline_updated),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_policy_inline_and_arns", "policy_arns.0", testAccAWSSecretBackendRolePolicyArn_updated),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "name", fmt.Sprintf("%s-role-arns", name)),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "backend", backend),
resource.TestCheckResourceAttr("vault_aws_secret_backend_role.test_role_arns", "role_arns.0", testAccAWSSecretBackendRoleRoleArn_updated),
),
},
},
Expand Down Expand Up @@ -192,7 +214,14 @@ resource "vault_aws_secret_backend_role" "test_policy_inline_and_arns" {
credential_type = "iam_user"
backend = "${vault_aws_secret_backend.test.path}"
}
`, path, accessKey, secretKey, name, testAccAWSSecretBackendRolePolicyInline_basic, name, testAccAWSSecretBackendRolePolicyArn_basic, name, testAccAWSSecretBackendRolePolicyInline_basic, testAccAWSSecretBackendRolePolicyArn_basic)

resource "vault_aws_secret_backend_role" "test_role_arns" {
name = "%s-role-arns"
role_arns = ["%s"]
credential_type = "assumed_role"
backend = "${vault_aws_secret_backend.test.path}"
}
`, path, accessKey, secretKey, name, testAccAWSSecretBackendRolePolicyInline_basic, name, testAccAWSSecretBackendRolePolicyArn_basic, name, testAccAWSSecretBackendRolePolicyInline_basic, testAccAWSSecretBackendRolePolicyArn_basic, name, testAccAWSSecretBackendRoleRoleArn_basic)
}

func testAccAWSSecretBackendRoleConfig_updated(name, path, accessKey, secretKey string) string {
Expand Down Expand Up @@ -224,5 +253,12 @@ resource "vault_aws_secret_backend_role" "test_policy_inline_and_arns" {
credential_type = "iam_user"
backend = "${vault_aws_secret_backend.test.path}"
}
`, path, accessKey, secretKey, name, testAccAWSSecretBackendRolePolicyInline_updated, name, testAccAWSSecretBackendRolePolicyArn_updated, name, testAccAWSSecretBackendRolePolicyInline_updated, testAccAWSSecretBackendRolePolicyArn_updated)

resource "vault_aws_secret_backend_role" "test_role_arns" {
name = "%s-role-arns"
role_arns = ["%s"]
credential_type = "assumed_role"
backend = "${vault_aws_secret_backend.test.path}"
}
`, path, accessKey, secretKey, name, testAccAWSSecretBackendRolePolicyInline_updated, name, testAccAWSSecretBackendRolePolicyArn_updated, name, testAccAWSSecretBackendRolePolicyInline_updated, testAccAWSSecretBackendRolePolicyArn_updated, name, testAccAWSSecretBackendRoleRoleArn_updated)
}
4 changes: 4 additions & 0 deletions website/docs/r/aws_secret_backend_role.html.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ role. Either `policy_document` or `policy_arns` must be specified.
* `policy_arns` - (Optional) The ARN for a pre-existing policy to associate
with this role. Either `policy_document` or `policy_arns` must be specified.

* `role_arns` - (Optional) Specifies the ARNs of the AWS roles this Vault role
is allowed to assume. Required when `credential_type` is `assumed_role` and
prohibited otherwise.

* `credential_type` - (Required) Specifies the type of credential to be used when
retrieving credentials from the role. Must be one of `iam_user`, `assumed_role`, or
`federation_token`.
Expand Down