Merge pull request #4281 from pascal-hofmann/action-annotation-with-target-group-name

feat: allow targetGroupName instead of targetGroupARN in forward action ingress annotations

Author: k8s-ci-robot

docs/guide/ingress/annotations.md

@@ -285,18 +285,19 @@ Traffic Routing can be controlled with following annotations:
 
     The `action-name` in the annotation must match the serviceName in the Ingress rules, and servicePort must be `use-annotation`.
 
-    !!!note "use ARN in forward Action"
-        ARN can be used in forward action(both simplified schema and advanced schema), it must be an targetGroup created outside of k8s, typically an targetGroup for legacy application.
+    !!!note "use TargetGroupARN/TargetGroupName in forward Action"
+        TargetGroupARN/TargetGroupName can be used in forward action (both simplified schema and advanced schema), it must be a target group created outside of k8s, typically a targetGroup for a legacy application.
     !!!note "use ServiceName/ServicePort in forward Action"
-        ServiceName/ServicePort can be used in forward action(advanced schema only).
+        ServiceName/ServicePort can be used in forward action (advanced schema only).
 
     !!!warning ""
-        [Auth related annotations](#authentication) on Service object will only be respected if a single TargetGroup in is used.
+        [Auth related annotations](#authentication) on a Service object will only be respected if a single TargetGroup is used.
 
     !!!example
         - response-503: return fixed 503 response
         - redirect-to-eks: redirect to an external url
         - forward-single-tg: forward to a single targetGroup [**simplified schema**]
+        - forward-single-tg-by-name: forward to a single targetGroup identified by its name  [**simplified schema**]
         - forward-multiple-tg: forward to multiple targetGroups with different weights and stickiness config [**advanced schema**]
 
         ```yaml
@@ -313,8 +314,10 @@ Traffic Routing can be controlled with following annotations:
               {"type":"redirect","redirectConfig":{"host":"aws.amazon.com","path":"/eks/","port":"443","protocol":"HTTPS","query":"k=v","statusCode":"HTTP_302"}}
             alb.ingress.kubernetes.io/actions.forward-single-tg: >
               {"type":"forward","targetGroupARN": "arn-of-your-target-group"}
+            alb.ingress.kubernetes.io/actions.forward-single-tg-by-name: >
+              {"type":"forward","targetGroupName": "name-of-your-target-group"}
             alb.ingress.kubernetes.io/actions.forward-multiple-tg: >
-              {"type":"forward","forwardConfig":{"targetGroups":[{"serviceName":"service-1","servicePort":"http","weight":20},{"serviceName":"service-2","servicePort":80,"weight":20},{"targetGroupARN":"arn-of-your-non-k8s-target-group","weight":60}],"targetGroupStickinessConfig":{"enabled":true,"durationSeconds":200}}}
+              {"type":"forward","forwardConfig":{"targetGroups":[{"serviceName":"service-1","servicePort":"http","weight":20},{"serviceName":"service-2","servicePort":80,"weight":20},{"targetGroupARN":"arn-of-your-non-k8s-target-group","weight":60},{"targetGroupName":"name-of-your-non-k8s-target-group","weight":80}],"targetGroupStickinessConfig":{"enabled":true,"durationSeconds":200}}}
         spec:
           ingressClassName: alb
           rules:
@@ -342,6 +345,13 @@ Traffic Routing can be controlled with following annotations:
                         port:
                           name: use-annotation
                   - path: /path2
+                    pathType: Exact
+                    backend:
+                      service:
+                        name: forward-single-tg-by-name
+                        port:
+                          name: use-annotation
+                  - path: /path3
                     pathType: Exact
                     backend:
                       service:
@@ -647,7 +657,7 @@ ALB supports authentication with Cognito or OIDC. See [Authenticate Users Using
 - <a name="auth-idp-oidc">`alb.ingress.kubernetes.io/auth-idp-oidc`</a> specifies the oidc idp configuration.
 
     !!!tip ""
-        You need to create an [secret](https://kubernetes.io/docs/concepts/configuration/secret/) within the same namespace as Ingress to hold your OIDC clientID and clientSecret. The format of secret is as below:
+        You need to create a [secret](https://kubernetes.io/docs/concepts/configuration/secret/) within the same namespace as Ingress to hold your OIDC clientID and clientSecret. The format of secret is as below:
         ```yaml
         apiVersion: v1
         kind: Secret
@@ -667,7 +677,7 @@ ALB supports authentication with Cognito or OIDC. See [Authenticate Users Using
 - <a name="auth-on-unauthenticated-request">`alb.ingress.kubernetes.io/auth-on-unauthenticated-request`</a> specifies the behavior if the user is not authenticated.
 
 	!!!info "options:"
-        * **authenticate**: try authenticate with configured IDP.
+        * **authenticate**: try to authenticate with configured IDP.
         * **deny**: return an HTTP 401 Unauthorized error.
         * **allow**: allow the request to be forwarded to the target.
 
@@ -837,10 +847,10 @@ TLS support can be controlled with the following annotations:
         - This annotation is not applicable for Outposts, Local Zones or Wavelength zones.
         - "Configuration Options"
             - `port: listen port ` 
-               - Must be a HTTPS port specified by [listen-ports](#listen-ports).
+               - Must be an HTTPS port specified by [listen-ports](#listen-ports).
             - `mode: "off" (default) | "passthrough" | "verify"`
                - `verify` mode requires an existing trust store resource.
-               -  See [Create a trust store](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/mutual-authentication.html#create-trust-store) in the AWS documentation for more details.
+               - See [Create a trust store](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/mutual-authentication.html#create-trust-store) in the AWS documentation for more details.
             - `trustStore: ARN (arn:aws:elasticloadbalancing:trustStoreArn) | Name (my-trust-store)`
                - Both ARN and Name of trustStore are supported values.
                - `trustStore` is required when mode is `verify`.

pkg/ingress/config_types.go

@@ -68,6 +68,10 @@ type TargetGroupTuple struct {
 	// The Amazon Resource Name (ARN) of the target group.
 	TargetGroupARN *string `json:"targetGroupARN"`
 
+	// The Name of the target group.
+	// +optional
+	TargetGroupName *string `json:"targetGroupName,omitempty"`
+
 	// the K8s service Name
 	ServiceName *string `json:"serviceName"`
 
@@ -80,8 +84,12 @@ type TargetGroupTuple struct {
 }
 
 func (t *TargetGroupTuple) validate() error {
-	if (t.TargetGroupARN != nil) == (t.ServiceName != nil) {
-		return errors.New("precisely one of targetGroupARN and serviceName can be specified")
+	if t.TargetGroupARN == nil && t.TargetGroupName == nil && t.ServiceName == nil {
+		return errors.New("missing serviceName or targetGroupARN/targetGroupName")
+	}
+
+	if (t.TargetGroupARN != nil || t.TargetGroupName != nil) && t.ServiceName != nil {
+		return errors.New("either serviceName or targetGroupARN/targetGroupName can be specified")
 	}
 
 	if t.ServiceName != nil && t.ServicePort == nil {
@@ -146,6 +154,9 @@ type Action struct {
 	// or more target groups, use ForwardConfig instead.
 	TargetGroupARN *string `json:"targetGroupARN"`
 
+	// Name of the target group. Can be specified instead of TargetGroupARN.
+	TargetGroupName *string `json:"targetGroupName,omitempty"`
+
 	// [Application Load Balancer] Information for creating an action that returns a custom HTTP response.
 	// +optional
 	FixedResponseConfig *FixedResponseActionConfig `json:"fixedResponseConfig,omitempty"`
@@ -176,8 +187,11 @@ func (a *Action) validate() error {
 			return errors.Wrap(err, "invalid RedirectConfig")
 		}
 	case ActionTypeForward:
-		if (a.TargetGroupARN != nil) == (a.ForwardConfig != nil) {
-			return errors.New("precisely one of TargetGroupArn and ForwardConfig can be specified")
+		if a.TargetGroupARN == nil && a.TargetGroupName == nil && a.ForwardConfig == nil {
+			return errors.New("missing ForwardConfig or TargetGroupARN/TargetGroupName")
+		}
+		if (a.TargetGroupARN != nil || a.TargetGroupName != nil) && (a.ForwardConfig != nil) {
+			return errors.New("either ForwardConfig or TargetGroupARN/TargetGroupName can be specified")
 		}
 		if a.ForwardConfig != nil {
 			if err := a.ForwardConfig.validate(); err != nil {

pkg/ingress/enhanced_backend_builder.go

@@ -208,13 +208,14 @@ func (b *defaultEnhancedBackendBuilder) buildActionViaServiceAndServicePort(_ co
 // normalizeSimplifiedSchemaForwardAction will normalize to the advanced schema for forward action to share common processing logic.
 // we support a simplified schema in action annotation when configure forward to a single TargetGroup.
 func (b *defaultEnhancedBackendBuilder) normalizeSimplifiedSchemaForwardAction(_ context.Context, action *Action) {
-	if action.Type == ActionTypeForward && action.TargetGroupARN != nil {
+	if action.Type == ActionTypeForward && (action.TargetGroupARN != nil || action.TargetGroupName != nil) {
 		*action = Action{
 			Type: ActionTypeForward,
 			ForwardConfig: &ForwardActionConfig{
 				TargetGroups: []TargetGroupTuple{
 					{
-						TargetGroupARN: action.TargetGroupARN,
+						TargetGroupARN:  action.TargetGroupARN,
+						TargetGroupName: action.TargetGroupName,
 					},
 				},
 			},

pkg/ingress/enhanced_backend_builder_test.go

@@ -1048,11 +1048,30 @@ func Test_defaultEnhancedBackendBuilder_buildActionViaAnnotation(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "forward action - simplified schema with target group name",
+			args: args{
+				ingAnnotation: map[string]string{
+					"alb.ingress.kubernetes.io/actions.forward-single-tg": `{"type":"forward","targetGroupName": "tg-name"}`,
+				},
+				svcName: "forward-single-tg",
+			},
+			want: Action{
+				Type: ActionTypeForward,
+				ForwardConfig: &ForwardActionConfig{
+					TargetGroups: []TargetGroupTuple{
+						{
+							TargetGroupName: awssdk.String("tg-name"),
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "forward action - advanced schema",
 			args: args{
 				ingAnnotation: map[string]string{
-					"alb.ingress.kubernetes.io/actions.forward-multiple-tg": `{"type":"forward","forwardConfig":{"targetGroups":[{"serviceName":"service-1","servicePort":"http","weight":20},{"serviceName":"service-2","servicePort":80,"weight":20},{"targetGroupARN":"tg-arn","weight":60}],"targetGroupStickinessConfig":{"enabled":true,"durationSeconds":200}}}`,
+					"alb.ingress.kubernetes.io/actions.forward-multiple-tg": `{"type":"forward","forwardConfig":{"targetGroups":[{"serviceName":"service-1","servicePort":"http","weight":20},{"serviceName":"service-2","servicePort":80,"weight":20},{"targetGroupARN":"tg-arn","weight":60},{"targetGroupName":"tg-name","weight":80}],"targetGroupStickinessConfig":{"enabled":true,"durationSeconds":200}}}`,
 				},
 				svcName: "forward-multiple-tg",
 			},
@@ -1074,6 +1093,10 @@ func Test_defaultEnhancedBackendBuilder_buildActionViaAnnotation(t *testing.T) {
 							TargetGroupARN: awssdk.String("tg-arn"),
 							Weight:         awssdk.Int32(60),
 						},
+						{
+							TargetGroupName: awssdk.String("tg-name"),
+							Weight:          awssdk.Int32(80),
+						},
 					},
 					TargetGroupStickinessConfig: &TargetGroupStickinessConfig{
 						Enabled:         awssdk.Bool(true),

pkg/ingress/model_build_actions.go

@@ -105,6 +105,12 @@ func (t *defaultModelBuildTask) buildForwardAction(ctx context.Context, ing Clas
 		var tgARN core.StringToken
 		if tgt.TargetGroupARN != nil {
 			tgARN = core.LiteralStringToken(*tgt.TargetGroupARN)
+		} else if tgt.TargetGroupName != nil {
+			targetGroupARN, err := t.targetGroupNameToArnMapper.getArnByName(ctx, *tgt.TargetGroupName)
+			if err != nil {
+				return elbv2model.Action{}, fmt.Errorf("searching TargetGroup with name %s: %w", *tgt.TargetGroupName, err)
+			}
+			tgARN = core.LiteralStringToken(targetGroupARN)
 		} else {
 			svcKey := types.NamespacedName{
 				Namespace: ing.Ing.Namespace,

pkg/ingress/model_build_actions_test.go

@@ -3,12 +3,17 @@ package ingress
 import (
 	"context"
 	awssdk "github.com/aws/aws-sdk-go-v2/aws"
+	elbv2sdk "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
+	elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
+	"github.com/golang/mock/gomock"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
 	elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 	testclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"testing"
@@ -339,3 +344,117 @@ func Test_defaultModelBuildTask_buildSSLRedirectAction(t *testing.T) {
 		})
 	}
 }
+
+func Test_defaultModelBuildTask_buildForwardActionWithTargetGroupName(t *testing.T) {
+	type describeTargetGroupsAsListCall struct {
+		req  *elbv2sdk.DescribeTargetGroupsInput
+		resp []elbv2types.TargetGroup
+		err  error
+	}
+	type args struct {
+		ingress                         ClassifiedIngress
+		forwardActionConfig             ForwardActionConfig
+		describeTargetGroupsAsListCalls []describeTargetGroupsAsListCall
+		cache                           map[string]string
+	}
+	tests := []struct {
+		name      string
+		args      args
+		want      elbv2model.Action
+		wantErr   error
+		wantCache map[string]string
+	}{
+		{
+			name: "Forward to target group identified by name",
+			args: args{
+				forwardActionConfig: ForwardActionConfig{
+					TargetGroups: []TargetGroupTuple{{TargetGroupName: awssdk.String("tg-name1")}},
+				},
+
+				describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{
+					{
+						req: &elbv2sdk.DescribeTargetGroupsInput{
+							Names: []string{"tg-name1"},
+						},
+						resp: []elbv2types.TargetGroup{
+							{
+								TargetGroupArn: awssdk.String("tg-arn1"),
+								TargetType:     elbv2types.TargetTypeEnum("instance"),
+							},
+						},
+					},
+				},
+				cache: map[string]string{},
+			},
+			want: elbv2model.Action{
+				Type: elbv2model.ActionTypeForward,
+				ForwardConfig: &elbv2model.ForwardActionConfig{
+					TargetGroups: []elbv2model.TargetGroupTuple{{
+						TargetGroupARN: core.LiteralStringToken("tg-arn1"),
+					}},
+				},
+			},
+			wantCache: map[string]string{
+				"tg-name1": "tg-arn1",
+			},
+		},
+		{
+			name: "Forward to target group identified by name is cached",
+			args: args{
+				forwardActionConfig: ForwardActionConfig{
+					TargetGroups: []TargetGroupTuple{{TargetGroupName: awssdk.String("tg-name2")}},
+				},
+
+				describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{},
+				cache: map[string]string{
+					"tg-name2": "tg-arn2",
+				},
+			},
+			want: elbv2model.Action{
+				Type: elbv2model.ActionTypeForward,
+				ForwardConfig: &elbv2model.ForwardActionConfig{
+					TargetGroups: []elbv2model.TargetGroupTuple{{
+						TargetGroupARN: core.LiteralStringToken("tg-arn2"),
+					}},
+				},
+			},
+			wantCache: map[string]string{
+				"tg-name2": "tg-arn2",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t1 *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			elbv2Client := services.NewMockELBV2(ctrl)
+			for _, call := range tt.args.describeTargetGroupsAsListCalls {
+				elbv2Client.EXPECT().DescribeTargetGroupsAsList(gomock.Any(), call.req).Return(call.resp, call.err)
+			}
+			task := &defaultModelBuildTask{
+				elbv2Client:                elbv2Client,
+				targetGroupNameToArnMapper: newTargetGroupNameToArnMapper(elbv2Client, defaultTargetGroupNameToARNCacheTTL),
+			}
+
+			for targetGroupName, cachedArn := range tt.args.cache {
+				task.targetGroupNameToArnMapper.cache.Set(targetGroupName, cachedArn, defaultTargetGroupNameToARNCacheTTL)
+			}
+
+			got, err := task.buildForwardAction(context.Background(), tt.args.ingress, Action{
+				Type:          ActionTypeForward,
+				ForwardConfig: &tt.args.forwardActionConfig,
+			})
+			assert.Equal(t, tt.want, got)
+			assert.Equal(t, tt.wantErr, err)
+			assert.Equal(t, len(tt.wantCache), task.targetGroupNameToArnMapper.cache.Len())
+			for targetGroupName, expectedArn := range tt.wantCache {
+				rawCacheItem, exists := task.targetGroupNameToArnMapper.cache.Get(targetGroupName)
+				assert.True(t, exists)
+				cachedArn := rawCacheItem.(string)
+				assert.Equal(t, expectedArn, cachedArn)
+			}
+		})
+	}
+}