Skip to content

Commit 0a176e4

Browse files
committed
Avoid creating unneeded ALB rules
Detect unconditional redirects and ignore any rules defined afterwards
1 parent 57484cb commit 0a176e4

File tree

2 files changed

+241
-0
lines changed

2 files changed

+241
-0
lines changed

internal/alb/ls/rules.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,13 @@ func (c *rulesController) getDesiredRules(ctx context.Context, listener *elbv2.L
128128
continue
129129
}
130130

131+
seenUnconditionalRedirect := false
132+
131133
for _, path := range ingressRule.HTTP.Paths {
134+
if seenUnconditionalRedirect {
135+
// Ignore rules that follow a unconditional redirect, they are moot
136+
continue
137+
}
132138
authCfg, err := c.authModule.NewConfig(ctx, ingress, path.Backend, aws.StringValue(listener.Protocol))
133139
if err != nil {
134140
return nil, err
@@ -146,6 +152,8 @@ func (c *rulesController) getDesiredRules(ctx context.Context, listener *elbv2.L
146152
}
147153
if createsRedirectLoop(listener, elbRule) {
148154
continue
155+
} else if isUnconditionalRedirect(listener, elbRule) {
156+
seenUnconditionalRedirect = true
149157
}
150158
output = append(output, elbRule)
151159
nextPriority++
@@ -529,3 +537,42 @@ func createsRedirectLoop(listener *elbv2.Listener, r elbv2.Rule) bool {
529537
}
530538
return false
531539
}
540+
541+
// isUnconditionalRedirect checks whether specified rule always redirects
542+
// We consider the rule is a unconditional redirect if
543+
// 1) The Path condition is nil, or at least one Path condition is /*
544+
// 2) All other rule conditions are nil (ignoring the Host condition).
545+
// 3) RedirectConfig is not nil.
546+
func isUnconditionalRedirect(listener *elbv2.Listener, r elbv2.Rule) bool {
547+
for _, action := range r.Actions {
548+
rc := action.RedirectConfig
549+
if rc == nil {
550+
continue
551+
}
552+
553+
var paths []string
554+
for _, c := range r.Conditions {
555+
switch aws.StringValue(c.Field) {
556+
case conditions.FieldPathPattern:
557+
paths = append(paths, aws.StringValueSlice(c.PathPatternConfig.Values)...)
558+
case conditions.FieldHTTPRequestMethod, conditions.FieldSourceIP, conditions.FieldHTTPHeader, conditions.FieldQueryString:
559+
// If there are any conditions, then the redirect is not unconditional
560+
return false
561+
}
562+
}
563+
564+
if len(paths) != 0 {
565+
// ALB path conditions are ORed, so if any of them are a wildcard, the redirect is unconditional
566+
for _, path := range paths {
567+
if path == "/*" {
568+
return true
569+
}
570+
}
571+
// The redirect isn't unconditional if none of the path conditions are a wildcard
572+
return false
573+
}
574+
575+
return true
576+
}
577+
return false
578+
}

internal/alb/ls/rules_test.go

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2878,6 +2878,200 @@ func Test_createsRedirectLoop(t *testing.T) {
28782878
}
28792879
}
28802880

2881+
func Test_isUnconditionalRedirect(t *testing.T) {
2882+
for _, tc := range []struct {
2883+
name string
2884+
listener elbv2.Listener
2885+
rule elbv2.Rule
2886+
2887+
expected bool
2888+
}{
2889+
{
2890+
name: "No RedirectConfig",
2891+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
2892+
rule: elbv2.Rule{},
2893+
expected: false,
2894+
},
2895+
{
2896+
name: "No conditions",
2897+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
2898+
rule: elbv2.Rule{
2899+
Actions: []*elbv2.Action{
2900+
{
2901+
Type: aws.String(elbv2.ActionTypeEnumRedirect),
2902+
RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
2903+
},
2904+
},
2905+
Conditions: nil,
2906+
},
2907+
expected: true,
2908+
},
2909+
{
2910+
name: "No Path conditions",
2911+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
2912+
rule: elbv2.Rule{
2913+
Actions: []*elbv2.Action{
2914+
{
2915+
Type: aws.String(elbv2.ActionTypeEnumRedirect),
2916+
RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
2917+
},
2918+
},
2919+
Conditions: []*elbv2.RuleCondition{
2920+
{
2921+
Field: aws.String(conditions.FieldHostHeader),
2922+
HostHeaderConfig: &elbv2.HostHeaderConditionConfig{
2923+
Values: aws.StringSlice([]string{"www.example.com", "anno.example.com"}),
2924+
},
2925+
},
2926+
},
2927+
},
2928+
expected: true,
2929+
},
2930+
{
2931+
name: "Path condition set to /*",
2932+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
2933+
rule: elbv2.Rule{
2934+
Actions: []*elbv2.Action{
2935+
{
2936+
Type: aws.String(elbv2.ActionTypeEnumRedirect),
2937+
RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
2938+
},
2939+
},
2940+
Conditions: []*elbv2.RuleCondition{
2941+
{
2942+
Field: aws.String(conditions.FieldPathPattern),
2943+
PathPatternConfig: &elbv2.PathPatternConditionConfig{
2944+
Values: aws.StringSlice([]string{"/*"}),
2945+
},
2946+
},
2947+
},
2948+
},
2949+
expected: true,
2950+
},
2951+
{
2952+
name: "Multiple Path conditions, one of which is /*",
2953+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
2954+
rule: elbv2.Rule{
2955+
Actions: []*elbv2.Action{
2956+
{
2957+
Type: aws.String(elbv2.ActionTypeEnumRedirect),
2958+
RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
2959+
},
2960+
},
2961+
Conditions: []*elbv2.RuleCondition{
2962+
{
2963+
Field: aws.String(conditions.FieldPathPattern),
2964+
PathPatternConfig: &elbv2.PathPatternConditionConfig{
2965+
Values: aws.StringSlice([]string{"/*", "/test", "/annothertest"}),
2966+
},
2967+
},
2968+
},
2969+
},
2970+
expected: true,
2971+
},
2972+
{
2973+
name: "Multiple Path conditions, one of which is /*, different ordering",
2974+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
2975+
rule: elbv2.Rule{
2976+
Actions: []*elbv2.Action{
2977+
{
2978+
Type: aws.String(elbv2.ActionTypeEnumRedirect),
2979+
RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
2980+
},
2981+
},
2982+
Conditions: []*elbv2.RuleCondition{
2983+
{
2984+
Field: aws.String(conditions.FieldPathPattern),
2985+
PathPatternConfig: &elbv2.PathPatternConditionConfig{
2986+
Values: aws.StringSlice([]string{"/test", "/anothertest", "/*"}),
2987+
},
2988+
},
2989+
},
2990+
},
2991+
expected: true,
2992+
},
2993+
{
2994+
name: "Multiple Path conditions, none of which is /*",
2995+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
2996+
rule: elbv2.Rule{
2997+
Actions: []*elbv2.Action{
2998+
{
2999+
Type: aws.String(elbv2.ActionTypeEnumRedirect),
3000+
RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
3001+
},
3002+
},
3003+
Conditions: []*elbv2.RuleCondition{
3004+
{
3005+
Field: aws.String(conditions.FieldPathPattern),
3006+
PathPatternConfig: &elbv2.PathPatternConditionConfig{
3007+
Values: aws.StringSlice([]string{"/test", "/anothertest", "anothertest2"}),
3008+
},
3009+
},
3010+
},
3011+
},
3012+
expected: false,
3013+
},
3014+
{
3015+
name: "Path condition set to /* and Host condition is set ",
3016+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
3017+
rule: elbv2.Rule{
3018+
Actions: []*elbv2.Action{
3019+
{
3020+
Type: aws.String(elbv2.ActionTypeEnumRedirect),
3021+
RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
3022+
},
3023+
},
3024+
Conditions: []*elbv2.RuleCondition{
3025+
{
3026+
Field: aws.String(conditions.FieldPathPattern),
3027+
PathPatternConfig: &elbv2.PathPatternConditionConfig{
3028+
Values: aws.StringSlice([]string{"/*"}),
3029+
},
3030+
},
3031+
{
3032+
Field: aws.String(conditions.FieldHostHeader),
3033+
HostHeaderConfig: &elbv2.HostHeaderConditionConfig{
3034+
Values: aws.StringSlice([]string{"www.example.com", "anno.example.com"}),
3035+
},
3036+
},
3037+
},
3038+
},
3039+
expected: true,
3040+
},
3041+
{
3042+
name: "Path condition set to /* but a SourceIP condition is also set",
3043+
listener: elbv2.Listener{Protocol: aws.String("HTTP"), Port: aws.Int64(80)},
3044+
rule: elbv2.Rule{
3045+
Actions: []*elbv2.Action{
3046+
{
3047+
Type: aws.String(elbv2.ActionTypeEnumRedirect),
3048+
RedirectConfig: redirectActionConfig(&elbv2.RedirectActionConfig{Path: aws.String("/#{path}")}),
3049+
},
3050+
},
3051+
Conditions: []*elbv2.RuleCondition{
3052+
{
3053+
Field: aws.String(conditions.FieldPathPattern),
3054+
PathPatternConfig: &elbv2.PathPatternConditionConfig{
3055+
Values: aws.StringSlice([]string{"/*"}),
3056+
},
3057+
},
3058+
{
3059+
Field: aws.String(conditions.FieldSourceIP),
3060+
SourceIpConfig: &elbv2.SourceIpConditionConfig{
3061+
Values: aws.StringSlice([]string{"192.168.0.0/16"}),
3062+
},
3063+
},
3064+
},
3065+
},
3066+
expected: false,
3067+
},
3068+
} {
3069+
t.Run(tc.name, func(t *testing.T) {
3070+
assert.Equal(t, tc.expected, isUnconditionalRedirect(&tc.listener, tc.rule))
3071+
})
3072+
}
3073+
}
3074+
28813075
func redirectActionConfig(override *elbv2.RedirectActionConfig) *elbv2.RedirectActionConfig {
28823076
r := &elbv2.RedirectActionConfig{
28833077
Host: aws.String("#{host}"),

0 commit comments

Comments
 (0)