From fc86bc1e03e71903291737640e234ee9f876c3a0 Mon Sep 17 00:00:00 2001 From: Anthony Wat Date: Fri, 1 Nov 2024 00:06:25 -0400 Subject: [PATCH 01/45] feat: Add description arg/attr for aws_cloudwatch_event_bus resource and data source --- .changelog/39980.txt | 6 +++ internal/service/events/bus.go | 19 ++++++++- internal/service/events/bus_data_source.go | 5 +++ .../service/events/bus_data_source_test.go | 2 + internal/service/events/bus_test.go | 39 ++++++++++++++++--- .../docs/d/cloudwatch_event_bus.html.markdown | 8 ++-- .../docs/r/cloudwatch_event_bus.html.markdown | 22 +++++++---- 7 files changed, 85 insertions(+), 16 deletions(-) create mode 100644 .changelog/39980.txt diff --git a/.changelog/39980.txt b/.changelog/39980.txt new file mode 100644 index 00000000000..5f700b35da9 --- /dev/null +++ b/.changelog/39980.txt @@ -0,0 +1,6 @@ +```release-note:enhancement +resource/aws_cloudwatch_event_bus: Add `description` argument +``` +```release-note:enhancement +data-source/aws_cloudwatch_event_bus: Add `description` attribute +``` \ No newline at end of file diff --git a/internal/service/events/bus.go b/internal/service/events/bus.go index be6e57c1d2a..0fab5e1873d 100644 --- a/internal/service/events/bus.go +++ b/internal/service/events/bus.go @@ -41,6 +41,11 @@ func resourceBus() *schema.Resource { Type: schema.TypeString, Computed: true, }, + names.AttrDescription: { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringLenBetween(0, 512), + }, "event_source_name": { Type: schema.TypeString, Optional: true, @@ -76,6 +81,10 @@ func resourceBusCreate(ctx context.Context, d *schema.ResourceData, meta interfa Tags: getTagsIn(ctx), } + if v, ok := d.GetOk(names.AttrDescription); ok { + input.Description = aws.String(v.(string)) + } + if v, ok := d.GetOk("event_source_name"); ok { input.EventSourceName = aws.String(v.(string)) } @@ -133,6 +142,7 @@ func resourceBusRead(ctx context.Context, d *schema.ResourceData, meta interface } d.Set(names.AttrARN, output.Arn) + d.Set(names.AttrDescription, output.Description) d.Set("kms_key_identifier", output.KmsKeyIdentifier) d.Set(names.AttrName, output.Name) @@ -143,11 +153,18 @@ func resourceBusUpdate(ctx context.Context, d *schema.ResourceData, meta interfa var diags diag.Diagnostics conn := meta.(*conns.AWSClient).EventsClient(ctx) - if d.HasChange("kms_key_identifier") { + if d.HasChanges(names.AttrDescription, "kms_key_identifier") { input := &eventbridge.UpdateEventBusInput{ Name: aws.String(d.Get(names.AttrName).(string)), } + // To unset the description, the only way is to explicitly set it to the empty string + if v, ok := d.GetOk(names.AttrDescription); ok { + input.Description = aws.String(v.(string)) + } else { + input.Description = aws.String("") + } + if v, ok := d.GetOk("kms_key_identifier"); ok { input.KmsKeyIdentifier = aws.String(v.(string)) } diff --git a/internal/service/events/bus_data_source.go b/internal/service/events/bus_data_source.go index f71ec0347b5..6cf4b03d359 100644 --- a/internal/service/events/bus_data_source.go +++ b/internal/service/events/bus_data_source.go @@ -23,6 +23,10 @@ func dataSourceBus() *schema.Resource { Type: schema.TypeString, Computed: true, }, + names.AttrDescription: { + Type: schema.TypeString, + Computed: true, + }, "kms_key_identifier": { Type: schema.TypeString, Computed: true, @@ -48,6 +52,7 @@ func dataSourceBusRead(ctx context.Context, d *schema.ResourceData, meta interfa d.SetId(eventBusName) d.Set(names.AttrARN, output.Arn) + d.Set(names.AttrDescription, output.Description) d.Set("kms_key_identifier", output.KmsKeyIdentifier) d.Set(names.AttrName, output.Name) diff --git a/internal/service/events/bus_data_source_test.go b/internal/service/events/bus_data_source_test.go index 9baad712d58..b7f210bbffe 100644 --- a/internal/service/events/bus_data_source_test.go +++ b/internal/service/events/bus_data_source_test.go @@ -49,6 +49,7 @@ func TestAccEventsBusDataSource_kmsKeyIdentifier(t *testing.T) { { Config: testAccBusDataSourceConfig_kmsKeyIdentifier(busName), Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttrPair(dataSourceName, names.AttrDescription, resourceName, names.AttrDescription), resource.TestCheckResourceAttrPair(dataSourceName, "kms_key_identifier", resourceName, "kms_key_identifier"), ), }, @@ -124,6 +125,7 @@ resource "aws_kms_key_policy" "test" { resource "aws_cloudwatch_event_bus" "test" { name = %[1]q + description = "Test event bus" kms_key_identifier = aws_kms_key.test.arn } diff --git a/internal/service/events/bus_test.go b/internal/service/events/bus_test.go index 16fd0f80d14..c7519fe6401 100644 --- a/internal/service/events/bus_test.go +++ b/internal/service/events/bus_test.go @@ -24,10 +24,11 @@ import ( func TestAccEventsBus_basic(t *testing.T) { ctx := acctest.Context(t) - var v1, v2, v3 eventbridge.DescribeEventBusOutput + var v1, v2, v3, v4, v5 eventbridge.DescribeEventBusOutput busName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) busNameModified := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_cloudwatch_event_bus.test" + description := "Test event bus" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -40,6 +41,7 @@ func TestAccEventsBus_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckBusExists(ctx, resourceName, &v1), acctest.CheckResourceAttrRegionalARN(resourceName, names.AttrARN, "events", fmt.Sprintf("event-bus/%s", busName)), + resource.TestCheckResourceAttr(resourceName, names.AttrDescription, ""), resource.TestCheckNoResourceAttr(resourceName, "event_source_name"), resource.TestCheckResourceAttr(resourceName, names.AttrName, busName), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"), @@ -51,11 +53,28 @@ func TestAccEventsBus_basic(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccBusConfig_basic(busNameModified), + Config: testAccBusConfig_description(busName, description), Check: resource.ComposeTestCheckFunc( testAccCheckBusExists(ctx, resourceName, &v2), - testAccCheckBusRecreated(&v1, &v2), + testAccCheckBusNotRecreated(&v1, &v2), + resource.TestCheckResourceAttr(resourceName, names.AttrDescription, description), + ), + }, + { + Config: testAccBusConfig_basic(busName), + Check: resource.ComposeTestCheckFunc( + testAccCheckBusExists(ctx, resourceName, &v3), + testAccCheckBusNotRecreated(&v2, &v3), + resource.TestCheckResourceAttr(resourceName, names.AttrDescription, ""), + ), + }, + { + Config: testAccBusConfig_basic(busNameModified), + Check: resource.ComposeTestCheckFunc( + testAccCheckBusExists(ctx, resourceName, &v4), + testAccCheckBusRecreated(&v3, &v4), acctest.CheckResourceAttrRegionalARN(resourceName, names.AttrARN, "events", fmt.Sprintf("event-bus/%s", busNameModified)), + resource.TestCheckResourceAttr(resourceName, names.AttrDescription, ""), resource.TestCheckNoResourceAttr(resourceName, "event_source_name"), resource.TestCheckResourceAttr(resourceName, names.AttrName, busNameModified), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"), @@ -64,8 +83,9 @@ func TestAccEventsBus_basic(t *testing.T) { { Config: testAccBusConfig_tags1(busNameModified, names.AttrKey, names.AttrValue), Check: resource.ComposeTestCheckFunc( - testAccCheckBusExists(ctx, resourceName, &v3), - testAccCheckBusNotRecreated(&v2, &v3), + testAccCheckBusExists(ctx, resourceName, &v5), + testAccCheckBusNotRecreated(&v4, &v5), + resource.TestCheckResourceAttr(resourceName, names.AttrDescription, ""), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "1"), resource.TestCheckResourceAttr(resourceName, "tags.key", names.AttrValue), ), @@ -302,6 +322,15 @@ resource "aws_cloudwatch_event_bus" "test" { `, name) } +func testAccBusConfig_description(name, description string) string { + return fmt.Sprintf(` +resource "aws_cloudwatch_event_bus" "test" { + name = %[1]q + description = %[2]q +} +`, name, description) +} + func testAccBusConfig_tags1(name, key, value string) string { return fmt.Sprintf(` resource "aws_cloudwatch_event_bus" "test" { diff --git a/website/docs/d/cloudwatch_event_bus.html.markdown b/website/docs/d/cloudwatch_event_bus.html.markdown index 3a804c6ad5d..b844acae3e0 100644 --- a/website/docs/d/cloudwatch_event_bus.html.markdown +++ b/website/docs/d/cloudwatch_event_bus.html.markdown @@ -22,11 +22,13 @@ data "aws_cloudwatch_event_bus" "example" { ## Argument Reference -* `name` - (Required) Friendly EventBridge event bus name. +* `name` - (Required) Name of the event bus. ## Attribute Reference This data source exports the following attributes in addition to the arguments above: -* `arn` - ARN. -* `kms_key_identifier` - The identifier of the AWS KMS customer managed key for EventBridge to use to encrypt events on this event bus, if one has been specified. +* `arn` - ARN of the event bus. +* `description` - Event bus description. +* `id` - Name of the event bus. +* `kms_key_identifier` - Identifier of the AWS KMS customer managed key for EventBridge to use to encrypt events on this event bus, if one has been specified. diff --git a/website/docs/r/cloudwatch_event_bus.html.markdown b/website/docs/r/cloudwatch_event_bus.html.markdown index cefcf691583..8760cb0fcb1 100644 --- a/website/docs/r/cloudwatch_event_bus.html.markdown +++ b/website/docs/r/cloudwatch_event_bus.html.markdown @@ -27,6 +27,7 @@ data "aws_cloudwatch_event_source" "examplepartner" { resource "aws_cloudwatch_event_bus" "examplepartner" { name = data.aws_cloudwatch_event_source.examplepartner.name + description = "Event bus for example partner events" event_source_name = data.aws_cloudwatch_event_source.examplepartner.name } ``` @@ -35,17 +36,24 @@ resource "aws_cloudwatch_event_bus" "examplepartner" { This resource supports the following arguments: -* `name` - (Required) The name of the new event bus. The names of custom event buses can't contain the / character. To create a partner event bus, ensure the `name` matches the `event_source_name`. -* `event_source_name` - (Optional) The partner event source that the new event bus will be matched with. Must match `name`. -* `kms_key_identifier` - (Optional) The identifier of the AWS KMS customer managed key for EventBridge to use, if you choose to use a customer managed key to encrypt events on this event bus. The identifier can be the key Amazon Resource Name (ARN), KeyId, key alias, or key alias ARN. -* `tags` - (Optional) A map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. +The following arguments are required: + +* `name` - (Required) Name of the new event bus. The names of custom event buses can't contain the / character. To create a partner event bus, ensure that the `name` matches the `event_source_name`. + +The following arguments are optional: + +* `description` - (Optional) Event bus description. +* `event_source_name` - (Optional) Partner event source that the new event bus will be matched with. Must match `name`. +* `kms_key_identifier` - (Optional) Identifier of the AWS KMS customer managed key for EventBridge to use, if you choose to use a customer managed key to encrypt events on this event bus. The identifier can be the key Amazon Resource Name (ARN), KeyId, key alias, or key alias ARN. +* `tags` - (Optional) Map of tags assigned to the resource. If configured with a provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. ## Attribute Reference This resource exports the following attributes in addition to the arguments above: -* `arn` - The Amazon Resource Name (ARN) of the event bus. -* `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block). +* `arn` - ARN of the event bus. +* `id` - Name of the event bus. +* `tags_all` - Map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block). ## Import @@ -58,7 +66,7 @@ import { } ``` -Using `terraform import`, import EventBridge event buses using the `name` (which can also be a partner event source name). For example: +Using `terraform import`, import EventBridge event buses using the name of the event bus (which can also be a partner event source name). For example: ```console % terraform import aws_cloudwatch_event_bus.messenger chat-messages From 9184c10109b2394944f3391b6d669570b477c641 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 1 Nov 2024 14:08:44 -0700 Subject: [PATCH 02/45] Correctly resets Account on delete --- internal/service/apigateway/account.go | 29 ++++++++++++++--- internal/service/apigateway/account_test.go | 32 ++++++++++++++++++- internal/service/apigateway/exports_test.go | 1 + .../docs/r/api_gateway_account.html.markdown | 2 -- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/internal/service/apigateway/account.go b/internal/service/apigateway/account.go index 329d6d6a2e5..39377a5f3f1 100644 --- a/internal/service/apigateway/account.go +++ b/internal/service/apigateway/account.go @@ -25,7 +25,7 @@ func resourceAccount() *schema.Resource { CreateWithoutTimeout: resourceAccountUpdate, ReadWithoutTimeout: resourceAccountRead, UpdateWithoutTimeout: resourceAccountUpdate, - DeleteWithoutTimeout: schema.NoopContext, + DeleteWithoutTimeout: resourceAccountDelete, Importer: &schema.ResourceImporter{ StateContext: schema.ImportStatePassthroughContext, @@ -66,7 +66,7 @@ func resourceAccount() *schema.Resource { } } -func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).APIGatewayClient(ctx) @@ -82,12 +82,12 @@ func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta int input.PatchOperations = []types.PatchOperation{{ Op: types.OpReplace, Path: aws.String("/cloudwatchRoleArn"), - Value: aws.String(""), + Value: nil, }} } _, err := tfresource.RetryWhen(ctx, propagationTimeout, - func() (interface{}, error) { + func() (any, error) { return conn.UpdateAccount(ctx, input) }, func(err error) (bool, error) { @@ -114,7 +114,7 @@ func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta int return append(diags, resourceAccountRead(ctx, d, meta)...) } -func resourceAccountRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func resourceAccountRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).APIGatewayClient(ctx) @@ -140,6 +140,25 @@ func resourceAccountRead(ctx context.Context, d *schema.ResourceData, meta inter return diags } +func resourceAccountDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).APIGatewayClient(ctx) + + input := &apigateway.UpdateAccountInput{} + + input.PatchOperations = []types.PatchOperation{{ + Op: types.OpReplace, + Path: aws.String("/cloudwatchRoleArn"), + Value: nil, + }} + + _, err := conn.UpdateAccount(ctx, input) + if err != nil { + return sdkdiag.AppendErrorf(diags, "resetting API Gateway Account: %s", err) + } + return diags +} + func findAccount(ctx context.Context, conn *apigateway.Client) (*apigateway.GetAccountOutput, error) { input := &apigateway.GetAccountInput{} diff --git a/internal/service/apigateway/account_test.go b/internal/service/apigateway/account_test.go index 2e81280fb17..096f2a5c8c9 100644 --- a/internal/service/apigateway/account_test.go +++ b/internal/service/apigateway/account_test.go @@ -4,12 +4,16 @@ package apigateway_test import ( + "context" "fmt" "testing" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" + "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfapigateway "github.com/hashicorp/terraform-provider-aws/internal/service/apigateway" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -22,7 +26,7 @@ func testAccAccount_basic(t *testing.T) { PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: acctest.CheckDestroyNoop, + CheckDestroy: testAccCheckAccountDestroy(ctx), Steps: []resource.TestStep{ { Config: testAccAccountConfig_role0(rName), @@ -56,6 +60,32 @@ func testAccAccount_basic(t *testing.T) { }) } +func testAccCheckAccountDestroy(ctx context.Context) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).APIGatewayClient(ctx) + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_api_gateway_stage" { + continue + } + + account, err := tfapigateway.FindAccount(ctx, conn) + if err != nil { + return err + } + + if account.CloudwatchRoleArn == nil { + // Settings have been reset + continue + } + + return fmt.Errorf("API Gateway Stage %s still exists", rs.Primary.ID) + } + + return nil + } +} + const testAccAccountConfig_empty = ` resource "aws_api_gateway_account" "test" {} ` diff --git a/internal/service/apigateway/exports_test.go b/internal/service/apigateway/exports_test.go index a0ad73271ce..185fe263672 100644 --- a/internal/service/apigateway/exports_test.go +++ b/internal/service/apigateway/exports_test.go @@ -31,6 +31,7 @@ var ( ResourceVPCLink = resourceVPCLink DefaultAuthorizerTTL = defaultAuthorizerTTL + FindAccount = findAccount FindAPIKeyByID = findAPIKeyByID FindAuthorizerByTwoPartKey = findAuthorizerByTwoPartKey FindBasePathMappingByTwoPartKey = findBasePathMappingByTwoPartKey diff --git a/website/docs/r/api_gateway_account.html.markdown b/website/docs/r/api_gateway_account.html.markdown index 010b038fa65..baf47364aa5 100644 --- a/website/docs/r/api_gateway_account.html.markdown +++ b/website/docs/r/api_gateway_account.html.markdown @@ -10,8 +10,6 @@ description: |- Provides a settings of an API Gateway Account. Settings is applied region-wide per `provider` block. --> **Note:** As there is no API method for deleting account settings or resetting it to defaults, destroying this resource will keep your account settings intact - ## Example Usage ```terraform From 01882e5386e7d17e81328224df877a00462fc079 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 1 Nov 2024 14:17:42 -0700 Subject: [PATCH 03/45] Adds sweeper for `aws_api_gateway_account` --- internal/service/apigateway/sweep.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/service/apigateway/sweep.go b/internal/service/apigateway/sweep.go index 7501f7eeeda..fffa5e0932b 100644 --- a/internal/service/apigateway/sweep.go +++ b/internal/service/apigateway/sweep.go @@ -4,6 +4,7 @@ package apigateway import ( + "context" "fmt" "log" @@ -11,11 +12,17 @@ import ( "github.com/aws/aws-sdk-go-v2/service/apigateway" "github.com/aws/aws-sdk-go-v2/service/apigateway/types" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/sweep" "github.com/hashicorp/terraform-provider-aws/internal/sweep/awsv2" + "github.com/hashicorp/terraform-provider-aws/internal/sweep/sdk" ) func RegisterSweepers() { + awsv2.Register("aws_api_gateway_account", sweepAccounts, + "aws_api_gateway_rest_api", + ) + resource.AddTestSweepers("aws_api_gateway_rest_api", &resource.Sweeper{ Name: "aws_api_gateway_rest_api", F: sweepRestAPIs, @@ -50,6 +57,15 @@ func RegisterSweepers() { }) } +func sweepAccounts(ctx context.Context, client *conns.AWSClient) ([]sweep.Sweepable, error) { + r := resourceAccount() + d := r.Data(nil) + + return []sweep.Sweepable{ + sdk.NewSweepResource(r, d, client), + }, nil +} + func sweepRestAPIs(region string) error { ctx := sweep.Context(region) client, err := sweep.SharedRegionalSweepClient(ctx, region) From 7a8e24392e17557a84b469540e9128b339153046 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 4 Nov 2024 10:36:26 -0800 Subject: [PATCH 04/45] Adds CHANGELOG entry --- .changelog/40004.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/40004.txt diff --git a/.changelog/40004.txt b/.changelog/40004.txt new file mode 100644 index 00000000000..31da035ab3f --- /dev/null +++ b/.changelog/40004.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_api_gateway_account: Supports unregistering account settings. +``` From 186a88b0833328edd22efb98bcbb7394718ffc2e Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 5 Nov 2024 12:20:02 -0800 Subject: [PATCH 05/45] Fix `tfsdk2fw` --- tools/tfsdk2fw/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/tfsdk2fw/main.go b/tools/tfsdk2fw/main.go index 37c55c5f895..b9c99be197a 100644 --- a/tools/tfsdk2fw/main.go +++ b/tools/tfsdk2fw/main.go @@ -11,7 +11,6 @@ import ( "io" "os" "path" - "sort" "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" From 0046e47ecebd295d0a007a55e974bf5df7552d02 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 5 Nov 2024 13:18:36 -0800 Subject: [PATCH 06/45] Improves `aws_api_gateway_account` tests --- internal/service/apigateway/account_test.go | 83 +++++++++++++++---- .../service/apigateway/apigateway_test.go | 3 +- 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/internal/service/apigateway/account_test.go b/internal/service/apigateway/account_test.go index 096f2a5c8c9..762928a5f3b 100644 --- a/internal/service/apigateway/account_test.go +++ b/internal/service/apigateway/account_test.go @@ -8,9 +8,13 @@ import ( "fmt" "testing" + "github.com/hashicorp/terraform-plugin-testing/compare" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/statecheck" "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfapigateway "github.com/hashicorp/terraform-provider-aws/internal/service/apigateway" @@ -18,6 +22,40 @@ import ( ) func testAccAccount_basic(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_api_gateway_account.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckAccountDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccAccountConfig_basic, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("api_key_version"), knownvalue.NotNull()), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("cloudwatch_role_arn"), knownvalue.StringExact("")), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("features"), knownvalue.NotNull()), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrID), knownvalue.StringExact("api-gateway-account")), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("throttle_settings"), knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "burst_limit": knownvalue.Int32Exact(5000), + "rate_limit": knownvalue.Float64Exact(10000), + }), + })), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccAccount_cloudwatchRoleARN(t *testing.T) { ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_account.test" @@ -30,12 +68,13 @@ func testAccAccount_basic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccAccountConfig_role0(rName), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrPair(resourceName, "cloudwatch_role_arn", "aws_iam_role.test.0", names.AttrARN), - resource.TestCheckResourceAttr(resourceName, "throttle_settings.#", "1"), - resource.TestCheckResourceAttrSet(resourceName, "api_key_version"), - resource.TestCheckResourceAttrSet(resourceName, "features.#"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.CompareValuePairs( + resourceName, tfjsonpath.New("cloudwatch_role_arn"), + "aws_iam_role.test[0]", tfjsonpath.New(names.AttrARN), + compare.ValuesSame(), + ), + }, }, { ResourceName: resourceName, @@ -44,17 +83,29 @@ func testAccAccount_basic(t *testing.T) { }, { Config: testAccAccountConfig_role1(rName), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrPair(resourceName, "cloudwatch_role_arn", "aws_iam_role.test.1", names.AttrARN), - resource.TestCheckResourceAttr(resourceName, "throttle_settings.#", "1"), - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.CompareValuePairs( + resourceName, tfjsonpath.New("cloudwatch_role_arn"), + "aws_iam_role.test[1]", tfjsonpath.New(names.AttrARN), + compare.ValuesSame(), + ), + }, }, { - Config: testAccAccountConfig_empty, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "cloudwatch_role_arn", ""), - resource.TestCheckResourceAttr(resourceName, "throttle_settings.#", "1"), - ), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAccountConfig_basic, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("cloudwatch_role_arn"), knownvalue.StringExact("")), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, }, }, }) @@ -86,7 +137,7 @@ func testAccCheckAccountDestroy(ctx context.Context) resource.TestCheckFunc { } } -const testAccAccountConfig_empty = ` +const testAccAccountConfig_basic = ` resource "aws_api_gateway_account" "test" {} ` diff --git a/internal/service/apigateway/apigateway_test.go b/internal/service/apigateway/apigateway_test.go index f34ce15901e..79b78501e57 100644 --- a/internal/service/apigateway/apigateway_test.go +++ b/internal/service/apigateway/apigateway_test.go @@ -25,7 +25,8 @@ func testAccErrorCheckSkip(t *testing.T) resource.ErrorCheckFunc { func TestAccAPIGateway_serial(t *testing.T) { testCases := map[string]map[string]func(t *testing.T){ "Account": { - acctest.CtBasic: testAccAccount_basic, + acctest.CtBasic: testAccAccount_basic, + "CloudwatchRoleARN": testAccAccount_cloudwatchRoleARN, }, // Some aws_api_gateway_method_settings tests require the account-level CloudWatch Logs role ARN to be set. // Serialize all this resource's acceptance tests. From 3920881def31e2cefd96594e2d1dfbe4b54dc122 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 6 Nov 2024 11:57:04 -0800 Subject: [PATCH 07/45] Converts `aws_api_gateway_account` to Framework --- internal/framework/flex/auto_flatten.go | 2 + internal/service/apigateway/account.go | 284 ++++++++++++------ internal/service/apigateway/account_test.go | 98 +++++- .../service/apigateway/apigateway_test.go | 7 +- internal/service/apigateway/exports_test.go | 1 - .../service/apigateway/service_package_gen.go | 11 +- internal/service/apigateway/sweep.go | 7 +- 7 files changed, 305 insertions(+), 105 deletions(-) diff --git a/internal/framework/flex/auto_flatten.go b/internal/framework/flex/auto_flatten.go index 0e7c346120e..2bc9cd4bc55 100644 --- a/internal/framework/flex/auto_flatten.go +++ b/internal/framework/flex/auto_flatten.go @@ -684,6 +684,8 @@ func (flattener autoFlattener) struct_(ctx context.Context, sourcePath path.Path return diags } + tflog.SubsystemError(ctx, subsystemName, "Flattening incompatible types") + return diags } diff --git a/internal/service/apigateway/account.go b/internal/service/apigateway/account.go index 39377a5f3f1..149480d8ec3 100644 --- a/internal/service/apigateway/account.go +++ b/internal/service/apigateway/account.go @@ -5,158 +5,262 @@ package apigateway import ( "context" - "log" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/apigateway" - "github.com/aws/aws-sdk-go-v2/service/apigateway/types" - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-provider-aws/internal/conns" + awstypes "github.com/aws/aws-sdk-go-v2/service/apigateway/types" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-provider-aws/internal/errs" - "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + "github.com/hashicorp/terraform-provider-aws/internal/errs/fwdiag" + "github.com/hashicorp/terraform-provider-aws/internal/framework" + "github.com/hashicorp/terraform-provider-aws/internal/framework/flex" + fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" + "github.com/hashicorp/terraform-provider-aws/internal/framework/validators" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" - "github.com/hashicorp/terraform-provider-aws/internal/verify" + "github.com/hashicorp/terraform-provider-aws/names" ) -// @SDKResource("aws_api_gateway_account", name="Account") -func resourceAccount() *schema.Resource { - return &schema.Resource{ - CreateWithoutTimeout: resourceAccountUpdate, - ReadWithoutTimeout: resourceAccountRead, - UpdateWithoutTimeout: resourceAccountUpdate, - DeleteWithoutTimeout: resourceAccountDelete, +// @FrameworkResource("aws_api_gateway_account") +func newResourceAccount(context.Context) (resource.ResourceWithConfigure, error) { + r := &resourceAccount{} - Importer: &schema.ResourceImporter{ - StateContext: schema.ImportStatePassthroughContext, - }, + return r, nil +} + +type resourceAccount struct { + framework.ResourceWithConfigure +} + +func (r *resourceAccount) Metadata(_ context.Context, request resource.MetadataRequest, response *resource.MetadataResponse) { + response.TypeName = "aws_api_gateway_account" +} - Schema: map[string]*schema.Schema{ - "api_key_version": { - Type: schema.TypeString, +func (r *resourceAccount) Schema(ctx context.Context, request resource.SchemaRequest, response *resource.SchemaResponse) { + s := schema.Schema{ + Attributes: map[string]schema.Attribute{ + "api_key_version": schema.StringAttribute{ Computed: true, }, - "cloudwatch_role_arn": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: verify.ValidARN, - }, - "features": { - Type: schema.TypeSet, - Elem: &schema.Schema{Type: schema.TypeString}, + "cloudwatch_role_arn": schema.StringAttribute{ + Optional: true, Computed: true, + Validators: []validator.String{ + stringvalidator.Any( + validators.ARN(), + stringvalidator.OneOf(""), + ), + }, + Default: stringdefault.StaticString(""), // Needed for backwards compatibility with SDK resource }, - "throttle_settings": { - Type: schema.TypeList, - Computed: true, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "burst_limit": { - Type: schema.TypeInt, - Computed: true, - }, - "rate_limit": { - Type: schema.TypeFloat, - Computed: true, - }, - }, + "features": schema.SetAttribute{ + ElementType: types.StringType, + Computed: true, + }, + names.AttrID: schema.StringAttribute{ + Computed: true, + DeprecationMessage: "This attribute will be removed in a future version.", + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), }, }, + "throttle_settings": framework.DataSourceComputedListOfObjectAttribute[throttleSettingsModel](ctx), }, } + + response.Schema = s } -func resourceAccountUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - var diags diag.Diagnostics - conn := meta.(*conns.AWSClient).APIGatewayClient(ctx) +func (r *resourceAccount) Create(ctx context.Context, request resource.CreateRequest, response *resource.CreateResponse) { + var data resourceAccountModel + response.Diagnostics.Append(request.Plan.Get(ctx, &data)...) + if response.Diagnostics.HasError() { + return + } + + conn := r.Meta().APIGatewayClient(ctx) input := &apigateway.UpdateAccountInput{} - if v, ok := d.GetOk("cloudwatch_role_arn"); ok { - input.PatchOperations = []types.PatchOperation{{ - Op: types.OpReplace, - Path: aws.String("/cloudwatchRoleArn"), - Value: aws.String(v.(string)), - }} + if data.CloudwatchRoleARN.IsNull() || data.CloudwatchRoleARN.ValueString() == "" { + input.PatchOperations = []awstypes.PatchOperation{ + { + Op: awstypes.OpReplace, + Path: aws.String("/cloudwatchRoleArn"), + Value: nil, + }, + } } else { - input.PatchOperations = []types.PatchOperation{{ - Op: types.OpReplace, - Path: aws.String("/cloudwatchRoleArn"), - Value: nil, - }} + input.PatchOperations = []awstypes.PatchOperation{ + { + Op: awstypes.OpReplace, + Path: aws.String("/cloudwatchRoleArn"), + Value: aws.String(data.CloudwatchRoleARN.ValueString()), + }, + } } - _, err := tfresource.RetryWhen(ctx, propagationTimeout, - func() (any, error) { + output, err := tfresource.RetryGWhen(ctx, propagationTimeout, + func() (*apigateway.UpdateAccountOutput, error) { return conn.UpdateAccount(ctx, input) }, func(err error) (bool, error) { - if errs.IsAErrorMessageContains[*types.BadRequestException](err, "The role ARN does not have required permissions") { + if errs.IsAErrorMessageContains[*awstypes.BadRequestException](err, "The role ARN does not have required permissions") { return true, err } - - if errs.IsAErrorMessageContains[*types.BadRequestException](err, "API Gateway could not successfully write to CloudWatch Logs using the ARN specified") { + if errs.IsAErrorMessageContains[*awstypes.BadRequestException](err, "API Gateway could not successfully write to CloudWatch Logs using the ARN specified") { return true, err } - return false, err }, ) - if err != nil { - return sdkdiag.AppendErrorf(diags, "updating API Gateway Account: %s", err) + response.Diagnostics.AddError("creating API Gateway Account", err.Error()) + return } - if d.IsNewResource() { - d.SetId("api-gateway-account") - } + response.Diagnostics.Append(flex.Flatten(ctx, output, &data)...) + data.ID = types.StringValue("api-gateway-account") - return append(diags, resourceAccountRead(ctx, d, meta)...) + response.Diagnostics.Append(response.State.Set(ctx, &data)...) } -func resourceAccountRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - var diags diag.Diagnostics - conn := meta.(*conns.AWSClient).APIGatewayClient(ctx) +func (r *resourceAccount) Read(ctx context.Context, request resource.ReadRequest, response *resource.ReadResponse) { + var data resourceAccountModel + response.Diagnostics.Append(request.State.Get(ctx, &data)...) + if response.Diagnostics.HasError() { + return + } + + conn := r.Meta().APIGatewayClient(ctx) account, err := findAccount(ctx, conn) + if tfresource.NotFound(err) { + response.Diagnostics.Append(fwdiag.NewResourceNotFoundWarningDiagnostic(err)) + response.State.RemoveResource(ctx) + return + } + + response.Diagnostics.Append(flex.Flatten(ctx, account, &data)...) - if !d.IsNewResource() && tfresource.NotFound(err) { - log.Printf("[WARN] API Gateway Account (%s) not found, removing from state", d.Id()) - d.SetId("") - return diags + response.Diagnostics.Append(response.State.Set(ctx, &data)...) +} + +func (r *resourceAccount) Update(ctx context.Context, request resource.UpdateRequest, response *resource.UpdateResponse) { + var state, plan resourceAccountModel + response.Diagnostics.Append(request.State.Get(ctx, &state)...) + if response.Diagnostics.HasError() { + return } - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading API Gateway Account: %s", err) + response.Diagnostics.Append(request.Plan.Get(ctx, &plan)...) + if response.Diagnostics.HasError() { + return } - d.Set("api_key_version", account.ApiKeyVersion) - d.Set("cloudwatch_role_arn", account.CloudwatchRoleArn) - d.Set("features", account.Features) - if err := d.Set("throttle_settings", flattenThrottleSettings(account.ThrottleSettings)); err != nil { - return sdkdiag.AppendErrorf(diags, "setting throttle_settings: %s", err) + diff, d := flex.Calculate(ctx, plan, state) + response.Diagnostics.Append(d...) + if response.Diagnostics.HasError() { + return } - return diags + if diff.HasChanges() { + conn := r.Meta().APIGatewayClient(ctx) + + input := &apigateway.UpdateAccountInput{} + + if plan.CloudwatchRoleARN.IsNull() || plan.CloudwatchRoleARN.ValueString() == "" { + input.PatchOperations = []awstypes.PatchOperation{ + { + Op: awstypes.OpReplace, + Path: aws.String("/cloudwatchRoleArn"), + Value: nil, + }, + } + } else { + input.PatchOperations = []awstypes.PatchOperation{ + { + Op: awstypes.OpReplace, + Path: aws.String("/cloudwatchRoleArn"), + Value: aws.String(plan.CloudwatchRoleARN.ValueString()), + }, + } + } + + output, err := tfresource.RetryGWhen(ctx, propagationTimeout, + func() (*apigateway.UpdateAccountOutput, error) { + return conn.UpdateAccount(ctx, input) + }, + func(err error) (bool, error) { + if errs.IsAErrorMessageContains[*awstypes.BadRequestException](err, "The role ARN does not have required permissions") { + return true, err + } + if errs.IsAErrorMessageContains[*awstypes.BadRequestException](err, "API Gateway could not successfully write to CloudWatch Logs using the ARN specified") { + return true, err + } + return false, err + }, + ) + if err != nil { + response.Diagnostics.AddError("updating API Gateway Account", err.Error()) + return + } + + response.Diagnostics.Append(flex.Flatten(ctx, output, &plan)...) + } + + response.Diagnostics.Append(response.State.Set(ctx, &plan)...) } -func resourceAccountDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - var diags diag.Diagnostics - conn := meta.(*conns.AWSClient).APIGatewayClient(ctx) +func (r *resourceAccount) Delete(ctx context.Context, request resource.DeleteRequest, response *resource.DeleteResponse) { + var data resourceAccountModel + response.Diagnostics.Append(request.State.Get(ctx, &data)...) + if response.Diagnostics.HasError() { + return + } + + conn := r.Meta().APIGatewayClient(ctx) input := &apigateway.UpdateAccountInput{} - input.PatchOperations = []types.PatchOperation{{ - Op: types.OpReplace, + input.PatchOperations = []awstypes.PatchOperation{{ + Op: awstypes.OpReplace, Path: aws.String("/cloudwatchRoleArn"), Value: nil, }} _, err := conn.UpdateAccount(ctx, input) if err != nil { - return sdkdiag.AppendErrorf(diags, "resetting API Gateway Account: %s", err) + response.Diagnostics.AddError("resetting API Gateway Account", err.Error()) } - return diags +} + +// ImportState is called when the provider must import the state of a resource instance. +// This method must return enough state so the Read method can properly refresh the full resource. +// +// If setting an attribute with the import identifier, it is recommended to use the ImportStatePassthroughID() call in this method. +func (r *resourceAccount) ImportState(ctx context.Context, request resource.ImportStateRequest, response *resource.ImportStateResponse) { + resource.ImportStatePassthroughID(ctx, path.Root("id"), request, response) +} + +type resourceAccountModel struct { + ApiKeyVersion types.String `tfsdk:"api_key_version"` + CloudwatchRoleARN types.String `tfsdk:"cloudwatch_role_arn" autoflex:",legacy"` + Features types.Set `tfsdk:"features"` + ID types.String `tfsdk:"id"` + ThrottleSettings fwtypes.ListNestedObjectValueOf[throttleSettingsModel] `tfsdk:"throttle_settings"` +} + +type throttleSettingsModel struct { + BurstLimit types.Int32 `tfsdk:"burst_limit"` + RateLimit types.Float64 `tfsdk:"rate_limit"` } func findAccount(ctx context.Context, conn *apigateway.Client) (*apigateway.GetAccountOutput, error) { diff --git a/internal/service/apigateway/account_test.go b/internal/service/apigateway/account_test.go index 762928a5f3b..fc11ef64f42 100644 --- a/internal/service/apigateway/account_test.go +++ b/internal/service/apigateway/account_test.go @@ -55,7 +55,7 @@ func testAccAccount_basic(t *testing.T) { }) } -func testAccAccount_cloudwatchRoleARN(t *testing.T) { +func testAccAccount_cloudwatchRoleARN_value(t *testing.T) { ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_account.test" @@ -111,6 +111,96 @@ func testAccAccount_cloudwatchRoleARN(t *testing.T) { }) } +func testAccAccount_cloudwatchRoleARN_empty(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_api_gateway_account.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckAccountDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccAccountConfig_empty, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("cloudwatch_role_arn"), knownvalue.StringExact("")), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccAccount_frameworkMigration_basic(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_api_gateway_account.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), + CheckDestroy: testAccCheckAccountDestroy(ctx), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "aws": { + Source: "hashicorp/aws", + VersionConstraint: "5.74.0", + }, + }, + Config: testAccAccountConfig_basic, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("cloudwatch_role_arn"), knownvalue.StringExact("")), + }, + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testAccAccountConfig_basic, + PlanOnly: true, + }, + }, + }) +} + +func testAccAccount_frameworkMigration_cloudwatchRoleARN(t *testing.T) { + ctx := acctest.Context(t) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_api_gateway_account.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), + CheckDestroy: testAccCheckAccountDestroy(ctx), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "aws": { + Source: "hashicorp/aws", + VersionConstraint: "5.74.0", + }, + }, + Config: testAccAccountConfig_role0(rName), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.CompareValuePairs( + resourceName, tfjsonpath.New("cloudwatch_role_arn"), + "aws_iam_role.test[0]", tfjsonpath.New(names.AttrARN), + compare.ValuesSame(), + ), + }, + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testAccAccountConfig_role0(rName), + PlanOnly: true, + }, + }, + }) +} + func testAccCheckAccountDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).APIGatewayClient(ctx) @@ -181,3 +271,9 @@ resource "aws_api_gateway_account" "test" { } `) } + +const testAccAccountConfig_empty = ` +resource "aws_api_gateway_account" "test" { + cloudwatch_role_arn = "" +} +` diff --git a/internal/service/apigateway/apigateway_test.go b/internal/service/apigateway/apigateway_test.go index 79b78501e57..1ca22b81abb 100644 --- a/internal/service/apigateway/apigateway_test.go +++ b/internal/service/apigateway/apigateway_test.go @@ -25,8 +25,11 @@ func testAccErrorCheckSkip(t *testing.T) resource.ErrorCheckFunc { func TestAccAPIGateway_serial(t *testing.T) { testCases := map[string]map[string]func(t *testing.T){ "Account": { - acctest.CtBasic: testAccAccount_basic, - "CloudwatchRoleARN": testAccAccount_cloudwatchRoleARN, + acctest.CtBasic: testAccAccount_basic, + "CloudwatchRoleARN_Value": testAccAccount_cloudwatchRoleARN_value, + "CloudwatchRoleARN_Empty": testAccAccount_cloudwatchRoleARN_empty, + "FrameworkMigration_Basic": testAccAccount_frameworkMigration_basic, + "FrameworkMigration_CloudwatchRoleARN": testAccAccount_frameworkMigration_cloudwatchRoleARN, }, // Some aws_api_gateway_method_settings tests require the account-level CloudWatch Logs role ARN to be set. // Serialize all this resource's acceptance tests. diff --git a/internal/service/apigateway/exports_test.go b/internal/service/apigateway/exports_test.go index 185fe263672..8933b86d518 100644 --- a/internal/service/apigateway/exports_test.go +++ b/internal/service/apigateway/exports_test.go @@ -5,7 +5,6 @@ package apigateway // Exports for use in tests only. var ( - ResourceAccount = resourceAccount ResourceAPIKey = resourceAPIKey ResourceAuthorizer = resourceAuthorizer ResourceBasePathMapping = resourceBasePathMapping diff --git a/internal/service/apigateway/service_package_gen.go b/internal/service/apigateway/service_package_gen.go index 0c9dbd98479..ae94df64315 100644 --- a/internal/service/apigateway/service_package_gen.go +++ b/internal/service/apigateway/service_package_gen.go @@ -17,7 +17,11 @@ func (p *servicePackage) FrameworkDataSources(ctx context.Context) []*types.Serv } func (p *servicePackage) FrameworkResources(ctx context.Context) []*types.ServicePackageFrameworkResource { - return []*types.ServicePackageFrameworkResource{} + return []*types.ServicePackageFrameworkResource{ + { + Factory: newResourceAccount, + }, + } } func (p *servicePackage) SDKDataSources(ctx context.Context) []*types.ServicePackageSDKDataSource { @@ -76,11 +80,6 @@ func (p *servicePackage) SDKDataSources(ctx context.Context) []*types.ServicePac func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePackageSDKResource { return []*types.ServicePackageSDKResource{ - { - Factory: resourceAccount, - TypeName: "aws_api_gateway_account", - Name: "Account", - }, { Factory: resourceAPIKey, TypeName: "aws_api_gateway_api_key", diff --git a/internal/service/apigateway/sweep.go b/internal/service/apigateway/sweep.go index fffa5e0932b..0e0e7bc229b 100644 --- a/internal/service/apigateway/sweep.go +++ b/internal/service/apigateway/sweep.go @@ -15,7 +15,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/sweep" "github.com/hashicorp/terraform-provider-aws/internal/sweep/awsv2" - "github.com/hashicorp/terraform-provider-aws/internal/sweep/sdk" + "github.com/hashicorp/terraform-provider-aws/internal/sweep/framework" ) func RegisterSweepers() { @@ -58,11 +58,8 @@ func RegisterSweepers() { } func sweepAccounts(ctx context.Context, client *conns.AWSClient) ([]sweep.Sweepable, error) { - r := resourceAccount() - d := r.Data(nil) - return []sweep.Sweepable{ - sdk.NewSweepResource(r, d, client), + framework.NewSweepResource(newResourceAccount, client), }, nil } From c0fea7110fb9f23e9f3771f06fd92d4641cc709f Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 6 Nov 2024 16:18:38 -0800 Subject: [PATCH 08/45] Adds attribute `reset_on_delete` to properly reset account settings for backwards compatibility. --- .changelog/40004.txt | 2 +- internal/service/apigateway/account.go | 40 +++-- internal/service/apigateway/account_test.go | 160 ++++++++++++++++-- .../service/apigateway/apigateway_test.go | 2 + .../apigateway/method_settings_test.go | 4 +- internal/service/apigateway/stage_test.go | 8 +- internal/service/apigateway/sweep.go | 4 +- .../docs/r/api_gateway_account.html.markdown | 5 + 8 files changed, 193 insertions(+), 32 deletions(-) diff --git a/.changelog/40004.txt b/.changelog/40004.txt index 31da035ab3f..52d99688db7 100644 --- a/.changelog/40004.txt +++ b/.changelog/40004.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -resource/aws_api_gateway_account: Supports unregistering account settings. +resource/aws_api_gateway_account: Add attribute `reset_on_delete` to properly reset CloudWatch Role ARN on deletion. ``` diff --git a/internal/service/apigateway/account.go b/internal/service/apigateway/account.go index 149480d8ec3..9bf80ee8909 100644 --- a/internal/service/apigateway/account.go +++ b/internal/service/apigateway/account.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" @@ -66,11 +67,18 @@ func (r *resourceAccount) Schema(ctx context.Context, request resource.SchemaReq }, names.AttrID: schema.StringAttribute{ Computed: true, - DeprecationMessage: "This attribute will be removed in a future version.", + DeprecationMessage: "This attribute is unused and will be removed in a future version of the provider", PlanModifiers: []planmodifier.String{ stringplanmodifier.UseStateForUnknown(), }, }, + "reset_on_delete": schema.BoolAttribute{ + Optional: true, + PlanModifiers: []planmodifier.Bool{ + boolplanmodifier.UseStateForUnknown(), + }, + DeprecationMessage: "This attribute will be removed in a future version of the provider", + }, "throttle_settings": framework.DataSourceComputedListOfObjectAttribute[throttleSettingsModel](ctx), }, } @@ -226,19 +234,28 @@ func (r *resourceAccount) Delete(ctx context.Context, request resource.DeleteReq return } - conn := r.Meta().APIGatewayClient(ctx) + if data.ResetOnDelete.ValueBool() { + conn := r.Meta().APIGatewayClient(ctx) - input := &apigateway.UpdateAccountInput{} + input := &apigateway.UpdateAccountInput{} - input.PatchOperations = []awstypes.PatchOperation{{ - Op: awstypes.OpReplace, - Path: aws.String("/cloudwatchRoleArn"), - Value: nil, - }} + input.PatchOperations = []awstypes.PatchOperation{{ + Op: awstypes.OpReplace, + Path: aws.String("/cloudwatchRoleArn"), + Value: nil, + }} - _, err := conn.UpdateAccount(ctx, input) - if err != nil { - response.Diagnostics.AddError("resetting API Gateway Account", err.Error()) + _, err := conn.UpdateAccount(ctx, input) + if err != nil { + response.Diagnostics.AddError("resetting API Gateway Account", err.Error()) + } + } else { + response.Diagnostics.AddWarning( + "Resource Destruction", + "This resource has only been removed from Terraform state. "+ + "Manually use the AWS Console to fully destroy this resource. "+ + "Setting the attribute \"reset_on_delete\" will also fully destroy resources of this type.", + ) } } @@ -255,6 +272,7 @@ type resourceAccountModel struct { CloudwatchRoleARN types.String `tfsdk:"cloudwatch_role_arn" autoflex:",legacy"` Features types.Set `tfsdk:"features"` ID types.String `tfsdk:"id"` + ResetOnDelete types.Bool `tfsdk:"reset_on_delete"` ThrottleSettings fwtypes.ListNestedObjectValueOf[throttleSettingsModel] `tfsdk:"throttle_settings"` } diff --git a/internal/service/apigateway/account_test.go b/internal/service/apigateway/account_test.go index fc11ef64f42..83b47c5dfb3 100644 --- a/internal/service/apigateway/account_test.go +++ b/internal/service/apigateway/account_test.go @@ -5,9 +5,13 @@ package apigateway_test import ( "context" + "errors" "fmt" "testing" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/apigateway" + awstypes "github.com/aws/aws-sdk-go-v2/service/apigateway/types" "github.com/hashicorp/terraform-plugin-testing/compare" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -23,6 +27,7 @@ import ( func testAccAccount_basic(t *testing.T) { ctx := acctest.Context(t) + t.Cleanup(accountCleanup(ctx, t)) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ @@ -38,6 +43,7 @@ func testAccAccount_basic(t *testing.T) { statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("cloudwatch_role_arn"), knownvalue.StringExact("")), statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("features"), knownvalue.NotNull()), statecheck.ExpectKnownValue(resourceName, tfjsonpath.New(names.AttrID), knownvalue.StringExact("api-gateway-account")), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("reset_on_delete"), knownvalue.Null()), statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("throttle_settings"), knownvalue.ListExact([]knownvalue.Check{ knownvalue.ObjectExact(map[string]knownvalue.Check{ "burst_limit": knownvalue.Int32Exact(5000), @@ -57,6 +63,7 @@ func testAccAccount_basic(t *testing.T) { func testAccAccount_cloudwatchRoleARN_value(t *testing.T) { ctx := acctest.Context(t) + t.Cleanup(accountCleanup(ctx, t)) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_account.test" @@ -113,6 +120,7 @@ func testAccAccount_cloudwatchRoleARN_value(t *testing.T) { func testAccAccount_cloudwatchRoleARN_empty(t *testing.T) { ctx := acctest.Context(t) + t.Cleanup(accountCleanup(ctx, t)) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ @@ -136,8 +144,79 @@ func testAccAccount_cloudwatchRoleARN_empty(t *testing.T) { }) } +func testAccAccount_resetOnDelete_false(t *testing.T) { + ctx := acctest.Context(t) + t.Cleanup(accountCleanup(ctx, t)) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_api_gateway_account.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckAccountNotDestroyed(ctx), + Steps: []resource.TestStep{ + { + Config: testAccAccountConfig_resetOnDelete(rName, false), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.CompareValuePairs( + resourceName, tfjsonpath.New("cloudwatch_role_arn"), + "aws_iam_role.test[0]", tfjsonpath.New(names.AttrARN), + compare.ValuesSame(), + ), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("reset_on_delete"), knownvalue.Bool(false)), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "reset_on_delete", + }, + }, + }, + }) +} + +func testAccAccount_resetOnDelete_true(t *testing.T) { + ctx := acctest.Context(t) + t.Cleanup(accountCleanup(ctx, t)) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_api_gateway_account.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckAccountDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccAccountConfig_resetOnDelete(rName, true), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.CompareValuePairs( + resourceName, tfjsonpath.New("cloudwatch_role_arn"), + "aws_iam_role.test[0]", tfjsonpath.New(names.AttrARN), + compare.ValuesSame(), + ), + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("reset_on_delete"), knownvalue.Bool(true)), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "reset_on_delete", + }, + }, + }, + }) +} + func testAccAccount_frameworkMigration_basic(t *testing.T) { ctx := acctest.Context(t) + t.Cleanup(accountCleanup(ctx, t)) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ @@ -168,13 +247,14 @@ func testAccAccount_frameworkMigration_basic(t *testing.T) { func testAccAccount_frameworkMigration_cloudwatchRoleARN(t *testing.T) { ctx := acctest.Context(t) + t.Cleanup(accountCleanup(ctx, t)) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), - CheckDestroy: testAccCheckAccountDestroy(ctx), + CheckDestroy: testAccCheckAccountNotDestroyed(ctx), Steps: []resource.TestStep{ { ExternalProviders: map[string]resource.ExternalProvider{ @@ -205,25 +285,58 @@ func testAccCheckAccountDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).APIGatewayClient(ctx) - for _, rs := range s.RootModule().Resources { - if rs.Type != "aws_api_gateway_stage" { - continue - } + account, err := tfapigateway.FindAccount(ctx, conn) + if err != nil { + return err + } + + if account.CloudwatchRoleArn == nil { + // Settings have been reset + return nil + + } + + return errors.New("API Gateway Account still exists") + } +} + +func testAccCheckAccountNotDestroyed(ctx context.Context) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).APIGatewayClient(ctx) + + account, err := tfapigateway.FindAccount(ctx, conn) + if err != nil { + return err + } + + if account.CloudwatchRoleArn != nil { + // Settings have not been reset + return nil + } - account, err := tfapigateway.FindAccount(ctx, conn) - if err != nil { - return err - } + return errors.New("API Gateway Account still exists") + } +} - if account.CloudwatchRoleArn == nil { - // Settings have been reset - continue - } +func accountCleanup(ctx context.Context, t *testing.T) func() { + return func() { + t.Helper() - return fmt.Errorf("API Gateway Stage %s still exists", rs.Primary.ID) + conn := acctest.Provider.Meta().(*conns.AWSClient).APIGatewayClient(ctx) + + input := &apigateway.UpdateAccountInput{ + PatchOperations: []awstypes.PatchOperation{ + { + Op: awstypes.OpReplace, + Path: aws.String("/cloudwatchRoleArn"), + Value: nil, + }, + }, } - return nil + if _, err := conn.UpdateAccount(ctx, input); err != nil { + t.Errorf("API Gateway Account cleanup: %s", err) + } } } @@ -257,7 +370,8 @@ resource "aws_iam_role" "test" { } func testAccAccountConfig_role0(rName string) string { - return acctest.ConfigCompose(testAccAccountConfig_base(rName), ` + return acctest.ConfigCompose( + testAccAccountConfig_base(rName), ` resource "aws_api_gateway_account" "test" { cloudwatch_role_arn = aws_iam_role.test[0].arn } @@ -265,7 +379,8 @@ resource "aws_api_gateway_account" "test" { } func testAccAccountConfig_role1(rName string) string { - return acctest.ConfigCompose(testAccAccountConfig_base(rName), ` + return acctest.ConfigCompose( + testAccAccountConfig_base(rName), ` resource "aws_api_gateway_account" "test" { cloudwatch_role_arn = aws_iam_role.test[1].arn } @@ -277,3 +392,14 @@ resource "aws_api_gateway_account" "test" { cloudwatch_role_arn = "" } ` + +func testAccAccountConfig_resetOnDelete(rName string, reset bool) string { + return acctest.ConfigCompose( + testAccAccountConfig_base(rName), + fmt.Sprintf(` +resource "aws_api_gateway_account" "test" { + cloudwatch_role_arn = aws_iam_role.test[0].arn + reset_on_delete = %[1]t +} +`, reset)) +} diff --git a/internal/service/apigateway/apigateway_test.go b/internal/service/apigateway/apigateway_test.go index 1ca22b81abb..b83d53d7532 100644 --- a/internal/service/apigateway/apigateway_test.go +++ b/internal/service/apigateway/apigateway_test.go @@ -30,6 +30,8 @@ func TestAccAPIGateway_serial(t *testing.T) { "CloudwatchRoleARN_Empty": testAccAccount_cloudwatchRoleARN_empty, "FrameworkMigration_Basic": testAccAccount_frameworkMigration_basic, "FrameworkMigration_CloudwatchRoleARN": testAccAccount_frameworkMigration_cloudwatchRoleARN, + "ResetOnDelete_false": testAccAccount_resetOnDelete_false, + "ResetOnDelete_true": testAccAccount_resetOnDelete_true, }, // Some aws_api_gateway_method_settings tests require the account-level CloudWatch Logs role ARN to be set. // Serialize all this resource's acceptance tests. diff --git a/internal/service/apigateway/method_settings_test.go b/internal/service/apigateway/method_settings_test.go index ee807ac65e5..fbb1fba5968 100644 --- a/internal/service/apigateway/method_settings_test.go +++ b/internal/service/apigateway/method_settings_test.go @@ -619,7 +619,9 @@ func testAccMethodSettingsImportStateIdFunc(resourceName string) resource.Import } func testAccMethodSettingsConfig_base(rName string) string { - return acctest.ConfigCompose(testAccAccountConfig_role0(rName), fmt.Sprintf(` + return acctest.ConfigCompose( + testAccAccountConfig_resetOnDelete(rName, true), + fmt.Sprintf(` resource "aws_api_gateway_rest_api" "test" { name = %[1]q } diff --git a/internal/service/apigateway/stage_test.go b/internal/service/apigateway/stage_test.go index 906af2e8daa..9508db920d8 100644 --- a/internal/service/apigateway/stage_test.go +++ b/internal/service/apigateway/stage_test.go @@ -551,7 +551,9 @@ func testAccStageImportStateIdFunc(resourceName string) resource.ImportStateIdFu } func testAccStageConfig_base(rName string) string { - return acctest.ConfigCompose(testAccAccountConfig_role0(rName), fmt.Sprintf(` + return acctest.ConfigCompose( + testAccAccountConfig_resetOnDelete(rName, true), + fmt.Sprintf(` resource "aws_api_gateway_rest_api" "test" { name = %[1]q } @@ -672,6 +674,10 @@ resource "aws_api_gateway_stage" "test" { destination_arn = aws_cloudwatch_log_group.test.arn format = %[2]q } + + depends_on = [ + aws_api_gateway_account.test + ] } `, rName, format)) } diff --git a/internal/service/apigateway/sweep.go b/internal/service/apigateway/sweep.go index 0e0e7bc229b..939b3b9241d 100644 --- a/internal/service/apigateway/sweep.go +++ b/internal/service/apigateway/sweep.go @@ -59,7 +59,9 @@ func RegisterSweepers() { func sweepAccounts(ctx context.Context, client *conns.AWSClient) ([]sweep.Sweepable, error) { return []sweep.Sweepable{ - framework.NewSweepResource(newResourceAccount, client), + framework.NewSweepResource(newResourceAccount, client, + framework.NewAttribute("reset_on_delete", true), + ), }, nil } diff --git a/website/docs/r/api_gateway_account.html.markdown b/website/docs/r/api_gateway_account.html.markdown index baf47364aa5..a6d8221d69a 100644 --- a/website/docs/r/api_gateway_account.html.markdown +++ b/website/docs/r/api_gateway_account.html.markdown @@ -10,6 +10,8 @@ description: |- Provides a settings of an API Gateway Account. Settings is applied region-wide per `provider` block. +-> **Note:** By default, destroying this resource will keep your account settings intact. Set `reset_on_delete` to `true` to reset the account setttings to default. In a future version, destroying the resource will reset account settings. + ## Example Usage ```terraform @@ -64,6 +66,9 @@ resource "aws_iam_role_policy" "cloudwatch" { This resource supports the following arguments: * `cloudwatch_role_arn` - (Optional) ARN of an IAM role for CloudWatch (to allow logging & monitoring). See more [in AWS Docs](https://docs.aws.amazon.com/apigateway/latest/developerguide/how-to-stage-settings.html#how-to-stage-settings-console). Logging & monitoring can be enabled/disabled and otherwise tuned on the API Gateway Stage level. +* `reset_on_delete` - (Optional) If `true`, destroying the resource will reset account settings to default, otherwise account settings are not modified. + Defaults to `false`. + Will be removed in a future version. ## Attribute Reference From 81c4a0b7230a7c15392bfcbe3ecfd217ff520639 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 6 Nov 2024 16:38:03 -0800 Subject: [PATCH 09/45] Adds destroy plan-time warning --- internal/service/apigateway/account.go | 20 ++++++++++++++++++++ internal/service/apigateway/account_test.go | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/internal/service/apigateway/account.go b/internal/service/apigateway/account.go index 9bf80ee8909..847a7ac3aa9 100644 --- a/internal/service/apigateway/account.go +++ b/internal/service/apigateway/account.go @@ -281,6 +281,26 @@ type throttleSettingsModel struct { RateLimit types.Float64 `tfsdk:"rate_limit"` } +func (r *resourceAccount) ModifyPlan(ctx context.Context, request resource.ModifyPlanRequest, response *resource.ModifyPlanResponse) { + // If the entire plan is null, the resource is planned for destruction. + if request.Plan.Raw.IsNull() { + var resetOnDelete types.Bool + response.Diagnostics.Append(request.State.GetAttribute(ctx, path.Root("reset_on_delete"), &resetOnDelete)...) + if response.Diagnostics.HasError() { + return + } + + if !resetOnDelete.ValueBool() { + response.Diagnostics.AddWarning( + "Resource Destruction", + "Applying this resource destruction will only remove the resource from Terraform state and will not reset account settings. "+ + "Either manually use the AWS Console to fully destroy this resource or "+ + "update the resource with \"reset_on_delete\" set to true.", + ) + } + } +} + func findAccount(ctx context.Context, conn *apigateway.Client) (*apigateway.GetAccountOutput, error) { input := &apigateway.GetAccountInput{} diff --git a/internal/service/apigateway/account_test.go b/internal/service/apigateway/account_test.go index 83b47c5dfb3..639f2d81634 100644 --- a/internal/service/apigateway/account_test.go +++ b/internal/service/apigateway/account_test.go @@ -314,7 +314,7 @@ func testAccCheckAccountNotDestroyed(ctx context.Context) resource.TestCheckFunc return nil } - return errors.New("API Gateway Account still exists") + return errors.New("API Gateway Account was reset") } } From cb24bce892b75813d444493ad2c7b76056b61243 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 6 Nov 2024 16:42:12 -0800 Subject: [PATCH 10/45] Linting --- internal/service/apigateway/account.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/service/apigateway/account.go b/internal/service/apigateway/account.go index 847a7ac3aa9..f92eed7af3a 100644 --- a/internal/service/apigateway/account.go +++ b/internal/service/apigateway/account.go @@ -110,7 +110,7 @@ func (r *resourceAccount) Create(ctx context.Context, request resource.CreateReq { Op: awstypes.OpReplace, Path: aws.String("/cloudwatchRoleArn"), - Value: aws.String(data.CloudwatchRoleARN.ValueString()), + Value: data.CloudwatchRoleARN.ValueStringPointer(), }, } } @@ -155,6 +155,10 @@ func (r *resourceAccount) Read(ctx context.Context, request resource.ReadRequest response.State.RemoveResource(ctx) return } + if err != nil { + response.Diagnostics.AddError("reading API Gateway Account", err.Error()) + return + } response.Diagnostics.Append(flex.Flatten(ctx, account, &data)...) @@ -197,7 +201,7 @@ func (r *resourceAccount) Update(ctx context.Context, request resource.UpdateReq { Op: awstypes.OpReplace, Path: aws.String("/cloudwatchRoleArn"), - Value: aws.String(plan.CloudwatchRoleARN.ValueString()), + Value: plan.CloudwatchRoleARN.ValueStringPointer(), }, } } @@ -264,7 +268,7 @@ func (r *resourceAccount) Delete(ctx context.Context, request resource.DeleteReq // // If setting an attribute with the import identifier, it is recommended to use the ImportStatePassthroughID() call in this method. func (r *resourceAccount) ImportState(ctx context.Context, request resource.ImportStateRequest, response *resource.ImportStateResponse) { - resource.ImportStatePassthroughID(ctx, path.Root("id"), request, response) + resource.ImportStatePassthroughID(ctx, path.Root(names.AttrID), request, response) } type resourceAccountModel struct { From aabc50a37f11e48ccf9f0827235edbafb39d0d03 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 7 Nov 2024 15:19:56 +0000 Subject: [PATCH 11/45] build(deps): bump github.com/golang-jwt/jwt/v4 in /.ci/tools Bumps [github.com/golang-jwt/jwt/v4](https://github.com/golang-jwt/jwt) from 4.5.0 to 4.5.1. - [Release notes](https://github.com/golang-jwt/jwt/releases) - [Changelog](https://github.com/golang-jwt/jwt/blob/main/VERSION_HISTORY.md) - [Commits](https://github.com/golang-jwt/jwt/compare/v4.5.0...v4.5.1) --- updated-dependencies: - dependency-name: github.com/golang-jwt/jwt/v4 dependency-type: indirect ... Signed-off-by: dependabot[bot] --- .ci/tools/go.mod | 2 +- .ci/tools/go.sum | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.ci/tools/go.mod b/.ci/tools/go.mod index a08a4ccd0c2..ec3db3d4645 100644 --- a/.ci/tools/go.mod +++ b/.ci/tools/go.mod @@ -120,7 +120,7 @@ require ( github.com/go-xmlfmt/xmlfmt v1.1.2 // indirect github.com/gobwas/glob v0.2.3 // indirect github.com/gofrs/flock v0.12.1 // indirect - github.com/golang-jwt/jwt/v4 v4.5.0 // indirect + github.com/golang-jwt/jwt/v4 v4.5.1 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a // indirect diff --git a/.ci/tools/go.sum b/.ci/tools/go.sum index 5b9960ada91..8ed28c3d9a6 100644 --- a/.ci/tools/go.sum +++ b/.ci/tools/go.sum @@ -524,8 +524,9 @@ github.com/gofrs/flock v0.12.1 h1:MTLVXXHf8ekldpJk3AKicLij9MdwOWkZ+a/jHHZby9E= github.com/gofrs/flock v0.12.1/go.mod h1:9zxTsyu5xtJ9DK+1tFZyibEV7y3uwDxPPfbxeeHCoD0= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= -github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo= +github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= From 5008597d9b7a0c5308aefec7b020da46b8692d6e Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Wed, 6 Nov 2024 16:54:29 -0500 Subject: [PATCH 12/45] r/aws_lb_listener: remove `tcp_idle_timeout_seconds` default The `tcp_idle_timeout_seconds` default value of `350` has been removed in favor or marking this argument optional and computed. In practice, this means the default value of `350` is still read and stored in state for protocols which support this feature, but unless explicitly configured by the user, the `ModifyListenerAttributes` API will not be called during create or update operations. This change fixes a regression introduced by the addition of the `tcp_idle_timeout_seconds` argument with a default value, which caused any existing configuration using the `aws_lb_listener` resource to now require the calling principal to include the `elasticloadbalancing:ModifyLoadBalancerAttributes` IAM action in an attached IAM policy. By only sending this value when explicitly configured, existing configurations can continue to operate as expected with no modifications to the IAM permissions of the calling principal. To confirm the API is no longer being called when `tcp_idle_timeout_seconds` is not explicitly configured, I've grepped the debug logs of the same configuration applied with `v5.74.0` of the AWS provider and a locally built binary from this branch. The former includes a result with the API request body where the TCP idle timeout is set to the default of `350`. The latter includes no result indicating the `ModifyListenerAttributes` API was not called, as expected. __`v5.74.0`__: ```console % rg "Action=ModifyListenerAttributes" apply-old.out 2672: | Action=ModifyListenerAttributes&Attributes.member.1.Key=tcp.idle_timeout.seconds&Attributes.member.1.Value=350&ListenerArn=arn%3Aaws%3Aelasticloadbalancing%3Aus-west-2%3A%3Alistener%2Fnet%2Fjb-test%2Fb0f926497af0c80f%2F819828f77724007f&Version=2015-12-01 ``` __This branch__: ```console % rg "Action=ModifyListenerAttributes" apply.out ``` ```console % make testacc PKG=elbv2 TESTARGS="-run=TestAccELBV2Listener_.*_basic" make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.23.2 test ./internal/service/elbv2/... -v -count 1 -parallel 20 -run=TestAccELBV2Listener_.*_basic -timeout 360m 2024/11/06 16:43:21 Initializing Terraform AWS Provider... --- PASS: TestAccELBV2Listener_Gateway_basic (191.64s) --- PASS: TestAccELBV2Listener_Network_basic (214.10s) --- PASS: TestAccELBV2Listener_Application_basic (229.81s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/elbv2 236.141s ``` --- internal/service/elbv2/listener.go | 19 +++++++++---------- internal/service/elbv2/listener_test.go | 3 +++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/internal/service/elbv2/listener.go b/internal/service/elbv2/listener.go index 3f42df19768..89da733492b 100644 --- a/internal/service/elbv2/listener.go +++ b/internal/service/elbv2/listener.go @@ -393,7 +393,7 @@ func resourceListener() *schema.Resource { "tcp_idle_timeout_seconds": { Type: schema.TypeInt, Optional: true, - Default: 350, + Computed: true, ValidateFunc: validation.IntBetween(60, 6000), // Attribute only valid for TCP (NLB) and GENEVE (GWLB) listeners DiffSuppressFunc: suppressIfListenerProtocolNot(awstypes.ProtocolEnumGeneve, awstypes.ProtocolEnumTcp), @@ -536,9 +536,7 @@ func resourceListenerCreate(ctx context.Context, d *schema.ResourceData, meta in } // Listener attributes like TCP idle timeout are not supported on create - var attributes []awstypes.ListenerAttribute - - attributes = append(attributes, listenerAttributes.expand(d, listenerProtocolType, false)...) + attributes := listenerAttributes.expand(d, listenerProtocolType, false) if len(attributes) > 0 { if err := modifyListenerAttributes(ctx, conn, d.Id(), attributes); err != nil { @@ -750,12 +748,13 @@ func (m listenerAttributeMap) expand(d *schema.ResourceData, listenerType awstyp continue } - if attributeInfo.tfType == schema.TypeInt { - v := (d.Get(tfAttributeName)).(int) - attributes = append(attributes, awstypes.ListenerAttribute{ - Key: aws.String(attributeInfo.apiAttributeKey), - Value: flex.IntValueToString(v), - }) + if v, ok := d.GetOk(tfAttributeName); ok { + if attributeInfo.tfType == schema.TypeInt { + attributes = append(attributes, awstypes.ListenerAttribute{ + Key: aws.String(attributeInfo.apiAttributeKey), + Value: flex.IntValueToString(v.(int)), + }) + } } } diff --git a/internal/service/elbv2/listener_test.go b/internal/service/elbv2/listener_test.go index 9265819937c..341ccca3cd9 100644 --- a/internal/service/elbv2/listener_test.go +++ b/internal/service/elbv2/listener_test.go @@ -62,6 +62,7 @@ func TestAccELBV2Listener_Application_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "ssl_policy", ""), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, "0"), + resource.TestCheckNoResourceAttr(resourceName, "tcp_idle_timeout_seconds"), ), }, { @@ -114,6 +115,7 @@ func TestAccELBV2Listener_Network_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "ssl_policy", ""), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, "0"), + resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", "350"), ), }, { @@ -165,6 +167,7 @@ func TestAccELBV2Listener_Gateway_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "ssl_policy", ""), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"), resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, "0"), + resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", "350"), ), }, { From a16df31ff4d10256949509efe0617b6ea3762d43 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Wed, 6 Nov 2024 16:34:14 -0500 Subject: [PATCH 13/45] r/aws_lb_listener: isolate `tcp_idle_timeout_seconds` update flow Updates to the `tcp_idle_timeout_seconds` argument are now handled distinctly from changes to other non-tag related arguments. Additionally, the logic applied to appropriately infer the protocol for gateway load balancers during the create operation has been replicated during update. Together these changes prevent failures when updating the `tcp_idle_timeout_seconds` argument for gateway load balancers like the following: ``` Error: modifying ELBv2 Listener (arn:aws:elasticloadbalancing:us-west-2::listener/gwy/tf-acc-test-5332588832756138226/541226271e9ad4ec/2bbf23d4354df440): operation error Elastic Load Balancing v2: ModifyListener, https response error StatusCode: 400, RequestID: c6f51e60-0511-443d-a98e-6f51b48fce11, api error ValidationError: An action must be specified ``` Also updates the `_attributes` acceptances tests to fully exercise the update workflow: ```console % make testacc PKG=elbv2 TESTS=TestAccELBV2Listener_attributes make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.23.2 test ./internal/service/elbv2/... -v -count 1 -parallel 20 -run='TestAccELBV2Listener_attributes' -timeout 360m 2024/11/06 16:12:02 Initializing Terraform AWS Provider... --- PASS: TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds (210.83s) --- PASS: TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds (238.02s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/elbv2 244.440s ``` --- internal/service/elbv2/listener.go | 29 ++++++++----- internal/service/elbv2/listener_test.go | 57 ++++++++++++++----------- 2 files changed, 49 insertions(+), 37 deletions(-) diff --git a/internal/service/elbv2/listener.go b/internal/service/elbv2/listener.go index 89da733492b..3bbf5f434dd 100644 --- a/internal/service/elbv2/listener.go +++ b/internal/service/elbv2/listener.go @@ -602,9 +602,7 @@ func resourceListenerUpdate(ctx context.Context, d *schema.ResourceData, meta in var diags diag.Diagnostics conn := meta.(*conns.AWSClient).ELBV2Client(ctx) - var attributes []awstypes.ListenerAttribute - - if d.HasChangesExcept(names.AttrTags, names.AttrTagsAll) { + if d.HasChangesExcept(names.AttrTags, names.AttrTagsAll, "tcp_idle_timeout_seconds") { input := &elasticloadbalancingv2.ModifyListenerInput{ ListenerArn: aws.String(d.Id()), } @@ -642,14 +640,6 @@ func resourceListenerUpdate(ctx context.Context, d *schema.ResourceData, meta in input.SslPolicy = aws.String(v.(string)) } - attributes = append(attributes, listenerAttributes.expand(d, awstypes.ProtocolEnum(d.Get(names.AttrProtocol).(string)), true)...) - - if len(attributes) > 0 { - if err := modifyListenerAttributes(ctx, conn, d.Id(), attributes); err != nil { - return sdkdiag.AppendFromErr(diags, err) - } - } - _, err := tfresource.RetryWhenIsA[*awstypes.CertificateNotFoundException](ctx, d.Timeout(schema.TimeoutUpdate), func() (interface{}, error) { return conn.ModifyListener(ctx, input) }) @@ -659,6 +649,23 @@ func resourceListenerUpdate(ctx context.Context, d *schema.ResourceData, meta in } } + if d.HasChanges("tcp_idle_timeout_seconds") { + lbARN := d.Get("load_balancer_arn").(string) + listenerProtocolType := awstypes.ProtocolEnum(d.Get(names.AttrProtocol).(string)) + // Protocol does not need to be explicitly set with GWLB listeners, nor is it returned by the API + // If protocol is not set, use the load balancer ARN to determine if listener is gateway type and set protocol appropriately + if listenerProtocolType == awstypes.ProtocolEnum("") && strings.Contains(lbARN, "loadbalancer/gwy/") { + listenerProtocolType = awstypes.ProtocolEnumGeneve + } + + attributes := listenerAttributes.expand(d, listenerProtocolType, true) + if len(attributes) > 0 { + if err := modifyListenerAttributes(ctx, conn, d.Id(), attributes); err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + } + } + return append(diags, resourceListenerRead(ctx, d, meta)...) } diff --git a/internal/service/elbv2/listener_test.go b/internal/service/elbv2/listener_test.go index 341ccca3cd9..cd55a5274dc 100644 --- a/internal/service/elbv2/listener_test.go +++ b/internal/service/elbv2/listener_test.go @@ -8,6 +8,7 @@ import ( "fmt" "regexp" "slices" + "strconv" "testing" "github.com/YakDriver/regexache" @@ -1016,7 +1017,7 @@ func TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds(t *testing.T) { lbResourceName := "aws_lb.test" resourceName := "aws_lb_listener.test" tcpTimeout1 := 60 - // tcpTimeout2 := 6000 + tcpTimeout2 := 6000 if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -1035,8 +1036,7 @@ func TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds(t *testing.T) { resource.TestCheckResourceAttrPair(resourceName, "load_balancer_arn", lbResourceName, names.AttrARN), resource.TestCheckResourceAttr(resourceName, names.AttrProtocol, ""), resource.TestCheckResourceAttr(resourceName, names.AttrPort, "0"), - resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", "60"), - resource.TestCheckResourceAttr(resourceName, "tags.Name", rName), + resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", strconv.Itoa(tcpTimeout1)), ), }, { @@ -1047,6 +1047,16 @@ func TestAccELBV2Listener_attributes_gwlb_TCPIdleTimeoutSeconds(t *testing.T) { "default_action.0.forward", }, }, + { + Config: testAccListenerConfig_attributes_gwlbTCPIdleTimeoutSeconds(rName, tcpTimeout2), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckListenerExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttrPair(resourceName, "load_balancer_arn", lbResourceName, names.AttrARN), + resource.TestCheckResourceAttr(resourceName, names.AttrProtocol, ""), + resource.TestCheckResourceAttr(resourceName, names.AttrPort, "0"), + resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", strconv.Itoa(tcpTimeout2)), + ), + }, }, }) } @@ -1056,6 +1066,8 @@ func TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds(t *testing.T) { var conf awstypes.Listener resourceName := "aws_lb_listener.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + tcpTimeout1 := 60 + tcpTimeout2 := 6000 resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -1064,31 +1076,13 @@ func TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds(t *testing.T) { CheckDestroy: testAccCheckListenerDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName), + Config: testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName, tcpTimeout1), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckListenerExists(ctx, resourceName, &conf), - resource.TestCheckResourceAttr("aws_lb.test", "load_balancer_type", "network"), resource.TestCheckResourceAttrPair(resourceName, "load_balancer_arn", "aws_lb.test", names.AttrARN), - acctest.MatchResourceAttrRegionalARN(resourceName, names.AttrARN, "elasticloadbalancing", regexache.MustCompile("listener/.+$")), - resource.TestCheckNoResourceAttr(resourceName, "alpn_policy"), - resource.TestCheckNoResourceAttr(resourceName, names.AttrCertificateARN), - resource.TestCheckResourceAttr(resourceName, "default_action.#", "1"), - resource.TestCheckResourceAttr(resourceName, "default_action.0.order", "1"), - resource.TestCheckResourceAttr(resourceName, "default_action.0.type", "forward"), - resource.TestCheckResourceAttrPair(resourceName, "default_action.0.target_group_arn", "aws_lb_target_group.test", names.AttrARN), - resource.TestCheckResourceAttr(resourceName, "default_action.0.authenticate_cognito.#", "0"), - resource.TestCheckResourceAttr(resourceName, "default_action.0.authenticate_oidc.#", "0"), - resource.TestCheckResourceAttr(resourceName, "default_action.0.fixed_response.#", "0"), - resource.TestCheckResourceAttr(resourceName, "default_action.0.forward.#", "0"), - resource.TestCheckResourceAttr(resourceName, "default_action.0.redirect.#", "0"), - resource.TestCheckResourceAttrPair(resourceName, names.AttrID, resourceName, names.AttrARN), - resource.TestCheckResourceAttr(resourceName, "mutual_authentication.#", "0"), resource.TestCheckResourceAttr(resourceName, names.AttrProtocol, "TCP"), resource.TestCheckResourceAttr(resourceName, names.AttrPort, "80"), - resource.TestCheckResourceAttr(resourceName, "ssl_policy", ""), - resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", "60"), - resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"), - resource.TestCheckResourceAttr(resourceName, acctest.CtTagsAllPercent, "0"), + resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", strconv.Itoa(tcpTimeout1)), ), }, { @@ -1099,6 +1093,16 @@ func TestAccELBV2Listener_attributes_nlb_TCPIdleTimeoutSeconds(t *testing.T) { "default_action.0.forward", }, }, + { + Config: testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName, tcpTimeout2), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckListenerExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttrPair(resourceName, "load_balancer_arn", "aws_lb.test", names.AttrARN), + resource.TestCheckResourceAttr(resourceName, names.AttrProtocol, "TCP"), + resource.TestCheckResourceAttr(resourceName, names.AttrPort, "80"), + resource.TestCheckResourceAttr(resourceName, "tcp_idle_timeout_seconds", strconv.Itoa(tcpTimeout2)), + ), + }, }, }) } @@ -2967,7 +2971,7 @@ resource "aws_lb_listener" "test" { `, rName, seconds)) } -func testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName string) string { +func testAccListenerConfig_attributes_nlbTCPIdleTimeoutSeconds(rName string, seconds int) string { return acctest.ConfigCompose( testAccListenerConfig_base(rName), fmt.Sprintf(` resource "aws_lb_listener" "test" { @@ -2979,7 +2983,8 @@ resource "aws_lb_listener" "test" { target_group_arn = aws_lb_target_group.test.id type = "forward" } - tcp_idle_timeout_seconds = 60 + + tcp_idle_timeout_seconds = %[2]d } resource "aws_lb" "test" { @@ -3015,7 +3020,7 @@ resource "aws_lb_target_group" "test" { Name = %[1]q } } -`, rName)) +`, rName, seconds)) } func testAccListenerConfig_Forward_changeWeightedToBasic(rName, rName2 string) string { From 5aa38a323fd38c410f60783beec37a408ed95d62 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 7 Nov 2024 11:22:36 -0500 Subject: [PATCH 14/45] chore: changelog --- .changelog/40039.txt | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changelog/40039.txt diff --git a/.changelog/40039.txt b/.changelog/40039.txt new file mode 100644 index 00000000000..abed6875eba --- /dev/null +++ b/.changelog/40039.txt @@ -0,0 +1,6 @@ +```release-note:bug +resource/aws_lb_listener: Remove the default `tcp_idle_timeout_seconds` value, preventing `ModifyListenerAttributes` API calls when a value is not explicitly configured +``` +```release-note:bug +resource/aws_lb_listener: Fix errors when updating the `tcp_idle_timeout_seconds` argument for gateway load balancers +``` From 33406fe1e2a65e133cb152b1703240643256f52b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 12:57:36 -0500 Subject: [PATCH 15/45] ipam_pool: Fix publicly_advertisable bug --- internal/service/ec2/ipam_pool.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/ec2/ipam_pool.go b/internal/service/ec2/ipam_pool.go index 6f674203e0a..138e10398c0 100644 --- a/internal/service/ec2/ipam_pool.go +++ b/internal/service/ec2/ipam_pool.go @@ -204,9 +204,9 @@ func resourceIPAMPoolCreate(ctx context.Context, d *schema.ResourceData, meta in if v, ok := d.GetOk("public_ip_source"); ok { input.PublicIpSource = awstypes.IpamPoolPublicIpSource(v.(string)) } - // PubliclyAdvertisable must be set if if the AddressFamily is IPv6 and PublicIpSource is byoip. - // The request can only contain PubliclyAdvertisable if the AddressFamily is IPv6 and PublicIpSource is byoip. - if addressFamily == awstypes.AddressFamilyIpv6 && scope.IpamScopeType == awstypes.IpamScopeTypePublic { + // PubliclyAdvertisable must be set if if the AddressFamily is IPv6 and PublicIpSource is byoip (either '' or 'byoip'). + // The request can't contain PubliclyAdvertisable if PublicIpSource is 'amazon'. + if addressFamily == awstypes.AddressFamilyIpv6 && scope.IpamScopeType == awstypes.IpamScopeTypePublic && input.PublicIpSource != awstypes.IpamPoolPublicIpSourceAmazon { input.PubliclyAdvertisable = aws.Bool(d.Get("publicly_advertisable").(bool)) } From 2b2fe389f5b109e505026647f98e4e95c8d76292 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:00:40 -0500 Subject: [PATCH 16/45] Add changelog --- .changelog/40042.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/40042.txt diff --git a/.changelog/40042.txt b/.changelog/40042.txt new file mode 100644 index 00000000000..bcc27c3e424 --- /dev/null +++ b/.changelog/40042.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_vpc_ipam_pool: Fix bug when `public_ip_source = "amazon"`: `The request can only contain PubliclyAdvertisable if the AddressFamily is IPv6 and PublicIpSource is byoip.` +``` From 9208140788593832683d8f2d00ea15f97dc84a06 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:01:18 -0500 Subject: [PATCH 17/45] ipam_pool: Add test for public_ip_source = amazon --- internal/service/ec2/ipam_pool_test.go | 40 ++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/internal/service/ec2/ipam_pool_test.go b/internal/service/ec2/ipam_pool_test.go index 4833936b162..c457bc0dfd5 100644 --- a/internal/service/ec2/ipam_pool_test.go +++ b/internal/service/ec2/ipam_pool_test.go @@ -132,6 +132,36 @@ func TestAccIPAMPool_ipv6Basic(t *testing.T) { }) } +func TestAccIPAMPool_ipv6PublicIPAmazon(t *testing.T) { + ctx := acctest.Context(t) + var pool awstypes.IpamPool + resourceName := "aws_vpc_ipam_pool.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.EC2ServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckIPAMPoolDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccIPAMPoolConfig_ipv6PublicIPAmazon, + Check: resource.ComposeTestCheckFunc( + testAccCheckIPAMPoolExists(ctx, resourceName, &pool), + resource.TestCheckResourceAttr(resourceName, "address_family", "ipv6"), + resource.TestCheckResourceAttr(resourceName, "public_ip_source", "amazon"), + resource.TestCheckResourceAttr(resourceName, "aws_service", "ec2"), + resource.TestCheckResourceAttr(resourceName, "publicly_advertisable", acctest.CtFalse), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccIPAMPool_ipv6Contiguous(t *testing.T) { ctx := acctest.Context(t) var pool awstypes.IpamPool @@ -368,6 +398,16 @@ resource "aws_vpc_ipam_pool" "test" { } `) +var testAccIPAMPoolConfig_ipv6PublicIPAmazon = acctest.ConfigCompose(testAccIPAMPoolConfig_base, ` +resource "aws_vpc_ipam_pool" "test" { + address_family = "ipv6" + ipam_scope_id = aws_vpc_ipam.test.public_default_scope_id + locale = data.aws_region.current.name + public_ip_source = "amazon" + aws_service = "ec2" +} +`) + var testAccIPAMPoolConfig_ipv6Contiguous = acctest.ConfigCompose(testAccIPAMPoolConfig_base, ` resource "aws_vpc_ipam_pool" "test" { address_family = "ipv6" From 75206ded01ea8dffa9955a7ca0c83ce2716710f6 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 7 Nov 2024 10:01:46 -0800 Subject: [PATCH 18/45] Cleanup --- internal/service/apigateway/account.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/service/apigateway/account.go b/internal/service/apigateway/account.go index f92eed7af3a..793e5a8cf45 100644 --- a/internal/service/apigateway/account.go +++ b/internal/service/apigateway/account.go @@ -263,10 +263,6 @@ func (r *resourceAccount) Delete(ctx context.Context, request resource.DeleteReq } } -// ImportState is called when the provider must import the state of a resource instance. -// This method must return enough state so the Read method can properly refresh the full resource. -// -// If setting an attribute with the import identifier, it is recommended to use the ImportStatePassthroughID() call in this method. func (r *resourceAccount) ImportState(ctx context.Context, request resource.ImportStateRequest, response *resource.ImportStateResponse) { resource.ImportStatePassthroughID(ctx, path.Root(names.AttrID), request, response) } From e0753cc85e93ad146356533bf005c26591969299 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 7 Nov 2024 10:04:52 -0800 Subject: [PATCH 19/45] Rephrasing from PR review --- internal/service/apigateway/account.go | 4 ++-- website/docs/r/api_gateway_account.html.markdown | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/service/apigateway/account.go b/internal/service/apigateway/account.go index 793e5a8cf45..fa431e794cc 100644 --- a/internal/service/apigateway/account.go +++ b/internal/service/apigateway/account.go @@ -67,7 +67,7 @@ func (r *resourceAccount) Schema(ctx context.Context, request resource.SchemaReq }, names.AttrID: schema.StringAttribute{ Computed: true, - DeprecationMessage: "This attribute is unused and will be removed in a future version of the provider", + DeprecationMessage: `The "id" attribute is unused and will be removed in a future version of the provider`, PlanModifiers: []planmodifier.String{ stringplanmodifier.UseStateForUnknown(), }, @@ -77,7 +77,7 @@ func (r *resourceAccount) Schema(ctx context.Context, request resource.SchemaReq PlanModifiers: []planmodifier.Bool{ boolplanmodifier.UseStateForUnknown(), }, - DeprecationMessage: "This attribute will be removed in a future version of the provider", + DeprecationMessage: `The "reset_on_delete" attribute will be removed in a future version of the provider`, }, "throttle_settings": framework.DataSourceComputedListOfObjectAttribute[throttleSettingsModel](ctx), }, diff --git a/website/docs/r/api_gateway_account.html.markdown b/website/docs/r/api_gateway_account.html.markdown index a6d8221d69a..44996a8cf79 100644 --- a/website/docs/r/api_gateway_account.html.markdown +++ b/website/docs/r/api_gateway_account.html.markdown @@ -10,7 +10,7 @@ description: |- Provides a settings of an API Gateway Account. Settings is applied region-wide per `provider` block. --> **Note:** By default, destroying this resource will keep your account settings intact. Set `reset_on_delete` to `true` to reset the account setttings to default. In a future version, destroying the resource will reset account settings. +-> **Note:** By default, destroying this resource will keep your account settings intact. Set `reset_on_delete` to `true` to reset the account setttings to default. In a future major version of the provider, destroying the resource will reset account settings. ## Example Usage @@ -68,7 +68,7 @@ This resource supports the following arguments: * `cloudwatch_role_arn` - (Optional) ARN of an IAM role for CloudWatch (to allow logging & monitoring). See more [in AWS Docs](https://docs.aws.amazon.com/apigateway/latest/developerguide/how-to-stage-settings.html#how-to-stage-settings-console). Logging & monitoring can be enabled/disabled and otherwise tuned on the API Gateway Stage level. * `reset_on_delete` - (Optional) If `true`, destroying the resource will reset account settings to default, otherwise account settings are not modified. Defaults to `false`. - Will be removed in a future version. + Will be removed in a future major version of the provider. ## Attribute Reference From 248c46219eadc3a161b0854c10e932846292dcc2 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 7 Nov 2024 10:14:42 -0800 Subject: [PATCH 20/45] Only register `t.Cleanup` function if the test is executing --- internal/service/apigateway/account_test.go | 42 ++++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/internal/service/apigateway/account_test.go b/internal/service/apigateway/account_test.go index 639f2d81634..4687b26dce5 100644 --- a/internal/service/apigateway/account_test.go +++ b/internal/service/apigateway/account_test.go @@ -27,11 +27,13 @@ import ( func testAccAccount_basic(t *testing.T) { ctx := acctest.Context(t) - t.Cleanup(accountCleanup(ctx, t)) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + t.Cleanup(accountCleanup(ctx, t)) + }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckAccountDestroy(ctx), @@ -63,12 +65,14 @@ func testAccAccount_basic(t *testing.T) { func testAccAccount_cloudwatchRoleARN_value(t *testing.T) { ctx := acctest.Context(t) - t.Cleanup(accountCleanup(ctx, t)) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + t.Cleanup(accountCleanup(ctx, t)) + }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckAccountDestroy(ctx), @@ -120,11 +124,13 @@ func testAccAccount_cloudwatchRoleARN_value(t *testing.T) { func testAccAccount_cloudwatchRoleARN_empty(t *testing.T) { ctx := acctest.Context(t) - t.Cleanup(accountCleanup(ctx, t)) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + t.Cleanup(accountCleanup(ctx, t)) + }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckAccountDestroy(ctx), @@ -146,12 +152,14 @@ func testAccAccount_cloudwatchRoleARN_empty(t *testing.T) { func testAccAccount_resetOnDelete_false(t *testing.T) { ctx := acctest.Context(t) - t.Cleanup(accountCleanup(ctx, t)) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + t.Cleanup(accountCleanup(ctx, t)) + }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckAccountNotDestroyed(ctx), @@ -181,12 +189,14 @@ func testAccAccount_resetOnDelete_false(t *testing.T) { func testAccAccount_resetOnDelete_true(t *testing.T) { ctx := acctest.Context(t) - t.Cleanup(accountCleanup(ctx, t)) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + t.Cleanup(accountCleanup(ctx, t)) + }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckAccountDestroy(ctx), @@ -216,11 +226,13 @@ func testAccAccount_resetOnDelete_true(t *testing.T) { func testAccAccount_frameworkMigration_basic(t *testing.T) { ctx := acctest.Context(t) - t.Cleanup(accountCleanup(ctx, t)) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + t.Cleanup(accountCleanup(ctx, t)) + }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), CheckDestroy: testAccCheckAccountDestroy(ctx), Steps: []resource.TestStep{ @@ -247,12 +259,14 @@ func testAccAccount_frameworkMigration_basic(t *testing.T) { func testAccAccount_frameworkMigration_cloudwatchRoleARN(t *testing.T) { ctx := acctest.Context(t) - t.Cleanup(accountCleanup(ctx, t)) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_api_gateway_account.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, + PreCheck: func() { + acctest.PreCheck(ctx, t) + t.Cleanup(accountCleanup(ctx, t)) + }, ErrorCheck: acctest.ErrorCheck(t, names.APIGatewayServiceID), CheckDestroy: testAccCheckAccountNotDestroyed(ctx), Steps: []resource.TestStep{ From 37b97bf22f78232b5f405a66a37c22ee35b6e4ac Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:23:05 -0500 Subject: [PATCH 21/45] ipam_pool: Move declaration to when used --- internal/service/ec2/ipam_pool.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/internal/service/ec2/ipam_pool.go b/internal/service/ec2/ipam_pool.go index 138e10398c0..e8f813e2356 100644 --- a/internal/service/ec2/ipam_pool.go +++ b/internal/service/ec2/ipam_pool.go @@ -155,11 +155,6 @@ func resourceIPAMPoolCreate(ctx context.Context, d *schema.ResourceData, meta in conn := meta.(*conns.AWSClient).EC2Client(ctx) scopeID := d.Get("ipam_scope_id").(string) - scope, err := findIPAMScopeByID(ctx, conn, scopeID) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IPAM Scope (%s): %s", scopeID, err) - } addressFamily := awstypes.AddressFamily(d.Get("address_family").(string)) input := &ec2.CreateIpamPoolInput{ @@ -204,6 +199,12 @@ func resourceIPAMPoolCreate(ctx context.Context, d *schema.ResourceData, meta in if v, ok := d.GetOk("public_ip_source"); ok { input.PublicIpSource = awstypes.IpamPoolPublicIpSource(v.(string)) } + + scope, err := findIPAMScopeByID(ctx, conn, scopeID) + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading IPAM Scope (%s): %s", scopeID, err) + } + // PubliclyAdvertisable must be set if if the AddressFamily is IPv6 and PublicIpSource is byoip (either '' or 'byoip'). // The request can't contain PubliclyAdvertisable if PublicIpSource is 'amazon'. if addressFamily == awstypes.AddressFamilyIpv6 && scope.IpamScopeType == awstypes.IpamScopeTypePublic && input.PublicIpSource != awstypes.IpamPoolPublicIpSourceAmazon { From ed80c854b53a36cd0ec7cd674da83a20d80cc634 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 7 Nov 2024 10:24:39 -0800 Subject: [PATCH 22/45] Linting --- internal/service/apigateway/account_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/service/apigateway/account_test.go b/internal/service/apigateway/account_test.go index 4687b26dce5..c63e66ce9c7 100644 --- a/internal/service/apigateway/account_test.go +++ b/internal/service/apigateway/account_test.go @@ -307,7 +307,6 @@ func testAccCheckAccountDestroy(ctx context.Context) resource.TestCheckFunc { if account.CloudwatchRoleArn == nil { // Settings have been reset return nil - } return errors.New("API Gateway Account still exists") From 418c45cc755e00632b519939801c69e8d8e7518d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:27:38 -0500 Subject: [PATCH 23/45] docs/ipam_pool: Add clarification --- website/docs/r/vpc_ipam_pool.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/vpc_ipam_pool.html.markdown b/website/docs/r/vpc_ipam_pool.html.markdown index dd56e5c1a30..8238167aa01 100644 --- a/website/docs/r/vpc_ipam_pool.html.markdown +++ b/website/docs/r/vpc_ipam_pool.html.markdown @@ -81,7 +81,7 @@ within the CIDR range in the pool. * `description` - (Optional) A description for the IPAM pool. * `ipam_scope_id` - (Required) The ID of the scope in which you would like to create the IPAM pool. * `locale` - (Optional) The locale in which you would like to create the IPAM pool. Locale is the Region where you want to make an IPAM pool available for allocations. You can only create pools with locales that match the operating Regions of the IPAM. You can only create VPCs from a pool whose locale matches the VPC's Region. Possible values: Any AWS region, such as `us-east-1`. -* `publicly_advertisable` - (Optional) Defines whether or not IPv6 pool space is publicly advertisable over the internet. This argument is required if `address_family = "ipv6"` and `public_ip_source = "byoip"`, default is `false`. This option is not available for IPv4 pool space or if `public_ip_source = "amazon"`. +* `publicly_advertisable` - (Optional) Defines whether or not IPv6 pool space is publicly advertisable over the internet. This argument is required if `address_family = "ipv6"` and `public_ip_source = "byoip"`, default is `false`. This option is not available for IPv4 pool space or if `public_ip_source = "amazon"`. Setting this argument to `true` when it is not available may result in erroneous differences being reported. * `public_ip_source` - (Optional) The IP address source for pools in the public scope. Only used for provisioning IP address CIDRs to pools in the public scope. Valid values are `byoip` or `amazon`. Default is `byoip`. * `source_ipam_pool_id` - (Optional) The ID of the source IPAM pool. Use this argument to create a child pool within an existing pool. * `tags` - (Optional) A map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. From b2dd8324d01eb02b9c3d9f641b888282179a110c Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:28:22 -0500 Subject: [PATCH 24/45] tests/ipam_pool: Fix format --- internal/service/ec2/ipam_pool_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/service/ec2/ipam_pool_test.go b/internal/service/ec2/ipam_pool_test.go index c457bc0dfd5..bbfdb169f50 100644 --- a/internal/service/ec2/ipam_pool_test.go +++ b/internal/service/ec2/ipam_pool_test.go @@ -400,11 +400,11 @@ resource "aws_vpc_ipam_pool" "test" { var testAccIPAMPoolConfig_ipv6PublicIPAmazon = acctest.ConfigCompose(testAccIPAMPoolConfig_base, ` resource "aws_vpc_ipam_pool" "test" { - address_family = "ipv6" - ipam_scope_id = aws_vpc_ipam.test.public_default_scope_id - locale = data.aws_region.current.name - public_ip_source = "amazon" - aws_service = "ec2" + address_family = "ipv6" + ipam_scope_id = aws_vpc_ipam.test.public_default_scope_id + locale = data.aws_region.current.name + public_ip_source = "amazon" + aws_service = "ec2" } `) From f95b677c6abcaadc6fa777da86b0d6b36a0f5b67 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:31:18 -0500 Subject: [PATCH 25/45] tests/ipam_pool: Fix format --- internal/service/ec2/ipam_pool_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/ec2/ipam_pool_test.go b/internal/service/ec2/ipam_pool_test.go index bbfdb169f50..b1f2852cae5 100644 --- a/internal/service/ec2/ipam_pool_test.go +++ b/internal/service/ec2/ipam_pool_test.go @@ -404,7 +404,7 @@ resource "aws_vpc_ipam_pool" "test" { ipam_scope_id = aws_vpc_ipam.test.public_default_scope_id locale = data.aws_region.current.name public_ip_source = "amazon" - aws_service = "ec2" + aws_service = "ec2" } `) From 619fecb91325019323723c93bd680fca6adff7c3 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 13:50:21 -0500 Subject: [PATCH 26/45] Lower memory use of golangci, avoid OOM doom --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 925ad7e64cb..7984d4ee338 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -100,4 +100,4 @@ jobs: # Trigger garbage collection more frequently to reduce the likelihood # of OOM errors. Higher values mean it runs faster but more likely to OOM, exit 137. # ref: https://golangci-lint.run/product/performance/ - GOGC: "150" # 100 is the default value + GOGC: "140" # 100 is the default value From c8fa0fa3eb40e37c59e7b28e44659b51609e35e5 Mon Sep 17 00:00:00 2001 From: changelogbot Date: Thu, 7 Nov 2024 19:15:38 +0000 Subject: [PATCH 27/45] Update CHANGELOG.md for #40039 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index da2de38e19a..3dcebf3def5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,9 @@ BUG FIXES: * resource/aws_codedeploy_deployment_group: Remove maximum items limit on the `alarm_configuration.alarms` argument ([#39971](https://github.com/hashicorp/terraform-provider-aws/issues/39971)) * resource/aws_eks_addon: Handle `ResourceNotFound` exceptions during resource destruction ([#38357](https://github.com/hashicorp/terraform-provider-aws/issues/38357)) * resource/aws_elasticache_reserved_cache_node: Fix `Value Conversion Error` during resource creation ([#39945](https://github.com/hashicorp/terraform-provider-aws/issues/39945)) +* resource/aws_lb_listener: Fix errors when updating the `tcp_idle_timeout_seconds` argument for gateway load balancers ([#40039](https://github.com/hashicorp/terraform-provider-aws/issues/40039)) +* resource/aws_lb_listener: Remove the default `tcp_idle_timeout_seconds` value, preventing `ModifyListenerAttributes` API calls when a value is not explicitly configured ([#40039](https://github.com/hashicorp/terraform-provider-aws/issues/40039)) +* resource/aws_vpc_ipam_pool: Fix bug when `public_ip_source = "amazon"`: `The request can only contain PubliclyAdvertisable if the AddressFamily is IPv6 and PublicIpSource is byoip.` ([#40042](https://github.com/hashicorp/terraform-provider-aws/issues/40042)) ## 5.74.0 (October 31, 2024) From b7fbc586c8b1d20c9e08c5ac44b0155cbac7b47a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:08:44 -0500 Subject: [PATCH 28/45] dynamodb/table: Fix ability to disable dynamodb table ttl --- internal/service/dynamodb/table.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/service/dynamodb/table.go b/internal/service/dynamodb/table.go index 6535dec9599..2a11aa797bf 100644 --- a/internal/service/dynamodb/table.go +++ b/internal/service/dynamodb/table.go @@ -473,6 +473,14 @@ func resourceTable() *schema.Resource { "attribute_name": { Type: schema.TypeString, Optional: true, + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + // AWS requires the attribute name to be set when disabling TTL but + // does not return it so it causes a diff. + if old == "" && new != "" && !d.Get("ttl.0.enabled").(bool) { + return true + } + return false + }, }, names.AttrEnabled: { Type: schema.TypeBool, @@ -2628,13 +2636,9 @@ func ttlPlantimeValidate(ttlPath cty.Path, ttl cty.Value, diags *diag.Diagnostic errs.PathString(ttlPath.GetAttr("attribute_name")), )) } - } else { - if !(attribute.IsNull() || attribute.AsString() == "") { - *diags = append(*diags, errs.NewAttributeConflictsWhenError( - ttlPath.GetAttr("attribute_name"), - ttlPath.GetAttr(names.AttrEnabled), - "false", - )) - } } + + // !! Not a validation error for attribute_name to be set when enabled is false !! + // AWS *requires* attribute_name to be set when disabling TTL but does not return it, causing a diff. + // The diff is handled by DiffSuppressFunc of AttrEnabled. } From 7fc2e3fb1875c8144371f8088bd0e2c9e02a689e Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:11:42 -0500 Subject: [PATCH 29/45] Add changelog --- .changelog/40046.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/40046.txt diff --git a/.changelog/40046.txt b/.changelog/40046.txt new file mode 100644 index 00000000000..656630be1ba --- /dev/null +++ b/.changelog/40046.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_dynamodb_table: Allow table TTL to be disabled by allowing `ttl[0].attribute_name` to be set +``` From 41c7e94c99c4f735d239dabba54344a377656633 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:12:13 -0500 Subject: [PATCH 30/45] dynamodb/table: Test ability to disable TTL --- internal/service/dynamodb/table_test.go | 78 ++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index 91a8f121b16..396dd3de55f 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -10,6 +10,7 @@ import ( "reflect" "regexp" "testing" + "time" "github.com/YakDriver/regexache" "github.com/aws/aws-sdk-go-v2/aws" @@ -1557,7 +1558,7 @@ func TestAccDynamoDBTable_TTL_disabled(t *testing.T) { // TTL tests must be split since it can only be updated once per hour // ValidationException: Time to live has been modified multiple times within a fixed interval -func TestAccDynamoDBTable_TTL_update(t *testing.T) { +func TestAccDynamoDBTable_TTL_updateEnable(t *testing.T) { ctx := acctest.Context(t) var table awstypes.TableDescription resourceName := "aws_dynamodb_table.test" @@ -1606,6 +1607,81 @@ func TestAccDynamoDBTable_TTL_update(t *testing.T) { }) } +// TestAccDynamoDBTable_TTL_updateDisable takes an hour because AWS does not allow disabling TTL +// for an hour after it was enabled. Otherwise, it will return the following error: +// ValidationException: Time to live has been modified multiple times within a fixed interval +// https://github.com/hashicorp/terraform-provider-aws/issues/39195 +func TestAccDynamoDBTable_TTL_updateDisable(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var table awstypes.TableDescription + resourceName := "aws_dynamodb_table.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.DynamoDBServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckTableDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccTableConfig_timeToLive(rName, rName, true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(ctx, resourceName, &table), + ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "attribute_name": knownvalue.StringExact(rName), + names.AttrEnabled: knownvalue.Bool(true), + }), + })), + }, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + PreConfig: func() { + // 5 & 10 have failed + time.Sleep(60 * time.Minute) + }, + Config: testAccTableConfig_timeToLive(rName, rName, false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(ctx, resourceName, &table), + ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "attribute_name": knownvalue.StringExact(""), + names.AttrEnabled: knownvalue.Bool(false), + }), + })), + }, + }, + { + Config: testAccTableConfig_timeToLive(rName, rName, false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInitialTableExists(ctx, resourceName, &table), + ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{ + knownvalue.ObjectExact(map[string]knownvalue.Check{ + "attribute_name": knownvalue.StringExact(""), + names.AttrEnabled: knownvalue.Bool(false), + }), + })), + }, + }, + }, + }) +} + func TestAccDynamoDBTable_TTL_validate(t *testing.T) { ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) From dd0cadee7fb98dac4f2fbad2f4e08e94ef002602 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:26:42 -0500 Subject: [PATCH 31/45] tests/dynamodb/table: Clean up comments --- internal/service/dynamodb/table_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index 396dd3de55f..766caae1f6c 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -1648,17 +1648,19 @@ func TestAccDynamoDBTable_TTL_updateDisable(t *testing.T) { }, { PreConfig: func() { - // 5 & 10 have failed + // AWS does not allow disabling TTL for an hour after it was enabled. Otherwise, it + // will return the following error: ValidationException: Time to live has been + // modified multiple times within a fixed interval time.Sleep(60 * time.Minute) }, - Config: testAccTableConfig_timeToLive(rName, rName, false), + Config: testAccTableConfig_timeToLive(rName, rName, false), // can't disable without attribute_name (2nd arg) Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInitialTableExists(ctx, resourceName, &table), ), ConfigStateChecks: []statecheck.StateCheck{ statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("ttl"), knownvalue.ListExact([]knownvalue.Check{ knownvalue.ObjectExact(map[string]knownvalue.Check{ - "attribute_name": knownvalue.StringExact(""), + "attribute_name": knownvalue.StringExact(""), // set attribute_name, but returned empty; diff suppressed names.AttrEnabled: knownvalue.Bool(false), }), })), From 3d6da2fe5d36d474592007ba9551c02628b0d3e5 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:36:51 -0500 Subject: [PATCH 32/45] Update changelog --- .changelog/40046.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/40046.txt b/.changelog/40046.txt index 656630be1ba..becdfe09b83 100644 --- a/.changelog/40046.txt +++ b/.changelog/40046.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_dynamodb_table: Allow table TTL to be disabled by allowing `ttl[0].attribute_name` to be set +resource/aws_dynamodb_table: Allow table TTL to be disabled by allowing `ttl[0].attribute_name` to be set when `ttl[0].enabled` is false ``` From 2c5cfdce14d90fde214342d541236c45b9f78669 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:38:00 -0500 Subject: [PATCH 33/45] Correct comment --- internal/service/dynamodb/table.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/dynamodb/table.go b/internal/service/dynamodb/table.go index 2a11aa797bf..ba455942330 100644 --- a/internal/service/dynamodb/table.go +++ b/internal/service/dynamodb/table.go @@ -2640,5 +2640,5 @@ func ttlPlantimeValidate(ttlPath cty.Path, ttl cty.Value, diags *diag.Diagnostic // !! Not a validation error for attribute_name to be set when enabled is false !! // AWS *requires* attribute_name to be set when disabling TTL but does not return it, causing a diff. - // The diff is handled by DiffSuppressFunc of AttrEnabled. + // The diff is handled by DiffSuppressFunc of attribute_name. } From e44f37d1989bc71a354b70d7df4006e2168b7d19 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:51:02 -0500 Subject: [PATCH 34/45] sagemaker/domain: Fix ValidationException with AppSecurityGroupMangement --- internal/service/sagemaker/domain.go | 30 ++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/service/sagemaker/domain.go b/internal/service/sagemaker/domain.go index a50bae15eeb..2358c25ca42 100644 --- a/internal/service/sagemaker/domain.go +++ b/internal/service/sagemaker/domain.go @@ -1474,7 +1474,7 @@ func resourceDomainCreate(ctx context.Context, d *schema.ResourceData, meta inte Tags: getTagsIn(ctx), } - if v, ok := d.GetOk("app_security_group_management"); ok { + if v, ok := d.GetOk("app_security_group_management"); ok && rstudioDomainEnabled(d.Get("domain_settings").([]interface{})) { input.AppSecurityGroupManagement = awstypes.AppSecurityGroupManagement(v.(string)) } @@ -1575,7 +1575,7 @@ func resourceDomainUpdate(ctx context.Context, d *schema.ResourceData, meta inte input.AppNetworkAccessType = awstypes.AppNetworkAccessType(v.(string)) } - if v, ok := d.GetOk("app_security_group_management"); ok { + if v, ok := d.GetOk("app_security_group_management"); ok && rstudioDomainEnabled(d.Get("domain_settings").([]interface{})) { input.AppSecurityGroupManagement = awstypes.AppSecurityGroupManagement(v.(string)) } @@ -1763,6 +1763,32 @@ func expandDomainSettingsUpdate(l []interface{}) *awstypes.DomainSettingsForUpda return config } +// rstudioDomainEnabled takes domain_settings and returns true if rstudio is enabled +func rstudioDomainEnabled(domainSettings []interface{}) bool { + if len(domainSettings) == 0 || domainSettings[0] == nil { + return false + } + + m := domainSettings[0].(map[string]interface{}) + + v, ok := m["r_studio_server_pro_domain_settings"].([]interface{}) + if !ok || len(v) < 1 { + return false + } + + rsspds, ok := v[0].(map[string]interface{}) + if !ok || len(rsspds) == 0 { + return false + } + + domainExecutionRoleArn, ok := rsspds["domain_execution_role_arn"].(string) + if !ok || domainExecutionRoleArn == "" { + return false + } + + return true +} + func expandRStudioServerProDomainSettingsUpdate(l []interface{}) *awstypes.RStudioServerProDomainSettingsForUpdate { if len(l) == 0 || l[0] == nil { return nil From 90a789f0a5afb91d69236e8d4a27a02bd5faa37c Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:53:40 -0500 Subject: [PATCH 35/45] Add changelog --- .changelog/40049.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/40049.txt diff --git a/.changelog/40049.txt b/.changelog/40049.txt new file mode 100644 index 00000000000..494a79cce06 --- /dev/null +++ b/.changelog/40049.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_sagemaker_domain: Fix issue causing a `ValidationException` on updates when RStudio is disabled on the domain +``` From 182998a84f9956775880bdbe366b3c72bfbdf5bc Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:54:08 -0500 Subject: [PATCH 36/45] sagemaker/test: Add new test --- internal/service/sagemaker/sagemaker_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/service/sagemaker/sagemaker_test.go b/internal/service/sagemaker/sagemaker_test.go index f4cf8747b6e..6ef6a2f5993 100644 --- a/internal/service/sagemaker/sagemaker_test.go +++ b/internal/service/sagemaker/sagemaker_test.go @@ -74,6 +74,7 @@ func TestAccSageMaker_serial(t *testing.T) { "rSessionAppSettings": testAccDomain_rSessionAppSettings, "rStudioServerProAppSettings": testAccDomain_rStudioServerProAppSettings, "rStudioServerProDomainSettings": testAccDomain_rStudioServerProDomainSettings, + "rStudioServerProDomainSettingsUpdate": testAccDomain_rStudioServerProDomainSettingsUpdate, "spaceSettingsKernelGatewayAppSettings": testAccDomain_spaceSettingsKernelGatewayAppSettings, "spaceSettingsJupyterLabAppSettings": testAccDomain_spaceSettingsJupyterLabAppSettings, "spaceSettingsSpaceStorageSettings": testAccDomain_spaceSettingsSpaceStorageSettings, From de7be565e5522709b5d45894481ecb2c79217245 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 15:54:43 -0500 Subject: [PATCH 37/45] sagemaker/domain/test: Test updating domain --- internal/service/sagemaker/domain_test.go | 77 +++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/internal/service/sagemaker/domain_test.go b/internal/service/sagemaker/domain_test.go index 70ce982913d..031c08151af 100644 --- a/internal/service/sagemaker/domain_test.go +++ b/internal/service/sagemaker/domain_test.go @@ -730,6 +730,42 @@ func testAccDomain_rStudioServerProDomainSettings(t *testing.T) { }) } +func testAccDomain_rStudioServerProDomainSettingsUpdate(t *testing.T) { + ctx := acctest.Context(t) + var domain sagemaker.DescribeDomainOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_sagemaker_domain.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.SageMakerServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckDomainDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccDomainConfig_rStudioServerProDomainSettingsUpdate(rName, "PublicInternetOnly", "https://connect.domain.com", "https://package.domain.com"), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(ctx, resourceName, &domain), + resource.TestCheckResourceAttr(resourceName, "domain_settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.0.r_studio_connect_url", "https://connect.domain.com"), + resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.0.r_studio_package_manager_url", "https://package.domain.com"), + ), + }, + { + Config: testAccDomainConfig_rStudioServerProDomainSettingsUpdate(rName, "VpcOnly", "https://connect.domain.com", "https://package.domain.com"), + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(ctx, resourceName, &domain), + resource.TestCheckResourceAttr(resourceName, "domain_settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.0.r_studio_connect_url", "https://connect.domain.com"), + resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.0.r_studio_package_manager_url", "https://package.domain.com"), + ), + }, + }, + }) +} + func testAccDomain_kernelGatewayAppSettings(t *testing.T) { ctx := acctest.Context(t) var domain sagemaker.DescribeDomainOutput @@ -2487,6 +2523,47 @@ resource "aws_sagemaker_domain" "test" { `, rName, connectURL, packageURL)) } +func testAccDomainConfig_rStudioServerProDomainSettingsUpdate(rName, networkAccess, connectURL, packageURL string) string { + return acctest.ConfigCompose(testAccDomainConfig_baseWithLicense(rName), fmt.Sprintf(` +resource "aws_sagemaker_domain" "test" { + domain_name = %[1]q + auth_mode = "IAM" + vpc_id = aws_vpc.test.id + subnet_ids = aws_subnet.test[*].id + app_network_access_type = %[2]q + + domain_settings { + + r_studio_server_pro_domain_settings { + r_studio_connect_url = %[3]q + r_studio_package_manager_url = %[4]q + domain_execution_role_arn = aws_iam_role.test.arn + default_resource_spec { + instance_type = "system" + } + } + } + + default_user_settings { + execution_role = aws_iam_role.test.arn + } + + retention_policy { + home_efs_file_system = "Delete" + } + + # ignoring default image + # it would be too hard to create the logic to find the default Rstudio image: https://docs.aws.amazon.com/sagemaker/latest/dg/rstudio-version.html + # it changes for every region + lifecycle { + ignore_changes = [ + domain_settings[0].r_studio_server_pro_domain_settings[0].default_resource_spec[0] + ] + } +} +`, rName, networkAccess, connectURL, packageURL)) +} + func testAccDomainConfig_baseWithLicense(rName string) string { return acctest.ConfigCompose(acctest.ConfigVPCWithSubnets(rName, 1), fmt.Sprintf(` data "aws_partition" "current" {} From 105e7fd7cab89cb1f0a3415b469cc889c0ac60a3 Mon Sep 17 00:00:00 2001 From: changelogbot Date: Thu, 7 Nov 2024 20:58:34 +0000 Subject: [PATCH 38/45] Update CHANGELOG.md after v5.75.0 --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dcebf3def5..a2bd01da1c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ -## 5.75.0 (Unreleased) +## 5.76.0 (Unreleased) +## 5.75.0 (November 7, 2024) BREAKING CHANGES: From 02f3e4150177b2c22d0c74a833d20eb2870e454a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 7 Nov 2024 16:09:38 -0500 Subject: [PATCH 39/45] sagemaker/tests: Rename test --- internal/service/sagemaker/domain_test.go | 44 ++++---------------- internal/service/sagemaker/sagemaker_test.go | 2 +- 2 files changed, 10 insertions(+), 36 deletions(-) diff --git a/internal/service/sagemaker/domain_test.go b/internal/service/sagemaker/domain_test.go index 031c08151af..e2210f04322 100644 --- a/internal/service/sagemaker/domain_test.go +++ b/internal/service/sagemaker/domain_test.go @@ -730,7 +730,7 @@ func testAccDomain_rStudioServerProDomainSettings(t *testing.T) { }) } -func testAccDomain_rStudioServerProDomainSettingsUpdate(t *testing.T) { +func testAccDomain_rStudioDomainDisabledNetworkUpdate(t *testing.T) { ctx := acctest.Context(t) var domain sagemaker.DescribeDomainOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -743,24 +743,19 @@ func testAccDomain_rStudioServerProDomainSettingsUpdate(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccDomainConfig_rStudioServerProDomainSettingsUpdate(rName, "PublicInternetOnly", "https://connect.domain.com", "https://package.domain.com"), + Config: testAccDomainConfig_rStudioDomainDisabledNetworkUpdate(rName, "PublicInternetOnly"), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(ctx, resourceName, &domain), - resource.TestCheckResourceAttr(resourceName, "domain_settings.#", "1"), - resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.#", "1"), - resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.0.r_studio_connect_url", "https://connect.domain.com"), - resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.0.r_studio_package_manager_url", "https://package.domain.com"), + resource.TestCheckResourceAttr(resourceName, "domain_settings.#", "0"), + resource.TestCheckResourceAttr(resourceName, "app_network_access_type", "PublicInternetOnly"), ), }, { - Config: testAccDomainConfig_rStudioServerProDomainSettingsUpdate(rName, "VpcOnly", "https://connect.domain.com", "https://package.domain.com"), + Config: testAccDomainConfig_rStudioDomainDisabledNetworkUpdate(rName, "VpcOnly"), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(ctx, resourceName, &domain), - resource.TestCheckResourceAttr(resourceName, "domain_settings.#", "1"), - resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.#", "1"), - resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.0.r_studio_connect_url", "https://connect.domain.com"), - resource.TestCheckResourceAttr(resourceName, "domain_settings.0.r_studio_server_pro_domain_settings.0.r_studio_package_manager_url", "https://package.domain.com"), - ), + resource.TestCheckResourceAttr(resourceName, "domain_settings.#", "0"), + resource.TestCheckResourceAttr(resourceName, "app_network_access_type", "VpcOnly")), }, }, }) @@ -2523,7 +2518,7 @@ resource "aws_sagemaker_domain" "test" { `, rName, connectURL, packageURL)) } -func testAccDomainConfig_rStudioServerProDomainSettingsUpdate(rName, networkAccess, connectURL, packageURL string) string { +func testAccDomainConfig_rStudioDomainDisabledNetworkUpdate(rName, networkAccess string) string { return acctest.ConfigCompose(testAccDomainConfig_baseWithLicense(rName), fmt.Sprintf(` resource "aws_sagemaker_domain" "test" { domain_name = %[1]q @@ -2532,18 +2527,6 @@ resource "aws_sagemaker_domain" "test" { subnet_ids = aws_subnet.test[*].id app_network_access_type = %[2]q - domain_settings { - - r_studio_server_pro_domain_settings { - r_studio_connect_url = %[3]q - r_studio_package_manager_url = %[4]q - domain_execution_role_arn = aws_iam_role.test.arn - default_resource_spec { - instance_type = "system" - } - } - } - default_user_settings { execution_role = aws_iam_role.test.arn } @@ -2551,17 +2534,8 @@ resource "aws_sagemaker_domain" "test" { retention_policy { home_efs_file_system = "Delete" } - - # ignoring default image - # it would be too hard to create the logic to find the default Rstudio image: https://docs.aws.amazon.com/sagemaker/latest/dg/rstudio-version.html - # it changes for every region - lifecycle { - ignore_changes = [ - domain_settings[0].r_studio_server_pro_domain_settings[0].default_resource_spec[0] - ] - } } -`, rName, networkAccess, connectURL, packageURL)) +`, rName, networkAccess)) } func testAccDomainConfig_baseWithLicense(rName string) string { diff --git a/internal/service/sagemaker/sagemaker_test.go b/internal/service/sagemaker/sagemaker_test.go index 6ef6a2f5993..64edde8d121 100644 --- a/internal/service/sagemaker/sagemaker_test.go +++ b/internal/service/sagemaker/sagemaker_test.go @@ -74,7 +74,7 @@ func TestAccSageMaker_serial(t *testing.T) { "rSessionAppSettings": testAccDomain_rSessionAppSettings, "rStudioServerProAppSettings": testAccDomain_rStudioServerProAppSettings, "rStudioServerProDomainSettings": testAccDomain_rStudioServerProDomainSettings, - "rStudioServerProDomainSettingsUpdate": testAccDomain_rStudioServerProDomainSettingsUpdate, + "rStudioDomainDisabledNetworkUpdate": testAccDomain_rStudioDomainDisabledNetworkUpdate, "spaceSettingsKernelGatewayAppSettings": testAccDomain_spaceSettingsKernelGatewayAppSettings, "spaceSettingsJupyterLabAppSettings": testAccDomain_spaceSettingsJupyterLabAppSettings, "spaceSettingsSpaceStorageSettings": testAccDomain_spaceSettingsSpaceStorageSettings, From 492c47b49c53ab2c875d40b2eba4a524d4b97dfd Mon Sep 17 00:00:00 2001 From: changelogbot Date: Thu, 7 Nov 2024 22:49:27 +0000 Subject: [PATCH 40/45] Update CHANGELOG.md for #40004 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2bd01da1c2..e861a5cb631 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,9 @@ ## 5.76.0 (Unreleased) + +ENHANCEMENTS: + +* resource/aws_api_gateway_account: Add attribute `reset_on_delete` to properly reset CloudWatch Role ARN on deletion. ([#40004](https://github.com/hashicorp/terraform-provider-aws/issues/40004)) + ## 5.75.0 (November 7, 2024) BREAKING CHANGES: From 2d9ede6a47fcd74865289894a331aa9e632f7c5e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 8 Nov 2024 09:15:00 -0500 Subject: [PATCH 41/45] build(deps): bump goreleaser/goreleaser-action from 6.0.0 to 6.1.0 (#40051) Bumps [goreleaser/goreleaser-action](https://github.com/goreleaser/goreleaser-action) from 6.0.0 to 6.1.0. - [Release notes](https://github.com/goreleaser/goreleaser-action/releases) - [Commits](https://github.com/goreleaser/goreleaser-action/compare/286f3b13b1b49da4ac219696163fb8c1c93e1200...9ed2f89a662bf1735a48bc8557fd212fa902bebf) --- updated-dependencies: - dependency-name: goreleaser/goreleaser-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/goreleaser-ci.yml | 4 ++-- .github/workflows/snapshot.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/goreleaser-ci.yml b/.github/workflows/goreleaser-ci.yml index fc5c952e5af..0f175105d45 100644 --- a/.github/workflows/goreleaser-ci.yml +++ b/.github/workflows/goreleaser-ci.yml @@ -48,7 +48,7 @@ jobs: path: ~/go/pkg/mod key: ${{ runner.os }}-go-pkg-mod-${{ hashFiles('go.sum') }} - name: goreleaser check - uses: goreleaser/goreleaser-action@286f3b13b1b49da4ac219696163fb8c1c93e1200 # v6.0.0 + uses: goreleaser/goreleaser-action@9ed2f89a662bf1735a48bc8557fd212fa902bebf # v6.1.0 with: args: check @@ -68,6 +68,6 @@ jobs: path: ~/go/pkg/mod key: ${{ runner.os }}-go-pkg-mod-${{ hashFiles('go.sum') }} - name: goreleaser build - uses: goreleaser/goreleaser-action@286f3b13b1b49da4ac219696163fb8c1c93e1200 # v6.0.0 + uses: goreleaser/goreleaser-action@9ed2f89a662bf1735a48bc8557fd212fa902bebf # v6.1.0 with: args: build --config .github/goreleaser-cross-compiler-test.yml --id 32-bit-arch --snapshot diff --git a/.github/workflows/snapshot.yml b/.github/workflows/snapshot.yml index fa00b3bf7cb..ab5034022f1 100644 --- a/.github/workflows/snapshot.yml +++ b/.github/workflows/snapshot.yml @@ -20,7 +20,7 @@ jobs: path: ~/go/pkg/mod key: ${{ runner.os }}-go-pkg-mod-${{ hashFiles('go.sum') }} - name: goreleaser release - uses: goreleaser/goreleaser-action@286f3b13b1b49da4ac219696163fb8c1c93e1200 # v6.0.0 + uses: goreleaser/goreleaser-action@9ed2f89a662bf1735a48bc8557fd212fa902bebf # v6.1.0 with: args: release --clean --skip=sign --snapshot --timeout 2h version: "~> v2" From 34d2ab20631a87e13b1276a7d4501a3c7bd647d8 Mon Sep 17 00:00:00 2001 From: Stefan Freitag Date: Fri, 8 Nov 2024 15:16:53 +0100 Subject: [PATCH 42/45] docs: add argument `validation_window_hours` (#40044) --- website/docs/r/backup_restore_testing_selection.html.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/r/backup_restore_testing_selection.html.markdown b/website/docs/r/backup_restore_testing_selection.html.markdown index 454af639818..65cbd6ae18c 100644 --- a/website/docs/r/backup_restore_testing_selection.html.markdown +++ b/website/docs/r/backup_restore_testing_selection.html.markdown @@ -56,6 +56,7 @@ The following arguments are supported: * `protected_resource_arns` - (Optional) The ARNs for the protected resources. * `protected_resource_conditions` - (Optional) The conditions for the protected resource. * `restore_metadata_overrides` - (Optional) Override certain restore metadata keys. See the complete list of [restore testing inferred metadata](https://docs.aws.amazon.com/aws-backup/latest/devguide/restore-testing-inferred-metadata.html) . +* `validation_window_hours` - (Optional) The amount of hours available to run a validation script on the data. Valid range is `1` to `168`. The `protected_resource_conditions` block supports the following arguments: From 7fc9ada394b262ef3eb0a0547b317406d5c61702 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 8 Nov 2024 14:58:21 -0500 Subject: [PATCH 43/45] dynamodb/table: TTL validation test no longer valid --- internal/service/dynamodb/table_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index 766caae1f6c..ea287aeb935 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -1694,10 +1694,6 @@ func TestAccDynamoDBTable_TTL_validate(t *testing.T) { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckTableDestroy(ctx), Steps: []resource.TestStep{ - { - Config: testAccTableConfig_timeToLive(rName, "TestTTL", false), - ExpectError: regexache.MustCompile(regexp.QuoteMeta(`Attribute "ttl[0].attribute_name" cannot be specified when "ttl[0].enabled" is "false"`)), - }, { Config: testAccTableConfig_timeToLive(rName, "", true), ExpectError: regexache.MustCompile(regexp.QuoteMeta(`Attribute "ttl[0].attribute_name" cannot have an empty value`)), From d48270f579446fb1544ede2626e7d3b6e0f912e2 Mon Sep 17 00:00:00 2001 From: changelogbot Date: Fri, 8 Nov 2024 19:59:00 +0000 Subject: [PATCH 44/45] Update CHANGELOG.md for #40046 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e861a5cb631..28768a56d23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,14 @@ ENHANCEMENTS: +* data-source/aws_cloudwatch_event_bus: Add `description` attribute ([#39980](https://github.com/hashicorp/terraform-provider-aws/issues/39980)) * resource/aws_api_gateway_account: Add attribute `reset_on_delete` to properly reset CloudWatch Role ARN on deletion. ([#40004](https://github.com/hashicorp/terraform-provider-aws/issues/40004)) +* resource/aws_cloudwatch_event_bus: Add `description` argument ([#39980](https://github.com/hashicorp/terraform-provider-aws/issues/39980)) + +BUG FIXES: + +* resource/aws_dynamodb_table: Allow table TTL to be disabled by allowing `ttl[0].attribute_name` to be set when `ttl[0].enabled` is false ([#40046](https://github.com/hashicorp/terraform-provider-aws/issues/40046)) +* resource/aws_sagemaker_domain: Fix issue causing a `ValidationException` on updates when RStudio is disabled on the domain ([#40049](https://github.com/hashicorp/terraform-provider-aws/issues/40049)) ## 5.75.0 (November 7, 2024) From 9d83ece5d8d7061f637dabcdda37bf3f012f5018 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 8 Nov 2024 15:07:16 -0500 Subject: [PATCH 45/45] Add short gates --- internal/service/dynamodb/table_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/service/dynamodb/table_test.go b/internal/service/dynamodb/table_test.go index ea287aeb935..7f4597d207e 100644 --- a/internal/service/dynamodb/table_test.go +++ b/internal/service/dynamodb/table_test.go @@ -776,6 +776,10 @@ func TestAccDynamoDBTable_BillingMode_provisionedToPayPerRequestIgnoreChanges(t func TestAccDynamoDBTable_BillingModeGSI_payPerRequestToProvisioned(t *testing.T) { ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + var conf awstypes.TableDescription resourceName := "aws_dynamodb_table.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -814,6 +818,10 @@ func TestAccDynamoDBTable_BillingModeGSI_payPerRequestToProvisioned(t *testing.T func TestAccDynamoDBTable_BillingModeGSI_provisionedToPayPerRequest(t *testing.T) { ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + var conf awstypes.TableDescription resourceName := "aws_dynamodb_table.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -849,6 +857,10 @@ func TestAccDynamoDBTable_BillingModeGSI_provisionedToPayPerRequest(t *testing.T func TestAccDynamoDBTable_BillingMode_payPerRequestBasic(t *testing.T) { ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + var conf awstypes.TableDescription resourceName := "aws_dynamodb_table.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)