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

r/secretsmanager_secret - policy wait for IAM eventual consistency #14459

Merged
merged 4 commits into from
Aug 6, 2020
Merged

r/secretsmanager_secret - policy wait for IAM eventual consistency #14459

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
756dabb
replace custom validation func with built in ones
DrFaust92 Aug 3, 2020
934a15b
Update aws/resource_aws_secretsmanager_secret.go
DrFaust92 Aug 6, 2020
0cc2538
Update aws/resource_aws_secretsmanager_secret.go
DrFaust92 Aug 6, 2020
b363659
use iam waiter
DrFaust92 Aug 6, 2020
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
84 changes: 52 additions & 32 deletions aws/resource_aws_secretsmanager_secret.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,10 @@ func resourceAwsSecretsManagerSecret() *schema.Resource {
Type: schema.TypeInt,
Optional: true,
Default: 30,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
value := v.(int)
if value == 0 {
return
}
if value >= 7 && value <= 30 {
return
}
errors = append(errors, fmt.Errorf("%q must be 0 or between 7 and 30", k))
return
},
ValidateFunc: validation.Any(
validation.IntBetween(7, 30),
validation.IntInSlice([]int{0}),
),
},
"rotation_enabled": {
Deprecated: "Use the aws_secretsmanager_secret_rotation resource instead",
Expand Down Expand Up @@ -154,7 +147,7 @@ func resourceAwsSecretsManagerSecretCreate(d *schema.ResourceData, meta interfac
output, err = conn.CreateSecret(input)
}
if err != nil {
return fmt.Errorf("error creating Secrets Manager Secret: %s", err)
return fmt.Errorf("error creating Secrets Manager Secret: %w", err)
}

d.SetId(aws.StringValue(output.ARN))
Expand All @@ -165,10 +158,23 @@ func resourceAwsSecretsManagerSecretCreate(d *schema.ResourceData, meta interfac
SecretId: aws.String(d.Id()),
}

log.Printf("[DEBUG] Setting Secrets Manager Secret resource policy; %s", input)
_, err := conn.PutResourcePolicy(input)
err := resource.Retry(2*time.Minute, func() *resource.RetryError {
DrFaust92 marked this conversation as resolved.
Show resolved Hide resolved
var err error
_, err = conn.PutResourcePolicy(input)
if isAWSErr(err, secretsmanager.ErrCodeMalformedPolicyDocumentException,
"This resource policy contains an unsupported principal") {
return resource.RetryableError(err)
}
if err != nil {
return resource.NonRetryableError(err)
}
return nil
})
if isResourceTimeoutError(err) {
_, err = conn.PutResourcePolicy(input)
}
if err != nil {
return fmt.Errorf("error setting Secrets Manager Secret %q policy: %s", d.Id(), err)
return fmt.Errorf("error setting Secrets Manager Secret %q policy: %w", d.Id(), err)
}
}

Expand All @@ -195,7 +201,7 @@ func resourceAwsSecretsManagerSecretCreate(d *schema.ResourceData, meta interfac
_, err = conn.RotateSecret(input)
}
if err != nil {
return fmt.Errorf("error enabling Secrets Manager Secret %q rotation: %s", d.Id(), err)
return fmt.Errorf("error enabling Secrets Manager Secret %q rotation: %w", d.Id(), err)
}
}

Expand All @@ -218,7 +224,7 @@ func resourceAwsSecretsManagerSecretRead(d *schema.ResourceData, meta interface{
d.SetId("")
return nil
}
return fmt.Errorf("error reading Secrets Manager Secret: %s", err)
return fmt.Errorf("error reading Secrets Manager Secret: %w", err)
}

d.Set("arn", output.ARN)
Expand All @@ -232,13 +238,13 @@ func resourceAwsSecretsManagerSecretRead(d *schema.ResourceData, meta interface{
log.Printf("[DEBUG] Reading Secrets Manager Secret policy: %s", pIn)
pOut, err := conn.GetResourcePolicy(pIn)
if err != nil {
return fmt.Errorf("error reading Secrets Manager Secret policy: %s", err)
return fmt.Errorf("error reading Secrets Manager Secret policy: %w", err)
}

if pOut.ResourcePolicy != nil {
policy, err := structure.NormalizeJsonString(aws.StringValue(pOut.ResourcePolicy))
if err != nil {
return fmt.Errorf("policy contains an invalid JSON: %s", err)
return fmt.Errorf("policy contains an invalid JSON: %w", err)
}
d.Set("policy", policy)
}
Expand All @@ -248,15 +254,15 @@ func resourceAwsSecretsManagerSecretRead(d *schema.ResourceData, meta interface{
if aws.BoolValue(output.RotationEnabled) {
d.Set("rotation_lambda_arn", output.RotationLambdaARN)
if err := d.Set("rotation_rules", flattenSecretsManagerRotationRules(output.RotationRules)); err != nil {
return fmt.Errorf("error setting rotation_rules: %s", err)
return fmt.Errorf("error setting rotation_rules: %w", err)
}
} else {
d.Set("rotation_lambda_arn", "")
d.Set("rotation_rules", []interface{}{})
}

if err := d.Set("tags", keyvaluetags.SecretsmanagerKeyValueTags(output.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
return fmt.Errorf("error setting tags: %w", err)
}

return nil
Expand All @@ -278,35 +284,49 @@ func resourceAwsSecretsManagerSecretUpdate(d *schema.ResourceData, meta interfac
log.Printf("[DEBUG] Updating Secrets Manager Secret: %s", input)
_, err := conn.UpdateSecret(input)
if err != nil {
return fmt.Errorf("error updating Secrets Manager Secret: %s", err)
return fmt.Errorf("error updating Secrets Manager Secret: %w", err)
}
}

if d.HasChange("policy") {
if v, ok := d.GetOk("policy"); ok && v.(string) != "" {
policy, err := structure.NormalizeJsonString(v.(string))
if err != nil {
return fmt.Errorf("policy contains an invalid JSON: %s", err)
return fmt.Errorf("policy contains an invalid JSON: %w", err)
}
input := &secretsmanager.PutResourcePolicyInput{
ResourcePolicy: aws.String(policy),
SecretId: aws.String(d.Id()),
}

log.Printf("[DEBUG] Setting Secrets Manager Secret resource policy; %s", input)
_, err = conn.PutResourcePolicy(input)
log.Printf("[DEBUG] Setting Secrets Manager Secret resource policy; %#v", input)
err = resource.Retry(2*time.Minute, func() *resource.RetryError {
DrFaust92 marked this conversation as resolved.
Show resolved Hide resolved
var err error
_, err = conn.PutResourcePolicy(input)
if isAWSErr(err, secretsmanager.ErrCodeMalformedPolicyDocumentException,
"This resource policy contains an unsupported principal") {
return resource.RetryableError(err)
}
if err != nil {
return resource.NonRetryableError(err)
}
return nil
})
if isResourceTimeoutError(err) {
_, err = conn.PutResourcePolicy(input)
}
if err != nil {
return fmt.Errorf("error setting Secrets Manager Secret %q policy: %s", d.Id(), err)
return fmt.Errorf("error setting Secrets Manager Secret %q policy: %w", d.Id(), err)
}
} else {
input := &secretsmanager.DeleteResourcePolicyInput{
SecretId: aws.String(d.Id()),
}

log.Printf("[DEBUG] Removing Secrets Manager Secret policy: %s", input)
log.Printf("[DEBUG] Removing Secrets Manager Secret policy: %#v", input)
_, err := conn.DeleteResourcePolicy(input)
if err != nil {
return fmt.Errorf("error removing Secrets Manager Secret %q policy: %s", d.Id(), err)
return fmt.Errorf("error removing Secrets Manager Secret %q policy: %w", d.Id(), err)
}
}
}
Expand Down Expand Up @@ -335,7 +355,7 @@ func resourceAwsSecretsManagerSecretUpdate(d *schema.ResourceData, meta interfac
_, err = conn.RotateSecret(input)
}
if err != nil {
return fmt.Errorf("error updating Secrets Manager Secret %q rotation: %s", d.Id(), err)
return fmt.Errorf("error updating Secrets Manager Secret %q rotation: %w", d.Id(), err)
}
} else {
input := &secretsmanager.CancelRotateSecretInput{
Expand All @@ -345,15 +365,15 @@ func resourceAwsSecretsManagerSecretUpdate(d *schema.ResourceData, meta interfac
log.Printf("[DEBUG] Cancelling Secrets Manager Secret rotation: %s", input)
_, err := conn.CancelRotateSecret(input)
if err != nil {
return fmt.Errorf("error cancelling Secret Manager Secret %q rotation: %s", d.Id(), err)
return fmt.Errorf("error cancelling Secret Manager Secret %q rotation: %w", d.Id(), err)
}
}
}

if d.HasChange("tags") {
o, n := d.GetChange("tags")
if err := keyvaluetags.SecretsmanagerUpdateTags(conn, d.Id(), o, n); err != nil {
return fmt.Errorf("error updating tags: %s", err)
return fmt.Errorf("error updating tags: %w", err)
}
}

Expand All @@ -380,7 +400,7 @@ func resourceAwsSecretsManagerSecretDelete(d *schema.ResourceData, meta interfac
if isAWSErr(err, secretsmanager.ErrCodeResourceNotFoundException, "") {
return nil
}
return fmt.Errorf("error deleting Secrets Manager Secret: %s", err)
return fmt.Errorf("error deleting Secrets Manager Secret: %w", err)
}

return nil
Expand Down
77 changes: 39 additions & 38 deletions aws/resource_aws_secretsmanager_secret_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/terraform"
awspolicy "github.com/jen20/awspolicyequivalence"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/secretsmanager/waiter"
)

Expand Down Expand Up @@ -393,7 +392,6 @@ func TestAccAwsSecretsManagerSecret_policy(t *testing.T) {
var secret secretsmanager.DescribeSecretOutput
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_secretsmanager_secret.test"
expectedPolicyText := `{"Version":"2012-10-17","Statement":[{"Sid":"EnableAllPermissions","Effect":"Allow","Principal":{"AWS":"*"},"Action":"secretsmanager:GetSecretValue","Resource":"*"}]}`

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSSecretsManager(t) },
Expand All @@ -404,7 +402,23 @@ func TestAccAwsSecretsManagerSecret_policy(t *testing.T) {
Config: testAccAwsSecretsManagerSecretConfig_Policy(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsSecretsManagerSecretExists(resourceName, &secret),
testAccCheckAwsSecretsManagerSecretHasPolicy(resourceName, expectedPolicyText),
resource.TestMatchResourceAttr(resourceName, "policy",
regexp.MustCompile(`{"Action":"secretsmanager:GetSecretValue".+`)),
),
},
{
Config: testAccAwsSecretsManagerSecretConfig_Name(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsSecretsManagerSecretExists(resourceName, &secret),
resource.TestCheckResourceAttr(resourceName, "policy", ""),
),
},
{
Config: testAccAwsSecretsManagerSecretConfig_Policy(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsSecretsManagerSecretExists(resourceName, &secret),
resource.TestMatchResourceAttr(resourceName, "policy",
regexp.MustCompile(`{"Action":"secretsmanager:GetSecretValue".+`)),
),
},
},
Expand Down Expand Up @@ -489,39 +503,6 @@ func testAccCheckAwsSecretsManagerSecretExists(resourceName string, secret *secr
}
}

func testAccCheckAwsSecretsManagerSecretHasPolicy(name string, expectedPolicyText string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[name]
if !ok {
return fmt.Errorf("Not found: %s", name)
}

conn := testAccProvider.Meta().(*AWSClient).secretsmanagerconn
input := &secretsmanager.GetResourcePolicyInput{
SecretId: aws.String(rs.Primary.ID),
}

out, err := conn.GetResourcePolicy(input)

if err != nil {
return err
}

actualPolicyText := *out.ResourcePolicy

equivalent, err := awspolicy.PoliciesAreEquivalent(actualPolicyText, expectedPolicyText)
if err != nil {
return fmt.Errorf("error testing policy equivalence: %w", err)
}
if !equivalent {
return fmt.Errorf("non-equivalent policy error:\n\nexpected: %s\n\n got: %s",
expectedPolicyText, actualPolicyText)
}

return nil
}
}

func testAccPreCheckAWSSecretsManager(t *testing.T) {
conn := testAccProvider.Meta().(*AWSClient).secretsmanagerconn

Expand Down Expand Up @@ -719,8 +700,28 @@ resource "aws_secretsmanager_secret" "test" {

func testAccAwsSecretsManagerSecretConfig_Policy(rName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "test" {
name = %[1]q

assume_role_policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Action": "sts:AssumeRole",
"Principal": {
"Service": "ec2.amazonaws.com"
},
"Effect": "Allow",
"Sid": ""
}
]
}
EOF
}

resource "aws_secretsmanager_secret" "test" {
name = "%s"
name = %[1]q

policy = <<POLICY
{
Expand All @@ -730,7 +731,7 @@ resource "aws_secretsmanager_secret" "test" {
"Sid": "EnableAllPermissions",
"Effect": "Allow",
"Principal": {
"AWS": "*"
"AWS": "${aws_iam_role.test.arn}"
},
"Action": "secretsmanager:GetSecretValue",
"Resource": "*"
Expand Down