Merge pull request #442 from weaveworks/header-ops

Use header operations in Istio router

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

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.

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

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)

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 {

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

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{