Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logs/resource_policy: Fix equivalent policy diffs #22135

Merged
merged 3 commits into from
Dec 9, 2021
Merged

logs/resource_policy: Fix equivalent policy diffs #22135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ff98fb
logs/resource_policy: Fix equivalent policy diffs
YakDriver Dec 9, 2021
0006872
Add changelog
YakDriver Dec 9, 2021
1f91f04
Lint Eastwood
YakDriver Dec 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/22135.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_cloudwatch_log_resource_policy: Fix erroneous diffs in `policy_document` when no changes made or policies are equivalent
```
30 changes: 27 additions & 3 deletions internal/service/cloudwatchlogs/resource_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/cloudwatchlogs"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/verify"
)
Expand Down Expand Up @@ -36,6 +37,10 @@ func ResourceResourcePolicy() *schema.Resource {
Required: true,
ValidateFunc: validResourcePolicyDocument,
DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
StateFunc: func(v interface{}) string {
json, _ := structure.NormalizeJsonString(v)
return json
},
},
},
}
Expand All @@ -44,15 +49,21 @@ func ResourceResourcePolicy() *schema.Resource {
func resourceResourcePolicyPut(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*conns.AWSClient).CloudWatchLogsConn

policy, err := structure.NormalizeJsonString(d.Get("policy_document").(string))

if err != nil {
return fmt.Errorf("policy (%s) is invalid JSON: %w", policy, err)
}

policyName := d.Get("policy_name").(string)

input := &cloudwatchlogs.PutResourcePolicyInput{
PolicyDocument: aws.String(d.Get("policy_document").(string)),
PolicyDocument: aws.String(policy),
PolicyName: aws.String(policyName),
}

log.Printf("[DEBUG] Writing CloudWatch log resource policy: %#v", input)
_, err := conn.PutResourcePolicy(input)
_, err = conn.PutResourcePolicy(input)

if err != nil {
return fmt.Errorf("Writing CloudWatch log resource policy failed: %s", err.Error())
Expand All @@ -75,7 +86,19 @@ func resourceResourcePolicyRead(d *schema.ResourceData, meta interface{}) error
return nil
}

d.Set("policy_document", resourcePolicy.PolicyDocument)
policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy_document").(string), aws.StringValue(resourcePolicy.PolicyDocument))

if err != nil {
return fmt.Errorf("while setting policy (%s), encountered: %w", policyToSet, err)
}

policyToSet, err = structure.NormalizeJsonString(policyToSet)

if err != nil {
return fmt.Errorf("policy (%s) is invalid JSON: %w", policyToSet, err)
}

d.Set("policy_document", policyToSet)

return nil
}
Expand All @@ -99,6 +122,7 @@ func LookupResourcePolicy(conn *cloudwatchlogs.CloudWatchLogs,
input := &cloudwatchlogs.DescribeResourcePoliciesInput{
NextToken: nextToken,
}

log.Printf("[DEBUG] Reading CloudWatch log resource policies: %#v", input)
resp, err := conn.DescribeResourcePolicies(input)
if err != nil {
Expand Down
117 changes: 102 additions & 15 deletions internal/service/cloudwatchlogs/resource_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@ import (
)

func TestAccCloudWatchLogsResourcePolicy_basic(t *testing.T) {
name := sdkacctest.RandString(5)
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_cloudwatch_log_resource_policy.test"
var resourcePolicy cloudwatchlogs.ResourcePolicy

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, cloudwatchlogs.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckCloudWatchLogResourcePolicyDestroy,
CheckDestroy: testAccCheckResourcePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckResourcePolicyResourceBasic1Config(name),
Config: testAccCheckResourcePolicyResourceBasic1Config(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogResourcePolicy(resourceName, &resourcePolicy),
resource.TestCheckResourceAttr(resourceName, "policy_name", name),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"rds.%s\"},\"Action\":[\"logs:PutLogEvents\",\"logs:CreateLogStream\"],\"Resource\":\"arn:%s:logs:*:*:log-group:/aws/rds/*\"}]}", acctest.PartitionDNSSuffix(), acctest.Partition())),
resource.TestCheckResourceAttr(resourceName, "policy_name", rName),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"Statement\":[{\"Action\":[\"logs:PutLogEvents\",\"logs:CreateLogStream\"],\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"rds.%s\"},\"Resource\":\"arn:%s:logs:*:*:log-group:/aws/rds/*\",\"Sid\":\"\"}],\"Version\":\"2012-10-17\"}", acctest.PartitionDNSSuffix(), acctest.Partition())),
),
},
{
Expand All @@ -38,11 +38,42 @@ func TestAccCloudWatchLogsResourcePolicy_basic(t *testing.T) {
ImportStateVerify: true,
},
{
Config: testAccCheckResourcePolicyResourceBasic2Config(name),
Config: testAccCheckResourcePolicyResourceBasic2Config(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogResourcePolicy(resourceName, &resourcePolicy),
resource.TestCheckResourceAttr(resourceName, "policy_name", name),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"rds.%s\"},\"Action\":[\"logs:PutLogEvents\",\"logs:CreateLogStream\"],\"Resource\":\"arn:%s:logs:*:*:log-group:/aws/rds/example.com\"}]}", acctest.PartitionDNSSuffix(), acctest.Partition())),
resource.TestCheckResourceAttr(resourceName, "policy_name", rName),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"Statement\":[{\"Action\":[\"logs:PutLogEvents\",\"logs:CreateLogStream\"],\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"rds.%s\"},\"Resource\":\"arn:%s:logs:*:*:log-group:/aws/rds/example.com\",\"Sid\":\"\"}],\"Version\":\"2012-10-17\"}", acctest.PartitionDNSSuffix(), acctest.Partition())),
),
},
},
})
}

func TestAccCloudWatchLogsResourcePolicy_ignoreEquivalent(t *testing.T) {
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_cloudwatch_log_resource_policy.test"
var resourcePolicy cloudwatchlogs.ResourcePolicy

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, cloudwatchlogs.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckResourcePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccCheckResourcePolicyResourceOrderConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogResourcePolicy(resourceName, &resourcePolicy),
resource.TestCheckResourceAttr(resourceName, "policy_name", rName),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"Statement\":[{\"Action\":[\"logs:CreateLogStream\",\"logs:PutLogEvents\"],\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"rds.%s\"]},\"Resource\":[\"arn:%s:logs:*:*:log-group:/aws/rds/example.com\"]}],\"Version\":\"2012-10-17\"}", acctest.PartitionDNSSuffix(), acctest.Partition())),
),
},
{
Config: testAccCheckResourcePolicyResourceNewOrderConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogResourcePolicy(resourceName, &resourcePolicy),
resource.TestCheckResourceAttr(resourceName, "policy_name", rName),
resource.TestCheckResourceAttr(resourceName, "policy_document", fmt.Sprintf("{\"Statement\":[{\"Action\":[\"logs:CreateLogStream\",\"logs:PutLogEvents\"],\"Effect\":\"Allow\",\"Principal\":{\"Service\":[\"rds.%s\"]},\"Resource\":[\"arn:%s:logs:*:*:log-group:/aws/rds/example.com\"]}],\"Version\":\"2012-10-17\"}", acctest.PartitionDNSSuffix(), acctest.Partition())),
),
},
},
Expand Down Expand Up @@ -75,7 +106,7 @@ func testAccCheckCloudWatchLogResourcePolicy(pr string, resourcePolicy *cloudwat
}
}

func testAccCheckCloudWatchLogResourcePolicyDestroy(s *terraform.State) error {
func testAccCheckResourcePolicyDestroy(s *terraform.State) error {
conn := acctest.Provider.Meta().(*conns.AWSClient).CloudWatchLogsConn

for _, rs := range s.RootModule().Resources {
Expand All @@ -97,7 +128,7 @@ func testAccCheckCloudWatchLogResourcePolicyDestroy(s *terraform.State) error {
return nil
}

func testAccCheckResourcePolicyResourceBasic1Config(name string) string {
func testAccCheckResourcePolicyResourceBasic1Config(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}

Expand All @@ -118,13 +149,13 @@ data "aws_iam_policy_document" "test" {
}

resource "aws_cloudwatch_log_resource_policy" "test" {
policy_name = "%s"
policy_name = %[1]q
policy_document = data.aws_iam_policy_document.test.json
}
`, name)
`, rName)
}

func testAccCheckResourcePolicyResourceBasic2Config(name string) string {
func testAccCheckResourcePolicyResourceBasic2Config(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}

Expand All @@ -145,8 +176,64 @@ data "aws_iam_policy_document" "test" {
}

resource "aws_cloudwatch_log_resource_policy" "test" {
policy_name = "%s"
policy_name = %[1]q
policy_document = data.aws_iam_policy_document.test.json
}
`, name)
`, rName)
}

func testAccCheckResourcePolicyResourceOrderConfig(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}

resource "aws_cloudwatch_log_resource_policy" "test" {
policy_name = %[1]q
policy_document = jsonencode({
Statement = [{
Action = [
"logs:CreateLogStream",
"logs:PutLogEvents",
]
Effect = "Allow"
Resource = [
"arn:${data.aws_partition.current.partition}:logs:*:*:log-group:/aws/rds/example.com",
]
Principal = {
Service = [
"rds.${data.aws_partition.current.dns_suffix}",
]
}
}]
Version = "2012-10-17"
})
}
`, rName)
}

func testAccCheckResourcePolicyResourceNewOrderConfig(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}

resource "aws_cloudwatch_log_resource_policy" "test" {
policy_name = %[1]q
policy_document = jsonencode({
Statement = [{
Action = [
"logs:PutLogEvents",
"logs:CreateLogStream",
]
Effect = "Allow"
Resource = [
"arn:${data.aws_partition.current.partition}:logs:*:*:log-group:/aws/rds/example.com",
]
Principal = {
Service = [
"rds.${data.aws_partition.current.dns_suffix}",
]
}
}]
Version = "2012-10-17"
})
}
`, rName)
}