Merge pull request #681 from o11n/preservePredicates

Skipper: preserve Predicates

Author: stefanprodan

pkg/router/skipper.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
 
 	"github.com/google/go-cmp/cmp"
 	"go.uber.org/zap"
@@ -176,7 +177,8 @@ func (skp *SkipperRouter) SetRoutes(canary *flaggerv1.Canary, primaryWeight, can
 
 	// Disable the canary-ingress route after the canary process
 	if canaryWeight == 0 {
-		iClone.Annotations[skipperpredicateAnnotationKey] = canaryRouteDisable
+		// ensuring False() is at first place
+		iClone.Annotations[skipperpredicateAnnotationKey] = insertPredicate(iClone.Annotations[skipperpredicateAnnotationKey], canaryRouteDisable)
 	}
 
 	_, err = skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Update(
@@ -212,7 +214,7 @@ func (skp *SkipperRouter) makeAnnotations(annotations map[string]string, backend
 	}
 	annotations[skipperBackendWeightsAnnotationKey] = string(b)
 	// adding more weight to canary route solves traffic bypassing through apexIngress
-	annotations[skipperpredicateAnnotationKey] = canaryRouteWeight
+	annotations[skipperpredicateAnnotationKey] = insertPredicate(annotations[skipperpredicateAnnotationKey], canaryRouteWeight)
 
 	return annotations
 }
@@ -233,3 +235,19 @@ func (skp *SkipperRouter) backendWeights(annotation map[string]string) (backendW
 func (skp *SkipperRouter) getIngressNames(name string) (apexName, canaryName string) {
 	return name, fmt.Sprintf(canaryPatternf, name)
 }
+
+func insertPredicate(raw, insert string) string {
+	// ensuring it at first place
+	predicates := []string{insert}
+	for _, x := range strings.Split(raw, "&&") {
+		predicate := strings.TrimSpace(x)
+		// dropping conflicting predicates
+		if predicate == "" ||
+			predicate == canaryRouteWeight ||
+			predicate == canaryRouteDisable {
+			continue
+		}
+		predicates = append(predicates, predicate)
+	}
+	return strings.Join(predicates, " && ")
+}

pkg/router/skipper_test.go

@@ -105,3 +105,49 @@ func TestSkipperRouter_GetSetRoutes(t *testing.T) {
 	}
 
 }
+
+func Test_insertPredicate(t *testing.T) {
+	tests := []struct {
+		name   string
+		raw    string
+		insert string
+		want   string
+	}{
+		{
+			name:   "a few Predicates lined up",
+			raw:    `Host(/^my-host-header\.example\.org$/) && Method("GET") && Path("/hello")`,
+			insert: "Weight(100)",
+			want:   `Weight(100) && Host(/^my-host-header\.example\.org$/) && Method("GET") && Path("/hello")`,
+		},
+		{
+			name:   "adds Predicate if none is set",
+			raw:    "",
+			insert: "Weight(100)",
+			want:   `Weight(100)`,
+		},
+		{
+			name:   "removes duplicated Predicate Weight(100)",
+			raw:    `Weight(100) && Host(/^my-host-header\.example\.org$/) && Method("GET") && Path("/hello")`,
+			insert: "Weight(100)",
+			want:   `Weight(100) && Host(/^my-host-header\.example\.org$/) && Method("GET") && Path("/hello")`,
+		},
+		{
+			name:   "removes duplicated Predicate False() and reorders them",
+			raw:    `Host(/^my-host-header\.example\.org$/) && Method("GET") && Path("/hello")&&False()`,
+			insert: "False()",
+			want:   `False() && Host(/^my-host-header\.example\.org$/) && Method("GET") && Path("/hello")`,
+		},
+		{
+			name:   "removes conflicting Predicate False()",
+			raw:    `Host(/^my-host-header\.example\.org$/) &&  False() && Method("GET") && Path("/hello")`,
+			insert: "Weight(100)",
+			want:   `Weight(100) && Host(/^my-host-header\.example\.org$/) && Method("GET") && Path("/hello")`,
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, insertPredicate(tt.raw, tt.insert))
+		})
+	}
+}