Add elasticloadbalancing:ModifyTargetGroupAttributes to aws lb controller

[olemarkus]

Fixes #11297

[peter-svensson]

When using the annotation
alb.ingress.kubernetes.io/actions.ssl-redirect: '{"Type": "redirect", "RedirectConfig": { "Protocol": "HTTPS", "Port": "443", "StatusCode": "HTTP_301"}}'

The following permissions are needed in order to setup HTTPs certificate:

{
    "Effect": "Allow",
    "Action": "acm:DescribeCertificate",
    "Resource": "arn:aws:acm:*:<accountid>:certificate/*"
},
{
    "Effect": "Allow",
    "Action": "acm:ListCertificates",
    "Resource": "*"
}

[peter-svensson]

When changing an ingress I also need to set

       {
            "Effect": "Allow",
            "Action": "elasticloadbalancing:DeleteRule",
            "Resource": [
                "arn:aws:elasticloadbalancing:*:<accountid>:listener-rule/app/*/*/*/*",
                "arn:aws:elasticloadbalancing:*:<accountid>:listener-rule/net/*/*/*/*"
            ]
        },
        {
            "Effect": "Allow",
            "Action": "elasticloadbalancing:ModifyRule",
            "Resource": [
                "arn:aws:elasticloadbalancing:*:<accountid>:listener-rule/app/*/*/*/*",
                "arn:aws:elasticloadbalancing:*:<accountid>:listener-rule/net/*/*/*/*"
            ]
        }

[olemarkus]

I think the ACM part needs more thinking. Maybe require that the certs have certain tags for them to be used by a cluster.

[peter-svensson]

I think the ACM part needs more thinking. Maybe require that the certs have certain tags for them to be used by a cluster.

I'd say the same might be applicable to the ALBs created by the ingress controller? But that might be harder to achieve?

[olemarkus]

I think the ACM part needs more thinking. Maybe require that the certs have certain tags for them to be used by a cluster.

I'd say the same might be applicable to the ALBs created by the ingress controller? But that might be harder to achieve?

ALBs created by the LB controller has the elbv2.k8s.aws/cluster: <cluster> tag. But those are created by the cluster.
ACM certs are never created by the cluster itself, so we need to use tags so that ALB controller suddenly doesn't get to use any cert for any purpose.

This is why we don't copy/paste the permissions from the LB controller docs.. they are just way to open for anyone security minded.

[peter-svensson]

I think the ACM part needs more thinking. Maybe require that the certs have certain tags for them to be used by a cluster.

I'd say the same might be applicable to the ALBs created by the ingress controller? But that might be harder to achieve?

ALBs created by the LB controller has the elbv2.k8s.aws/cluster: <cluster> tag. But those are created by the cluster.
ACM certs are never created by the cluster itself, so we need to use tags so that ALB controller suddenly doesn't get to use any cert for any purpose.

This is why we don't copy/paste the permissions from the LB controller docs.. they are just way to open for anyone security minded.

I can image that.
What I meant was the the permission that we set with kops when enabling the AWS ALB addon should reflect that as well. So the permission to delete/update rules for example should be only for the ALBs with the correct tags as well. I haven't looked at the code so it might already be the case that it is considered.

[olemarkus]

ah yes. more rules could have

		Condition: Condition{
			"StringEquals": map[string]string{
				"ec2:ResourceTag/elbv2.k8s.aws/cluster": clusterName,
			},
		},

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [hakman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment