Support conditionals for allowed services

[johncblandii]

Have a question? Please checkout our Slack Community or visit our Slack Archive.

Describe the Feature

Support principal conditions for allowed ECS services

Expected Behavior

Update this section of code to allow a dynamic set of conditionals to further restrict the allowed services.

    dynamic "principals" {
      for_each = length(var.allowed_aws_services_for_sns_published) > 0 ? ["_enable"] : []
      content {
        type        = "Service"
        identifiers = var.allowed_aws_services_for_sns_published
      }
    }

Use Case

Allow ecs.amazonaws.com and a specific ECS task role to limit access.

Example expected policy:

    {
      "Sid": "Grant my-service permission to publish to the topic.",
      "Effect": "Allow",
      "Principal": {
        "Service": "ecs.amazonaws.com"
      },
      "Action": "SNS:Publish",
      "Resource": "arn:aws:sns:us-east-1:1234567890:sns-topic",
      "Condition": {
        "ArnEquals": {
          "aws:SourceArn": "arn:aws:iam::1234567890:role/my-service-ecsTaskRole-6L1TOAC7MPTEC"
        }
      }
    },

Describe Ideal Solution

Since the allowed_aws_services_for_sns_published is just a list, it will be a breaking change, but we could update that to be a map which could enable more values for the list.

Current (in an Atmos stack):

        allowed_aws_services_for_sns_published:
          - ecs.amazonaws.com

Proposed:

        allowed_aws_services_for_sns_published:
          - service: ecs.amazonaws.com
             conditional:
               ArnEquals:
                 "aws:SourceArn": arn:aws:iam::1234567890:role/my-service-ecsTaskRole-6L1TOAC7MPTEC

Alternatives Considered

• Keep the current list and add a new variable for conditionals to be zipmapped internally. This limits it to more of a global option and mixing services becomes more problematic, but it supports backwards compat.

        allowed_aws_services_for_sns_published:
          - ecs.amazonaws.com
          - otherservice.amazonaws.com
        allowed_aws_services_for_sns_published_conditionals:
          - service: ecs.amazonaws.com
             conditionals:
               - ArnEquals: "aws:SourceArn": arn:aws:iam::1234567890:role/my-service-ecsTaskRole-6L1TOAC7MPTEC
               - ArnEquals: "aws:SourceArn": arn:aws:iam::1234567890:role/other-service-ecsTaskRole-6L1TOAC7MPTEC
               - ArnEquals: "aws:SourceArn": arn:aws:iam::1234567890:role/some-app-ecsTaskRole-6L1TOAC7MPTEC
          - service: otherservice.amazonaws.com
             conditionals:
               - ArnEquals: "aws:SourceArn": arn:aws:iam::1234567890:role/something-else

This would be zipmapped to combine the service with the conditionals.

Additional Context

[Nuru]

@johncblandii Thank you for your suggestion.

If we were going to do something like this, we would want to maintain backwards compatibility, so we would introduce a new variable with all the features and allow people to use the new variable instead of the old one.

However, in the big picture, we are moving to simplify our modules by not having them take complex inputs for ancillary tasks, and this is a perfect example. The logical conclusion (or argumentum ad absurdum) of your proposal is that this module would take every possible component of a policy (twice, once for SNS topic and once for SQS queue) and feed them into a policy generator like our (not recommended) iam-policy module. This is a huge duplication of effort and a maintenance nightmare.

Instead, the direction we are moving in is to simplify these modules and focus them on their primary task as much as is reasonable. So we will continue to support creating an SNS topic policy from allowed_aws_services_for_sns_published and allowed_iam_arns_for_sns_publish because that is a relatively simple interface and covers 80% of the use cases. Beyond that, we ask that you create your desired policy outside of this module and feed it in via var.sns_topic_policy_json. That provides a separation of concerns and allows for centralizing tools to cover all the special cases of each kind of resource, rather than trying to support, for example, arbitrary conditionals configuration inputs to every Terraform module that creates an IAM policy.