Add sorting template helper code to sort haproxy map files so that wildcard

routes don't take precedence over routes that start with a number.
Based on discussions in PR openshift#19076 to fix issue openshift#16724

Author: ramr

images/router/haproxy/conf/haproxy-config.template

@@ -580,3 +580,18 @@ backend be_tcp:{{$cfgIdx}}
 {{      end -}}
 {{     end -}}
 {{ end }}{{/* end cert_config map template */}}
+
+
+{{/*
+   Sort the haproxy map files (based on map paths and group wildcards at end).
+   Note: We don't sort the cert_config map `cert_config.map` as it doesn't matter.
+*/}}
+{{ define "/tmp/sort-haproxy-maps" -}}
+{{sortMap "/var/lib/haproxy/conf/os_wildcard_domain.map"}}
+{{sortMap "/var/lib/haproxy/conf/os_http_be.map"}}
+{{sortMap "/var/lib/haproxy/conf/os_edge_reencrypt_be.map"}}
+{{sortMap "/var/lib/haproxy/conf/os_route_http_redirect.map"}}
+{{sortMap "/var/lib/haproxy/conf/os_tcp_be.map"}}
+{{sortMap "/var/lib/haproxy/conf/os_sni_passthrough.map"}}
+{{ end -}}{{/* end sort maps */}}
+

images/router/haproxy/reload-haproxy

@@ -72,11 +72,6 @@ function haproxyHealthCheck() {
 retries=20
 
 
-# sort the path based map files for the haproxy map_beg function
-for mapfile in "$haproxy_conf_dir"/*.map; do
-  sort -r "$mapfile" -o "$mapfile"
-done
-
 old_pids=$(ps -A -opid,args | grep haproxy | egrep -v -e 'grep|reload-haproxy' | awk '{print $1}' | tr '\n' ' ')
 
 reload_status=0

pkg/router/template/template_helper.go

@@ -2,6 +2,7 @@ package templaterouter
 
 import (
 	"fmt"
+	"io/ioutil"
 	"math/rand"
 	"os"
 	"regexp"
@@ -12,6 +13,8 @@ import (
 	"github.com/golang/glog"
 
 	routeapi "github.com/openshift/origin/pkg/route/apis/route"
+	templateutil "github.com/openshift/origin/pkg/router/template/util"
+	fileutil "github.com/openshift/origin/pkg/util/file"
 )
 
 func isTrue(s string) bool {
@@ -170,6 +173,24 @@ func endpointsForAlias(alias ServiceAliasConfig, svc ServiceUnit) []Endpoint {
 	return endpoints
 }
 
+// sortMap sorts a map file based on longest path first (a reverse sort)
+// with the sorted wildcard paths added at the end.
+func sortMap(name string) bool {
+	lines, err := fileutil.ReadLines(name)
+	if err != nil {
+		glog.Errorf("Error reading map file %s: %v", name, err)
+		return false
+	}
+
+	data := templateutil.SortMapPaths(lines, `^[^\.]*\.`)
+	if err := ioutil.WriteFile(name, []byte(strings.Join(data, "\n")), 0664); err != nil {
+		glog.Errorf("Error writing map file %s: %v", name, err)
+		return false
+	}
+
+	return true
+}
+
 var helperFunctions = template.FuncMap{
 	"endpointsForAlias":        endpointsForAlias,        //returns the list of valid endpoints
 	"processEndpointsForAlias": processEndpointsForAlias, //returns the list of valid endpoints after processing them
@@ -184,4 +205,5 @@ var helperFunctions = template.FuncMap{
 
 	"isTrue":     isTrue,     //determines if a given variable is a true value
 	"firstMatch": firstMatch, //anchors provided regular expression and evaluates against given strings, returns the first matched string or ""
+	"sortMap":    sortMap,    //sorts a map file based on the longest path first and sorted wildcard paths at the end
 }

pkg/router/template/template_helper_test.go

@@ -1,8 +1,12 @@
 package templaterouter
 
 import (
+	"io/ioutil"
 	"regexp"
+	"strings"
 	"testing"
+
+	fileutil "github.com/openshift/origin/pkg/util/file"
 )
 
 func TestFirstMatch(t *testing.T) {
@@ -297,3 +301,63 @@ func TestMatchPattern(t *testing.T) {
 		}
 	}
 }
+
+func TestSortMap(t *testing.T) {
+	testData := []string{
+		`^api-stg\.127\.0\.0\.1\.nip\.io(:[0-9]+)?(/.*)?$ stg:api-route`,
+		`^api-prod\.127\.0\.0\.1\.nip\.io(:[0-9]+)?(/.*)?$ prod:api-route`,
+		`^[^\.]*\.127\.0\.0\.1\.nip\.io(:[0-9]+)?(/.*)?$ prod:wildcard-route`,
+		`^3dev\.127\.0\.0\.1\.nip\.io(:[0-9]+)?(/.*)?$ dev:api-route`,
+		`^api-prod\.127\.0\.0\.1\.nip\.io(:[0-9]+)?/x/y/z(/.*)?$ prod:api-path-route`,
+		`^3app-admin\.127\.0\.0\.1\.nip\.io(:[0-9]+)?(/.*)?$ dev:admin-route`,
+		`^[^\.]*\.foo\.127\.0\.0\.1\.nip\.io(:[0-9]+)?(/.*)?$ devel2:foo-wildcard-route`,
+		`^zzz-production\.wildcard\.test(:[0-9]+)?/x/y/z(/.*)?$ test:api-route`,
+		`^backend-app\.127\.0\.0\.1\.nip\.io(:[0-9]+)?(/.*)?$ prod:backend-route`,
+		`^[^\.]*\.foo\.wildcard\.test(:[0-9]+)?(/.*)?$ devel2:foo-wildcard-test`,
+	}
+
+	expectedOrder := []string{
+		"test:api-route",
+		"prod:backend-route",
+		"stg:api-route",
+		"prod:api-path-route",
+		"prod:api-route",
+		"dev:api-route",
+		"dev:admin-route",
+		"devel2:foo-wildcard-test",
+		"devel2:foo-wildcard-route",
+		"prod:wildcard-route",
+	}
+
+	fileName := ""
+	tempFile, err := ioutil.TempFile("", "sort-map-test")
+	if err == nil {
+		fileName = tempFile.Name()
+		err = tempFile.Close()
+	}
+	if err != nil || len(fileName) == 0 {
+		t.Errorf("TestSortMap unexpected error creating a temp file: %s", fileName)
+	}
+	if err := ioutil.WriteFile(fileName, []byte(strings.Join(testData, "\n")), 0664); err != nil {
+		t.Errorf("TestSortMap unexpected error writing temp file %s: %v", fileName, err)
+	}
+
+	if !sortMap(fileName) {
+		t.Errorf("TestSortMap temp file %s sort failed", fileName)
+	}
+
+	lines, err := fileutil.ReadLines(fileName)
+	if err != nil {
+		t.Errorf("TestSortMap error reading sorted temp file %s: %v", fileName, err)
+	}
+	if len(lines) != len(expectedOrder) {
+		t.Errorf("TestSortMap sorted data length %d did not match expected length %d",
+			len(lines), len(expectedOrder))
+	}
+	for idx, suffix := range expectedOrder {
+		if !strings.HasSuffix(lines[idx], suffix) {
+			t.Errorf("TestSortMap sorted data %s at index %d did not match expectation %s",
+				lines[idx], idx, suffix)
+		}
+	}
+}

pkg/router/template/util/map_paths.go

@@ -0,0 +1,65 @@
+package util
+
+import (
+	"sort"
+	"strings"
+)
+
+// sorterFunc is the type of "Less" function that defines map path order.
+type sorterFunc func(s1, s2 string) bool
+
+// mapPathSorter sorts a slice of map paths using the sort function.
+type mapPathSorter struct {
+	data []string
+	fn   sorterFunc // closure used in the Less method.
+}
+
+// Len returns the length of the data and is part of sort.Interface
+func (s *mapPathSorter) Len() int {
+	return len(s.data)
+}
+
+// Swap swaps the entries for the given indexes and (part of sort.Interface)
+func (s *mapPathSorter) Swap(i, j int) {
+	s.data[i], s.data[j] = s.data[j], s.data[i]
+}
+
+// Less compares two map paths using a closure and is part of sort.Interface
+func (s *mapPathSorter) Less(i, j int) bool {
+	return s.fn(s.data[i], s.data[j])
+}
+
+// sortByGroup sorts the data with any matching prefixes sorted last.
+// `reverse` indicates whether or not the data in each group (with and
+// without prefixes) needs to be reverse sorted.
+// Note that the matching prefixes are sorted based on length.
+func sortByGroup(data []string, prefix string, reverse bool) []string {
+	patternsAtEnd := func(s1, s2 string) bool {
+		if len(prefix) > 0 {
+			if strings.HasPrefix(s1, prefix) {
+				if !strings.HasPrefix(s2, prefix) {
+					return false
+				}
+			} else if strings.HasPrefix(s2, prefix) {
+				return true
+			}
+		}
+
+		// No prefix or both or neither strings have the prefix.
+		if reverse {
+			return s1 > s2
+		}
+
+		return s1 < s2
+	}
+
+	mps := &mapPathSorter{data: data, fn: patternsAtEnd}
+	sort.Sort(mps)
+	return mps.data
+}
+
+// SortMapPaths sorts the data by groups with any matching prefixes sorted at
+// the end. The data in each group is reverse sorted.
+func SortMapPaths(data []string, prefix string) []string {
+	return sortByGroup(data, prefix, true)
+}