From 4005ced1adab385f435f429c8bc83d3be6aa651e Mon Sep 17 00:00:00 2001 From: vasiliyparfenov Date: Sun, 4 Dec 2022 18:34:01 +0200 Subject: [PATCH 01/11] fix: sns topic subcription attributes order --- internal/service/sns/topic_subscription.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/sns/topic_subscription.go b/internal/service/sns/topic_subscription.go index 99aa586c6d9..84dcc0fc273 100644 --- a/internal/service/sns/topic_subscription.go +++ b/internal/service/sns/topic_subscription.go @@ -114,8 +114,8 @@ var ( "confirmation_was_authenticated": SubscriptionAttributeNameConfirmationWasAuthenticated, "delivery_policy": SubscriptionAttributeNameDeliveryPolicy, "endpoint": SubscriptionAttributeNameEndpoint, - "filter_policy": SubscriptionAttributeNameFilterPolicy, "filter_policy_scope": SubscriptionAttributeNameFilterPolicyScope, + "filter_policy": SubscriptionAttributeNameFilterPolicy, "owner_id": SubscriptionAttributeNameOwner, "pending_confirmation": SubscriptionAttributeNamePendingConfirmation, "protocol": SubscriptionAttributeNameProtocol, From 7dfaa9f42e17b77ad52c83379250db624075d310 Mon Sep 17 00:00:00 2001 From: vasiliyparfenov Date: Mon, 5 Dec 2022 22:25:18 +0200 Subject: [PATCH 02/11] added acceptance test --- .../service/sns/topic_subscription_test.go | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/internal/service/sns/topic_subscription_test.go b/internal/service/sns/topic_subscription_test.go index e9f1ad75667..231321af1ea 100644 --- a/internal/service/sns/topic_subscription_test.go +++ b/internal/service/sns/topic_subscription_test.go @@ -295,6 +295,55 @@ func TestAccSNSTopicSubscription_filterPolicyScope_policyNotSet(t *testing.T) { }) } +func TestAccSNSTopicSubscription_filterPolicyScope_filterPolicyWithComplexObject(t *testing.T) { + + var attributes map[string]string + resourceName := "aws_sns_topic_subscription.test" + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, sns.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckTopicSubscriptionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccTopicSubscriptionConfig_filterPolicyScope(rName, strconv.Quote("MessageAttributes")), + Check: resource.ComposeTestCheckFunc( + testAccCheckTopicSubscriptionExists(resourceName, &attributes), + resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageAttributes"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "confirmation_timeout_in_minutes", + "endpoint_auto_confirms", + }, + }, + { + Config: testAccTopicSubscriptionConfig_filterPolicyScope_filterPolicyWithComplexObject(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckTopicSubscriptionExists(resourceName, &attributes), + resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageBody"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "confirmation_timeout_in_minutes", + "endpoint_auto_confirms", + }, + }, + }, + }) +} + func TestAccSNSTopicSubscription_deliveryPolicy(t *testing.T) { var attributes map[string]string resourceName := "aws_sns_topic_subscription.test" @@ -792,6 +841,28 @@ resource "aws_sns_topic_subscription" "test" { `, rName, scope) } +func testAccTopicSubscriptionConfig_filterPolicyScope_filterPolicyWithComplexObject(rName string) string { + return fmt.Sprintf(` +resource "aws_sns_topic" "test" { + name = %[1]q +} + +resource "aws_sqs_queue" "test" { + name = %[1]q + + sqs_managed_sse_enabled = true +} + +resource "aws_sns_topic_subscription" "test" { + topic_arn = aws_sns_topic.test.arn + protocol = "sqs" + endpoint = aws_sqs_queue.test.arn + filter_policy = jsonencode({ key1 = { object = {key : ["value"]}}}) + filter_policy_scope = "MessageBody" +} +`, rName) +} + func testAccTopicSubscriptionConfig_filterPolicyScope_policyNotSet(rName string) string { return fmt.Sprintf(` resource "aws_sns_topic" "test" { From ae75b6ac9639e62ce3603eeaf53982125a18a450 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Sat, 24 Dec 2022 10:11:50 -0300 Subject: [PATCH 03/11] Acceptance tests --- .../service/sns/topic_subscription_test.go | 65 +++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/internal/service/sns/topic_subscription_test.go b/internal/service/sns/topic_subscription_test.go index fc3c24c0535..b36aee02db1 100644 --- a/internal/service/sns/topic_subscription_test.go +++ b/internal/service/sns/topic_subscription_test.go @@ -274,6 +274,56 @@ func TestAccSNSTopicSubscription_filterPolicyScope(t *testing.T) { "endpoint_auto_confirms", }, }, + // Test transition from MessageAttributes to nested MessageBody ... + { + Config: testAccTopicSubscriptionConfig_filterPolicyScope(rName, strconv.Quote("MessageAttributes")), + Check: resource.ComposeTestCheckFunc( + testAccCheckTopicSubscriptionExists(resourceName, &attributes), + resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageAttributes"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "confirmation_timeout_in_minutes", + "endpoint_auto_confirms", + }, + }, + { + Config: testAccTopicSubscriptionConfig_nestedFilterPolicyScope(rName, strconv.Quote("MessageBody"), true), + Check: resource.ComposeTestCheckFunc( + testAccCheckTopicSubscriptionExists(resourceName, &attributes), + resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageBody"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "confirmation_timeout_in_minutes", + "endpoint_auto_confirms", + }, + }, + // ... and transition from nested MessageBody back to flat MessageAttributes + { + Config: testAccTopicSubscriptionConfig_filterPolicyScope(rName, strconv.Quote("MessageAttributes")), + Check: resource.ComposeTestCheckFunc( + testAccCheckTopicSubscriptionExists(resourceName, &attributes), + resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageAttributes"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "confirmation_timeout_in_minutes", + "endpoint_auto_confirms", + }, + }, }, }) } @@ -770,7 +820,11 @@ resource "aws_sns_topic_subscription" "test" { `, rName, policy) } -func testAccTopicSubscriptionConfig_filterPolicyScope(rName, scope string) string { +func testAccTopicSubscriptionConfig_nestedFilterPolicyScope(rName, scope string, nested bool) string { + filterPolicy := `jsonencode({ key1 = ["value1"] })` + if nested { + filterPolicy = `jsonencode({ key1 = { key2 = {"value1"} } })` + } return fmt.Sprintf(` resource "aws_sns_topic" "test" { name = %[1]q @@ -786,10 +840,13 @@ resource "aws_sns_topic_subscription" "test" { topic_arn = aws_sns_topic.test.arn protocol = "sqs" endpoint = aws_sqs_queue.test.arn - filter_policy = jsonencode({ key1 = ["value1"] }) - filter_policy_scope = %[2]s + filter_policy = %[2]s + filter_policy_scope = %[3]s +} +`, rName, filterPolicy, scope) } -`, rName, scope) +func testAccTopicSubscriptionConfig_filterPolicyScope(rName, scope string) string { + return testAccTopicSubscriptionConfig_nestedFilterPolicyScope(rName, scope, false) } func testAccTopicSubscriptionConfig_filterPolicyScope_policyNotSet(rName string) string { From 49745fc7dc06dd90f079274009fe8805c58ca7d8 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Tue, 27 Dec 2022 08:38:05 -0300 Subject: [PATCH 04/11] Fix subscription policy scope update order --- internal/service/sns/topic_subscription.go | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/internal/service/sns/topic_subscription.go b/internal/service/sns/topic_subscription.go index 0a8784f09aa..f787bd1551b 100644 --- a/internal/service/sns/topic_subscription.go +++ b/internal/service/sns/topic_subscription.go @@ -261,6 +261,21 @@ func resourceTopicSubscriptionDelete(ctx context.Context, d *schema.ResourceData } func putSubscriptionAttributes(ctx context.Context, conn *sns.SNS, arn string, attributes map[string]string) error { + // Filter policy order matters + filterPolicyScope, ok := attributes[SubscriptionAttributeNameFilterPolicyScope] + + if ok { + delete(attributes, SubscriptionAttributeNameFilterPolicyScope) + } + + // MessageBody is backwards-compatible so it should always be applied first + if filterPolicyScope == SubscriptionFilterPolicyScopeMessageBody { + err := putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicyScope, filterPolicyScope) + if err != nil { + return err + } + } + for name, value := range attributes { err := putSubscriptionAttribute(ctx, conn, arn, name, value) @@ -269,6 +284,16 @@ func putSubscriptionAttributes(ctx context.Context, conn *sns.SNS, arn string, a } } + // MessageAttributes isn't compatible with nested policies, so it should always be last + // in case the update also includes a change from a nested policy to a flat policy + if filterPolicyScope == SubscriptionFilterPolicyScopeMessageAttributes { + err := putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicyScope, filterPolicyScope) + + if err != nil { + return err + } + } + return nil } From a908f9b48982801d8e57c906b3d99c8288aeb87a Mon Sep 17 00:00:00 2001 From: vasiliyparfenov Date: Wed, 7 Dec 2022 13:00:19 +0200 Subject: [PATCH 05/11] sns filter_policy update handling --- .changelog/28193.txt | 3 + internal/service/sns/topic_subscription.go | 64 ++++++++++++++++++- .../service/sns/topic_subscription_test.go | 33 ++++++++-- 3 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 .changelog/28193.txt diff --git a/.changelog/28193.txt b/.changelog/28193.txt new file mode 100644 index 00000000000..6b3f1faa836 --- /dev/null +++ b/.changelog/28193.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_sns_topic_subscription: Fix filter_policy update from MessageAttributes to MessageBody scope with nested objects +``` \ No newline at end of file diff --git a/internal/service/sns/topic_subscription.go b/internal/service/sns/topic_subscription.go index 84dcc0fc273..00f12c94709 100644 --- a/internal/service/sns/topic_subscription.go +++ b/internal/service/sns/topic_subscription.go @@ -114,8 +114,8 @@ var ( "confirmation_was_authenticated": SubscriptionAttributeNameConfirmationWasAuthenticated, "delivery_policy": SubscriptionAttributeNameDeliveryPolicy, "endpoint": SubscriptionAttributeNameEndpoint, - "filter_policy_scope": SubscriptionAttributeNameFilterPolicyScope, "filter_policy": SubscriptionAttributeNameFilterPolicy, + "filter_policy_scope": SubscriptionAttributeNameFilterPolicyScope, "owner_id": SubscriptionAttributeNameOwner, "pending_confirmation": SubscriptionAttributeNamePendingConfirmation, "protocol": SubscriptionAttributeNameProtocol, @@ -260,7 +260,69 @@ func resourceTopicSubscriptionDelete(ctx context.Context, d *schema.ResourceData return nil } +func handleFilterPolicyUpdate(ctx context.Context, conn *sns.SNS, arn string, attributes map[string]string) error { + + // In case both FilterPolicy and FilterPolicyScope are going to be updated, those changes should be made in specific order + // to mitigate issue with updating MessageAttribute filter policy to MessageBody nested fiter policy, + // when policy is updated before scope and fails due to MessageAttributes does not support nested filter policy. + // + // Error: setting SNS Topic Subscription (arn:aws:sns:eu-west-1::) attribute (FilterPolicy): + // InvalidParameter: Invalid parameter: Filter policy scope MessageAttributes does not support nested filter policy + + filterPolicyScope, containsFilterPolicyScopeUpdate := attributes[SubscriptionAttributeNameFilterPolicyScope] + filterPolicy, containsFilterPolicyUpdate := attributes[SubscriptionAttributeNameFilterPolicy] + + if containsFilterPolicyScopeUpdate && containsFilterPolicyUpdate { + + //MessageBody filterScope supports both nested and simple objects filterPolicy + + switch filterPolicyScope { + + case SubscriptionFilterPolicyScopeMessageBody: + + err := putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicyScope, filterPolicyScope) + + if err != nil { + return err + } + + err = putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicy, filterPolicy) + + if err != nil { + return err + } + + return nil + + case SubscriptionFilterPolicyScopeMessageAttributes: + + err := putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicy, filterPolicy) + + if err != nil { + return err + } + + err = putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicyScope, filterPolicyScope) + + if err != nil { + return err + } + + return nil + } + } + + return nil +} + func putSubscriptionAttributes(ctx context.Context, conn *sns.SNS, arn string, attributes map[string]string) error { + + err := handleFilterPolicyUpdate(ctx, conn, arn, attributes) + + if err != nil { + return err + } + for name, value := range attributes { err := putSubscriptionAttribute(ctx, conn, arn, name, value) diff --git a/internal/service/sns/topic_subscription_test.go b/internal/service/sns/topic_subscription_test.go index 231321af1ea..56be6f8bf12 100644 --- a/internal/service/sns/topic_subscription_test.go +++ b/internal/service/sns/topic_subscription_test.go @@ -295,7 +295,7 @@ func TestAccSNSTopicSubscription_filterPolicyScope_policyNotSet(t *testing.T) { }) } -func TestAccSNSTopicSubscription_filterPolicyScope_filterPolicyWithComplexObject(t *testing.T) { +func TestAccSNSTopicSubscription_filterPolicy_messageAttributesFilterPolicy_to_messageBodyNestedFilterPolicy(t *testing.T) { var attributes map[string]string resourceName := "aws_sns_topic_subscription.test" @@ -325,7 +325,16 @@ func TestAccSNSTopicSubscription_filterPolicyScope_filterPolicyWithComplexObject }, }, { - Config: testAccTopicSubscriptionConfig_filterPolicyScope_filterPolicyWithComplexObject(rName), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "confirmation_timeout_in_minutes", + "endpoint_auto_confirms", + }, + }, + { + Config: testAccTopicSubscriptionConfig_filterPolicy_messageBodyNestedFilterPolicy(rName), Check: resource.ComposeTestCheckFunc( testAccCheckTopicSubscriptionExists(resourceName, &attributes), resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageBody"), @@ -340,6 +349,22 @@ func TestAccSNSTopicSubscription_filterPolicyScope_filterPolicyWithComplexObject "endpoint_auto_confirms", }, }, + { + Config: testAccTopicSubscriptionConfig_filterPolicyScope(rName, strconv.Quote("MessageAttributes")), + Check: resource.ComposeTestCheckFunc( + testAccCheckTopicSubscriptionExists(resourceName, &attributes), + resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageAttributes"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "confirmation_timeout_in_minutes", + "endpoint_auto_confirms", + }, + }, }, }) } @@ -841,7 +866,7 @@ resource "aws_sns_topic_subscription" "test" { `, rName, scope) } -func testAccTopicSubscriptionConfig_filterPolicyScope_filterPolicyWithComplexObject(rName string) string { +func testAccTopicSubscriptionConfig_filterPolicy_messageBodyNestedFilterPolicy(rName string) string { return fmt.Sprintf(` resource "aws_sns_topic" "test" { name = %[1]q @@ -857,7 +882,7 @@ resource "aws_sns_topic_subscription" "test" { topic_arn = aws_sns_topic.test.arn protocol = "sqs" endpoint = aws_sqs_queue.test.arn - filter_policy = jsonencode({ key1 = { object = {key : ["value"]}}}) + filter_policy = jsonencode({ nested = { object = {key = ["value"]}}}) filter_policy_scope = "MessageBody" } `, rName) From c4cb35f3c0d06460a5615d01235683459a6715af Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 21 Feb 2023 11:56:14 -0500 Subject: [PATCH 06/11] Revert "sns filter_policy update handling" This reverts commit a908f9b48982801d8e57c906b3d99c8288aeb87a. --- .changelog/28193.txt | 3 - internal/service/sns/topic_subscription.go | 64 +------------------ .../service/sns/topic_subscription_test.go | 33 ++-------- 3 files changed, 5 insertions(+), 95 deletions(-) delete mode 100644 .changelog/28193.txt diff --git a/.changelog/28193.txt b/.changelog/28193.txt deleted file mode 100644 index 6b3f1faa836..00000000000 --- a/.changelog/28193.txt +++ /dev/null @@ -1,3 +0,0 @@ -```release-note:bug -resource/aws_sns_topic_subscription: Fix filter_policy update from MessageAttributes to MessageBody scope with nested objects -``` \ No newline at end of file diff --git a/internal/service/sns/topic_subscription.go b/internal/service/sns/topic_subscription.go index 00f12c94709..84dcc0fc273 100644 --- a/internal/service/sns/topic_subscription.go +++ b/internal/service/sns/topic_subscription.go @@ -114,8 +114,8 @@ var ( "confirmation_was_authenticated": SubscriptionAttributeNameConfirmationWasAuthenticated, "delivery_policy": SubscriptionAttributeNameDeliveryPolicy, "endpoint": SubscriptionAttributeNameEndpoint, - "filter_policy": SubscriptionAttributeNameFilterPolicy, "filter_policy_scope": SubscriptionAttributeNameFilterPolicyScope, + "filter_policy": SubscriptionAttributeNameFilterPolicy, "owner_id": SubscriptionAttributeNameOwner, "pending_confirmation": SubscriptionAttributeNamePendingConfirmation, "protocol": SubscriptionAttributeNameProtocol, @@ -260,69 +260,7 @@ func resourceTopicSubscriptionDelete(ctx context.Context, d *schema.ResourceData return nil } -func handleFilterPolicyUpdate(ctx context.Context, conn *sns.SNS, arn string, attributes map[string]string) error { - - // In case both FilterPolicy and FilterPolicyScope are going to be updated, those changes should be made in specific order - // to mitigate issue with updating MessageAttribute filter policy to MessageBody nested fiter policy, - // when policy is updated before scope and fails due to MessageAttributes does not support nested filter policy. - // - // Error: setting SNS Topic Subscription (arn:aws:sns:eu-west-1::) attribute (FilterPolicy): - // InvalidParameter: Invalid parameter: Filter policy scope MessageAttributes does not support nested filter policy - - filterPolicyScope, containsFilterPolicyScopeUpdate := attributes[SubscriptionAttributeNameFilterPolicyScope] - filterPolicy, containsFilterPolicyUpdate := attributes[SubscriptionAttributeNameFilterPolicy] - - if containsFilterPolicyScopeUpdate && containsFilterPolicyUpdate { - - //MessageBody filterScope supports both nested and simple objects filterPolicy - - switch filterPolicyScope { - - case SubscriptionFilterPolicyScopeMessageBody: - - err := putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicyScope, filterPolicyScope) - - if err != nil { - return err - } - - err = putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicy, filterPolicy) - - if err != nil { - return err - } - - return nil - - case SubscriptionFilterPolicyScopeMessageAttributes: - - err := putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicy, filterPolicy) - - if err != nil { - return err - } - - err = putSubscriptionAttribute(ctx, conn, arn, SubscriptionAttributeNameFilterPolicyScope, filterPolicyScope) - - if err != nil { - return err - } - - return nil - } - } - - return nil -} - func putSubscriptionAttributes(ctx context.Context, conn *sns.SNS, arn string, attributes map[string]string) error { - - err := handleFilterPolicyUpdate(ctx, conn, arn, attributes) - - if err != nil { - return err - } - for name, value := range attributes { err := putSubscriptionAttribute(ctx, conn, arn, name, value) diff --git a/internal/service/sns/topic_subscription_test.go b/internal/service/sns/topic_subscription_test.go index 56be6f8bf12..231321af1ea 100644 --- a/internal/service/sns/topic_subscription_test.go +++ b/internal/service/sns/topic_subscription_test.go @@ -295,7 +295,7 @@ func TestAccSNSTopicSubscription_filterPolicyScope_policyNotSet(t *testing.T) { }) } -func TestAccSNSTopicSubscription_filterPolicy_messageAttributesFilterPolicy_to_messageBodyNestedFilterPolicy(t *testing.T) { +func TestAccSNSTopicSubscription_filterPolicyScope_filterPolicyWithComplexObject(t *testing.T) { var attributes map[string]string resourceName := "aws_sns_topic_subscription.test" @@ -325,16 +325,7 @@ func TestAccSNSTopicSubscription_filterPolicy_messageAttributesFilterPolicy_to_m }, }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{ - "confirmation_timeout_in_minutes", - "endpoint_auto_confirms", - }, - }, - { - Config: testAccTopicSubscriptionConfig_filterPolicy_messageBodyNestedFilterPolicy(rName), + Config: testAccTopicSubscriptionConfig_filterPolicyScope_filterPolicyWithComplexObject(rName), Check: resource.ComposeTestCheckFunc( testAccCheckTopicSubscriptionExists(resourceName, &attributes), resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageBody"), @@ -349,22 +340,6 @@ func TestAccSNSTopicSubscription_filterPolicy_messageAttributesFilterPolicy_to_m "endpoint_auto_confirms", }, }, - { - Config: testAccTopicSubscriptionConfig_filterPolicyScope(rName, strconv.Quote("MessageAttributes")), - Check: resource.ComposeTestCheckFunc( - testAccCheckTopicSubscriptionExists(resourceName, &attributes), - resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageAttributes"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{ - "confirmation_timeout_in_minutes", - "endpoint_auto_confirms", - }, - }, }, }) } @@ -866,7 +841,7 @@ resource "aws_sns_topic_subscription" "test" { `, rName, scope) } -func testAccTopicSubscriptionConfig_filterPolicy_messageBodyNestedFilterPolicy(rName string) string { +func testAccTopicSubscriptionConfig_filterPolicyScope_filterPolicyWithComplexObject(rName string) string { return fmt.Sprintf(` resource "aws_sns_topic" "test" { name = %[1]q @@ -882,7 +857,7 @@ resource "aws_sns_topic_subscription" "test" { topic_arn = aws_sns_topic.test.arn protocol = "sqs" endpoint = aws_sqs_queue.test.arn - filter_policy = jsonencode({ nested = { object = {key = ["value"]}}}) + filter_policy = jsonencode({ key1 = { object = {key : ["value"]}}}) filter_policy_scope = "MessageBody" } `, rName) From db9372cae9fad09ec5ebe5f889fd3ed86b953e9d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 21 Feb 2023 11:58:07 -0500 Subject: [PATCH 07/11] Revert "added acceptance test" This reverts commit 7dfaa9f42e17b77ad52c83379250db624075d310. --- .../service/sns/topic_subscription_test.go | 71 ------------------- 1 file changed, 71 deletions(-) diff --git a/internal/service/sns/topic_subscription_test.go b/internal/service/sns/topic_subscription_test.go index 231321af1ea..e9f1ad75667 100644 --- a/internal/service/sns/topic_subscription_test.go +++ b/internal/service/sns/topic_subscription_test.go @@ -295,55 +295,6 @@ func TestAccSNSTopicSubscription_filterPolicyScope_policyNotSet(t *testing.T) { }) } -func TestAccSNSTopicSubscription_filterPolicyScope_filterPolicyWithComplexObject(t *testing.T) { - - var attributes map[string]string - resourceName := "aws_sns_topic_subscription.test" - - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, sns.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckTopicSubscriptionDestroy, - Steps: []resource.TestStep{ - { - Config: testAccTopicSubscriptionConfig_filterPolicyScope(rName, strconv.Quote("MessageAttributes")), - Check: resource.ComposeTestCheckFunc( - testAccCheckTopicSubscriptionExists(resourceName, &attributes), - resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageAttributes"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{ - "confirmation_timeout_in_minutes", - "endpoint_auto_confirms", - }, - }, - { - Config: testAccTopicSubscriptionConfig_filterPolicyScope_filterPolicyWithComplexObject(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckTopicSubscriptionExists(resourceName, &attributes), - resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageBody"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{ - "confirmation_timeout_in_minutes", - "endpoint_auto_confirms", - }, - }, - }, - }) -} - func TestAccSNSTopicSubscription_deliveryPolicy(t *testing.T) { var attributes map[string]string resourceName := "aws_sns_topic_subscription.test" @@ -841,28 +792,6 @@ resource "aws_sns_topic_subscription" "test" { `, rName, scope) } -func testAccTopicSubscriptionConfig_filterPolicyScope_filterPolicyWithComplexObject(rName string) string { - return fmt.Sprintf(` -resource "aws_sns_topic" "test" { - name = %[1]q -} - -resource "aws_sqs_queue" "test" { - name = %[1]q - - sqs_managed_sse_enabled = true -} - -resource "aws_sns_topic_subscription" "test" { - topic_arn = aws_sns_topic.test.arn - protocol = "sqs" - endpoint = aws_sqs_queue.test.arn - filter_policy = jsonencode({ key1 = { object = {key : ["value"]}}}) - filter_policy_scope = "MessageBody" -} -`, rName) -} - func testAccTopicSubscriptionConfig_filterPolicyScope_policyNotSet(rName string) string { return fmt.Sprintf(` resource "aws_sns_topic" "test" { From 03fbe6dd59a3918a64f89efa8acb245b74f1ada8 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 21 Feb 2023 11:58:15 -0500 Subject: [PATCH 08/11] Revert "fix: sns topic subcription attributes order" This reverts commit 4005ced1adab385f435f429c8bc83d3be6aa651e. --- internal/service/sns/topic_subscription.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/sns/topic_subscription.go b/internal/service/sns/topic_subscription.go index 84dcc0fc273..99aa586c6d9 100644 --- a/internal/service/sns/topic_subscription.go +++ b/internal/service/sns/topic_subscription.go @@ -114,8 +114,8 @@ var ( "confirmation_was_authenticated": SubscriptionAttributeNameConfirmationWasAuthenticated, "delivery_policy": SubscriptionAttributeNameDeliveryPolicy, "endpoint": SubscriptionAttributeNameEndpoint, - "filter_policy_scope": SubscriptionAttributeNameFilterPolicyScope, "filter_policy": SubscriptionAttributeNameFilterPolicy, + "filter_policy_scope": SubscriptionAttributeNameFilterPolicyScope, "owner_id": SubscriptionAttributeNameOwner, "pending_confirmation": SubscriptionAttributeNamePendingConfirmation, "protocol": SubscriptionAttributeNameProtocol, From 3aa1e7f77caebc1dd61f3ebaeb0ccdc26d79205f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 21 Feb 2023 12:02:25 -0500 Subject: [PATCH 09/11] Add CHANGELOG entry. --- .changelog/28572.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/28572.txt diff --git a/.changelog/28572.txt b/.changelog/28572.txt new file mode 100644 index 00000000000..f0dedabebe1 --- /dev/null +++ b/.changelog/28572.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_sns_topic_subscription: Fix `filter_policy_scope` update from `MessageAttributes` to `MessageBody` with nested objects in `filter_policy` +``` \ No newline at end of file From 7ee61d420c10cdb9e0213aefb3203439b15923d3 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 21 Feb 2023 12:03:37 -0500 Subject: [PATCH 10/11] Add 'Context' argument to 'testAccCheckTopicSubscriptionExists' calls. --- internal/service/sns/topic_subscription_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/sns/topic_subscription_test.go b/internal/service/sns/topic_subscription_test.go index 4b3d8699c54..dc4de42e9bb 100644 --- a/internal/service/sns/topic_subscription_test.go +++ b/internal/service/sns/topic_subscription_test.go @@ -283,7 +283,7 @@ func TestAccSNSTopicSubscription_filterPolicyScope(t *testing.T) { { Config: testAccTopicSubscriptionConfig_filterPolicyScope(rName, strconv.Quote("MessageAttributes")), Check: resource.ComposeTestCheckFunc( - testAccCheckTopicSubscriptionExists(resourceName, &attributes), + testAccCheckTopicSubscriptionExists(ctx, resourceName, &attributes), resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageAttributes"), ), }, @@ -299,7 +299,7 @@ func TestAccSNSTopicSubscription_filterPolicyScope(t *testing.T) { { Config: testAccTopicSubscriptionConfig_nestedFilterPolicyScope(rName, strconv.Quote("MessageBody"), true), Check: resource.ComposeTestCheckFunc( - testAccCheckTopicSubscriptionExists(resourceName, &attributes), + testAccCheckTopicSubscriptionExists(ctx, resourceName, &attributes), resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageBody"), ), }, @@ -316,7 +316,7 @@ func TestAccSNSTopicSubscription_filterPolicyScope(t *testing.T) { { Config: testAccTopicSubscriptionConfig_filterPolicyScope(rName, strconv.Quote("MessageAttributes")), Check: resource.ComposeTestCheckFunc( - testAccCheckTopicSubscriptionExists(resourceName, &attributes), + testAccCheckTopicSubscriptionExists(ctx, resourceName, &attributes), resource.TestCheckResourceAttr(resourceName, "filter_policy_scope", "MessageAttributes"), ), }, From 279c343a2750b127df2aa918c9d4a87db74a3a8a Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 21 Feb 2023 13:12:53 -0500 Subject: [PATCH 11/11] Fix jsonencode 'Expected an equals sign ("=") to mark the beginning of the attribute value.' errors. --- internal/service/sns/topic_subscription_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/sns/topic_subscription_test.go b/internal/service/sns/topic_subscription_test.go index dc4de42e9bb..84e185dbf3d 100644 --- a/internal/service/sns/topic_subscription_test.go +++ b/internal/service/sns/topic_subscription_test.go @@ -838,9 +838,9 @@ resource "aws_sns_topic_subscription" "test" { } func testAccTopicSubscriptionConfig_nestedFilterPolicyScope(rName, scope string, nested bool) string { - filterPolicy := `jsonencode({ key1 = ["value1"] })` + filterPolicy := `jsonencode({"key1"=["value1"]})` if nested { - filterPolicy = `jsonencode({ key1 = { key2 = {"value1"} } })` + filterPolicy = `jsonencode({"key2"={"key1"=["value1"]}})` } return fmt.Sprintf(` resource "aws_sns_topic" "test" {