fix assume role tgb

Author: zac-nixon

docs/guide/targetgroupbinding/targetgroupbinding.md

@@ -118,6 +118,59 @@ The way to do that is assuming a role from such account. The following spec fiel
 * `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 order to use this feature, the source account (The cluster owner) must allow the controller role to assume role. By default, the installed permissions
+_do not_ allow this. Augment the load balancer controller role by adding:
+
+```json
+        {
+            "Effect": "Allow",
+            "Action": [
+                "sts:AssumeRole"
+            ],
+            "Resource": "*"
+        }
+```
+
+In the target account, which owns the Target Group the assumed role must allow for Target Group Management, at the minimum:
+
+```json
+      {
+            "Effect": "Allow",
+            "Action": [
+                "elasticloadbalancing:RegisterTargets",
+                "elasticloadbalancing:DeregisterTargets",
+                "elasticloadbalancing:DescribeTargetHealth",
+                "elasticloadbalancing:DescribeTargetGroups"
+            ],
+            "Resource": "*"
+        }
+```
+
+
+
 ## Sample YAML
 
 ```yaml

helm/aws-load-balancer-controller/crds/crds.yaml

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

pkg/aws/aws_config.go

@@ -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{}

pkg/aws/cloud.go

@@ -10,18 +10,11 @@ import (
 	"sync"
 	"time"
 
-	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/credentials"
 	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
 
-	smithymiddleware "github.com/aws/smithy-go/middleware"
-	"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/throttle"
-	"sigs.k8s.io/aws-load-balancer-controller/pkg/version"
-
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
@@ -35,7 +28,6 @@ import (
 )
 
 const (
-	userAgent          = "elbv2.k8s.aws"
 	cacheTTLBufferTime = 30 * time.Second
 )
 
@@ -81,29 +73,11 @@ func NewCloud(cfg CloudConfig, clusterName string, metricsCollector *aws_metrics
 		}
 		cfg.Region = region
 	}
-	awsConfig, err := config.LoadDefaultConfig(context.TODO(),
-		config.WithRegion(cfg.Region),
-		config.WithRetryer(func() aws.Retryer {
-			return retry.NewStandard(func(o *retry.StandardOptions) {
-				o.RateLimiter = ratelimit.None
-				o.MaxAttempts = cfg.MaxRetries
-			})
-		}),
-		config.WithEC2IMDSEndpointMode(ec2IMDSEndpointMode),
-		config.WithAPIOptions([]func(stack *smithymiddleware.Stack) error{
-			awsmiddleware.AddUserAgentKeyValue(userAgent, version.GitVersion),
-		}),
-	)
-
-	if cfg.ThrottleConfig != nil {
-		throttler := throttle.NewThrottler(cfg.ThrottleConfig)
-		awsConfig.APIOptions = append(awsConfig.APIOptions, func(stack *smithymiddleware.Stack) error {
-			return throttle.WithSDKRequestThrottleMiddleware(throttler)(stack)
-		})
-	}
 
-	if metricsCollector != nil {
-		awsConfig.APIOptions = aws_metrics.WithSDKMetricCollector(metricsCollector, awsConfig.APIOptions)
+	awsConfigGenerator := NewAWSConfigGenerator(cfg, ec2IMDSEndpointMode, metricsCollector)
+	awsConfig, err := awsConfigGenerator.GenerateAWSConfig()
+	if err != nil {
+		return nil, errors.Wrap(err, "Unable to generate AWS config")
 	}
 
 	if awsClientsProvider == nil {
@@ -132,6 +106,8 @@ func NewCloud(cfg CloudConfig, clusterName string, metricsCollector *aws_metrics
 		shield:      services.NewShield(awsClientsProvider),
 		rgt:         services.NewRGT(awsClientsProvider),
 
+		awsConfigGenerator: awsConfigGenerator,
+
 		assumeRoleElbV2Cache: cache.NewExpiring(),
 
 		awsClientsProvider: awsClientsProvider,
@@ -229,6 +205,8 @@ type defaultCloud struct {
 
 	clusterName string
 
+	awsConfigGenerator AWSConfigGenerator
+
 	// A cache holding elbv2 clients that are assuming a role.
 	assumeRoleElbV2Cache *cache.Expiring
 	// assumeRoleElbV2CacheMutex protects assumeRoleElbV2Cache
@@ -251,31 +229,33 @@ func (c *defaultCloud) GetAssumedRoleELBV2(ctx context.Context, assumeRoleArn st
 	if exists {
 		return assumedRoleELBV2.(services.ELBV2), nil
 	}
-	c.logger.Info("awsCloud", "method", "GetAssumedRoleELBV2", "AssumeRoleArn", assumeRoleArn, "externalId", externalId)
+	c.logger.Info("Constructing new elbv2 client", "AssumeRoleArn", assumeRoleArn, "externalId", externalId)
 
-	existingAwsConfig, _ := c.awsClientsProvider.GetAWSConfig(ctx, "GetAWSConfigForIAMRoleImpersonation")
+	stsClient, err := c.awsClientsProvider.GetSTSClient(ctx, "AssumeRole")
+	if err != nil {
+		// This should never happen, but let's be forward-looking.
+		return nil, err
+	}
 
-	sourceAccount := sts.NewFromConfig(*existingAwsConfig)
-	response, err := sourceAccount.AssumeRole(ctx, &sts.AssumeRoleInput{
+	response, err := stsClient.AssumeRole(ctx, &sts.AssumeRoleInput{
 		RoleArn:         aws.String(assumeRoleArn),
 		RoleSessionName: aws.String(generateAssumeRoleSessionName(c.clusterName)),
 		ExternalId:      aws.String(externalId),
 	})
 	if err != nil {
-		c.logger.Error(err, "Unable to assume target role, %v")
+		c.logger.Error(err, "Unable to assume target role", "roleArn", assumeRoleArn)
 		return nil, err
 	}
 	assumedRoleCreds := response.Credentials
 	newCreds := credentials.NewStaticCredentialsProvider(*assumedRoleCreds.AccessKeyId, *assumedRoleCreds.SecretAccessKey, *assumedRoleCreds.SessionToken)
-	newAwsConfig, err := config.LoadDefaultConfig(ctx, config.WithRegion(c.cfg.Region), config.WithCredentialsProvider(newCreds))
+	newAwsConfig, err := c.awsConfigGenerator.GenerateAWSConfig(config.WithCredentialsProvider(newCreds))
 	if err != nil {
-		c.logger.Error(err, "Unable to load static credentials for service client config, %v. Attempting to use default client")
+		c.logger.Error(err, "Create new service client config service client config", "roleArn", assumeRoleArn)
 		return nil, err
 	}
 
 	cacheTTL := assumedRoleCreds.Expiration.Sub(time.Now())
-	existingAwsConfig.Credentials = newAwsConfig.Credentials
-	elbv2WithAssumedRole := services.NewELBV2(c.awsClientsProvider, c)
+	elbv2WithAssumedRole := services.NewELBV2FromStaticClient(c.awsClientsProvider.GenerateNewELBv2Client(newAwsConfig), c)
 
 	c.assumeRoleElbV2CacheMutex.Lock()
 	defer c.assumeRoleElbV2CacheMutex.Unlock()

pkg/aws/provider/default_aws_clients_provider.go

@@ -8,6 +8,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
 	"github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi"
 	"github.com/aws/aws-sdk-go-v2/service/shield"
+	"github.com/aws/aws-sdk-go-v2/service/sts"
 	"github.com/aws/aws-sdk-go-v2/service/wafregional"
 	"github.com/aws/aws-sdk-go-v2/service/wafv2"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/endpoints"
@@ -21,29 +22,30 @@ type defaultAWSClientsProvider struct {
 	wafRegionClient *wafregional.Client
 	shieldClient    *shield.Client
 	rgtClient       *resourcegroupstaggingapi.Client
+	stsClient       *sts.Client
 
-	awsConfig *aws.Config
+	// used for dynamic creation of ELBv2 client
+	elbv2CustomEndpoint *string
 }
 
-func NewDefaultAWSClientsProvider(cfg aws.Config, endpointsResolver *endpoints.Resolver) (*defaultAWSClientsProvider, error) {
+func NewDefaultAWSClientsProvider(cfg aws.Config, endpointsResolver *endpoints.Resolver) (AWSClientsProvider, error) {
 	ec2CustomEndpoint := endpointsResolver.EndpointFor(ec2.ServiceID)
 	elbv2CustomEndpoint := endpointsResolver.EndpointFor(elasticloadbalancingv2.ServiceID)
 	acmCustomEndpoint := endpointsResolver.EndpointFor(acm.ServiceID)
 	wafv2CustomEndpoint := endpointsResolver.EndpointFor(wafv2.ServiceID)
 	wafregionalCustomEndpoint := endpointsResolver.EndpointFor(wafregional.ServiceID)
 	shieldCustomEndpoint := endpointsResolver.EndpointFor(shield.ServiceID)
 	rgtCustomEndpoint := endpointsResolver.EndpointFor(resourcegroupstaggingapi.ServiceID)
+	stsCustomEndpoint := endpointsResolver.EndpointFor(sts.ServiceID)
 
 	ec2Client := ec2.NewFromConfig(cfg, func(o *ec2.Options) {
 		if ec2CustomEndpoint != nil {
 			o.BaseEndpoint = ec2CustomEndpoint
 		}
 	})
-	elbv2Client := elasticloadbalancingv2.NewFromConfig(cfg, func(o *elasticloadbalancingv2.Options) {
-		if elbv2CustomEndpoint != nil {
-			o.BaseEndpoint = elbv2CustomEndpoint
-		}
-	})
+
+	elbv2Client := generateNewELBv2ClientHelper(cfg, elbv2CustomEndpoint)
+
 	acmClient := acm.NewFromConfig(cfg, func(o *acm.Options) {
 		if acmCustomEndpoint != nil {
 			o.BaseEndpoint = acmCustomEndpoint
@@ -68,6 +70,12 @@ func NewDefaultAWSClientsProvider(cfg aws.Config, endpointsResolver *endpoints.R
 		}
 	})
 
+	stsClient := sts.NewFromConfig(cfg, func(o *sts.Options) {
+		if stsCustomEndpoint != nil {
+			o.BaseEndpoint = stsCustomEndpoint
+		}
+	})
+
 	return &defaultAWSClientsProvider{
 		ec2Client:       ec2Client,
 		elbv2Client:     elbv2Client,
@@ -76,8 +84,9 @@ func NewDefaultAWSClientsProvider(cfg aws.Config, endpointsResolver *endpoints.R
 		wafRegionClient: wafregionalClient,
 		shieldClient:    shieldClient,
 		rgtClient:       rgtClient,
+		stsClient:       stsClient,
 
-		awsConfig: &cfg,
+		elbv2CustomEndpoint: elbv2CustomEndpoint,
 	}, nil
 }
 
@@ -112,6 +121,18 @@ func (p *defaultAWSClientsProvider) GetRGTClient(ctx context.Context, operationN
 	return p.rgtClient, nil
 }
 
-func (p *defaultAWSClientsProvider) GetAWSConfig(ctx context.Context, operationName string) (*aws.Config, error) {
-	return p.awsConfig, nil
+func (p *defaultAWSClientsProvider) GetSTSClient(ctx context.Context, operationName string) (*sts.Client, error) {
+	return p.stsClient, nil
+}
+
+func (p *defaultAWSClientsProvider) GenerateNewELBv2Client(cfg aws.Config) *elasticloadbalancingv2.Client {
+	return generateNewELBv2ClientHelper(cfg, p.elbv2CustomEndpoint)
+}
+
+func generateNewELBv2ClientHelper(cfg aws.Config, elbv2CustomEndpoint *string) *elasticloadbalancingv2.Client {
+	return elasticloadbalancingv2.NewFromConfig(cfg, func(o *elasticloadbalancingv2.Options) {
+		if elbv2CustomEndpoint != nil {
+			o.BaseEndpoint = elbv2CustomEndpoint
+		}
+	})
 }