Merge pull request #1976 from M00nF1sh/tag_ingClass

add support for tags in ingressClassParams

Author: k8s-ci-robot

apis/elbv2/v1beta1/ingressclassparams_types.go

@@ -47,6 +47,15 @@ type IngressGroup struct {
 Name string `json:"name"`
 }
 
+// Tag defines a AWS Tag on resources.
+type Tag struct {
+// The key of the tag.
+Key string `json:"key"`
+
+// The value of the tag.
+Value string `json:"value"`
+}
+
 // IngressClassParamsSpec defines the desired state of IngressClassParams
 type IngressClassParamsSpec struct {
 // NamespaceSelector restrict the namespaces of Ingresses that are allowed to specify the IngressClass with this IngressClassParams.
@@ -65,6 +74,9 @@ type IngressClassParamsSpec struct {
 // IPAddressType defines the ip address type for all Ingresses that belong to IngressClass with this IngressClassParams.
 // +optional
 IPAddressType *IPAddressType `json:"ipAddressType,omitempty"`
+
+// Tags defines list of Tags on AWS resources provisioned for Ingresses that belong to IngressClass with this IngressClassParams.
+Tags []Tag `json:"tags,omitempty"`
 }
 
 // +kubebuilder:object:root=true

apis/elbv2/v1beta1/zz_generated.deepcopy.go

@@ -103,6 +103,23 @@ spec:
  - internal
  - internet-facing
  type: string
+ tags:
+ description: Tags defines list of Tags on AWS resources provisioned
+ for Ingresses that belong to IngressClass with this IngressClassParams.
+ items:
+ description: Tag defines a AWS Tag on resources.
+ properties:
+ key:
+ description: The key of the tag.
+ type: string
+ value:
+ description: The value of the tag.
+ type: string
+ required:
+ - key
+ - value
+ type: object
+ type: array
  type: object
  type: object
  version: v1beta1

config/crd/bases/elbv2.k8s.aws_ingressclassparams.yaml

@@ -6,18 +6,17 @@ import (
 awssdk "github.com/aws/aws-sdk-go/aws"
 "github.com/pkg/errors"
 corev1 "k8s.io/api/core/v1"
-networking "k8s.io/api/networking/v1beta1"
 "k8s.io/apimachinery/pkg/types"
 "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
 elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 "strings"
 "unicode"
 )
 
-func (t *defaultModelBuildTask) buildActions(ctx context.Context, protocol elbv2model.Protocol, ing *networking.Ingress, backend EnhancedBackend) ([]elbv2model.Action, error) {
+func (t *defaultModelBuildTask) buildActions(ctx context.Context, protocol elbv2model.Protocol, ing ClassifiedIngress, backend EnhancedBackend) ([]elbv2model.Action, error) {
 var actions []elbv2model.Action
 if protocol == elbv2model.ProtocolHTTPS {
-authAction, err := t.buildAuthAction(ctx, ing, backend)
+authAction, err := t.buildAuthAction(ctx, ing.Ing.Namespace, backend)
 if err != nil {
 return nil, err
 }
@@ -33,7 +32,7 @@ func (t *defaultModelBuildTask) buildActions(ctx context.Context, protocol elbv2
 return actions, nil
 }
 
-func (t *defaultModelBuildTask) buildBackendAction(ctx context.Context, ing *networking.Ingress, actionCfg Action) (elbv2model.Action, error) {
+func (t *defaultModelBuildTask) buildBackendAction(ctx context.Context, ing ClassifiedIngress, actionCfg Action) (elbv2model.Action, error) {
 switch actionCfg.Type {
 case ActionTypeFixedResponse:
 return t.buildFixedResponseAction(ctx, actionCfg)
@@ -45,7 +44,7 @@ func (t *defaultModelBuildTask) buildBackendAction(ctx context.Context, ing *net
 return elbv2model.Action{}, errors.Errorf("unknown action type: %v", actionCfg.Type)
 }
 
-func (t *defaultModelBuildTask) buildAuthAction(ctx context.Context, ing *networking.Ingress, backend EnhancedBackend) (*elbv2model.Action, error) {
+func (t *defaultModelBuildTask) buildAuthAction(ctx context.Context, namespace string, backend EnhancedBackend) (*elbv2model.Action, error) {
 authCfg := backend.AuthConfig
 switch authCfg.Type {
 case AuthTypeCognito:
@@ -55,7 +54,7 @@ func (t *defaultModelBuildTask) buildAuthAction(ctx context.Context, ing *networ
 }
 return &action, nil
 case AuthTypeOIDC:
-action, err := t.buildAuthenticateOIDCAction(ctx, authCfg, ing.Namespace)
+action, err := t.buildAuthenticateOIDCAction(ctx, namespace, authCfg)
 if err != nil {
 return nil, err
 }
@@ -96,7 +95,7 @@ func (t *defaultModelBuildTask) buildRedirectAction(_ context.Context, actionCfg
 }, nil
 }
 
-func (t *defaultModelBuildTask) buildForwardAction(ctx context.Context, ing *networking.Ingress, actionCfg Action) (elbv2model.Action, error) {
+func (t *defaultModelBuildTask) buildForwardAction(ctx context.Context, ing ClassifiedIngress, actionCfg Action) (elbv2model.Action, error) {
 if actionCfg.ForwardConfig == nil {
 return elbv2model.Action{}, errors.New("missing ForwardConfig")
 }
@@ -108,7 +107,7 @@ func (t *defaultModelBuildTask) buildForwardAction(ctx context.Context, ing *net
 tgARN = core.LiteralStringToken(*tgt.TargetGroupARN)
 } else {
 svcKey := types.NamespacedName{
-Namespace: ing.Namespace,
+Namespace: ing.Ing.Namespace,
 Name: awssdk.StringValue(tgt.ServiceName),
 }
 svc := t.backendServices[svcKey]
@@ -160,7 +159,7 @@ func (t *defaultModelBuildTask) buildAuthenticateCognitoAction(_ context.Context
 }, nil
 }
 
-func (t *defaultModelBuildTask) buildAuthenticateOIDCAction(ctx context.Context, authCfg AuthConfig, namespace string) (elbv2model.Action, error) {
+func (t *defaultModelBuildTask) buildAuthenticateOIDCAction(ctx context.Context, namespace string, authCfg AuthConfig) (elbv2model.Action, error) {
 if authCfg.IDPConfigOIDC == nil {
 return elbv2model.Action{}, errors.New("missing IDPConfigOIDC")
 }

config/crd/bases/elbv2.k8s.aws_targetgroupbindings.yaml

@@ -238,7 +238,7 @@ func Test_defaultModelBuildTask_buildAuthenticateOIDCAction(t *testing.T) {
 task := &defaultModelBuildTask{
 k8sClient: k8sClient,
 }
-got, err := task.buildAuthenticateOIDCAction(context.Background(), tt.args.authCfg, tt.args.namespace)
+got, err := task.buildAuthenticateOIDCAction(context.Background(), tt.args.namespace, tt.args.authCfg)
 if tt.wantErr != nil {
 assert.EqualError(t, err, tt.wantErr.Error())
 } else {

pkg/ingress/model_build_actions.go

@@ -19,7 +19,7 @@ import (
 elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 )
 
-func (t *defaultModelBuildTask) buildListener(ctx context.Context, lbARN core.StringToken, port int64, config listenPortConfig, ingList []*networking.Ingress) (*elbv2model.Listener, error) {
+func (t *defaultModelBuildTask) buildListener(ctx context.Context, lbARN core.StringToken, port int64, config listenPortConfig, ingList []ClassifiedIngress) (*elbv2model.Listener, error) {
 lsSpec, err := t.buildListenerSpec(ctx, lbARN, port, config, ingList)
 if err != nil {
 return nil, err
@@ -29,12 +29,12 @@ func (t *defaultModelBuildTask) buildListener(ctx context.Context, lbARN core.St
 return ls, nil
 }
 
-func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, lbARN core.StringToken, port int64, config listenPortConfig, ingList []*networking.Ingress) (elbv2model.ListenerSpec, error) {
+func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, lbARN core.StringToken, port int64, config listenPortConfig, ingList []ClassifiedIngress) (elbv2model.ListenerSpec, error) {
 defaultActions, err := t.buildListenerDefaultActions(ctx, config.protocol, ingList)
 if err != nil {
 return elbv2model.ListenerSpec{}, err
 }
-tags, err := t.modelBuildListenerTags(ctx, ingList)
+tags, err := t.buildListenerTags(ctx, ingList)
 if err != nil {
 return elbv2model.ListenerSpec{}, err
 }
@@ -55,14 +55,14 @@ func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, lbARN cor
 }, nil
 }
 
-func (t *defaultModelBuildTask) buildListenerDefaultActions(ctx context.Context, protocol elbv2model.Protocol, ingList []*networking.Ingress) ([]elbv2model.Action, error) {
+func (t *defaultModelBuildTask) buildListenerDefaultActions(ctx context.Context, protocol elbv2model.Protocol, ingList []ClassifiedIngress) ([]elbv2model.Action, error) {
 if t.sslRedirectConfig != nil && protocol == elbv2model.ProtocolHTTP {
 return []elbv2model.Action{t.buildSSLRedirectAction(ctx, *t.sslRedirectConfig)}, nil
 }
 
-ingsWithDefaultBackend := make([]*networking.Ingress, 0, len(ingList))
+ingsWithDefaultBackend := make([]ClassifiedIngress, 0, len(ingList))
 for _, ing := range ingList {
-if ing.Spec.Backend != nil {
+if ing.Ing.Spec.Backend != nil {
 ingsWithDefaultBackend = append(ingsWithDefaultBackend, ing)
 }
 }
@@ -73,12 +73,12 @@ func (t *defaultModelBuildTask) buildListenerDefaultActions(ctx context.Context,
 if len(ingsWithDefaultBackend) > 1 {
 ingKeys := make([]types.NamespacedName, 0, len(ingsWithDefaultBackend))
 for _, ing := range ingsWithDefaultBackend {
-ingKeys = append(ingKeys, k8s.NamespacedName(ing))
+ingKeys = append(ingKeys, k8s.NamespacedName(ing.Ing))
 }
 return nil, errors.Errorf("multiple ingress defined default backend: %v", ingKeys)
 }
 ing := ingsWithDefaultBackend[0]
-enhancedBackend, err := t.enhancedBackendBuilder.Build(ctx, ing, *ing.Spec.Backend,
+enhancedBackend, err := t.enhancedBackendBuilder.Build(ctx, ing.Ing, *ing.Ing.Spec.Backend,
 WithLoadBackendServices(true, t.backendServices),
 WithLoadAuthConfig(true))
 if err != nil {
@@ -87,6 +87,14 @@ func (t *defaultModelBuildTask) buildListenerDefaultActions(ctx context.Context,
 return t.buildActions(ctx, protocol, ing, enhancedBackend)
 }
 
+func (t *defaultModelBuildTask) buildListenerTags(_ context.Context, ingList []ClassifiedIngress) (map[string]string, error) {
+ingGroupTags, err := t.buildIngressGroupResourceTags(ingList)
+if err != nil {
+return nil, err
+}
+return algorithm.MergeStringMap(t.defaultTags, ingGroupTags), nil
+}
+
 // the listen port config for specific listener port.
 type listenPortConfig struct {
 protocol elbv2model.Protocol
@@ -227,26 +235,3 @@ func (t *defaultModelBuildTask) computeIngressExplicitSSLPolicy(_ context.Contex
 }
 return &rawSSLPolicy
 }
-
-func (t *defaultModelBuildTask) modelBuildListenerTags(_ context.Context, ingList []*networking.Ingress) (map[string]string, error) {
-annotationTags := make(map[string]string)
-for _, ing := range ingList {
-var rawTags map[string]string
-if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixTags, &rawTags, ing.Annotations); err != nil {
-return nil, err
-}
-for tagKey, tagValue := range rawTags {
-if t.externalManagedTags.Has(tagKey) {
-return nil, errors.Errorf("external managed tag key %v cannot be specified on Ingress %v",
-tagKey, k8s.NamespacedName(ing).String())
-}
-
-if existingTagValue, exists := annotationTags[tagKey]; exists && existingTagValue != tagValue {
-return nil, errors.Errorf("conflicting tag %v: %v | %v", tagKey, existingTagValue, tagValue)
-}
-annotationTags[tagKey] = tagValue
-}
-}
-mergedTags := algorithm.MergeStringMap(t.defaultTags, annotationTags)
-return mergedTags, nil
-}

pkg/ingress/model_build_actions_test.go

@@ -8,41 +8,40 @@ import (
 "github.com/pkg/errors"
 networking "k8s.io/api/networking/v1beta1"
 "sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm"
-"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
 "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
 "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
 elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 )
 
-func (t *defaultModelBuildTask) buildListenerRules(ctx context.Context, lsARN core.StringToken, port int64, protocol elbv2model.Protocol, ingList []*networking.Ingress) error {
+func (t *defaultModelBuildTask) buildListenerRules(ctx context.Context, lsARN core.StringToken, port int64, protocol elbv2model.Protocol, ingList []ClassifiedIngress) error {
 if t.sslRedirectConfig != nil && protocol == elbv2model.ProtocolHTTP {
 return nil
 }
 
 var rules []Rule
 for _, ing := range ingList {
-for _, rule := range ing.Spec.Rules {
+for _, rule := range ing.Ing.Spec.Rules {
 if rule.HTTP == nil {
 continue
 }
 for _, path := range rule.HTTP.Paths {
-enhancedBackend, err := t.enhancedBackendBuilder.Build(ctx, ing, path.Backend,
+enhancedBackend, err := t.enhancedBackendBuilder.Build(ctx, ing.Ing, path.Backend,
 WithLoadBackendServices(true, t.backendServices),
 WithLoadAuthConfig(true))
 if err != nil {
-return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing))
+return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
 }
 conditions, err := t.buildRuleConditions(ctx, rule, path, enhancedBackend)
 if err != nil {
-return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing))
+return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
 }
 actions, err := t.buildActions(ctx, protocol, ing, enhancedBackend)
 if err != nil {
-return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing))
+return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
 }
-tags, err := t.modelBuildListenerRuleTags(ctx, ing)
+tags, err := t.buildListenerRuleTags(ctx, ing)
 if err != nil {
-return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing))
+return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
 }
 rules = append(rules, Rule{
 Conditions: conditions,
@@ -261,17 +260,11 @@ func (t *defaultModelBuildTask) buildPathPatternCondition(_ context.Context, pat
 }
 }
 
-func (t *defaultModelBuildTask) modelBuildListenerRuleTags(_ context.Context, ing *networking.Ingress) (map[string]string, error) {
-var rawTags map[string]string
-if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixTags, &rawTags, ing.Annotations); err != nil {
+func (t *defaultModelBuildTask) buildListenerRuleTags(_ context.Context, ing ClassifiedIngress) (map[string]string, error) {
+ingTags, err := t.buildIngressResourceTags(ing)
+if err != nil {
 return nil, err
 }
-for tagKey := range rawTags {
-if t.externalManagedTags.Has(tagKey) {
-return nil, errors.Errorf("external managed tag key %v cannot be specified", tagKey)
-}
-}
 
-mergedTags := algorithm.MergeStringMap(t.defaultTags, rawTags)
-return mergedTags, nil
+return algorithm.MergeStringMap(t.defaultTags, ingTags), nil
 }

pkg/ingress/model_build_listener.go

@@ -302,26 +302,11 @@ func (t *defaultModelBuildTask) buildLoadBalancerAttributes(_ context.Context) (
 }
 
 func (t *defaultModelBuildTask) buildLoadBalancerTags(_ context.Context) (map[string]string, error) {
-annotationTags := make(map[string]string)
-for _, member := range t.ingGroup.Members {
-var rawTags map[string]string
-if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixTags, &rawTags, member.Ing.Annotations); err != nil {
-return nil, err
-}
-for tagKey, tagValue := range rawTags {
-if t.externalManagedTags.Has(tagKey) {
-return nil, errors.Errorf("external managed tag key %v cannot be specified on Ingress %v",
-tagKey, k8s.NamespacedName(member.Ing).String())
-}
-
-if existingTagValue, exists := annotationTags[tagKey]; exists && existingTagValue != tagValue {
-return nil, errors.Errorf("conflicting tag %v: %v | %v", tagKey, existingTagValue, tagValue)
-}
-annotationTags[tagKey] = tagValue
-}
+ingGroupTags, err := t.buildIngressGroupResourceTags(t.ingGroup.Members)
+if err != nil {
+return nil, err
 }
-mergedTags := algorithm.MergeStringMap(t.defaultTags, annotationTags)
-return mergedTags, nil
+return algorithm.MergeStringMap(t.defaultTags, ingGroupTags), nil
 }
 
 func (t *defaultModelBuildTask) resolveSecurityGroupIDsViaNameOrIDSlice(ctx context.Context, sgNameOrIDs []string) ([]string, error) {

pkg/ingress/model_build_listener_rules.go

@@ -458,7 +458,7 @@ func Test_defaultModelBuildTask_buildLoadBalancerTags(t *testing.T) {
 },
 externalManagedTags: sets.NewString("k2"),
 },
-wantErr: errors.New("external managed tag key k2 cannot be specified on Ingress awesome-ns/ing-2"),
+wantErr: errors.New("failed build tags for Ingress awesome-ns/ing-2: external managed tag key k2 cannot be specified"),
 },
 }
 for _, tt := range tests {