diff --git a/pkg/equality/elbv2/compare_option_for_rule_conditions.go b/pkg/equality/elbv2/compare_option_for_rule_conditions.go index f858cfefd2..871bda03f0 100644 --- a/pkg/equality/elbv2/compare_option_for_rule_conditions.go +++ b/pkg/equality/elbv2/compare_option_for_rule_conditions.go @@ -17,7 +17,7 @@ func CompareOptionForRuleCondition() cmp.Option { cmpopts.IgnoreUnexported(elbv2types.QueryStringConditionConfig{}), cmpopts.IgnoreUnexported(elbv2types.QueryStringKeyValuePair{}), cmpopts.IgnoreUnexported(elbv2types.SourceIpConditionConfig{}), - cmpopts.IgnoreFields(elbv2types.RuleCondition{}, "Values"), + cmpopts.IgnoreFields(elbv2types.RuleCondition{}, "Values", "RegexValues"), } } diff --git a/pkg/equality/elbv2/compare_option_for_rule_conditions_test.go b/pkg/equality/elbv2/compare_option_for_rule_conditions_test.go index 8210478319..361b4de926 100644 --- a/pkg/equality/elbv2/compare_option_for_rule_conditions_test.go +++ b/pkg/equality/elbv2/compare_option_for_rule_conditions_test.go @@ -79,6 +79,60 @@ func Test_CompareOptionForRuleConditions(t *testing.T) { }, expected: true, }, + { + name: "equal - regex path pattern with AWS-mirrored top-level RegexValues", + desiredRuleCondition: []types.RuleCondition{ + { + Field: awssdk.String("host-header"), + HostHeaderConfig: &types.HostHeaderConditionConfig{ + Values: []string{"example.com"}, + }, + }, + { + Field: awssdk.String("path-pattern"), + PathPatternConfig: &types.PathPatternConditionConfig{ + RegexValues: []string{"^/api/v1/.*"}, + }, + }, + }, + actualRuleCondition: []types.RuleCondition{ + { + Field: awssdk.String("host-header"), + HostHeaderConfig: &types.HostHeaderConditionConfig{ + Values: []string{"example.com"}, + }, + }, + { + Field: awssdk.String("path-pattern"), + PathPatternConfig: &types.PathPatternConditionConfig{ + RegexValues: []string{"^/api/v1/.*"}, + }, + RegexValues: []string{"^/api/v1/.*"}, + }, + }, + expected: true, + }, + { + name: "not equal - different regex path pattern", + desiredRuleCondition: []types.RuleCondition{ + { + Field: awssdk.String("path-pattern"), + PathPatternConfig: &types.PathPatternConditionConfig{ + RegexValues: []string{"^/api/v1/.*"}, + }, + }, + }, + actualRuleCondition: []types.RuleCondition{ + { + Field: awssdk.String("path-pattern"), + PathPatternConfig: &types.PathPatternConditionConfig{ + RegexValues: []string{"^/api/v2/.*"}, + }, + RegexValues: []string{"^/api/v2/.*"}, + }, + }, + expected: false, + }, } for _, tc := range testCase { diff --git a/pkg/ingress/rule_optimizer.go b/pkg/ingress/rule_optimizer.go index 6b1cbb47a1..1a67cd35e4 100644 --- a/pkg/ingress/rule_optimizer.go +++ b/pkg/ingress/rule_optimizer.go @@ -98,6 +98,10 @@ func isInfiniteRedirectRule(port int32, protocol elbv2model.Protocol, rule Rule) case condition.Field == elbv2model.RuleConditionFieldHostHeader && condition.HostHeaderConfig != nil: ruleHosts.Insert(condition.HostHeaderConfig.Values...) case condition.Field == elbv2model.RuleConditionFieldPathPattern && condition.PathPatternConfig != nil: + // We dont check for infinite rules for regex paths + if len(condition.PathPatternConfig.RegexValues) > 0 { + return false + } rulePaths.Insert(condition.PathPatternConfig.Values...) } } @@ -146,7 +150,11 @@ func isSupersetConditions(lhsConditions []elbv2model.RuleCondition, rhsCondition case condition.Field == elbv2model.RuleConditionFieldHostHeader && condition.HostHeaderConfig != nil: lhsHosts.Insert(condition.HostHeaderConfig.Values...) case condition.Field == elbv2model.RuleConditionFieldPathPattern && condition.PathPatternConfig != nil: - lhsPaths.Insert(condition.PathPatternConfig.Values...) + if len(condition.PathPatternConfig.RegexValues) > 0 { + lhsPaths.Insert(condition.PathPatternConfig.RegexValues...) + } else { + lhsPaths.Insert(condition.PathPatternConfig.Values...) + } default: // if there are any other conditions, then we treat it as not superset. return false @@ -160,7 +168,11 @@ func isSupersetConditions(lhsConditions []elbv2model.RuleCondition, rhsCondition case condition.Field == elbv2model.RuleConditionFieldHostHeader && condition.HostHeaderConfig != nil: rhsHosts.Insert(condition.HostHeaderConfig.Values...) case condition.Field == elbv2model.RuleConditionFieldPathPattern && condition.PathPatternConfig != nil: - rhsPaths.Insert(condition.PathPatternConfig.Values...) + if len(condition.PathPatternConfig.RegexValues) > 0 { + rhsPaths.Insert(condition.PathPatternConfig.RegexValues...) + } else { + rhsPaths.Insert(condition.PathPatternConfig.Values...) + } } } diff --git a/pkg/ingress/rule_optimizer_test.go b/pkg/ingress/rule_optimizer_test.go index b76ba11150..46dfdac1f6 100644 --- a/pkg/ingress/rule_optimizer_test.go +++ b/pkg/ingress/rule_optimizer_test.go @@ -231,6 +231,886 @@ func Test_defaultRuleOptimizer_Optimize(t *testing.T) { }, }, }, + { + name: "catch-all regex rule should NOT overshadow specific regex rules", + args: args{ + port: 443, + protocol: elbv2model.ProtocolHTTPS, + rules: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/.*$"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("default.example.com"), + Path: awssdk.String("/#{path}"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/api/.*"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeFixedResponse, + FixedResponseConfig: &elbv2model.FixedResponseActionConfig{ + StatusCode: "404", + MessageBody: awssdk.String("Not found"), + }, + }, + }, + }, + }, + }, + want: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/.*$"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("default.example.com"), + Path: awssdk.String("/#{path}"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/api/.*"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeFixedResponse, + FixedResponseConfig: &elbv2model.FixedResponseActionConfig{ + StatusCode: "404", + MessageBody: awssdk.String("Not found"), + }, + }, + }, + }, + }, + }, + { + name: "identical regex paths with different actions should be deduplicated", + args: args{ + port: 443, + protocol: elbv2model.ProtocolHTTPS, + rules: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/(sos/packages/embed-button)/.*$"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("b.example.com"), + Path: awssdk.String("/#{path}"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/(sos/packages/embed-button)/.*$"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeFixedResponse, + FixedResponseConfig: &elbv2model.FixedResponseActionConfig{ + StatusCode: "403", + MessageBody: awssdk.String("403 permission denied"), + }, + }, + }, + }, + }, + }, + want: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/(sos/packages/embed-button)/.*$"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("b.example.com"), + Path: awssdk.String("/#{path}"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + { + name: "two different regex paths with redirect actions should be preserved", + args: args{ + port: 443, + protocol: elbv2model.ProtocolHTTPS, + rules: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/w/(custom_pin_code|form|survey)/?$"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("b.example.com"), + Path: awssdk.String("/#{path}/not-found"), + Protocol: awssdk.String("HTTPS"), + Port: awssdk.String("443"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/(w/(survey|form|custom_pin_code)|sos/packages/embed-button)/.*"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("b.example.com"), + Path: awssdk.String("/#{path}"), + Protocol: awssdk.String("HTTPS"), + Port: awssdk.String("443"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + want: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/w/(custom_pin_code|form|survey)/?$"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("b.example.com"), + Path: awssdk.String("/#{path}/not-found"), + Protocol: awssdk.String("HTTPS"), + Port: awssdk.String("443"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/(w/(survey|form|custom_pin_code)|sos/packages/embed-button)/.*"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("b.example.com"), + Path: awssdk.String("/#{path}"), + Protocol: awssdk.String("HTTPS"), + Port: awssdk.String("443"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + { + name: "rules with different query strings should be preserved", + args: args{ + port: 443, + protocol: elbv2model.ProtocolHTTPS, + rules: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/api"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldQueryString, + QueryStringConfig: &elbv2model.QueryStringConditionConfig{ + Values: []elbv2model.QueryStringKeyValuePair{ + {Key: awssdk.String("version"), Value: "v1"}, + }, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("api-v1.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/api"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldQueryString, + QueryStringConfig: &elbv2model.QueryStringConditionConfig{ + Values: []elbv2model.QueryStringKeyValuePair{ + {Key: awssdk.String("version"), Value: "v2"}, + }, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("api-v2.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + want: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/api"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldQueryString, + QueryStringConfig: &elbv2model.QueryStringConditionConfig{ + Values: []elbv2model.QueryStringKeyValuePair{ + {Key: awssdk.String("version"), Value: "v1"}, + }, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("api-v1.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/api"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldQueryString, + QueryStringConfig: &elbv2model.QueryStringConditionConfig{ + Values: []elbv2model.QueryStringKeyValuePair{ + {Key: awssdk.String("version"), Value: "v2"}, + }, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("api-v2.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + { + name: "rules with different HTTP methods should be preserved", + args: args{ + port: 443, + protocol: elbv2model.ProtocolHTTPS, + rules: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/api"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHTTPRequestMethod, + HTTPRequestMethodConfig: &elbv2model.HTTPRequestMethodConditionConfig{ + Values: []string{"GET"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("readonly.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/api"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHTTPRequestMethod, + HTTPRequestMethodConfig: &elbv2model.HTTPRequestMethodConditionConfig{ + Values: []string{"POST"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("writeapi.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + want: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/api"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHTTPRequestMethod, + HTTPRequestMethodConfig: &elbv2model.HTTPRequestMethodConditionConfig{ + Values: []string{"GET"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("readonly.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/api"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHTTPRequestMethod, + HTTPRequestMethodConfig: &elbv2model.HTTPRequestMethodConditionConfig{ + Values: []string{"POST"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("writeapi.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + { + name: "rules with different source IPs should be preserved", + args: args{ + port: 443, + protocol: elbv2model.ProtocolHTTPS, + rules: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/admin"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldSourceIP, + SourceIPConfig: &elbv2model.SourceIPConditionConfig{ + Values: []string{"10.0.0.0/8"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("internal-admin.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/admin"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldSourceIP, + SourceIPConfig: &elbv2model.SourceIPConditionConfig{ + Values: []string{"192.168.0.0/16"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("vpn-admin.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + want: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/admin"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldSourceIP, + SourceIPConfig: &elbv2model.SourceIPConditionConfig{ + Values: []string{"10.0.0.0/8"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("internal-admin.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + Values: []string{"/admin"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldSourceIP, + SourceIPConfig: &elbv2model.SourceIPConditionConfig{ + Values: []string{"192.168.0.0/16"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("vpn-admin.example.com"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + { + name: "complex scenario with mixed condition types", + args: args{ + port: 443, + protocol: elbv2model.ProtocolHTTPS, + rules: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/api/v[1-2]/users.*"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHTTPRequestMethod, + HTTPRequestMethodConfig: &elbv2model.HTTPRequestMethodConditionConfig{ + Values: []string{"GET"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("users-api.example.com"), + Path: awssdk.String("/#{path}"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/api/v3/.*"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldQueryString, + QueryStringConfig: &elbv2model.QueryStringConditionConfig{ + Values: []elbv2model.QueryStringKeyValuePair{ + {Key: awssdk.String("beta"), Value: "true"}, + }, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("beta-api.example.com"), + Path: awssdk.String("/#{path}"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, + want: []Rule{ + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/api/v[1-2]/users.*"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHTTPRequestMethod, + HTTPRequestMethodConfig: &elbv2model.HTTPRequestMethodConditionConfig{ + Values: []string{"GET"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("users-api.example.com"), + Path: awssdk.String("/#{path}"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + { + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/api/v3/.*"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldQueryString, + QueryStringConfig: &elbv2model.QueryStringConditionConfig{ + Values: []elbv2model.QueryStringKeyValuePair{ + {Key: awssdk.String("beta"), Value: "true"}, + }, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + Host: awssdk.String("beta-api.example.com"), + Path: awssdk.String("/#{path}"), + StatusCode: "HTTP_302", + }, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -248,6 +1128,120 @@ func Test_defaultRuleOptimizer_Optimize(t *testing.T) { } } +// TestIsSupersetConditions_DifferentRegexPaths tests that different regex paths are NOT considered supersets +func TestIsSupersetConditions_DifferentRegexPaths(t *testing.T) { + tests := []struct { + name string + lhsRules []elbv2model.RuleCondition + rhsRules []elbv2model.RuleCondition + expected bool + }{ + { + name: "Different regex paths should NOT be supersets", + lhsRules: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/w/(custom_pin_code|form|survey)/?$"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + }, + rhsRules: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/(w/(survey|form|custom_pin_code)|sos/packages/embed-button)/.*"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + }, + expected: false, // Different regex patterns should NOT be superset + }, + { + name: "Identical regex paths should be supersets", + lhsRules: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/(sos/packages/embed-button)/.*$"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + }, + rhsRules: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/(sos/packages/embed-button)/.*$"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + }, + expected: true, // Identical regex patterns should be superset + }, + { + name: "Catch-all regex with other paths", + lhsRules: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/.*$"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + }, + rhsRules: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/specific/path$"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"a.example.com"}, + }, + }, + }, + expected: false, // Different regex patterns, not superset + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isSupersetConditions(tt.lhsRules, tt.rhsRules) + assert.Equal(t, tt.expected, result) + }) + } +} + func Test_isInfiniteRedirectRule(t *testing.T) { type args struct { port int32 @@ -291,6 +1285,38 @@ func Test_isInfiniteRedirectRule(t *testing.T) { }, want: true, }, + { + name: "isnt infinite redirect rule when for regex paths", + args: args{ + port: 443, + protocol: elbv2model.ProtocolHTTPS, + rule: Rule{ + Conditions: []elbv2model.RuleCondition{ + { + Field: elbv2model.RuleConditionFieldHostHeader, + HostHeaderConfig: &elbv2model.HostHeaderConditionConfig{ + Values: []string{"www.example.com", "app.example.com"}, + }, + }, + { + Field: elbv2model.RuleConditionFieldPathPattern, + PathPatternConfig: &elbv2model.PathPatternConditionConfig{ + RegexValues: []string{"^/specific/path$"}, + }, + }, + }, + Actions: []elbv2model.Action{ + { + Type: elbv2model.ActionTypeRedirect, + RedirectConfig: &elbv2model.RedirectActionConfig{ + StatusCode: "HTTP_301", + }, + }, + }, + }, + }, + want: false, + }, { name: "is infinite redirect rule when all fields are set to default value", args: args{ diff --git a/pkg/runtime/retry_utils.go b/pkg/runtime/retry_utils.go index 38a2f3370a..1c12074d18 100644 --- a/pkg/runtime/retry_utils.go +++ b/pkg/runtime/retry_utils.go @@ -1,20 +1,29 @@ package runtime import ( - "k8s.io/apimachinery/pkg/util/wait" + "fmt" "time" + + "k8s.io/apimachinery/pkg/util/wait" ) // RetryImmediateOnError tries to run fn every interval until it succeeds, a non-retryable error occurs or the timeout is reached. +// If the timeout is reached while retrying, the returned error wraps both the timeout and the last retryable error. func RetryImmediateOnError(interval time.Duration, timeout time.Duration, retryable func(error) bool, fn func() error) error { - return wait.PollImmediate(interval, timeout, func() (bool, error) { + var lastRetryableErr error + err := wait.PollImmediate(interval, timeout, func() (bool, error) { err := fn() if err != nil { if retryable(err) { + lastRetryableErr = err return false, nil } return false, err } return true, nil }) + if wait.Interrupted(err) && lastRetryableErr != nil { + return fmt.Errorf("%w: %v", err, lastRetryableErr) + } + return err } diff --git a/pkg/runtime/retry_utils_test.go b/pkg/runtime/retry_utils_test.go index b1ac0a1d7a..85f2581d5c 100644 --- a/pkg/runtime/retry_utils_test.go +++ b/pkg/runtime/retry_utils_test.go @@ -2,9 +2,10 @@ package runtime import ( "errors" - "github.com/stretchr/testify/assert" "testing" "time" + + "github.com/stretchr/testify/assert" ) func Test_RetryImmediateOnError(t *testing.T) { @@ -38,54 +39,63 @@ func Test_RetryImmediateOnError(t *testing.T) { fn func() error } tests := []struct { - name string - args args - wantCount int - wantErr error + name string + args args + wantMinCount int + wantMaxCount int + wantErr error }{ { name: "retry 4 times before failure", args: args{ - interval: 10 * time.Millisecond, - timeout: 100 * time.Millisecond, + interval: 50 * time.Millisecond, + timeout: 500 * time.Millisecond, retryable: retryable, fn: failureAfterRetryCountFnGen(4), }, - wantCount: 5, - wantErr: errors.New("failure"), + wantMinCount: 5, + wantMaxCount: 5, + wantErr: errors.New("failure"), }, { name: "retry 4 times before failure - but timeout after 2nd retry", args: args{ - interval: 10 * time.Millisecond, - timeout: 19 * time.Millisecond, + interval: 50 * time.Millisecond, + timeout: 120 * time.Millisecond, retryable: retryable, fn: failureAfterRetryCountFnGen(4), }, - wantCount: 3, - wantErr: errors.New("timed out waiting for the condition"), + // PollImmediate calls fn at t=0, t=50ms, t=100ms. Timeout at 120ms + // may race with the next tick, so count can be 3 or 4. + wantMinCount: 3, + wantMaxCount: 4, + wantErr: errors.New("timed out waiting for the condition: retryable"), }, { name: "retry 4 times before success", args: args{ - interval: 10 * time.Millisecond, - timeout: 100 * time.Millisecond, + interval: 50 * time.Millisecond, + timeout: 500 * time.Millisecond, retryable: retryable, fn: successAfterRetryCountFnGen(4), }, - wantCount: 5, - wantErr: nil, + wantMinCount: 5, + wantMaxCount: 5, + wantErr: nil, }, { name: "retry 4 times before success - but timeout after 2nd retry", args: args{ - interval: 10 * time.Millisecond, - timeout: 19 * time.Millisecond, + interval: 50 * time.Millisecond, + timeout: 120 * time.Millisecond, retryable: retryable, fn: successAfterRetryCountFnGen(4), }, - wantCount: 3, - wantErr: errors.New("timed out waiting for the condition"), + // PollImmediate calls fn at t=0, t=50ms, t=100ms. Timeout at 120ms + // may race with the next tick, so count can be 3 or 4. + wantMinCount: 3, + wantMaxCount: 4, + wantErr: errors.New("timed out waiting for the condition: retryable"), }, } for _, tt := range tests { @@ -100,7 +110,8 @@ func Test_RetryImmediateOnError(t *testing.T) { } else { assert.NoError(t, err) } - assert.Equal(t, tt.wantCount, count) + assert.GreaterOrEqual(t, count, tt.wantMinCount, "retry count below minimum") + assert.LessOrEqual(t, count, tt.wantMaxCount, "retry count above maximum") }) } }