Merge rules per ingress by the same host, pathType and backend service

pkg/algorithm/integers.go

@@ -0,0 +1,9 @@
+package algorithm
+
+// GetMin returns the smaller of x or y
+func GetMin(x, y int) int {
+	if x < y {
+		return x
+	}
+	return y
+}

pkg/ingress/model_build_listener_rules.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -14,46 +15,33 @@ import (
 	elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 )
 
+const maxConditionValuesCount = int(5)
+
 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.Ing.Spec.Rules {
-			if rule.HTTP == nil {
-				continue
-			}
-			paths, err := t.sortIngressPaths(rule.HTTP.Paths)
+		rulesWithReplicateHosts, rulesWithUniqueHost, err := t.classifyRulesByHost(ing.Ing.Spec.Rules)
+		if err != nil {
+			return err
+		}
+		if len(rulesWithReplicateHosts) != 0 {
+			mergedRules := t.mergePathsAcrossRules(rulesWithReplicateHosts)
+			builtMergedRules, err := t.buildMergedListenerRules(ctx, protocol, ing, mergedRules)
 			if err != nil {
 				return err
 			}
-			for _, path := range paths {
-				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.Ing))
-				}
-				conditions, err := t.buildRuleConditions(ctx, rule, path, enhancedBackend)
-				if err != nil {
-					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.Ing))
-				}
-				tags, err := t.buildListenerRuleTags(ctx, ing)
-				if err != nil {
-					return errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
-				}
-				rules = append(rules, Rule{
-					Conditions: conditions,
-					Actions:    actions,
-					Tags:       tags,
-				})
+			rules = append(rules, builtMergedRules...)
+		}
+		if len(rulesWithUniqueHost) != 0 {
+			builtUnmergedRules, err := t.buildUnmergedListenerRules(ctx, protocol, ing, rulesWithUniqueHost)
+			if err != nil {
+				return err
 			}
+			rules = append(rules, builtUnmergedRules...)
 		}
 	}
 	optimizedRules, err := t.ruleOptimizer.Optimize(ctx, port, protocol, rules)
@@ -73,10 +61,92 @@ func (t *defaultModelBuildTask) buildListenerRules(ctx context.Context, lsARN co
 		})
 		priority += 1
 	}
-
 	return nil
 }
 
+// classifyRulesByHost classifies rules based on whether the rule has a unique host
+// only the rules with replicate hosts need merge
+func (t *defaultModelBuildTask) classifyRulesByHost(rules []networking.IngressRule) ([]networking.IngressRule, []networking.IngressRule, error) {
+	var rulesWithReplicateHosts []networking.IngressRule
+	var rulesWithUniqueHost []networking.IngressRule
+	hostToRulesMap := make(map[string][]networking.IngressRule)
+	for _, rule := range rules {
+		host := rule.Host
+		_, exists := hostToRulesMap[host]
+		if exists {
+			hostToRulesMap[host] = append(hostToRulesMap[host], rule)
+		} else {
+			hostToRulesMap[host] = []networking.IngressRule{rule}
+		}
+	}
+	for host, rules := range hostToRulesMap {
+		if len(rules) == 1 {
+			rulesWithUniqueHost = append(rulesWithUniqueHost, rules...)
+		} else if len(rules) > 1 {
+			rulesWithReplicateHosts = append(rulesWithReplicateHosts, rules...)
+		} else {
+			return nil, nil, errors.Errorf("no rules for Host %s", host)
+		}
+	}
+	return rulesWithReplicateHosts, rulesWithUniqueHost, nil
+}
+
+// mergePathsAcrossRules generates new rules with paths merged
+func (t *defaultModelBuildTask) mergePathsAcrossRules(rules []networking.IngressRule) []networking.IngressRule {
+	// iterate the mergeRefMap and append all paths in a group to the rule of the min ruleIdx
+	mergePathsRefMap := t.getMergeRuleRefMaps(rules)
+	var mergedRule networking.IngressRule
+	var mergedRules []networking.IngressRule
+	for k, paths := range mergePathsRefMap {
+		mergedRule = networking.IngressRule{
+			Host: k[0],
+			IngressRuleValue: networking.IngressRuleValue{
+				HTTP: &networking.HTTPIngressRuleValue{
+					Paths: paths,
+				},
+			},
+		}
+		mergedRules = append(mergedRules, mergedRule)
+	}
+	return mergedRules
+}
+
+// getMergeRuleRefMaps gets a map to help merge paths of rules by host, pathType and backend
+// e.g. {(hostA, pathTypeA, serviceNameA, PortNameA, PortNumberA): [path1, path2,...],
+//
+//		 (hostB, pathTypeB, serviceNameB, PortNameB, PortNumberB): [path3, path4,...],
+//	 	...}
+func (t *defaultModelBuildTask) getMergeRuleRefMaps(rules []networking.IngressRule) map[[5]string][]networking.HTTPIngressPath {
+	mergePathsRefMap := make(map[[5]string][]networking.HTTPIngressPath)
+	//// pathToRuleMap stores {path: ruleIdx} relationship
+	//pathToRuleMap := make(map[networking.HTTPIngressPath]int)
+
+	for _, rule := range rules {
+		if rule.HTTP == nil {
+			continue
+		}
+		host := rule.Host
+		for _, path := range rule.HTTP.Paths {
+			//pathToRuleMap[path] = idx
+			pathType := ""
+			if path.PathType != nil {
+				pathType = string(*path.PathType)
+			}
+			serviceName := path.Backend.Service.Name
+			portName := path.Backend.Service.Port.Name
+			portNumber := strconv.Itoa(int(path.Backend.Service.Port.Number))
+			_, exist := mergePathsRefMap[[5]string{host, pathType, serviceName, portName, portNumber}]
+			if !exist {
+				mergePathsRefMap[[5]string{host, pathType, serviceName, portName, portNumber}] = []networking.HTTPIngressPath{path}
+			} else {
+				mergePathsRefMap[[5]string{host, pathType, serviceName, portName, portNumber}] = append(mergePathsRefMap[[5]string{host, pathType, serviceName, portName, portNumber}],
+					path)
+			}
+		}
+	}
+	return mergePathsRefMap
+}
+
 // sortIngressPaths will sort the paths following the strategy:
 // all exact match paths come first, no need to sort since exact match has to be unique
 // followed by prefix paths, sort by lengths - longer paths get precedence
@@ -119,20 +189,116 @@ func (t *defaultModelBuildTask) classifyIngressPathsByType(paths []networking.HT
 	return exactPaths, prefixPaths, implementationSpecificPaths, nil
 }
 
-func (t *defaultModelBuildTask) buildRuleConditions(ctx context.Context, rule networking.IngressRule,
-	path networking.HTTPIngressPath, backend EnhancedBackend) ([]elbv2model.RuleCondition, error) {
-	var hosts []string
-	if rule.Host != "" {
-		hosts = append(hosts, rule.Host)
+func (t *defaultModelBuildTask) buildMergedListenerRules(ctx context.Context, protocol elbv2model.Protocol, ing ClassifiedIngress, rulesToMerge []networking.IngressRule) ([]Rule, error) {
+	var rules []Rule
+	for _, ruleToMerge := range rulesToMerge {
+		if ruleToMerge.HTTP == nil {
+			continue
+		}
+		host := ruleToMerge.Host
+		// all the paths in one mergedRule should have the same backend and pathType
+		mergedPaths := ruleToMerge.HTTP.Paths
+		if len(mergedPaths) == 0 {
+			continue
+		}
+		currPathType := networking.PathTypeImplementationSpecific
+		if mergedPaths[0].PathType != nil {
+			currPathType = *mergedPaths[0].PathType
+		}
+		currBackend := ruleToMerge.HTTP.Paths[0].Backend
+		var paths []string
+		for _, path := range mergedPaths {
+			if path.Path != "" {
+				paths = append(paths, path.Path)
+			}
+		}
+		enhancedBackend, err := t.enhancedBackendBuilder.Build(ctx, ing.Ing, currBackend,
+			WithLoadBackendServices(true, t.backendServices),
+			WithLoadAuthConfig(true))
+		conditions, err := t.buildRuleConditions(ctx, host, paths, enhancedBackend, currPathType)
+		if err != nil {
+			return nil, errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
+		}
+		if err != nil {
+			return nil, errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
+		}
+		actions, err := t.buildActions(ctx, protocol, ing, enhancedBackend)
+		if err != nil {
+			return nil, errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
+		}
+		tags, err := t.buildListenerRuleTags(ctx, ing)
+		if err != nil {
+			return nil, errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
+		}
+		for _, condition := range conditions {
+			rules = append(rules, Rule{
+				Conditions: condition,
+				Actions:    actions,
+				Tags:       tags,
+			})
+		}
 	}
-	var paths []string
-	if path.Path != "" {
-		pathPatterns, err := t.buildPathPatterns(path.Path, path.PathType)
+	return rules, nil
+}
+
+func (t *defaultModelBuildTask) buildUnmergedListenerRules(ctx context.Context, protocol elbv2model.Protocol, ing ClassifiedIngress, rulesToBuild []networking.IngressRule) ([]Rule, error) {
+	var rules []Rule
+	for _, ruleToBuild := range rulesToBuild {
+		if ruleToBuild.HTTP == nil {
+			continue
+		}
+		paths, err := t.sortIngressPaths(ruleToBuild.HTTP.Paths)
 		if err != nil {
 			return nil, err
 		}
-		paths = append(paths, pathPatterns...)
+		for _, path := range paths {
+			enhancedBackend, err := t.enhancedBackendBuilder.Build(ctx, ing.Ing, path.Backend,
+				WithLoadBackendServices(true, t.backendServices),
+				WithLoadAuthConfig(true))
+			if err != nil {
+				return nil, errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
+			}
+			pathType := networking.PathTypeImplementationSpecific
+			if path.PathType != nil {
+				pathType = *path.PathType
+			}
+			conditions, err := t.buildRuleConditions(ctx, ruleToBuild.Host, []string{path.Path}, enhancedBackend, pathType)
+			if err != nil {
+				return nil, errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
+			}
+			actions, err := t.buildActions(ctx, protocol, ing, enhancedBackend)
+			if err != nil {
+				return nil, errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
+			}
+			tags, err := t.buildListenerRuleTags(ctx, ing)
+			if err != nil {
+				return nil, errors.Wrapf(err, "ingress: %v", k8s.NamespacedName(ing.Ing))
+			}
+			for _, condition := range conditions {
+				rules = append(rules, Rule{
+					Conditions: condition,
+					Actions:    actions,
+					Tags:       tags,
+				})
+			}
+
+		}
 	}
+	return rules, nil
+}
+
+func (t *defaultModelBuildTask) buildRuleConditions(ctx context.Context, host string,
+	pathsToBuild []string, backend EnhancedBackend, pathType networking.PathType) ([][]elbv2model.RuleCondition, error) {
+	var hosts []string
+	if host != "" {
+		hosts = append(hosts, host)
+	}
+	var paths []string
+	pathPatterns, err := t.buildPathPatterns(pathsToBuild, &pathType)
+	if err != nil {
+		return nil, err
+	}
+	paths = append(paths, pathPatterns...)
 	var conditions []elbv2model.RuleCondition
 	for _, condition := range backend.Conditions {
 		switch condition.Field {
@@ -175,28 +341,61 @@ func (t *defaultModelBuildTask) buildRuleConditions(ctx context.Context, rule ne
 	if len(hosts) != 0 {
 		conditions = append(conditions, t.buildHostHeaderCondition(ctx, hosts))
 	}
-	if len(paths) != 0 {
-		conditions = append(conditions, t.buildPathPatternCondition(ctx, paths))
+	if len(conditions) > 5 {
+		return nil, errors.New("A rule can only have at most 5 condition values.")
 	}
-	if len(conditions) == 0 {
-		conditions = append(conditions, t.buildPathPatternCondition(ctx, []string{"/*"}))
+	// flow over the path values to new conditions if the total count of values exceeds 5
+	var builtConditions [][]elbv2model.RuleCondition
+	if len(paths) == 0 {
+		if len(conditions) == 0 {
+			conditions = append(conditions, t.buildPathPatternCondition(ctx, []string{"/*"}))
+		}
+		builtConditions = append(builtConditions, conditions)
+	} else {
+		step := maxConditionValuesCount - len(conditions)
+		for i := 0; i < len(paths); i += step {
+			end := algorithm.GetMin(i+step, len(paths))
+			conditionsWithPathPattern := append(conditions, t.buildPathPatternCondition(ctx, paths[i:end]))
+			builtConditions = append(builtConditions, conditionsWithPathPattern)
+		}
 	}
-	return conditions, nil
+	return builtConditions, nil
 }
 
-// buildPathPatterns will build ELBv2's path patterns for given path and pathType.
-func (t *defaultModelBuildTask) buildPathPatterns(path string, pathType *networking.PathType) ([]string, error) {
+func (t *defaultModelBuildTask) buildPathPatterns(paths []string, pathType *networking.PathType) ([]string, error) {
 	normalizedPathType := networking.PathTypeImplementationSpecific
 	if pathType != nil {
 		normalizedPathType = *pathType
 	}
+	var builtPaths []string
 	switch normalizedPathType {
 	case networking.PathTypeImplementationSpecific:
-		return t.buildPathPatternsForImplementationSpecificPathType(path)
+		for _, path := range paths {
+			builtPath, err := t.buildPathPatternsForImplementationSpecificPathType(path)
+			if err != nil {
+				return nil, err
+			}
+			builtPaths = append(builtPaths, builtPath...)
+		}
+		return builtPaths, nil
 	case networking.PathTypeExact:
-		return t.buildPathPatternsForExactPathType(path)
+		for _, path := range paths {
+			builtPath, err := t.buildPathPatternsForExactPathType(path)
+			if err != nil {
+				return nil, err
+			}
+			builtPaths = append(builtPaths, builtPath...)
+		}
+		return builtPaths, nil
 	case networking.PathTypePrefix:
-		return t.buildPathPatternsForPrefixPathType(path)
+		for _, path := range paths {
+			builtPath, err := t.buildPathPatternsForPrefixPathType(path)
+			if err != nil {
+				return nil, err
+			}
+			builtPaths = append(builtPaths, builtPath...)
+		}
+		return builtPaths, nil
 	default:
 		return nil, errors.Errorf("unsupported pathType: %v", normalizedPathType)
 	}
@@ -208,7 +407,7 @@ func (t *defaultModelBuildTask) buildPathPatternsForImplementationSpecificPathTy
 }
 
 // buildPathPatternsForExactPathType will build path patterns for exact pathType.
-// exact path shouldn't contains any wildcards.
+// exact path shouldn't contain any wildcards.
 func (t *defaultModelBuildTask) buildPathPatternsForExactPathType(path string) ([]string, error) {
 	if strings.ContainsAny(path, "*?") {
 		return nil, errors.Errorf("exact path shouldn't contain wildcards: %v", path)
@@ -217,8 +416,8 @@ func (t *defaultModelBuildTask) buildPathPatternsForExactPathType(path string) (
 }
 
 // buildPathPatternsForPrefixPathType will build path patterns for prefix pathType.
-// prefix path shouldn't contains any wildcards.
-// with prefixType type, both "/foo" or "/foo/" should matches path like "/foo" or "/foo/" or "/foo/bar".
+// prefix path shouldn't contain any wildcards.
+// with prefixType type, both "/foo" or "/foo/" should match path like "/foo" or "/foo/" or "/foo/bar".
 // for above case, we'll generate two path pattern: "/foo/" and "/foo/*".
 // an special case is "/", which matches all paths, thus we generate the path pattern as "/*"
 func (t *defaultModelBuildTask) buildPathPatternsForPrefixPathType(path string) ([]string, error) {