hashicorp / terraform-aws-consul-ecs

Consul Service Mesh on AWS ECS (Elastic Container Service)
https://www.consul.io/docs/ecs
Mozilla Public License 2.0
52 stars 31 forks source link

feature/bugfix: allow usage of cross account AWS SecretManager secrets #150

Closed v-rosa closed 1 year ago

v-rosa commented 1 year ago

Current implementation assumes that AWS SecretManager instances to store Consul secrets are located in the same AWS account as the ECS services (acl-controller and mesh-task) and using the default AWS KMS keys.

It would be helpful to have the option to use CMK/external AWS Secrets instance. The only change needed would be to pass the KMS key and adjust the service execution policies.

I've created a small PR with this approach. Yet it's not clear to me how to proceed with this secret consul_https_ca_cert_arn, but we can review it in the PR: TODO.

cthain commented 1 year ago

:wave: Hi @v-rosa, Thanks for bringing this up. AFAIK we have not added explicit support for cross-account secrets; however, I think that the mesh-task and gateway-task modules should already support what you are trying to do.

I've had a look over your PR and I'm curious to know if you have tried using the var.additional_execution_role_policies mechanism? This variable is supported for both mesh-task and gateway-task and I believe it should allow you to achieve the same result. Basically you need to create a policy that grants the perms for reading and decrypting the secrets. Then you pass that policy to the module using var.additional_execution_role_policies:

resource "aws_iam_policy" "cross_account_secrets" {
...
  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "secretsmanager:GetSecretValue"
      ],
      "Resource": [
        "<SECRET_ARN>"
      ]
    },
    {
      "Effect": "Allow",
      "Action": [
        "kms:Decrypt"
      ],
      "Resource": [
        "<KMS_KEY_ARN>"
      ]
    }
  ]
}
EOF
}

module "example" {
  source                       = "../../modules/mesh-task"
  ...
  additional_execution_role_policies = [aws_iam_policy.cross_account_secrets.arn]
  ...
}

Please let me know if you've already tried this and it isn't working as expected.

Unfortunately, this mechanism isn't supported for the acl-controller so we would need to add something analogous for that module.

cthain commented 1 year ago

With respect to your question about consul_https_ca_cert_arn, it should be the same as the other SM secrets. The task execution role has to include permission to read the secret (secretsmanager:GetSecretValue) and access the KMS key needed to decrypt that secret (kms:Decrypt).

Please correct me if I'm wrong but I think your question comes from the fact that there isn't an explicit policy statement for consul_https_ca_cert_arn in either the mesh-task or gateway-task module. I think this is indeed an oversight on our part. All of our tests and examples use the same CA certificate for both var.consul_server_ca_cert_arn and var.consul_https_ca_cert_arn. It doesn't look like we've exercised the path where they are using a different cert, so we missed this.

v-rosa commented 1 year ago

Hey @cthain Thanks for the detailed feedback. Indeed I started all of this from the acl-controller module, and just applied the same recipe in the others, wrongly :) For sure var.additional_execution_role_policies its all we need in all modules. If you don't mine I can re-do this PR and add the the analogous mechanism to acl-controller. WDYT?

cthain commented 1 year ago

Hi @v-rosa, if you are willing to update your PR to add this support to the acl-controller it would be a greatly appreciated addition to the ECS integration! If it's easier for you to create a new PR, that's fine too. Thanks!

v-rosa commented 1 year ago

Done. Here it goes: https://github.com/hashicorp/terraform-aws-consul-ecs/pull/151/files