Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-1.18] Fix invalid XDS configuration for wildcard Ingress HTTP path (#44898) #45168

Merged
merged 1 commit into from
Jun 8, 2023
Merged

[release-1.18] Fix invalid XDS configuration for wildcard Ingress HTTP path (#44898) #45168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions pilot/pkg/config/kube/ingress/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ func ConvertIngressVirtualService(ingress knetworking.Ingress, domainSuffix stri
}
default:
// Fallback to the legacy string matching
// If the httpPath.Path is a wildcard path, Uri will be nil
httpMatch.Uri = createFallbackStringMatch(httpPath.Path)
}
} else {
Expand All @@ -175,7 +176,10 @@ func ConvertIngressVirtualService(ingress knetworking.Ingress, domainSuffix stri
log.Infof("invalid ingress rule %s:%s for host %q, no backend defined for path", ingress.Namespace, ingress.Name, rule.Host)
continue
}
httpRoute.Match = []*networking.HTTPMatchRequest{httpMatch}
// Only create a match if Uri is not nil. HttpMatchRequest cannot be empty
if httpMatch.Uri != nil {
httpRoute.Match = []*networking.HTTPMatchRequest{httpMatch}
}
httpRoutes = append(httpRoutes, httpRoute)
}

Expand Down Expand Up @@ -218,8 +222,14 @@ func ConvertIngressVirtualService(ingress knetworking.Ingress, domainSuffix stri
// see https://kubernetes.io/docs/concepts/services-networking/ingress/#multiple-matches
vs := ingressByHost[host].Spec.(*networking.VirtualService)
sort.SliceStable(vs.Http, func(i, j int) bool {
r1Len, r1Ex := getMatchURILength(vs.Http[i].Match[0])
r2Len, r2Ex := getMatchURILength(vs.Http[j].Match[0])
var r1Len, r2Len int
var r1Ex, r2Ex bool
if vs.Http[i].Match != nil || len(vs.Http[i].Match) != 0 {
r1Len, r1Ex = getMatchURILength(vs.Http[i].Match[0])
}
if vs.Http[j].Match != nil || len(vs.Http[j].Match) != 0 {
r2Len, r2Ex = getMatchURILength(vs.Http[j].Match[0])
}
// TODO: default at the end
if r1Len == r2Len {
return r1Ex && !r2Ex
Expand Down Expand Up @@ -335,7 +345,8 @@ func shouldProcessIngressWithClass(mesh *meshconfig.MeshConfig, ingress *knetwor
}

func createFallbackStringMatch(s string) *networking.StringMatch {
if s == "" {
// If the string is empty or a wildcard, return nil
if s == "" || s == "*" || s == "/*" || s == ".*" {
return nil
}

Expand Down
32 changes: 30 additions & 2 deletions pilot/pkg/config/kube/ingress/conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,24 @@ func TestConversion(t *testing.T) {
},
},
},
{
Host: "my4.host.com",
IngressRuleValue: knetworking.IngressRuleValue{
HTTP: &knetworking.HTTPIngressRuleValue{
Paths: []knetworking.HTTPIngressPath{
{
Path: "/*",
Backend: knetworking.IngressBackend{
Service: &knetworking.IngressServiceBackend{
Name: "bar",
Port: knetworking.ServiceBackendPort{Number: 8000},
},
},
},
},
},
},
},
},
},
}
Expand Down Expand Up @@ -255,8 +273,8 @@ func TestConversion(t *testing.T) {
ConvertIngressVirtualService(ingress, "mydomain", cfgs, serviceLister)
ConvertIngressVirtualService(ingress2, "mydomain", cfgs, serviceLister)

if len(cfgs) != 3 {
t.Error("VirtualServices, expected 3 got ", len(cfgs))
if len(cfgs) != 4 {
t.Error("VirtualServices, expected 4 got ", len(cfgs))
}

expectedLength := [5]int{13, 13, 9, 6, 5}
Expand All @@ -279,6 +297,16 @@ func TestConversion(t *testing.T) {
i, expectedLength[i], expectedExact[i], length, exact)
}
}
} else if n == "my4.host.com" {
if vs.Hosts[0] != "my4.host.com" {
t.Error("Unexpected host", vs)
}
if len(vs.Http) != 1 {
t.Error("Unexpected rules", vs.Http)
}
if vs.Http[0].Match != nil {
t.Error("Expected HTTPMatchRequest to be nil, got {}")
}
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions pilot/pkg/config/kube/ingress/testdata/overlay.yaml.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ spec:
port:
number: 4200
weight: 100
- match:
- {}
route:
- route:
- destination:
host: service1.ns.svc.mydomain
port:
Expand Down
5 changes: 1 addition & 4 deletions pilot/pkg/config/kube/ingress/testdata/tls-no-secret.yaml.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ spec:
hosts:
- foo.org
http:
- match:
- uri:
prefix: ""
route:
- route:
- destination:
host: httpbin.bar.svc.mydomain
port:
Expand Down
5 changes: 1 addition & 4 deletions pilot/pkg/config/kube/ingress/testdata/tls.yaml.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ spec:
hosts:
- foo.org
http:
- match:
- uri:
prefix: ""
route:
- route:
- destination:
host: httpbin.bar.svc.mydomain
port:
Expand Down