From 59f62c14374c8b02dff63bd266eded7c33bcfec6 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 30 Oct 2023 16:41:05 -0400 Subject: [PATCH 1/3] r/aws_secretsmanager_secret_rotation: support managed secrets This change makes the rotation_lambda_arn argument optional, allowing for use cases where secrets are managed by AWS and do not require a custom lambda function ARN to be specified. --- .changelog/34180.txt | 3 + .../service/secretsmanager/secret_rotation.go | 72 +++++++++---------- ...cretsmanager_secret_rotation.html.markdown | 2 +- 3 files changed, 36 insertions(+), 41 deletions(-) create mode 100644 .changelog/34180.txt diff --git a/.changelog/34180.txt b/.changelog/34180.txt new file mode 100644 index 00000000000..2b0c9f1aef6 --- /dev/null +++ b/.changelog/34180.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_secretsmanager_secret_rotation: The `rotation_lambda_arn` argument is now optional to support modifying the rotation schedule of AWS-managed secrets. +``` diff --git a/internal/service/secretsmanager/secret_rotation.go b/internal/service/secretsmanager/secret_rotation.go index f0faf31ef8e..aa04c2bdd7f 100644 --- a/internal/service/secretsmanager/secret_rotation.go +++ b/internal/service/secretsmanager/secret_rotation.go @@ -38,7 +38,7 @@ func ResourceSecretRotation() *schema.Resource { }, "rotation_lambda_arn": { Type: schema.TypeString, - Required: true, + Optional: true, }, "rotation_rules": { Type: schema.TypeList, @@ -87,25 +87,26 @@ func resourceSecretRotationCreate(ctx context.Context, d *schema.ResourceData, m conn := meta.(*conns.AWSClient).SecretsManagerConn(ctx) secretID := d.Get("secret_id").(string) - if v, ok := d.GetOk("rotation_lambda_arn"); ok && v.(string) != "" { - input := &secretsmanager.RotateSecretInput{ - RotationLambdaARN: aws.String(v.(string)), - RotationRules: expandRotationRules(d.Get("rotation_rules").([]interface{})), - SecretId: aws.String(secretID), - } + input := &secretsmanager.RotateSecretInput{ + RotationRules: expandRotationRules(d.Get("rotation_rules").([]interface{})), + SecretId: aws.String(secretID), + } - // AccessDeniedException: Secrets Manager cannot invoke the specified Lambda function. - outputRaw, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 1*time.Minute, func() (interface{}, error) { - return conn.RotateSecretWithContext(ctx, input) - }, "AccessDeniedException") + if v, ok := d.GetOk("rotation_lambda_arn"); ok { + input.RotationLambdaARN = aws.String(v.(string)) + } - if err != nil { - return sdkdiag.AppendErrorf(diags, "creating Secrets Manager Secret Rotation (%s): %s", secretID, err) - } + // AccessDeniedException: Secrets Manager cannot invoke the specified Lambda function. + outputRaw, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 1*time.Minute, func() (interface{}, error) { + return conn.RotateSecretWithContext(ctx, input) + }, "AccessDeniedException") - d.SetId(aws.StringValue(outputRaw.(*secretsmanager.RotateSecretOutput).ARN)) + if err != nil { + return sdkdiag.AppendErrorf(diags, "creating Secrets Manager Secret Rotation (%s): %s", secretID, err) } + d.SetId(aws.StringValue(outputRaw.(*secretsmanager.RotateSecretOutput).ARN)) + return append(diags, resourceSecretRotationRead(ctx, d, meta)...) } @@ -150,31 +151,22 @@ func resourceSecretRotationUpdate(ctx context.Context, d *schema.ResourceData, m secretID := d.Get("secret_id").(string) if d.HasChanges("rotation_lambda_arn", "rotation_rules") { - if v, ok := d.GetOk("rotation_lambda_arn"); ok && v.(string) != "" { - input := &secretsmanager.RotateSecretInput{ - RotationLambdaARN: aws.String(v.(string)), - RotationRules: expandRotationRules(d.Get("rotation_rules").([]interface{})), - SecretId: aws.String(secretID), - } - - // AccessDeniedException: Secrets Manager cannot invoke the specified Lambda function. - _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 1*time.Minute, func() (interface{}, error) { - return conn.RotateSecretWithContext(ctx, input) - }, "AccessDeniedException") - - if err != nil { - return sdkdiag.AppendErrorf(diags, "updating Secrets Manager Secret Rotation (%s): %s", d.Id(), err) - } - } else { - input := &secretsmanager.CancelRotateSecretInput{ - SecretId: aws.String(d.Id()), - } - - _, err := conn.CancelRotateSecretWithContext(ctx, input) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "cancelling Secrets Manager Secret Rotation (%s): %s", d.Id(), err) - } + input := &secretsmanager.RotateSecretInput{ + RotationRules: expandRotationRules(d.Get("rotation_rules").([]interface{})), + SecretId: aws.String(secretID), + } + + if v, ok := d.GetOk("rotation_lambda_arn"); ok { + input.RotationLambdaARN = aws.String(v.(string)) + } + + // AccessDeniedException: Secrets Manager cannot invoke the specified Lambda function. + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 1*time.Minute, func() (interface{}, error) { + return conn.RotateSecretWithContext(ctx, input) + }, "AccessDeniedException") + + if err != nil { + return sdkdiag.AppendErrorf(diags, "updating Secrets Manager Secret Rotation (%s): %s", d.Id(), err) } } diff --git a/website/docs/r/secretsmanager_secret_rotation.html.markdown b/website/docs/r/secretsmanager_secret_rotation.html.markdown index 0f101d7c11d..eb8189c8202 100644 --- a/website/docs/r/secretsmanager_secret_rotation.html.markdown +++ b/website/docs/r/secretsmanager_secret_rotation.html.markdown @@ -38,7 +38,7 @@ To enable automatic secret rotation, the Secrets Manager service requires usage This resource supports the following arguments: * `secret_id` - (Required) Specifies the secret to which you want to add a new version. You can specify either the Amazon Resource Name (ARN) or the friendly name of the secret. The secret must already exist. -* `rotation_lambda_arn` - (Required) Specifies the ARN of the Lambda function that can rotate the secret. +* `rotation_lambda_arn` - (Optional) Specifies the ARN of the Lambda function that can rotate the secret. Must be supplied if the secret is not managed by AWS. * `rotation_rules` - (Required) A structure that defines the rotation configuration for this secret. Defined below. ### rotation_rules From 0a76c42aa05dc89e4b21a086105a994fde1016c1 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 30 Oct 2023 16:42:17 -0400 Subject: [PATCH 2/3] r/aws_secretsmanager_secret_rotation(test): simplify test config --- .../secretsmanager/secret_rotation_test.go | 224 ++++++------------ 1 file changed, 68 insertions(+), 156 deletions(-) diff --git a/internal/service/secretsmanager/secret_rotation_test.go b/internal/service/secretsmanager/secret_rotation_test.go index 51beff9738d..b13328f2413 100644 --- a/internal/service/secretsmanager/secret_rotation_test.go +++ b/internal/service/secretsmanager/secret_rotation_test.go @@ -25,39 +25,24 @@ func TestAccSecretsManagerSecretRotation_basic(t *testing.T) { var secret secretsmanager.DescribeSecretOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_secretsmanager_secret_rotation.test" - lambdaFunctionResourceName := "aws_lambda_function.test1" - days01 := 7 + lambdaFunctionResourceName := "aws_lambda_function.test" + days := 7 resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, secretsmanager.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckSecretRotationDestroy(ctx), Steps: []resource.TestStep{ - // Test creating secret rotation resource { - Config: testAccSecretRotationConfig_basic(rName, days01), + Config: testAccSecretRotationConfig_basic(rName, days), Check: resource.ComposeTestCheckFunc( testAccCheckSecretRotationExists(ctx, resourceName, &secret), resource.TestCheckResourceAttr(resourceName, "rotation_enabled", "true"), resource.TestCheckResourceAttrPair(resourceName, "rotation_lambda_arn", lambdaFunctionResourceName, "arn"), resource.TestCheckResourceAttr(resourceName, "rotation_rules.#", "1"), - resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.automatically_after_days", strconv.Itoa(days01)), + resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.automatically_after_days", strconv.Itoa(days)), ), }, - // Test updating rotation - // We need a valid rotation function for this testing - // InvalidRequestException: A previous rotation isn’t complete. That rotation will be reattempted. - /* - { - Config: testAccSecretRotationConfig_managerUpdated(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckSecretRotationExists(resourceName, &secret), - resource.TestCheckResourceAttr(resourceName, "rotation_enabled", "true"), - resource.TestMatchResourceAttr(resourceName, "rotation_lambda_arn", regexache.MustCompile(fmt.Sprintf("^arn:[^:]+:lambda:[^:]+:[^:]+:function:%s-2$", rName))), - ), - }, - */ - // Test importing secret rotation { ResourceName: resourceName, ImportState: true, @@ -72,7 +57,7 @@ func TestAccSecretsManagerSecretRotation_scheduleExpression(t *testing.T) { var secret secretsmanager.DescribeSecretOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_secretsmanager_secret_rotation.test" - lambdaFunctionResourceName := "aws_lambda_function.test1" + lambdaFunctionResourceName := "aws_lambda_function.test" scheduleExpression := "rate(10 days)" scheduleExpression02 := "rate(10 days)" resource.ParallelTest(t, resource.TestCase{ @@ -81,7 +66,6 @@ func TestAccSecretsManagerSecretRotation_scheduleExpression(t *testing.T) { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckSecretRotationDestroy(ctx), Steps: []resource.TestStep{ - // Test creating secret rotation resource { Config: testAccSecretRotationConfig_scheduleExpression(rName, scheduleExpression), Check: resource.ComposeTestCheckFunc( @@ -102,7 +86,6 @@ func TestAccSecretsManagerSecretRotation_scheduleExpression(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.schedule_expression", scheduleExpression02), ), }, - // Test importing secret rotation { ResourceName: resourceName, ImportState: true, @@ -117,7 +100,7 @@ func TestAccSecretsManagerSecretRotation_scheduleExpressionHours(t *testing.T) { var secret secretsmanager.DescribeSecretOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_secretsmanager_secret_rotation.test" - lambdaFunctionResourceName := "aws_lambda_function.test1" + lambdaFunctionResourceName := "aws_lambda_function.test" scheduleExpression := "rate(6 hours)" scheduleExpression02 := "rate(10 hours)" resource.ParallelTest(t, resource.TestCase{ @@ -126,7 +109,6 @@ func TestAccSecretsManagerSecretRotation_scheduleExpressionHours(t *testing.T) { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckSecretRotationDestroy(ctx), Steps: []resource.TestStep{ - // Test creating secret rotation resource { Config: testAccSecretRotationConfig_scheduleExpression(rName, scheduleExpression), Check: resource.ComposeTestCheckFunc( @@ -148,7 +130,6 @@ func TestAccSecretsManagerSecretRotation_scheduleExpressionHours(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.schedule_expression", scheduleExpression02), ), }, - // Test importing secret rotation { ResourceName: resourceName, ImportState: true, @@ -163,8 +144,8 @@ func TestAccSecretsManagerSecretRotation_duration(t *testing.T) { var secret secretsmanager.DescribeSecretOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_secretsmanager_secret_rotation.test" - lambdaFunctionResourceName := "aws_lambda_function.test1" - days01 := 7 + lambdaFunctionResourceName := "aws_lambda_function.test" + days := 7 duration := "3h" resource.ParallelTest(t, resource.TestCase{ @@ -173,19 +154,17 @@ func TestAccSecretsManagerSecretRotation_duration(t *testing.T) { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckSecretRotationDestroy(ctx), Steps: []resource.TestStep{ - // Test creating secret rotation resource { - Config: testAccSecretRotationConfig_duration(rName, days01, duration), + Config: testAccSecretRotationConfig_duration(rName, days, duration), Check: resource.ComposeTestCheckFunc( testAccCheckSecretRotationExists(ctx, resourceName, &secret), resource.TestCheckResourceAttr(resourceName, "rotation_enabled", "true"), resource.TestCheckResourceAttrPair(resourceName, "rotation_lambda_arn", lambdaFunctionResourceName, "arn"), resource.TestCheckResourceAttr(resourceName, "rotation_rules.#", "1"), - resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.automatically_after_days", strconv.Itoa(days01)), + resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.automatically_after_days", strconv.Itoa(days)), resource.TestCheckResourceAttr(resourceName, "rotation_rules.0.duration", duration), ), }, - // Test importing secret rotation { ResourceName: resourceName, ImportState: true, @@ -254,10 +233,43 @@ func testAccCheckSecretRotationExists(ctx context.Context, n string, v *secretsm } } -func testAccSecretRotationConfig_basic(rName string, automaticallyAfterDays int) string { - return acctest.ConfigCompose(acctest.ConfigLambdaBase(rName, rName, rName), fmt.Sprintf(` +func testSecretValueIsCurrent(ctx context.Context, rName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).SecretsManagerConn(ctx) + // Write secret value to clear in-rotation state, otherwise updating the secret rotation + // will fail with "A previous rotation isn't complete. That rotation will be reattempted." + put_secret_input := &secretsmanager.PutSecretValueInput{ + SecretId: aws.String(rName), + SecretString: aws.String("secret-value"), + } + _, err := conn.PutSecretValueWithContext(ctx, put_secret_input) + if err != nil { + return err + } + input := &secretsmanager.DescribeSecretInput{ + SecretId: aws.String(rName), + } + output, err := conn.DescribeSecretWithContext(ctx, input) + if err != nil { + return err + } else { + // Ensure that the current version of the secret is in the AWSCURRENT stage + for _, stage := range output.VersionIdsToStages { + if *stage[0] == "AWSCURRENT" { + return nil + } else { + return fmt.Errorf("Secret version is not in AWSCURRENT stage: %s", *stage[0]) + } + } + return nil + } + } +} + +func testAccSecretRotationConfigBase(rName string) string { + return fmt.Sprintf(` # Not a real rotation function -resource "aws_lambda_function" "test1" { +resource "aws_lambda_function" "test" { filename = "test-fixtures/lambdatest.zip" function_name = "%[1]s-1" handler = "exports.example" @@ -265,178 +277,78 @@ resource "aws_lambda_function" "test1" { runtime = "nodejs16.x" } -resource "aws_lambda_permission" "test1" { +resource "aws_lambda_permission" "test" { action = "lambda:InvokeFunction" - function_name = aws_lambda_function.test1.function_name + function_name = aws_lambda_function.test.function_name principal = "secretsmanager.amazonaws.com" statement_id = "AllowExecutionFromSecretsManager1" } - -# Not a real rotation function -resource "aws_lambda_function" "test2" { - filename = "test-fixtures/lambdatest.zip" - function_name = "%[1]s-2" - handler = "exports.example" - role = aws_iam_role.iam_for_lambda.arn - runtime = "nodejs16.x" -} - -resource "aws_lambda_permission" "test2" { - action = "lambda:InvokeFunction" - function_name = aws_lambda_function.test2.function_name - principal = "secretsmanager.amazonaws.com" - statement_id = "AllowExecutionFromSecretsManager2" +`, rName) } +func testAccSecretRotationConfig_basic(rName string, automaticallyAfterDays int) string { + return acctest.ConfigCompose( + acctest.ConfigLambdaBase(rName, rName, rName), + testAccSecretRotationConfigBase(rName), + fmt.Sprintf(` resource "aws_secretsmanager_secret" "test" { name = %[1]q } resource "aws_secretsmanager_secret_rotation" "test" { secret_id = aws_secretsmanager_secret.test.id - rotation_lambda_arn = aws_lambda_function.test1.arn + rotation_lambda_arn = aws_lambda_function.test.arn rotation_rules { automatically_after_days = %[2]d } - depends_on = [aws_lambda_permission.test1] + depends_on = [aws_lambda_permission.test] } `, rName, automaticallyAfterDays)) } func testAccSecretRotationConfig_scheduleExpression(rName string, scheduleExpression string) string { - return acctest.ConfigCompose(acctest.ConfigLambdaBase(rName, rName, rName), fmt.Sprintf(` -# Not a real rotation function -resource "aws_lambda_function" "test1" { - filename = "test-fixtures/lambdatest.zip" - function_name = "%[1]s-1" - handler = "exports.example" - role = aws_iam_role.iam_for_lambda.arn - runtime = "nodejs16.x" -} - -resource "aws_lambda_permission" "test1" { - action = "lambda:InvokeFunction" - function_name = aws_lambda_function.test1.function_name - principal = "secretsmanager.amazonaws.com" - statement_id = "AllowExecutionFromSecretsManager1" -} - -# Not a real rotation function -resource "aws_lambda_function" "test2" { - filename = "test-fixtures/lambdatest.zip" - function_name = "%[1]s-2" - handler = "exports.example" - role = aws_iam_role.iam_for_lambda.arn - runtime = "nodejs16.x" -} - -resource "aws_lambda_permission" "test2" { - action = "lambda:InvokeFunction" - function_name = aws_lambda_function.test2.function_name - principal = "secretsmanager.amazonaws.com" - statement_id = "AllowExecutionFromSecretsManager2" -} - + return acctest.ConfigCompose( + acctest.ConfigLambdaBase(rName, rName, rName), + testAccSecretRotationConfigBase(rName), + fmt.Sprintf(` resource "aws_secretsmanager_secret" "test" { name = %[1]q } resource "aws_secretsmanager_secret_rotation" "test" { secret_id = aws_secretsmanager_secret.test.id - rotation_lambda_arn = aws_lambda_function.test1.arn + rotation_lambda_arn = aws_lambda_function.test.arn rotation_rules { schedule_expression = "%[2]s" } - depends_on = [aws_lambda_permission.test1] + depends_on = [aws_lambda_permission.test] } `, rName, scheduleExpression)) } func testAccSecretRotationConfig_duration(rName string, automaticallyAfterDays int, duration string) string { - return acctest.ConfigCompose(acctest.ConfigLambdaBase(rName, rName, rName), fmt.Sprintf(` -# Not a real rotation function -resource "aws_lambda_function" "test1" { - filename = "test-fixtures/lambdatest.zip" - function_name = "%[1]s-1" - handler = "exports.example" - role = aws_iam_role.iam_for_lambda.arn - runtime = "nodejs16.x" -} - -resource "aws_lambda_permission" "test1" { - action = "lambda:InvokeFunction" - function_name = aws_lambda_function.test1.function_name - principal = "secretsmanager.amazonaws.com" - statement_id = "AllowExecutionFromSecretsManager1" -} - -# Not a real rotation function -resource "aws_lambda_function" "test2" { - filename = "test-fixtures/lambdatest.zip" - function_name = "%[1]s-2" - handler = "exports.example" - role = aws_iam_role.iam_for_lambda.arn - runtime = "nodejs16.x" -} - -resource "aws_lambda_permission" "test2" { - action = "lambda:InvokeFunction" - function_name = aws_lambda_function.test2.function_name - principal = "secretsmanager.amazonaws.com" - statement_id = "AllowExecutionFromSecretsManager2" -} - + return acctest.ConfigCompose( + acctest.ConfigLambdaBase(rName, rName, rName), + testAccSecretRotationConfigBase(rName), + fmt.Sprintf(` resource "aws_secretsmanager_secret" "test" { name = %[1]q } resource "aws_secretsmanager_secret_rotation" "test" { secret_id = aws_secretsmanager_secret.test.id - rotation_lambda_arn = aws_lambda_function.test1.arn + rotation_lambda_arn = aws_lambda_function.test.arn rotation_rules { automatically_after_days = %[2]d duration = "%[3]s" } - depends_on = [aws_lambda_permission.test1] + depends_on = [aws_lambda_permission.test] } `, rName, automaticallyAfterDays, duration)) } - -func testSecretValueIsCurrent(ctx context.Context, rName string) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).SecretsManagerConn(ctx) - // Write secret value to clear in-rotation state, otherwise updating the secret rotation - // will fail with "A previous rotation isn't complete. That rotation will be reattempted." - put_secret_input := &secretsmanager.PutSecretValueInput{ - SecretId: aws.String(rName), - SecretString: aws.String("secret-value"), - } - _, err := conn.PutSecretValueWithContext(ctx, put_secret_input) - if err != nil { - return err - } - input := &secretsmanager.DescribeSecretInput{ - SecretId: aws.String(rName), - } - output, err := conn.DescribeSecretWithContext(ctx, input) - if err != nil { - return err - } else { - // Ensure that the current version of the secret is in the AWSCURRENT stage - for _, stage := range output.VersionIdsToStages { - if *stage[0] == "AWSCURRENT" { - return nil - } else { - return fmt.Errorf("Secret version is not in AWSCURRENT stage: %s", *stage[0]) - } - } - return nil - } - } -} From 7d9e281970f12de6221a17cd57777700d1cfb4e4 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Tue, 31 Oct 2023 14:04:50 -0400 Subject: [PATCH 3/3] r/aws_secretsmanager_secret_rotation: add arn validation to rotation_lambda_arn --- internal/service/secretsmanager/secret_rotation.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/service/secretsmanager/secret_rotation.go b/internal/service/secretsmanager/secret_rotation.go index aa04c2bdd7f..15ba3a5ae4c 100644 --- a/internal/service/secretsmanager/secret_rotation.go +++ b/internal/service/secretsmanager/secret_rotation.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/internal/verify" ) // @SDKResource("aws_secretsmanager_secret_rotation") @@ -37,8 +38,9 @@ func ResourceSecretRotation() *schema.Resource { Computed: true, }, "rotation_lambda_arn": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Optional: true, + ValidateFunc: verify.ValidARN, }, "rotation_rules": { Type: schema.TypeList,