Skip to content

Commit

Permalink
Merge pull request #21969 from hashicorp/b-equivalent-policies-diffs
Browse files Browse the repository at this point in the history
kms/key: Preserve order in JSON policy lists
  • Loading branch information
YakDriver authored Dec 1, 2021
2 parents 4063920 + e7ede03 commit de40d0c
Show file tree
Hide file tree
Showing 10 changed files with 850 additions and 353 deletions.
3 changes: 3 additions & 0 deletions .changelog/21969.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_kms_key: Fix order-related diffs in `policy`
```
9 changes: 8 additions & 1 deletion internal/service/kms/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,14 @@ func resourceKeyRead(d *schema.ResourceData, meta interface{}) error {
d.Set("key_id", key.metadata.KeyId)
d.Set("key_usage", key.metadata.KeyUsage)
d.Set("multi_region", key.metadata.MultiRegion)
d.Set("policy", key.policy)

policyToSet, err := verify.SecondJSONUnlessEquivalent(d.Get("policy").(string), key.policy)

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

d.Set("policy", policyToSet)

tags := key.tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig)

Expand Down
262 changes: 212 additions & 50 deletions internal/service/kms/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ func TestAccKMSKey_asymmetricKey(t *testing.T) {
})
}

func TestAccKMSKey_policy(t *testing.T) {
func TestAccKMSKey_Policy_basic(t *testing.T) {
var key kms.KeyMetadata
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_kms_key.test"
expectedPolicyText := `{"Version":"2012-10-17","Id":"kms-tf-1","Statement":[{"Sid":"Enable IAM User Permissions","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`
expectedPolicyText := fmt.Sprintf(`{"Version":"2012-10-17","Id":%[1]q,"Statement":[{"Sid":"Enable IAM User Permissions","Effect":"Allow","Principal":{"AWS":"*"},"Action":"kms:*","Resource":"*"}]}`, rName)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
Expand Down Expand Up @@ -156,7 +156,7 @@ func TestAccKMSKey_policy(t *testing.T) {
})
}

func TestAccKMSKey_policyBypass(t *testing.T) {
func TestAccKMSKey_Policy_bypass(t *testing.T) {
var key kms.KeyMetadata
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_kms_key.test"
Expand Down Expand Up @@ -188,7 +188,7 @@ func TestAccKMSKey_policyBypass(t *testing.T) {
})
}

func TestAccKMSKey_policyBypassUpdate(t *testing.T) {
func TestAccKMSKey_Policy_bypassUpdate(t *testing.T) {
var before, after kms.KeyMetadata
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_kms_key.test"
Expand Down Expand Up @@ -244,6 +244,49 @@ func TestAccKMSKey_Policy_iamRole(t *testing.T) {
})
}

// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/11801
func TestAccKMSKey_Policy_iamRoleOrder(t *testing.T) {
var key kms.KeyMetadata
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_kms_key.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, kms.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckKeyDestroy,
Steps: []resource.TestStep{
{
Config: testAccKeyPolicyIAMMultiRoleConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckKeyExists(resourceName, &key),
),
},
{
Config: testAccKeyPolicyIAMMultiRoleConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckKeyExists(resourceName, &key),
),
PlanOnly: true,
},
{
Config: testAccKeyPolicyIAMMultiRoleConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckKeyExists(resourceName, &key),
),
PlanOnly: true,
},
{
Config: testAccKeyPolicyIAMMultiRoleConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckKeyExists(resourceName, &key),
),
PlanOnly: true,
},
},
})
}

// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/7646
func TestAccKMSKey_Policy_iamServiceLinkedRole(t *testing.T) {
var key kms.KeyMetadata
Expand Down Expand Up @@ -495,23 +538,19 @@ resource "aws_kms_key" "test" {
description = %[1]q
deletion_window_in_days = 7
policy = <<POLICY
{
"Version": "2012-10-17",
"Id": "kms-tf-1",
"Statement": [
{
"Sid": "Enable IAM User Permissions",
"Effect": "Allow",
"Principal": {
"AWS": "*"
},
"Action": "kms:*",
"Resource": "*"
}
]
}
POLICY
policy = jsonencode({
Id = %[1]q
Statement = [{
Sid = "Enable IAM User Permissions"
Effect = "Allow"
Principal = {
AWS = "*"
}
Action = "kms:*"
Resource = "*"
}]
Version = "2012-10-17"
})
}
`, rName)
}
Expand All @@ -526,32 +565,30 @@ resource "aws_kms_key" "test" {
bypass_policy_lockout_safety_check = %[2]t
policy = <<-POLICY
{
"Version": "2012-10-17",
"Id": "kms-tf-1",
"Statement": [
{
"Sid": "Enable IAM User Permissions",
"Effect": "Allow",
"Principal": {
"AWS": "${data.aws_caller_identity.current.arn}"
},
"Action": [
"kms:CreateKey",
"kms:DescribeKey",
"kms:ScheduleKeyDeletion",
"kms:Describe*",
"kms:Get*",
"kms:List*",
"kms:TagResource",
"kms:UntagResource"
],
"Resource": "*"
policy = jsonencode({
Id = %[1]q
Statement = [
{
Action = [
"kms:CreateKey",
"kms:DescribeKey",
"kms:ScheduleKeyDeletion",
"kms:Describe*",
"kms:Get*",
"kms:List*",
"kms:TagResource",
"kms:UntagResource",
]
Effect = "Allow"
Principal = {
AWS = data.aws_caller_identity.current.arn
}
]
}
POLICY
Resource = "*"
Sid = "Enable IAM User Permissions"
},
]
Version = "2012-10-17"
})
}
`, rName, bypassFlag)
}
Expand Down Expand Up @@ -580,7 +617,7 @@ resource "aws_kms_key" "test" {
deletion_window_in_days = 7
policy = jsonencode({
Id = "kms-tf-1"
Id = %[1]q
Statement = [
{
Action = "kms:*"
Expand All @@ -602,7 +639,7 @@ resource "aws_kms_key" "test" {
]
Effect = "Allow"
Principal = {
AWS = [aws_iam_role.test.arn]
AWS = aws_iam_role.test.arn
}
Resource = "*"
Expand All @@ -615,6 +652,131 @@ resource "aws_kms_key" "test" {
`, rName)
}

func testAccKeyPolicyIAMMultiRoleConfig(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}
resource "aws_iam_role" "test1" {
name = "%[1]s-sultan"
assume_role_policy = jsonencode({
Statement = [{
Action = "sts:AssumeRole"
Effect = "Allow"
Principal = {
Service = "ec2.${data.aws_partition.current.dns_suffix}"
}
}]
Version = "2012-10-17"
})
}
resource "aws_iam_role" "test2" {
name = "%[1]s-shepard"
assume_role_policy = jsonencode({
Statement = [{
Action = "sts:AssumeRole"
Effect = "Allow"
Principal = {
Service = "ec2.${data.aws_partition.current.dns_suffix}"
}
}]
Version = "2012-10-17"
})
}
resource "aws_iam_role" "test3" {
name = "%[1]s-tritonal"
assume_role_policy = jsonencode({
Statement = [{
Action = "sts:AssumeRole"
Effect = "Allow"
Principal = {
Service = "ec2.${data.aws_partition.current.dns_suffix}"
}
}]
Version = "2012-10-17"
})
}
resource "aws_iam_role" "test4" {
name = "%[1]s-artlec"
assume_role_policy = jsonencode({
Statement = [{
Action = "sts:AssumeRole"
Effect = "Allow"
Principal = {
Service = "ec2.${data.aws_partition.current.dns_suffix}"
}
}]
Version = "2012-10-17"
})
}
resource "aws_iam_role" "test5" {
name = "%[1]s-cazzette"
assume_role_policy = jsonencode({
Statement = [{
Action = "sts:AssumeRole"
Effect = "Allow"
Principal = {
Service = "ec2.${data.aws_partition.current.dns_suffix}"
}
}]
Version = "2012-10-17"
})
}
data "aws_iam_policy_document" "test" {
policy_id = %[1]q
statement {
actions = [
"kms:*",
]
effect = "Allow"
principals {
identifiers = ["*"]
type = "AWS"
}
resources = ["*"]
}
statement {
actions = [
"kms:Encrypt",
"kms:Decrypt",
"kms:ReEncrypt*",
"kms:GenerateDataKey*",
"kms:DescribeKey",
]
effect = "Allow"
principals {
identifiers = [
aws_iam_role.test2.arn,
aws_iam_role.test1.arn,
aws_iam_role.test4.arn,
aws_iam_role.test3.arn,
aws_iam_role.test5.arn,
]
type = "AWS"
}
resources = ["*"]
}
}
resource "aws_kms_key" "test" {
description = %[1]q
deletion_window_in_days = 7
policy = data.aws_iam_policy_document.test.json
}
`, rName)
}

func testAccKeyPolicyIAMServiceLinkedRoleConfig(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}
Expand All @@ -629,7 +791,7 @@ resource "aws_kms_key" "test" {
deletion_window_in_days = 7
policy = jsonencode({
Id = "kms-tf-1"
Id = %[1]q
Statement = [
{
Action = "kms:*"
Expand All @@ -651,7 +813,7 @@ resource "aws_kms_key" "test" {
]
Effect = "Allow"
Principal = {
AWS = [aws_iam_service_linked_role.test.arn]
AWS = aws_iam_service_linked_role.test.arn
}
Resource = "*"
Expand Down
Loading

0 comments on commit de40d0c

Please sign in to comment.