aws-ia / terraform-aws-eks-blueprints-addons

Terraform module which provisions addons on Amazon EKS clusters
https://aws-ia.github.io/terraform-aws-eks-blueprints-addons/main/
Apache License 2.0
272 stars 127 forks source link

Support for non-implicit AddTags in Create* API calls #200

Closed gmercadal closed 1 year ago

gmercadal commented 1 year ago

Please describe your question here

We've been receiving notifications from AWS due to a policy change where the AddTags permission is no longer implicit for Create* API calls. We've reviewed the latest v4 IAM policy for the AWS load balancer controller and apparently it is missing some permissions in order to avoid those notifications. After August these calls will start failing.

Checking the latest IAM policy it apparently adds a new statement to cover that change:

   {
        "Effect": "Allow",
        "Action": [
            "elasticloadbalancing:AddTags"
        ],
        "Resource": [
            "arn:aws:elasticloadbalancing:*:*:targetgroup/*/*",
            "arn:aws:elasticloadbalancing:*:*:loadbalancer/net/*/*",
            "arn:aws:elasticloadbalancing:*:*:loadbalancer/app/*/*"
        ],
        "Condition": {
            "StringEquals": {
                "elasticloadbalancing:CreateAction": [
                    "CreateTargetGroup",
                    "CreateLoadBalancer"
                ]
            },
            "Null": {
                "aws:RequestTag/elbv2.k8s.aws/cluster": "false"
            }
        }
    }

Do you plan to update the policy accordingly and create a new version?

Provide a link to the example/module related to the question

https://github.com/aws-ia/terraform-aws-eks-blueprints/blob/v4/modules/kubernetes-addons/aws-load-balancer-controller/data.tf

Additional context

armujahid commented 1 year ago

Attaching screeshot of the email received from AWS for reference.
image

As per my understanding, elasticloadbalancing:AddTags is only used while creating new load balancers. That's why I think we don't need to worry much. But it will be good if the fix can also be backported to the v4 blueprints.

gmercadal commented 1 year ago

Attaching screeshot of the email received from AWS for reference. image

As per my understanding, elasticloadbalancing:AddTags is only used while creating new load balancers. That's why I think we don't need to worry much. But it will be good if the fix can also be backported to the v4 blueprints.

Yes, that's correct, I believe the only case that is covering that statement is the first creation of the load balancers.

yardenw-terasky commented 1 year ago

When a solution will be merged,is it going to be applied to older versions of eks-blueprints?

bryantbiggs commented 1 year ago

is it going to be applied to older versions of eks-blueprints

No, this is the new home of EKS Blueprint addons and changes will be made here (if required)

ktibi commented 1 year ago

Hello @bryantbiggs

If I understand, now we need to use aws-ia/eks-blueprints-addons/aws instead of aws-ia/terraform-aws-eks-blueprints//modules.

So any idea on when we fix this issue ?

The fix is to remove l160 :

            "Condition": {
                "Null": {
                    "aws:RequestTag/elbv2.k8s.aws/cluster": "true",
                    "aws:ResourceTag/elbv2.k8s.aws/cluster": "false"
                }
            }
cdesch commented 1 year ago

@gmercadal or @bryantbiggs How do you add the IAM policy as a workaround in the meantime while PR 229 gets released?

The would be adding the following or linked IAM profile into the aws_load_balancer_controller_helm_config in the addons blueprint terraform module.

   {
        "Effect": "Allow",
        "Action": [
            "elasticloadbalancing:AddTags"
        ],
        "Resource": [
            "arn:aws:elasticloadbalancing:*:*:targetgroup/*/*",
            "arn:aws:elasticloadbalancing:*:*:loadbalancer/net/*/*",
            "arn:aws:elasticloadbalancing:*:*:loadbalancer/app/*/*"
        ],
        "Condition": {
            "StringEquals": {
                "elasticloadbalancing:CreateAction": [
                    "CreateTargetGroup",
                    "CreateLoadBalancer"
                ]
            },
            "Null": {
                "aws:RequestTag/elbv2.k8s.aws/cluster": "false"
            }
        }
    }

into the

aws_load_balancer_controller_helm_config = {
  ...
}

Alternatively, is it possible to use 1.6.0 here?

bryantbiggs commented 1 year ago

PR #229 was released under v1.6.0

gmercadal commented 1 year ago

@cdesch We did not have it yet, but I see that the issue has been fixed in v1.6.0 as per @bryantbiggs comment. We're going to bump it.