From 16312f548041eaf7e36504fe98f63e0f777a7309 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Date: Tue, 3 May 2022 11:39:03 +0200 Subject: [PATCH] feat: Add experimental "smart" sort plugin (#90) * feat: Add experimental "smart" sort plugin * Add tests and ci for tests * fix main test error --- .github/workflows/tests.yml | 24 +++++ cmd/caddy/main.go | 12 +-- internal/caddy/global/ingress_sort.go | 76 +++++++++++++++ internal/caddy/global/ingress_sort_test.go | 108 +++++++++++++++++++++ pkg/store/configmap_parser.go | 1 + 5 files changed, 215 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/tests.yml create mode 100644 internal/caddy/global/ingress_sort.go create mode 100644 internal/caddy/global/ingress_sort_test.go diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..490d9d4 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,24 @@ +name: Go + +on: + push: + branches: [ master ] + pull_request: + +jobs: + build-and-test: + name: Build and Test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: 1.16 + + - name: Build + run: go build -v ./... + + - name: Test + run: go test -v ./... \ No newline at end of file diff --git a/cmd/caddy/main.go b/cmd/caddy/main.go index e45e54e..a68ddbf 100644 --- a/cmd/caddy/main.go +++ b/cmd/caddy/main.go @@ -46,7 +46,7 @@ func main() { } // get client to access the kubernetes service api - kubeClient, err := createApiserverClient(logger) + kubeClient, _, err := createApiserverClient(logger) if err != nil { logger.Fatalf("Could not establish a connection to the Kubernetes API Server. %v", err) } @@ -71,10 +71,10 @@ func main() { // createApiserverClient creates a new Kubernetes REST client. We assume the // controller runs inside Kubernetes and use the in-cluster config. -func createApiserverClient(logger *zap.SugaredLogger) (*kubernetes.Clientset, error) { +func createApiserverClient(logger *zap.SugaredLogger) (*kubernetes.Clientset, *version.Info, error) { cfg, err := clientcmd.BuildConfigFromFlags("", "") if err != nil { - return nil, err + return nil, nil, err } logger.Infof("Creating API client for %s", cfg.Host) @@ -84,7 +84,7 @@ func createApiserverClient(logger *zap.SugaredLogger) (*kubernetes.Clientset, er cfg.ContentType = "application/vnd.kubernetes.protobuf" client, err := kubernetes.NewForConfig(cfg) if err != nil { - return nil, err + return nil, nil, err } // The client may fail to connect to the API server on the first request @@ -113,12 +113,12 @@ func createApiserverClient(logger *zap.SugaredLogger) (*kubernetes.Clientset, er // err is returned in case of timeout in the exponential backoff (ErrWaitTimeout) if err != nil { - return nil, lastErr + return nil, nil, lastErr } if retries > 0 { logger.Warnf("Initial connection to the Kubernetes API server was retried %d times.", retries) } - return client, nil + return client, v, nil } diff --git a/internal/caddy/global/ingress_sort.go b/internal/caddy/global/ingress_sort.go new file mode 100644 index 0000000..c2787c1 --- /dev/null +++ b/internal/caddy/global/ingress_sort.go @@ -0,0 +1,76 @@ +package global + +import ( + "encoding/json" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" + "github.com/caddyserver/ingress/pkg/converter" + "github.com/caddyserver/ingress/pkg/store" + "sort" + "strings" +) + +type IngressSortPlugin struct{} + +func (p IngressSortPlugin) IngressPlugin() converter.PluginInfo { + return converter.PluginInfo{ + Name: "ingress_sort", + // Must go after ingress are configured + Priority: -2, + New: func() converter.Plugin { return new(IngressSortPlugin) }, + } +} + +func init() { + converter.RegisterPlugin(IngressSortPlugin{}) +} + +func getFirstItemFromJSON(data json.RawMessage) string { + var arr []string + err := json.Unmarshal(data, &arr) + if err != nil { + return "" + } + return arr[0] +} + +func sortRoutes(routes caddyhttp.RouteList) { + sort.SliceStable(routes, func(i, j int) bool { + iPath := getFirstItemFromJSON(routes[i].MatcherSetsRaw[0]["path"]) + jPath := getFirstItemFromJSON(routes[j].MatcherSetsRaw[0]["path"]) + iPrefixed := strings.HasSuffix(iPath, "*") + jPrefixed := strings.HasSuffix(jPath, "*") + + // If both same type check by length + if iPrefixed == jPrefixed { + return len(jPath) < len(iPath) + } + // Empty path will be moved last + if jPath == "" || iPath == "" { + return jPath == "" + } + // j path is exact so should go first + return jPrefixed + }) +} + +// GlobalHandler in IngressSortPlugin tries to sort routes to have the less conflict. +// +// It only supports basic conflicts for now. It doesn't support multiple matchers in the same route +// nor multiple path/host in the matcher. It shouldn't be an issue with the ingress.matcher plugin. +// Sort will prioritize exact paths then prefix paths and finally empty paths. +// When 2 exacts paths or 2 prefixed paths are on the same host, we choose the longer first. +func (p IngressSortPlugin) GlobalHandler(config *converter.Config, store *store.Store) error { + if !store.ConfigMap.ExperimentalSmartSort { + return nil + } + + routes := config.GetHTTPServer().Routes + + sortRoutes(routes) + return nil +} + +// Interface guards +var ( + _ = converter.GlobalMiddleware(IngressSortPlugin{}) +) diff --git a/internal/caddy/global/ingress_sort_test.go b/internal/caddy/global/ingress_sort_test.go new file mode 100644 index 0000000..77df7b5 --- /dev/null +++ b/internal/caddy/global/ingress_sort_test.go @@ -0,0 +1,108 @@ +package global + +import ( + "encoding/json" + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/caddyconfig" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" + "reflect" + "testing" +) + +func TestIngressSort(t *testing.T) { + tests := []struct { + name string + routes []struct { + id int + path string + } + expect []int + }{ + + { + name: "multiple exact paths", + routes: []struct { + id int + path string + }{ + {id: 0, path: "/path/a"}, + {id: 1, path: "/path/"}, + {id: 2, path: "/other"}, + }, + expect: []int{0, 1, 2}, + }, + { + name: "multiple prefix paths", + routes: []struct { + id int + path string + }{ + {id: 0, path: "/path/*"}, + {id: 1, path: "/path/auth/*"}, + {id: 2, path: "/other/*"}, + {id: 3, path: "/login/*"}, + }, + expect: []int{1, 2, 3, 0}, + }, + { + name: "mixed exact and prefixed", + routes: []struct { + id int + path string + }{ + {id: 0, path: "/path/*"}, + {id: 1, path: "/path/auth/"}, + {id: 2, path: "/path/v2/*"}, + {id: 3, path: "/path/new"}, + }, + expect: []int{1, 3, 2, 0}, + }, + { + name: "mixed exact, prefix and empty", + routes: []struct { + id int + path string + }{ + {id: 0, path: "/path/*"}, + {id: 1, path: ""}, + {id: 2, path: "/path/v2/*"}, + {id: 3, path: "/path/new"}, + {id: 4, path: ""}, + }, + expect: []int{3, 2, 0, 1, 4}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + routes := []caddyhttp.Route{} + + for _, route := range test.routes { + match := caddy.ModuleMap{} + match["id"] = caddyconfig.JSON(route.id, nil) + + if route.path != "" { + match["path"] = caddyconfig.JSON(caddyhttp.MatchPath{route.path}, nil) + } + + r := caddyhttp.Route{MatcherSetsRaw: []caddy.ModuleMap{match}} + routes = append(routes, r) + } + + sortRoutes(routes) + + var got []int + for i := range test.expect { + var currentId int + err := json.Unmarshal(routes[i].MatcherSetsRaw[0]["id"], ¤tId) + if err != nil { + t.Fatalf("error unmarshaling id for i %v, %v", i, err) + } + got = append(got, currentId) + } + + if !reflect.DeepEqual(test.expect, got) { + t.Errorf("expected order to match: got %v, expected %v, %s", got, test.expect, routes[1].MatcherSetsRaw) + } + }) + } +} diff --git a/pkg/store/configmap_parser.go b/pkg/store/configmap_parser.go index 5164f3b..e810d57 100644 --- a/pkg/store/configmap_parser.go +++ b/pkg/store/configmap_parser.go @@ -14,6 +14,7 @@ type ConfigMapOptions struct { Debug bool `json:"debug,omitempty"` AcmeCA string `json:"acmeCA,omitempty"` Email string `json:"email,omitempty"` + ExperimentalSmartSort bool `json:"experimentalSmartSort,omitempty"` ProxyProtocol bool `json:"proxyProtocol,omitempty"` Metrics bool `json:"metrics,omitempty"` OnDemandTLS bool `json:"onDemandTLS,omitempty"`