Skipper: preserve Predicates

Current implementation did overwrite potentially existing Predicates.

We face the situation that we need to add further Predicates which we need to keep in order to have  a proper route setup

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