Skip to content

Commit

Permalink
Merge pull request #4026 from zac-nixon/znixon/multi-acc-tgb
Browse files Browse the repository at this point in the history
Follow up for 3691 [TargetGroupBindings can now manipulate target groups from different aws accounts]
  • Loading branch information
k8s-ci-robot authored Jan 24, 2025
2 parents 4d5ce64 + dfae145 commit 388f1df
Show file tree
Hide file tree
Showing 27 changed files with 636 additions and 294 deletions.
8 changes: 8 additions & 0 deletions apis/elbv2/v1alpha1/targetgroupbinding_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ type TargetGroupBindingSpec struct {
// networking provides the networking setup for ELBV2 LoadBalancer to access targets in TargetGroup.
// +optional
Networking *TargetGroupBindingNetworking `json:"networking,omitempty"`

// IAM Role ARN to assume when calling AWS APIs. Useful if the target group is in a different AWS account
// +optional
IamRoleArnToAssume string `json:"iamRoleArnToAssume,omitempty"`

// IAM Role ARN to assume when calling AWS APIs. Needed to assume a role in another account and prevent the confused deputy problem. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html
// +optional
AssumeRoleExternalId string `json:"assumeRoleExternalId,omitempty"`
}

// TargetGroupBindingStatus defines the observed state of TargetGroupBinding
Expand Down
4 changes: 2 additions & 2 deletions apis/elbv2/v1beta1/targetgroupbinding_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,11 @@ type TargetGroupBindingSpec struct {

// IAM Role ARN to assume when calling AWS APIs. Useful if the target group is in a different AWS account
// +optional
IamRoleArnToAssume string `json:"-"` // `json:"iamRoleArnToAssume,omitempty"`
IamRoleArnToAssume string `json:"iamRoleArnToAssume,omitempty"`

// IAM Role ARN to assume when calling AWS APIs. Needed to assume a role in another account and prevent the confused deputy problem. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html
// +optional
AssumeRoleExternalId string `json:"-"` // `json:"assumeRoleExternalId,omitempty"`
AssumeRoleExternalId string `json:"assumeRoleExternalId,omitempty"`
}

// TargetGroupBindingStatus defines the observed state of TargetGroupBinding
Expand Down
18 changes: 18 additions & 0 deletions config/crd/bases/elbv2.k8s.aws_targetgroupbindings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ spec:
spec:
description: TargetGroupBindingSpec defines the desired state of TargetGroupBinding
properties:
assumeRoleExternalId:
description: IAM Role ARN to assume when calling AWS APIs. Needed
to assume a role in another account and prevent the confused deputy
problem. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html
type: string
iamRoleArnToAssume:
description: IAM Role ARN to assume when calling AWS APIs. Useful
if the target group is in a different AWS account
type: string
multiClusterTargetGroup:
description: MultiClusterTargetGroup Denotes if the TargetGroup is
shared among multiple clusters
Expand Down Expand Up @@ -242,6 +251,15 @@ spec:
spec:
description: TargetGroupBindingSpec defines the desired state of TargetGroupBinding
properties:
assumeRoleExternalId:
description: IAM Role ARN to assume when calling AWS APIs. Needed
to assume a role in another account and prevent the confused deputy
problem. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html
type: string
iamRoleArnToAssume:
description: IAM Role ARN to assume when calling AWS APIs. Useful
if the target group is in a different AWS account
type: string
ipAddressType:
description: ipAddressType specifies whether the target group is of
type IPv4 or IPv6. If unspecified, it will be automatically inferred.
Expand Down
9 changes: 0 additions & 9 deletions docs/guide/targetgroupbinding/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,6 @@ Kubernetes meta/v1.ObjectMeta
<table>
<tr><td><code>annotations</code></td><td>

<table>
<tr><td><code>alb.ingress.kubernetes.io/IamRoleArnToAssume</code><br><i>string</i></td>
<td><i>(Optional)</i> In case the target group is in a differet AWS account, you put here the role that needs to be assumed in order to manipulate the target group.
</td></tr>
<tr><td><code>alb.ingress.kubernetes.io/AssumeRoleExternalId</code><br><i>string</i></td>
<td><i>(Optional)</i> The external ID for the assume role operation. Optional, but recommended. It helps you to prevent the <a href="https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html" target="_blank">confused deputy problem</a>.
</td></tr>
</table>

<tr><td colspan=2>
Refer to the Kubernetes API documentation for the other fields of the
<code>metadata</code> field.
Expand Down
109 changes: 103 additions & 6 deletions docs/guide/targetgroupbinding/targetgroupbinding.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,108 @@ spec:
### AssumeRole

Sometimes the AWS LoadBalancer controller needs to manipulate target groups from different AWS accounts.
The way to do that is assuming a role from such account. There are annotations that can help you with that:
The way to do that is assuming a role from such an account. The following spec fields help you with that.

* `alb.ingress.kubernetes.io/IamRoleArnToAssume`: the ARN that you need to assume
* `alb.ingress.kubernetes.io/AssumeRoleExternalId`: the external ID for the assume role operation. Optional, but recommended. It helps you to prevent the confused deputy problem ( https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html )
* `iamRoleArnToAssume`: the ARN that you need to assume
* `assumeRoleExternalId`: the external ID for the assume role operation. Optional, but recommended. It helps you to prevent the confused deputy problem ( https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html )


```yaml
apiVersion: elbv2.k8s.aws/v1beta1
kind: TargetGroupBinding
metadata:
name: peered-tg
namespace: nlb-game-2048-1
spec:
assumeRoleExternalId: very-secret-string-2
iamRoleArnToAssume: arn:aws:iam::155642222660:role/tg-management-role
networking:
ingress:
- from:
- securityGroup:
groupID: sg-0b6a41a2fd959623f
ports:
- port: 80
protocol: TCP
serviceRef:
name: service-2048
port: 80
targetGroupARN: arn:aws:elasticloadbalancing:us-west-2:155642222660:targetgroup/peered-tg/6a4ecf7bfae473c1
```

In the following examples, we will refer to Cluster Owner (CO) and Target Group Owner (TGO) accounts.

First, in the TGO account creates a role that will allow the AWS LBC in the CO account to assume it.
For improved security, we only allow the AWS LBC role in CO account to assume the role.

```json
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "",
"Effect": "Allow",
"Principal": {
"AWS": "arn:aws:iam::565768096483:role/eksctl-awslbc-loadtest-addon-iamserviceaccoun-Role1-13RdJCMqV6p2"
},
"Action": "sts:AssumeRole",
"Condition": {
"StringEquals": {
"sts:ExternalId": "very-secret-string"
}
}
}
]
}
```

Next, still in the TGO account we need to add the following permissions to the Role created in the first step.

```json
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "VisualEditor0",
"Effect": "Allow",
"Action": [
"elasticloadbalancing:RegisterTargets",
"elasticloadbalancing:DeregisterTargets"
],
"Resource": [
"arn:aws:elasticloadbalancing:us-west-2:155642222660:targetgroup/tg1/*",
"arn:aws:elasticloadbalancing:us-west-2:155642222660:targetgroup/tg2/*"
// add more here //
]
},
{
"Sid": "VisualEditor1",
"Effect": "Allow",
"Action": [
"elasticloadbalancing:DescribeTargetGroups",
"elasticloadbalancing:DescribeTargetHealth"
],
"Resource": "*"
}
]
}
```


Next, in the CO account, we need to allow the AWS LBC to perform the AssumeRole call.
By default, this permission is not a part of the standard IAM policy that is vended with the LBC installation scripts.
For improved security, it is possible to scope the AssumeRole permissions down to only roles that you know ahead of time the
LBC will need to Assume.

```json
{
"Effect": "Allow",
"Action": [
"sts:AssumeRole"
],
"Resource": "*"
}
```


## Sample YAML
Expand All @@ -125,10 +223,9 @@ apiVersion: elbv2.k8s.aws/v1beta1
kind: TargetGroupBinding
metadata:
name: my-tgb
annotations:
alb.ingress.kubernetes.io/IamRoleArnToAssume: "arn:aws:iam::999999999999:role/alb-controller-policy-to-assume"
alb.ingress.kubernetes.io/AssumeRoleExternalId: "some-magic-string"
spec:
iamRoleArnToAssume: "arn:aws:iam::999999999999:role/alb-controller-policy-to-assume"
assumeRoleExternalId: "some-magic-string"
...
```

Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ github.com/aws/aws-sdk-go-v2/service/ec2 v1.173.0 h1:ta62lid9JkIpKZtZZXSj6rP2AqY
github.com/aws/aws-sdk-go-v2/service/ec2 v1.173.0/go.mod h1:o6QDjdVKpP5EF0dp/VlvqckzuSDATr1rLdHt3A5m0YY=
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.43.1 h1:L9Wt9zgtoYKIlaeFTy+EztGjL4oaXBBGtVXA+jaeYko=
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.43.1/go.mod h1:yxzLdxt7bVGvIOPYIKFtiaJCJnx2ChlIIvlhW4QgI6M=
github.com/aws/aws-sdk-go-v2/service/iam v1.36.3/go.mod h1:HSvujsK8xeEHMIB18oMXjSfqaN9cVqpo/MtHJIksQRk=
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3 h1:dT3MqvGhSoaIhRseqw2I0yH81l7wiR2vjs57O51EAm8=
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3/go.mod h1:GlAeCkHwugxdHaueRr4nhPuY+WW+gR8UjlcqzPr1SPI=
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.17 h1:HGErhhrxZlQ044RiM+WdoZxp0p+EGM62y3L6pwA4olE=
Expand Down
18 changes: 18 additions & 0 deletions helm/aws-load-balancer-controller/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,15 @@ spec:
spec:
description: TargetGroupBindingSpec defines the desired state of TargetGroupBinding
properties:
assumeRoleExternalId:
description: IAM Role ARN to assume when calling AWS APIs. Needed
to assume a role in another account and prevent the confused deputy
problem. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html
type: string
iamRoleArnToAssume:
description: IAM Role ARN to assume when calling AWS APIs. Useful
if the target group is in a different AWS account
type: string
multiClusterTargetGroup:
description: MultiClusterTargetGroup Denotes if the TargetGroup is
shared among multiple clusters
Expand Down Expand Up @@ -494,6 +503,15 @@ spec:
spec:
description: TargetGroupBindingSpec defines the desired state of TargetGroupBinding
properties:
assumeRoleExternalId:
description: IAM Role ARN to assume when calling AWS APIs. Needed
to assume a role in another account and prevent the confused deputy
problem. https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html
type: string
iamRoleArnToAssume:
description: IAM Role ARN to assume when calling AWS APIs. Useful
if the target group is in a different AWS account
type: string
ipAddressType:
description: ipAddressType specifies whether the target group is of
type IPv4 or IPv6. If unspecified, it will be automatically inferred.
Expand Down
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func main() {
awsMetricsCollector = awsmetrics.NewCollector(metrics.Registry)
}

cloud, err := aws.NewCloud(controllerCFG.AWSConfig, awsMetricsCollector, ctrl.Log, nil)
cloud, err := aws.NewCloud(controllerCFG.AWSConfig, controllerCFG.ClusterName, awsMetricsCollector, ctrl.Log, nil)
if err != nil {
setupLog.Error(err, "unable to initialize AWS cloud")
os.Exit(1)
Expand Down
81 changes: 81 additions & 0 deletions pkg/aws/aws_config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package aws

import (
"context"
"github.com/aws/aws-sdk-go-v2/aws"
awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
"github.com/aws/aws-sdk-go-v2/aws/ratelimit"
"github.com/aws/aws-sdk-go-v2/aws/retry"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
smithymiddleware "github.com/aws/smithy-go/middleware"
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/throttle"
awsmetrics "sigs.k8s.io/aws-load-balancer-controller/pkg/metrics/aws"
"sigs.k8s.io/aws-load-balancer-controller/pkg/version"
)

const (
userAgent = "elbv2.k8s.aws"
)

func NewAWSConfigGenerator(cfg CloudConfig, ec2IMDSEndpointMode imds.EndpointModeState, metricsCollector *awsmetrics.Collector) AWSConfigGenerator {
return &awsConfigGeneratorImpl{
cfg: cfg,
ec2IMDSEndpointMode: ec2IMDSEndpointMode,
metricsCollector: metricsCollector,
}

}

// AWSConfigGenerator is responsible for generating an aws config based on the running environment
type AWSConfigGenerator interface {
GenerateAWSConfig(optFns ...func(*config.LoadOptions) error) (aws.Config, error)
}

type awsConfigGeneratorImpl struct {
cfg CloudConfig
ec2IMDSEndpointMode imds.EndpointModeState
metricsCollector *awsmetrics.Collector
}

func (gen *awsConfigGeneratorImpl) GenerateAWSConfig(optFns ...func(*config.LoadOptions) error) (aws.Config, error) {

defaultOpts := []func(*config.LoadOptions) error{
config.WithRegion(gen.cfg.Region),
config.WithRetryer(func() aws.Retryer {
return retry.NewStandard(func(o *retry.StandardOptions) {
o.RateLimiter = ratelimit.None
o.MaxAttempts = gen.cfg.MaxRetries
})
}),
config.WithEC2IMDSEndpointMode(gen.ec2IMDSEndpointMode),
config.WithAPIOptions([]func(stack *smithymiddleware.Stack) error{
awsmiddleware.AddUserAgentKeyValue(userAgent, version.GitVersion),
}),
}

defaultOpts = append(defaultOpts, optFns...)

awsConfig, err := config.LoadDefaultConfig(context.TODO(),
defaultOpts...,
)

if err != nil {
return aws.Config{}, err
}

if gen.cfg.ThrottleConfig != nil {
throttler := throttle.NewThrottler(gen.cfg.ThrottleConfig)
awsConfig.APIOptions = append(awsConfig.APIOptions, func(stack *smithymiddleware.Stack) error {
return throttle.WithSDKRequestThrottleMiddleware(throttler)(stack)
})
}

if gen.metricsCollector != nil {
awsConfig.APIOptions = awsmetrics.WithSDKMetricCollector(gen.metricsCollector, awsConfig.APIOptions)
}

return awsConfig, nil
}

var _ AWSConfigGenerator = &awsConfigGeneratorImpl{}
Loading

0 comments on commit 388f1df

Please sign in to comment.