Skip to content

Commit

Permalink
Fix NPE on missing matchers in CLI (#1564)
Browse files Browse the repository at this point in the history
* Fix NPE on missing matchers in CLI
  • Loading branch information
marcogschmidt authored and soloio-bulldozer[bot] committed Oct 31, 2019
1 parent 8f794aa commit dc0c0e3
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 8 deletions.
4 changes: 4 additions & 0 deletions changelog/v0.20.13/cli-fix-missing-matcher-panic.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changelog:
- type: FIX
description: Avoid panicking on routes without matchers when running `glooctl get vs`.
issueLink: https://github.com/solo-io/gloo/issues/1563
10 changes: 5 additions & 5 deletions projects/gloo/cli/pkg/printers/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func routeDefaultTableRow(r *v1.Route, index int, customItems []string) []string
func Matcher(r *v1.Route) (string, string, string, string) {
var path string
var rType string
switch p := r.Matcher.PathSpecifier.(type) {
switch p := r.GetMatcher().GetPathSpecifier().(type) {
case *gloov1.Matcher_Exact:
path = p.Exact
rType = "Exact Path"
Expand All @@ -103,13 +103,13 @@ func Matcher(r *v1.Route) (string, string, string, string) {
rType = "Unknown"
}
verb := "*"
if r.Matcher.Methods != nil {
verb = strings.Join(r.Matcher.Methods, " ")
if methods := r.GetMatcher().GetMethods(); methods != nil {
verb = strings.Join(methods, " ")
}
headers := ""
if r.Matcher.Headers != nil {
if headerMatchers := r.GetMatcher().GetHeaders(); headerMatchers != nil {
builder := bytes.Buffer{}
for _, v := range r.Matcher.Headers {
for _, v := range headerMatchers {
header := *v
builder.WriteString(string(header.Name))
builder.WriteString(":")
Expand Down
2 changes: 1 addition & 1 deletion projects/gloo/cli/pkg/printers/virtualservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func vhPlugins(v *v1.VirtualService) string {
}

func matcherString(matcher *gloov1.Matcher) string {
switch ps := matcher.PathSpecifier.(type) {
switch ps := matcher.GetPathSpecifier().(type) {
case *gloov1.Matcher_Exact:
return ps.Exact
case *gloov1.Matcher_Prefix:
Expand Down
32 changes: 32 additions & 0 deletions projects/gloo/cli/pkg/printers/virtualservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package printers

import (
"fmt"
"io/ioutil"
"strings"

"github.com/solo-io/gloo/projects/gloo/pkg/defaults"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
v1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1"
Expand Down Expand Up @@ -166,4 +169,33 @@ var _ = Describe("getStatus", func() {
Expect(out).To(MatchRegexp(genericErrorFormat(thing1, core.Status_Rejected.String(), reasonUpstreamList)))
Expect(out).To(MatchRegexp(genericErrorFormat(thing2, core.Status_Rejected.String(), reasonUpstreamList)))
})

It("does not panic on routes with no matchers", func() {
routes := []*v1.Route{
{
Action: &v1.Route_DirectResponseAction{
DirectResponseAction: &gloov1.DirectResponseAction{
Status: 200,
Body: "OK",
},
},
},
}
vs := &v1.VirtualService{
VirtualHost: &v1.VirtualHost{
Domains: []string{"*"},
Routes: routes,
},
Status: core.Status{
State: core.Status_Rejected,
},
Metadata: core.Metadata{
Name: "no-matcher",
Namespace: defaults.GlooSystem,
},
}

Expect(func() { VirtualServiceTable([]*v1.VirtualService{vs}, ioutil.Discard) }).NotTo(Panic())
Expect(func() { RouteTable(routes, ioutil.Discard) }).NotTo(Panic())
})
})
2 changes: 1 addition & 1 deletion projects/gloo/cli/pkg/surveyutils/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func SelectRouteFromVirtualServiceInteractive(vs *gatewayv1.VirtualService, rout

var routes []string
for i, r := range vs.VirtualHost.Routes {
routes = append(routes, fmt.Sprintf("%v: %+v", i, r.Matcher.PathSpecifier))
routes = append(routes, fmt.Sprintf("%v: %+v", i, r.GetMatcher().GetPathSpecifier()))
}

var chosenRoute string
Expand Down
11 changes: 10 additions & 1 deletion projects/gloo/pkg/utils/sort_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ func SortGatewayRoutesByPath(routes []*gatewayv1.Route) {
}

func lessMatcher(m1, m2 *v1.Matcher) bool {

// Handle nil matchers by de-prioritizing them.
// This is just to handle panics, as Gloo will reject routes with nil matchers
if m1 == nil {
return false
} else if m2 == nil {
return true
}

if len(m1.Methods) != len(m2.Methods) {
return len(m1.Methods) > len(m2.Methods)
}
Expand All @@ -43,7 +52,7 @@ const (
)

func pathTypePriority(m *v1.Matcher) int {
switch m.PathSpecifier.(type) {
switch m.GetPathSpecifier().(type) {
case *v1.Matcher_Exact:
return pathPriorityExact
case *v1.Matcher_Regex:
Expand Down

0 comments on commit dc0c0e3

Please sign in to comment.