Merge pull request #1286 from M00nF1sh/fix_1274

bug fix for #1274

Author: k8s-ci-robot

internal/alb/ls/rules.go

@@ -154,7 +154,7 @@ func (c *rulesController) getDesiredRules(ctx context.Context, listener *elbv2.L
 			}
 			if createsRedirectLoop(listener, elbRule) {
 				continue
-			} else if isUnconditionalRedirect(listener, elbRule) {
+			} else if isUnconditionalRedirect(listener, elbRule, ingressRule.Host) {
 				seenUnconditionalRedirect = true
 			}
 			output = append(output, elbRule)
@@ -543,25 +543,34 @@ func createsRedirectLoop(listener *elbv2.Listener, r elbv2.Rule) bool {
 // isUnconditionalRedirect checks whether specified rule always redirects
 // We consider the rule is a unconditional redirect if
 // 1) The Path condition is nil, or at least one Path condition is /*
-// 2) All other rule conditions are nil (ignoring the Host condition).
-// 3) RedirectConfig is not nil.
-func isUnconditionalRedirect(listener *elbv2.Listener, r elbv2.Rule) bool {
+// 2) The Host condition don't contain any other element than host passed-in
+// 3) All other rule conditions are nil.
+// 4) RedirectConfig is not nil.
+func isUnconditionalRedirect(listener *elbv2.Listener, r elbv2.Rule, ruleHost string) bool {
 	for _, action := range r.Actions {
 		rc := action.RedirectConfig
 		if rc == nil {
 			continue
 		}
 
+		var hosts []string
 		var paths []string
 		for _, c := range r.Conditions {
 			switch aws.StringValue(c.Field) {
 			case conditions.FieldPathPattern:
 				paths = append(paths, aws.StringValueSlice(c.PathPatternConfig.Values)...)
+			case conditions.FieldHostHeader:
+				hosts = append(hosts, aws.StringValueSlice(c.HostHeaderConfig.Values)...)
 			case conditions.FieldHTTPRequestMethod, conditions.FieldSourceIP, conditions.FieldHTTPHeader, conditions.FieldQueryString:
 				// If there are any conditions, then the redirect is not unconditional
 				return false
 			}
 		}
+		for _, host := range hosts {
+			if host != ruleHost {
+				return false
+			}
+		}
 
 		if len(paths) != 0 {
 			// ALB path conditions are ORed, so if any of them are a wildcard, the redirect is unconditional
@@ -573,7 +582,6 @@ func isUnconditionalRedirect(listener *elbv2.Listener, r elbv2.Rule) bool {
 			// The redirect isn't unconditional if none of the path conditions are a wildcard
 			return false
 		}
-
 		return true
 	}
 	return false

internal/alb/ls/rules_test.go

@@ -2883,6 +2883,7 @@ func Test_isUnconditionalRedirect(t *testing.T) {
 		name     string
 		listener elbv2.Listener
 		rule     elbv2.Rule
+		ruleHost string
 
 		expected bool
 	}{
@@ -2920,11 +2921,12 @@ func Test_isUnconditionalRedirect(t *testing.T) {
 					{
 						Field: aws.String(conditions.FieldHostHeader),
 						HostHeaderConfig: &elbv2.HostHeaderConditionConfig{
-							Values: aws.StringSlice([]string{"www.example.com", "anno.example.com"}),
+							Values: aws.StringSlice([]string{"www.example.com"}),
 						},
 					},
 				},
 			},
+			ruleHost: "www.example.com",
 			expected: true,
 		},
 		{
@@ -3012,7 +3014,7 @@ func Test_isUnconditionalRedirect(t *testing.T) {
 			expected: false,
 		},
 		{
-			name:     "Path condition set to /* and Host condition is set ",
+			name:     "Path condition set to /* and Host condition is set to same as ruleHost",
 			listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
 			rule: elbv2.Rule{
 				Actions: []*elbv2.Action{
@@ -3031,13 +3033,42 @@ func Test_isUnconditionalRedirect(t *testing.T) {
 					{
 						Field: aws.String(conditions.FieldHostHeader),
 						HostHeaderConfig: &elbv2.HostHeaderConditionConfig{
-							Values: aws.StringSlice([]string{"www.example.com", "anno.example.com"}),
+							Values: aws.StringSlice([]string{"www.example.com"}),
 						},
 					},
 				},
 			},
+			ruleHost: "www.example.com",
 			expected: true,
 		},
+		{
+			name:     "Path condition set to /* and Host condition is set to more than ruleHost",
+			listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
+			rule: elbv2.Rule{
+				Actions: []*elbv2.Action{
+					{
+						Type:           aws.String(elbv2.ActionTypeEnumRedirect),
+						RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
+					},
+				},
+				Conditions: []*elbv2.RuleCondition{
+					{
+						Field: aws.String(conditions.FieldPathPattern),
+						PathPatternConfig: &elbv2.PathPatternConditionConfig{
+							Values: aws.StringSlice([]string{"/*"}),
+						},
+					},
+					{
+						Field: aws.String(conditions.FieldHostHeader),
+						HostHeaderConfig: &elbv2.HostHeaderConditionConfig{
+							Values: aws.StringSlice([]string{"www.example.com", "annos.example.com"}),
+						},
+					},
+				},
+			},
+			ruleHost: "www.example.com",
+			expected: false,
+		},
 		{
 			name:     "Path condition set to /* but a SourceIP condition is also set",
 			listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
@@ -3067,7 +3098,7 @@ func Test_isUnconditionalRedirect(t *testing.T) {
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			assert.Equal(t, tc.expected, isUnconditionalRedirect(&tc.listener, tc.rule))
+			assert.Equal(t, tc.expected, isUnconditionalRedirect(&tc.listener, tc.rule, tc.ruleHost))
 		})
 	}
 }