Repository: weaveworks/flagger
Pull request #442 (2020-02-14 11:49:31)

Use header operations in Istio router
Allow all header operations when Flagger service mesh provider is set to Istio fix: #260

Changes:
- remove deprecated `appendHeaders/removeResponseHeaders` from Istio client
- propagate header operations from canary service headers to Istio virtual service
- add Istio router tests for request/response header removal
- update header operations examples in docs


Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 7375d3d3..f83c8d1e 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -35,9 +35,6 @@ jobs:
       - run:
           name: Verify code gen
           command: make test-codegen
-      - run:
-          name: Release notes
-          command: make release-notes
       - save_cache:
           key: go-mod-v3-{{ checksum "go.sum" }}
           paths:
diff --git a/Makefile b/Makefile
index 64dc3156..8255b5b6 100644
--- a/Makefile
+++ b/Makefile
@@ -106,11 +106,6 @@ release-set: fmt version-set helm-package
 	git tag $(VERSION)
 	git push origin $(VERSION)
 
-release-notes:
-	cd /tmp && GH_REL_URL="https://github.com/buchanae/github-release-notes/releases/download/0.2.0/github-release-notes-linux-amd64-0.2.0.tar.gz" && \
-    curl -sSL $${GH_REL_URL} | tar xz && sudo mv github-release-notes /usr/local/bin/ && \
-	github-release-notes -org weaveworks -repo flagger -since-latest-release
-
 reset-test:
 	kubectl delete -f ./artifacts/namespaces
 	kubectl apply -f ./artifacts/namespaces
diff --git a/docs/gitbook/faq.md b/docs/gitbook/faq.md
index a6236a8b..e6e143a0 100644
--- a/docs/gitbook/faq.md
+++ b/docs/gitbook/faq.md
@@ -452,9 +452,7 @@ spec:
     - frontend.example.com
     - frontend
   http:
-  - appendHeaders:
-      x-some-header: "value"
-    corsPolicy:
+  - corsPolicy:
       allowHeaders:
       - x-some-header
       allowMethods:
@@ -462,6 +460,10 @@ spec:
       allowOrigin:
       - example.com
       maxAge: 24h
+    headers:
+      request:
+        add:
+          x-some-header: "value"
     match:
     - uri:
         prefix: /
diff --git a/docs/gitbook/tutorials/zero-downtime-deployments.md b/docs/gitbook/tutorials/zero-downtime-deployments.md
index e40d8e67..f5479718 100644
--- a/docs/gitbook/tutorials/zero-downtime-deployments.md
+++ b/docs/gitbook/tutorials/zero-downtime-deployments.md
@@ -196,10 +196,10 @@ spec:
     - public-gateway.istio-system.svc.cluster.local
     hosts:
     - app.example.com
-    appendHeaders:
-      x-envoy-upstream-rq-timeout-ms: "15000"
-      x-envoy-max-retries: "10"
-      x-envoy-retry-on: "gateway-error,connect-failure,refused-stream"
+    retries:
+      attempts: 10
+      perTryTimeout: 5s
+      retryOn: "gateway-error,connect-failure,refused-stream"
 ```
 
 When the HPA scales down your app, your users could run into 503 errors.
diff --git a/pkg/apis/istio/v1alpha3/virtual_service.go b/pkg/apis/istio/v1alpha3/virtual_service.go
index e911f8ee..d800142b 100644
--- a/pkg/apis/istio/v1alpha3/virtual_service.go
+++ b/pkg/apis/istio/v1alpha3/virtual_service.go
@@ -330,13 +330,6 @@ type HTTPRoute struct {
 	// for further details about cross origin resource sharing.
 	CorsPolicy *CorsPolicy `json:"corsPolicy,omitempty"`
 
-	// Additional HTTP headers to add before forwarding a request to the
-	// destination service.
-	AppendHeaders map[string]string `json:"appendHeaders,omitempty"`
-
-	// Http headers to remove before returning the response to the caller
-	RemoveResponseHeaders map[string]string `json:"removeResponseHeaders,omitempty"`
-
 	// Header manipulation rules
 	Headers *Headers `json:"headers,omitempty"`
 }
@@ -355,14 +348,14 @@ type Headers struct {
 // HeaderOperations Describes the header manipulations to apply
 type HeaderOperations struct {
 	// Overwrite the headers specified by key with the given values
-	Set map[string]string `json:"set"`
+	Set map[string]string `json:"set,omitempty"`
 
 	// Append the given values to the headers specified by keys
 	// (will create a comma-separated list of values)
-	Add map[string]string `json:"add"`
+	Add map[string]string `json:"add,omitempty"`
 
 	// Remove the specified headers
-	Remove []string `json:"remove"`
+	Remove []string `json:"remove,omitempty"`
 }
 
 // HttpMatchRequest specifies a set of criterion to be met in order for the
@@ -518,7 +511,7 @@ type TCPRoute struct {
 	// activated. All conditions inside a single match block have AND
 	// semantics, while the list of match blocks have OR semantics. The rule
 	// is matched if any one of the match blocks succeed.
-	Match []L4MatchAttributes `json:"match"`
+	Match []L4MatchAttributes `json:"match,omitempty"`
 
 	// The destination to which the connection should be forwarded to.
 	// Currently, only one destination is allowed for TCP services. When TCP
@@ -651,17 +644,17 @@ type HTTPRetry struct {
 	// REQUIRED. Number of retries for a given request. The interval
 	// between retries will be determined automatically (25ms+). Actual
 	// number of retries attempted depends on the httpReqTimeout.
-	Attempts int `json:"attempts"`
+	Attempts int `json:"attempts,omitempty"`
 
 	// Timeout per retry attempt for a given request. format: 1h/1m/1s/1ms. MUST BE >=1ms.
-	PerTryTimeout string `json:"perTryTimeout"`
+	PerTryTimeout string `json:"perTryTimeout,omitempty"`
 
 	// Specifies the conditions under which retry takes place.
 	// One or more policies can be specified using a ‘,’ delimited list.
 	// The supported policies can be found in
 	// <https://www.envoyproxy.io/docs/envoy/latest/configuration/http_filters/router_filter#x-envoy-retry-on>
 	// and <https://www.envoyproxy.io/docs/envoy/latest/configuration/http_filters/router_filter#x-envoy-retry-grpc-on>
-	RetryOn string `json:"retryOn"`
+	RetryOn string `json:"retryOn,omitempty"`
 }
 
 // Describes the Cross-Origin Resource Sharing (CORS) policy, for a given
diff --git a/pkg/apis/istio/v1alpha3/zz_generated.deepcopy.go b/pkg/apis/istio/v1alpha3/zz_generated.deepcopy.go
index 3f7fa490..9447a78e 100644
--- a/pkg/apis/istio/v1alpha3/zz_generated.deepcopy.go
+++ b/pkg/apis/istio/v1alpha3/zz_generated.deepcopy.go
@@ -426,20 +426,6 @@ func (in *HTTPRoute) DeepCopyInto(out *HTTPRoute) {
 		*out = new(CorsPolicy)
 		(*in).DeepCopyInto(*out)
 	}
-	if in.AppendHeaders != nil {
-		in, out := &in.AppendHeaders, &out.AppendHeaders
-		*out = make(map[string]string, len(*in))
-		for key, val := range *in {
-			(*out)[key] = val
-		}
-	}
-	if in.RemoveResponseHeaders != nil {
-		in, out := &in.RemoveResponseHeaders, &out.RemoveResponseHeaders
-		*out = make(map[string]string, len(*in))
-		for key, val := range *in {
-			(*out)[key] = val
-		}
-	}
 	if in.Headers != nil {
 		in, out := &in.Headers, &out.Headers
 		*out = new(Headers)
diff --git a/pkg/router/istio.go b/pkg/router/istio.go
index c8f04849..015698cf 100644
--- a/pkg/router/istio.go
+++ b/pkg/router/istio.go
@@ -141,13 +141,13 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error {
 		Gateways: gateways,
 		Http: []istiov1alpha3.HTTPRoute{
 			{
-				Match:         canary.Spec.Service.Match,
-				Rewrite:       canary.Spec.Service.Rewrite,
-				Timeout:       canary.Spec.Service.Timeout,
-				Retries:       canary.Spec.Service.Retries,
-				CorsPolicy:    canary.Spec.Service.CorsPolicy,
-				AppendHeaders: addHeaders(canary),
-				Route:         canaryRoute,
+				Match:      canary.Spec.Service.Match,
+				Rewrite:    canary.Spec.Service.Rewrite,
+				Timeout:    canary.Spec.Service.Timeout,
+				Retries:    canary.Spec.Service.Retries,
+				CorsPolicy: canary.Spec.Service.CorsPolicy,
+				Headers:    canary.Spec.Service.Headers,
+				Route:      canaryRoute,
 			},
 		},
 	}
@@ -156,21 +156,21 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error {
 		canaryMatch := mergeMatchConditions(canary.Spec.CanaryAnalysis.Match, canary.Spec.Service.Match)
 		newSpec.Http = []istiov1alpha3.HTTPRoute{
 			{
-				Match:         canaryMatch,
-				Rewrite:       canary.Spec.Service.Rewrite,
-				Timeout:       canary.Spec.Service.Timeout,
-				Retries:       canary.Spec.Service.Retries,
-				CorsPolicy:    canary.Spec.Service.CorsPolicy,
-				AppendHeaders: addHeaders(canary),
-				Route:         canaryRoute,
+				Match:      canaryMatch,
+				Rewrite:    canary.Spec.Service.Rewrite,
+				Timeout:    canary.Spec.Service.Timeout,
+				Retries:    canary.Spec.Service.Retries,
+				CorsPolicy: canary.Spec.Service.CorsPolicy,
+				Headers:    canary.Spec.Service.Headers,
+				Route:      canaryRoute,
 			},
 			{
-				Match:         canary.Spec.Service.Match,
-				Rewrite:       canary.Spec.Service.Rewrite,
-				Timeout:       canary.Spec.Service.Timeout,
-				Retries:       canary.Spec.Service.Retries,
-				CorsPolicy:    canary.Spec.Service.CorsPolicy,
-				AppendHeaders: addHeaders(canary),
+				Match:      canary.Spec.Service.Match,
+				Rewrite:    canary.Spec.Service.Rewrite,
+				Timeout:    canary.Spec.Service.Timeout,
+				Retries:    canary.Spec.Service.Retries,
+				CorsPolicy: canary.Spec.Service.CorsPolicy,
+				Headers:    canary.Spec.Service.Headers,
 				Route: []istiov1alpha3.DestinationWeight{
 					makeDestination(canary, primaryName, 100),
 				},
@@ -303,12 +303,12 @@ func (ir *IstioRouter) SetRoutes(
 	// weighted routing (progressive canary)
 	vsCopy.Spec.Http = []istiov1alpha3.HTTPRoute{
 		{
-			Match:         canary.Spec.Service.Match,
-			Rewrite:       canary.Spec.Service.Rewrite,
-			Timeout:       canary.Spec.Service.Timeout,
-			Retries:       canary.Spec.Service.Retries,
-			CorsPolicy:    canary.Spec.Service.CorsPolicy,
-			AppendHeaders: addHeaders(canary),
+			Match:      canary.Spec.Service.Match,
+			Rewrite:    canary.Spec.Service.Rewrite,
+			Timeout:    canary.Spec.Service.Timeout,
+			Retries:    canary.Spec.Service.Retries,
+			CorsPolicy: canary.Spec.Service.CorsPolicy,
+			Headers:    canary.Spec.Service.Headers,
 			Route: []istiov1alpha3.DestinationWeight{
 				makeDestination(canary, primaryName, primaryWeight),
 				makeDestination(canary, canaryName, canaryWeight),
@@ -328,24 +328,24 @@ func (ir *IstioRouter) SetRoutes(
 		canaryMatch := mergeMatchConditions(canary.Spec.CanaryAnalysis.Match, canary.Spec.Service.Match)
 		vsCopy.Spec.Http = []istiov1alpha3.HTTPRoute{
 			{
-				Match:         canaryMatch,
-				Rewrite:       canary.Spec.Service.Rewrite,
-				Timeout:       canary.Spec.Service.Timeout,
-				Retries:       canary.Spec.Service.Retries,
-				CorsPolicy:    canary.Spec.Service.CorsPolicy,
-				AppendHeaders: addHeaders(canary),
+				Match:      canaryMatch,
+				Rewrite:    canary.Spec.Service.Rewrite,
+				Timeout:    canary.Spec.Service.Timeout,
+				Retries:    canary.Spec.Service.Retries,
+				CorsPolicy: canary.Spec.Service.CorsPolicy,
+				Headers:    canary.Spec.Service.Headers,
 				Route: []istiov1alpha3.DestinationWeight{
 					makeDestination(canary, primaryName, primaryWeight),
 					makeDestination(canary, canaryName, canaryWeight),
 				},
 			},
 			{
-				Match:         canary.Spec.Service.Match,
-				Rewrite:       canary.Spec.Service.Rewrite,
-				Timeout:       canary.Spec.Service.Timeout,
-				Retries:       canary.Spec.Service.Retries,
-				CorsPolicy:    canary.Spec.Service.CorsPolicy,
-				AppendHeaders: addHeaders(canary),
+				Match:      canary.Spec.Service.Match,
+				Rewrite:    canary.Spec.Service.Rewrite,
+				Timeout:    canary.Spec.Service.Timeout,
+				Retries:    canary.Spec.Service.Retries,
+				CorsPolicy: canary.Spec.Service.CorsPolicy,
+				Headers:    canary.Spec.Service.Headers,
 				Route: []istiov1alpha3.DestinationWeight{
 					makeDestination(canary, primaryName, primaryWeight),
 				},
@@ -361,18 +361,6 @@ func (ir *IstioRouter) SetRoutes(
 	return nil
 }
 
-// addHeaders applies headers before forwarding a request to the destination service
-// compatible with Istio 1.0.x and 1.1.0
-func addHeaders(canary *flaggerv1.Canary) (headers map[string]string) {
-	if canary.Spec.Service.Headers != nil &&
-		canary.Spec.Service.Headers.Request != nil &&
-		len(canary.Spec.Service.Headers.Request.Add) > 0 {
-		headers = canary.Spec.Service.Headers.Request.Add
-	}
-
-	return
-}
-
 // mergeMatchConditions appends the URI match rules to canary conditions
 func mergeMatchConditions(canary, defaults []istiov1alpha3.HTTPMatchRequest) []istiov1alpha3.HTTPMatchRequest {
 	for i := range canary {


Test patch:
diff --git a/pkg/router/istio_test.go b/pkg/router/istio_test.go
index afca0d4b..a917237b 100644
--- a/pkg/router/istio_test.go
+++ b/pkg/router/istio_test.go
@@ -335,10 +335,20 @@ func TestIstioRouter_HTTPRequestHeaders(t *testing.T) {
 		t.Fatalf("Got HTTPRoute %v wanted %v", len(vs.Spec.Http), 1)
 	}
 
-	timeout := vs.Spec.Http[0].AppendHeaders["x-envoy-upstream-rq-timeout-ms"]
+	timeout := vs.Spec.Http[0].Headers.Request.Add["x-envoy-upstream-rq-timeout-ms"]
 	if timeout != "15000" {
 		t.Errorf("Got timeout %v wanted %v", timeout, "15000")
 	}
+
+	reqRemove := vs.Spec.Http[0].Headers.Request.Remove[0]
+	if reqRemove != "test" {
+		t.Errorf("Got Headers.Request.Remove %v wanted %v", reqRemove, "test")
+	}
+
+	resRemove := vs.Spec.Http[0].Headers.Response.Remove[0]
+	if resRemove != "token" {
+		t.Errorf("Got Headers.Response.Remove %v wanted %v", reqRemove, "token")
+	}
 }
 
 func TestIstioRouter_CORS(t *testing.T) {
diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go
index 45c61a14..9e4ad3ef 100644
--- a/pkg/router/router_test.go
+++ b/pkg/router/router_test.go
@@ -87,6 +87,10 @@ func newTestCanary() *flaggerv1.Canary {
 						Add: map[string]string{
 							"x-envoy-upstream-rq-timeout-ms": "15000",
 						},
+						Remove: []string{"test"},
+					},
+					Response: &istiov1alpha3.HeaderOperations{
+						Remove: []string{"token"},
 					},
 				},
 				CorsPolicy: &istiov1alpha3.CorsPolicy{
diff --git a/test/goreleaser.sh b/test/goreleaser.sh
index 88d9151a..41a30578 100755
--- a/test/goreleaser.sh
+++ b/test/goreleaser.sh
@@ -3,8 +3,6 @@ set -e
 
 TAR_FILE="/tmp/goreleaser.tar.gz"
 RELEASES_URL="https://github.com/goreleaser/goreleaser/releases"
-GH_REL_DIR="github-release-notes-linux-amd64-0.2.0"
-GH_REL_URL="https://github.com/buchanae/github-release-notes/releases/download/0.2.0/${GH_REL_DIR}.tar.gz"
 test -z "$TMPDIR" && TMPDIR="$(mktemp -d)"
 
 last_version() {
@@ -23,10 +21,8 @@ download() {
   rm -f "$TAR_FILE"
   curl -s -L -o "$TAR_FILE" \
     "$RELEASES_URL/download/$VERSION/goreleaser_$(uname -s)_$(uname -m).tar.gz"
-
-  curl -sSL ${GH_REL_URL} | tar xz && sudo mv ${GH_REL_DIR}/github-release-notes /usr/local/bin/ && rm -rf ${GH_REL_DIR}
 }
 
 download
 tar -xf "$TAR_FILE" -C "$TMPDIR"
-"${TMPDIR}/goreleaser" --release-notes <(github-release-notes -org weaveworks -repo flagger -since-latest-release)
+"${TMPDIR}/goreleaser" "$@"