Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
gmercadal opened this issue Jun 26, 2023 · 8 comments · Fixed by #229
Closed
1 task done

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

gmercadal opened this issue Jun 26, 2023 · 8 comments · Fixed by #229
Labels
enhancement New feature or request

Comments

@gmercadal
Copy link

  • ✋ I have searched the open/closed issues and my issue is not listed.

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

@askulkarni2 askulkarni2 transferred this issue from aws-ia/terraform-aws-eks-blueprints Jun 29, 2023
@askulkarni2 askulkarni2 added the enhancement New feature or request label Jun 29, 2023
@askulkarni2 askulkarni2 added P0 and removed P0 labels Jun 29, 2023
@armujahid
Copy link

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
Copy link
Author

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
Copy link

yardenw-terasky commented Jul 11, 2023

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

@bryantbiggs
Copy link
Contributor

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
Copy link

ktibi commented Jul 12, 2023

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
Copy link

cdesch commented Aug 15, 2023

@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
Copy link
Contributor

PR #229 was released under v1.6.0

@gmercadal
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
7 participants