feat: Add experimental "smart" sort plugin (#90)

* feat: Add experimental "smart" sort plugin

* Add tests and ci for tests

* fix main test error

Author: Embraser01

.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 ./...

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
 }

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{})
+)

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"], &currentId)
+				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)
+			}
+		})
+	}
+}

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"`