s3 bucket policy with not_principal always changes due to ordering

[craiggenner]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

$ terraform -v
Terraform v0.12.24

• provider.aws v2.60.0
• provider.template v2.1.2

Affected Resource(s)

• aws_s3_bucket_policy
• aws_iam_policy_document
• aws_s3_bucket

Terraform Configuration Files

resource "aws_s3_bucket" "av" {
  bucket = "app-storage"
  acl    = "private"
}

data "aws_iam_policy_document" "s3_av" {
  statement {
    sid       = "DenyInfected"
    effect    = "Deny"
    actions   = ["s3:GetObject", "s3:PutObjectTagging"]
    resources = ["arn:aws:s3:::${var.s3_bucket}/*"]

    not_principals {
      type        = "AWS"
      identifiers = [
        aws_iam_role.antivirus.arn,
        "arn:aws:sts::${var.account_id}:assumed-role/${aws_iam_role.antivirus.name}/${aws_lambda_function.antivirus.function_name}"
      ]
    }

    # Deny where 'av-status' tag is 'INFECTED'
    condition {
      test     = "StringEquals"
      variable = "s3:ExistingObjectTag/av-status"
      values   = ["INFECTED"]
    }
  }
}

resource "aws_s3_bucket_policy" "av" {
  bucket = aws_s3_bucket.av.id
  policy = data.aws_iam_policy_document.s3_av.json
}

Debug Output

https://gist.github.com/craiggenner/b315384e6ca2cead486b14665840807e

Panic Output

None

Expected Behavior

Terraform shouldn't expect to make any changes

Actual Behavior

Terraform always wants to replace the policy of the s3 bucket

Steps to Reproduce

1. Create terraform code as above

2. terraform apply

3. terraform apply

Important Factoids

Running this against eu-west-1 region.

The issue appears to be related to the use of the sts assumed-role in the not_principals, a regular iam role entry doesn't produce this. With multiple entries the order of the not_principals block is moved around within the json policy

References

• Terraform aws s3 bucket policy are planned for change every time terraform#4948

[roccomuscaritolo-okta]

I can confirm this issue is still occurring in AWS hashicorp/aws version v3.18.0.

Even adding a sort on the principals directly does not change this diff behavior.

principals {
      type        = "AWS"
      identifiers = sort([for role in data.aws_iam_role.decrypt_allowed_roles : role.arn])
    }

While the end result makes no difference, at a much larger scale this puts an undo burden on the reviewer to manually parse the output. I personally have had to review about 9 prinicple changes for someone simply adding 1 new role to a list.

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # aws_s3_bucket_policy.main will be updated in-place
  ~ resource "aws_s3_bucket_policy" "main" {
        bucket = "rocco-testing"
        id     = "rocco-testing"
      ~ policy = jsonencode(
          ~ {
              ~ Statement = [
                  ~ {
                        Action    = "s3:*"
                        Effect    = "Allow"
                      ~ Principal = {
                          ~ AWS = [
                                "arn:aws:iam::123456789:role/test",
                              + "arn:aws:iam::123456789:role/rundeck-test",
                                "arn:aws:iam::123456789:role/kallentest",
                                "arn:aws:iam::123456789:role/career-test",
                              + "arn:aws:iam::123456789:role/career-test123",
                                "arn:aws:iam::123456789:root",
                              - "arn:aws:iam::123456789:role/rundeck-test",
                            ]
                        }
                        Resource  = [
                            "arn:aws:s3:::rocco-testing/*",
                            "arn:aws:s3:::rocco-testing",
                        ]
                        Sid       = "Allow test/root access"
                    },
                ]
                Version   = "2012-10-17"
            }
        )
    }

Plan: 0 to add, 1 to change, 0 to destroy.

------------------------------------------------------------------------

[iainelder]

@craiggenner I can confirm what @roccomuscaritolo-okta sees. It happens in the normal principal field as well.

The crazy thing is that if you test an expression such as [for role in data.aws_iam_role.decrypt_allowed_roles : role.arn] in terraform console then it always returns the same order of elements.

It looks like the bucket policy resource is jumbling them up at some point.

[iainelder]

The Principals and NotPrincipals elements of IAM policy statements appear to be handled in exactly the same way, so that would explain why the behavior is observed in both places.

terraform-provider-aws/aws/iam_policy_model.go

Lines 22 to 23 in 3b05635

	Principals IAMPolicyStatementPrincipalSet `json:"Principal,omitempty"`
	NotPrincipals IAMPolicyStatementPrincipalSet `json:"NotPrincipal,omitempty"`

The problem appears to occur in the unmarshalling of the policy document. That's a guess because it only happens when the result of encodejson has been set on the bucket policy resource.

I'm unsure if a change is required in UnmarshalJSON. It looks like it just iterates over whatever its given without reodering anything.

terraform-provider-aws/aws/iam_policy_model.go

Lines 127 to 159 in e9a98cf

	func (ps *IAMPolicyStatementPrincipalSet) UnmarshalJSON(b []byte) error {
	var out IAMPolicyStatementPrincipalSet
	var data interface{}
	if err := json.Unmarshal(b, &data); err != nil {
	return err
	}
	switch t := data.(type) {
	case string:
	out = append(out, IAMPolicyStatementPrincipal{Type: "*", Identifiers: []string{"*"}})
	case map[string]interface{}:
	for key, value := range data.(map[string]interface{}) {
	switch vt := value.(type) {
	case string:
	out = append(out, IAMPolicyStatementPrincipal{Type: key, Identifiers: value.(string)})
	case []interface{}:
	values := []string{}
	for _, v := range value.([]interface{}) {
	values = append(values, v.(string))
	}
	out = append(out, IAMPolicyStatementPrincipal{Type: key, Identifiers: values})
	default:
	return fmt.Errorf("Unsupported data type %T for IAMPolicyStatementPrincipalSet.Identifiers", vt)
	}
	}
	default:
	return fmt.Errorf("Unsupported data type %T for IAMPolicyStatementPrincipalSet", t)
	}
	*ps = out
	return nil
	}

The schema for the principal policy element says that identifiers are a set, and so unordered. As it turns out, the order matters for diffs. Instead of using schema.TypeSet, should it instead be using schema.TypeList?

terraform-provider-aws/aws/data_source_aws_iam_policy_document.go

Lines 344 to 364 in ac06ced

	func dataSourceAwsIamPolicyPrincipalSchema() *schema.Schema {
	return &schema.Schema{
	Type: schema.TypeSet,
	Optional: true,
	Elem: &schema.Resource{
	Schema: map[string]*schema.Schema{
	"type": {
	Type: schema.TypeString,
	Required: true,
	},
	"identifiers": {
	Type: schema.TypeSet,
	Required: true,
	Elem: &schema.Schema{
	Type: schema.TypeString,
	},
	},
	},
	},
	}
	}

A similar change was applied to preserve the order of condition values. The type of the values in the condition schema was changes from TypeSet to TypeList.

e9a98cf

#17556

@YakDriver you were here recently (it was your PR that fixed the condition ordering!) Do you think changing TypeSet to TypeList here would be enough to fix this?

[iainelder]

See below for a small Terraform configuration that reproduces the issue. Change to a temporary directory and copy the configuration to a file in the directory called main.tf.

Before running the Terraform commands, first set your environment with credentials and a region.

export AWS_PROFILE=...
export AWS_DEFAULT_REGION=...

Output after first apply shows that the principal elements in the bucket policy are ordered according to the configuration.

terraform apply
terraform output --raw test | jq

{
  "Statement": [
    {
      "Action": "*",
      "Effect": "Deny",
      "Principal": {
        "AWS": [
          "arn:aws:iam::111111111111:role/role-0",
          "arn:aws:iam::111111111111:role/role-1",
          "arn:aws:iam::111111111111:role/role-2",
          "arn:aws:iam::111111111111:role/role-3",
          "arn:aws:iam::111111111111:role/role-4",
          "arn:aws:iam::111111111111:role/role-5",
          "arn:aws:iam::111111111111:role/role-6",
          "arn:aws:iam::111111111111:role/role-7",
          "arn:aws:iam::111111111111:role/role-8",
          "arn:aws:iam::111111111111:role/role-9"
        ]
      },
      "Resource": "arn:aws:s3:::tftest20210525081545966100000001"
    }
  ],
  "Version": "2012-10-17"
}

Second apply shows a change to the output. Without changing the effective permissions of the bucket policy, the principals are reordered.

terraform apply
terraform output --raw test | jq

Changes to Outputs:
  ~ test = jsonencode(
      ~ {
          ~ Statement = [
              ~ {
                  ~ Principal = {
                      ~ AWS = [
                          - "arn:aws:iam::111111111111:role/role-0",
                          - "arn:aws:iam::111111111111:role/role-1",
                          - "arn:aws:iam::111111111111:role/role-2",
                          - "arn:aws:iam::111111111111:role/role-3",
                            "arn:aws:iam::111111111111:role/role-4",
                            "arn:aws:iam::111111111111:role/role-5",
                          - "arn:aws:iam::111111111111:role/role-6",
                          - "arn:aws:iam::111111111111:role/role-7",
                          + "arn:aws:iam::111111111111:role/role-2",
                          + "arn:aws:iam::111111111111:role/role-3",
                          + "arn:aws:iam::111111111111:role/role-0",
                          + "arn:aws:iam::111111111111:role/role-1",
                            "arn:aws:iam::111111111111:role/role-8",
                            "arn:aws:iam::111111111111:role/role-9",
                          + "arn:aws:iam::111111111111:role/role-6",
                          + "arn:aws:iam::111111111111:role/role-7",
                        ]
                    }
                    # (3 unchanged elements hidden)
                },
            ]
            # (1 unchanged element hidden)
        }
    )

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Deny",
      "Principal": {
        "AWS": [
          "arn:aws:iam::111111111111:role/role-4",
          "arn:aws:iam::111111111111:role/role-5",
          "arn:aws:iam::111111111111:role/role-2",
          "arn:aws:iam::111111111111:role/role-3",
          "arn:aws:iam::111111111111:role/role-0",
          "arn:aws:iam::111111111111:role/role-1",
          "arn:aws:iam::111111111111:role/role-8",
          "arn:aws:iam::111111111111:role/role-9",
          "arn:aws:iam::111111111111:role/role-6",
          "arn:aws:iam::111111111111:role/role-7"
        ]
      },
      "Action": "*",
      "Resource": "arn:aws:s3:::tftest20210525081545966100000001"
    }
  ]
}

The order of the principals changes arbitrarily with each successive apply.

---

The Terraform configuration:

output "test" {
  value = aws_s3_bucket_policy.test.policy
}

resource "aws_s3_bucket" "test" {
  bucket_prefix = "tftest"
}

resource "aws_s3_bucket_policy" "test" {
  bucket = aws_s3_bucket.test.id

  policy = jsonencode({
      Statement = [
        {
          Principal = {
            AWS = [for r in aws_iam_role.test: r.arn]
          }
          Resource = "${aws_s3_bucket.test.arn}"
          Action = "*"
          Effect = "Deny"
        }
      ]
      Version = "2012-10-17"
  })
}

resource "aws_iam_role" "test" {
  count = 10
    
  name = "role-${count.index}"

  assume_role_policy = jsonencode({
    Version = "2012-10-17"
    Statement = [
      {
        Effect = "Deny"
        Action = "sts:AssumeRole"
        Principal = {
          AWS = "*"
        }
      }
    ]
  })
}

[ewbankkit]

Potentially related: jen20/awspolicyequivalence#10.

[crawforde]

I can confirm I'm having this same problem for both Principal and NotPrincipal blocks using terraform v1.0.0 and provider version v3.52.0. I recall this used to be an issue and then I believe it was corrected and I hadn't see it for a while. I was surprised to see it rearing its ugly head again. Is this a regression?

[virgofx]

Also observing this issue with hashicorp/aws 3.60.0 and Terraform 1.0.8.

[github-actions]

This functionality has been released in v3.68.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.