From f48d134469bef08d1474409fe7103f9e8f23ea28 Mon Sep 17 00:00:00 2001 From: Jian Qiu Date: Mon, 30 Oct 2017 13:33:28 +0800 Subject: [PATCH 1/2] Add websocket-services annotation This PR is to add websocket-services annotation, user can specify a comma separated services list in the annotation and websocket is enabled on these backend services. --- internal/ingress/annotations/annotations.go | 3 + .../ingress/annotations/annotations_test.go | 22 +++++ .../ingress/annotations/websocket/main.go | 46 ++++++++++ .../annotations/websocket/main_test.go | 87 +++++++++++++++++++ internal/ingress/controller/controller.go | 2 + internal/ingress/types.go | 3 + internal/ingress/types_equals.go | 3 + rootfs/etc/nginx/template/nginx.tmpl | 12 ++- 8 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 internal/ingress/annotations/websocket/main.go create mode 100644 internal/ingress/annotations/websocket/main_test.go diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index b6064de6b..812242e7a 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -47,6 +47,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/upstreamhashby" "k8s.io/ingress-nginx/internal/ingress/annotations/upstreamvhost" "k8s.io/ingress-nginx/internal/ingress/annotations/vtsfilterkey" + "k8s.io/ingress-nginx/internal/ingress/annotations/websocket" "k8s.io/ingress-nginx/internal/ingress/annotations/xforwardedprefix" "k8s.io/ingress-nginx/internal/ingress/errors" "k8s.io/ingress-nginx/internal/ingress/resolver" @@ -83,6 +84,7 @@ type Ingress struct { VtsFilterKey string Whitelist ipwhitelist.SourceRange XForwardedPrefix bool + EnableWebSocket bool } // Extractor defines the annotation parsers to be used in the extraction of annotations @@ -118,6 +120,7 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor { "VtsFilterKey": vtsfilterkey.NewParser(cfg), "Whitelist": ipwhitelist.NewParser(cfg), "XForwardedPrefix": xforwardedprefix.NewParser(cfg), + "EnableWebSocket": websocket.NewParser(cfg), }, } } diff --git a/internal/ingress/annotations/annotations_test.go b/internal/ingress/annotations/annotations_test.go index 4657de41e..85823efbe 100644 --- a/internal/ingress/annotations/annotations_test.go +++ b/internal/ingress/annotations/annotations_test.go @@ -46,6 +46,7 @@ var ( annotationAffinityCookieName = parser.GetAnnotationWithPrefix("session-cookie-name") annotationAffinityCookieHash = parser.GetAnnotationWithPrefix("session-cookie-hash") annotationUpstreamHashBy = parser.GetAnnotationWithPrefix("upstream-hash-by") + annotationEnableWebSocket = parser.GetAnnotationWithPrefix("enable-websocket") ) type mockCfg struct { @@ -328,6 +329,27 @@ func TestCors(t *testing.T) { } } +func TestEnableWebSocket(t *testing.T) { + ec := NewAnnotationExtractor(mockCfg{}) + ing := buildIngress() + + fooAnns := []struct { + annotations map[string]string + er bool + }{ + {map[string]string{annotationEnableWebSocket: "true"}, true}, + {map[string]string{annotationEnableWebSocket: "false"}, false}, + } + + for _, foo := range fooAnns { + ing.SetAnnotations(foo.annotations) + r := ec.Extract(ing).EnableWebSocket + if r != foo.er { + t.Errorf("Returned %v but expected %v", r, foo.er) + } + } +} + /* func TestMergeLocationAnnotations(t *testing.T) { // initial parameters diff --git a/internal/ingress/annotations/websocket/main.go b/internal/ingress/annotations/websocket/main.go new file mode 100644 index 000000000..3c41a7aef --- /dev/null +++ b/internal/ingress/annotations/websocket/main.go @@ -0,0 +1,46 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package websocket + +import ( + extensions "k8s.io/api/extensions/v1beta1" + + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +type websocket struct { + r resolver.Resolver +} + +// NewParser creates a new Alias annotation parser +func NewParser(r resolver.Resolver) parser.IngressAnnotation { + return websocket{r} +} + +// Parse parses the annotations contained in the ingress rule +// used to add an alias to the provided hosts +func (a websocket) Parse(ing *extensions.Ingress) (interface{}, error) { + enabled, err := parser.GetBoolAnnotation("enable-websocket", ing) + + // If annotation is not set, enable websocket by default + if err != nil { + return true, err + } + + return enabled, nil +} diff --git a/internal/ingress/annotations/websocket/main_test.go b/internal/ingress/annotations/websocket/main_test.go new file mode 100644 index 000000000..7321c3b97 --- /dev/null +++ b/internal/ingress/annotations/websocket/main_test.go @@ -0,0 +1,87 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package websocket + +import ( + "testing" + + api "k8s.io/api/core/v1" + extensions "k8s.io/api/extensions/v1beta1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +func TestParse(t *testing.T) { + annotation := parser.GetAnnotationWithPrefix("enable-websocket") + ap := NewParser(&resolver.Mock{}) + if ap == nil { + t.Fatalf("expected a parser.IngressAnnotation but returned nil") + } + + testCases := []struct { + annotations map[string]string + expected bool + }{ + {map[string]string{annotation: "true"}, true}, + {map[string]string{annotation: "false"}, false}, + {map[string]string{annotation: ""}, true}, + {map[string]string{}, true}, + {nil, true}, + } + + ing := &extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: "foo.bar.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/foo", + Backend: extensions.IngressBackend{ + ServiceName: "test1", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + }, + }, + } + + for _, testCase := range testCases { + ing.SetAnnotations(testCase.annotations) + result, _ := ap.Parse(ing) + ws, ok := result.(bool) + if !ok { + t.Errorf("expected a Config type") + } + + if !ws == testCase.expected { + t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, ws, testCase.annotations) + } + } +} diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 5d5f15d02..010da83a7 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -426,6 +426,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] loc.Denied = anns.Denied loc.XForwardedPrefix = anns.XForwardedPrefix loc.UsePortInRedirects = anns.UsePortInRedirects + loc.EnableWebSocket = anns.EnableWebSocket if loc.Redirect.FromToWWW { server.RedirectFromToWWW = true @@ -458,6 +459,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([] Denied: anns.Denied, XForwardedPrefix: anns.XForwardedPrefix, UsePortInRedirects: anns.UsePortInRedirects, + EnableWebSocket: anns.EnableWebSocket, } if loc.Redirect.FromToWWW { diff --git a/internal/ingress/types.go b/internal/ingress/types.go index b58856d05..cbe9f829b 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -251,6 +251,9 @@ type Location struct { // original location. // +optional XForwardedPrefix bool `json:"xForwardedPrefix,omitempty"` + // EnableWebSocket indicates if websocket is enabled for the location + // +optional + EnableWebSocket bool `json:"webSocket,omitempty"` } // SSLPassthroughBackend describes a SSL upstream server configured diff --git a/internal/ingress/types_equals.go b/internal/ingress/types_equals.go index a5902646a..27a8921b4 100644 --- a/internal/ingress/types_equals.go +++ b/internal/ingress/types_equals.go @@ -370,6 +370,9 @@ func (l1 *Location) Equal(l2 *Location) bool { if l1.XForwardedPrefix != l2.XForwardedPrefix { return false } + if l1.EnableWebSocket != l2.EnableWebSocket { + return false + } return true } diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index c9f1323dc..9db4d3a4e 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -119,7 +119,7 @@ http { include /etc/nginx/mime.types; default_type text/html; - + {{ if $cfg.EnableBrotli }} brotli on; brotli_comp_level {{ $cfg.BrotliLevel }}; @@ -181,7 +181,11 @@ http { # Retain the default nginx handling of requests without a "Connection" header map $http_upgrade $connection_upgrade { default upgrade; + {{ if (and (not ($location.EnableWebSocket)) (gt $all.Cfg.UpstreamKeepaliveConnections 0)) }} + '' keep-alive + {{ else }} '' close; + {{ end }} } map {{ buildForwardedFor $cfg.ForwardedForHeader }} $the_real_ip { @@ -292,7 +296,7 @@ http { {{ range $header := $cfg.HideHeaders }}proxy_hide_header {{ $header }}; {{ end }} - + {{ if not (empty $cfg.HTTPSnippet) }} # Custom code snippet configured in the configuration configmap {{ $cfg.HTTPSnippet }} @@ -695,10 +699,10 @@ stream { {{/* redirect to HTTPS can be achieved forcing the redirect or having a SSL Certificate configured for the server */}} {{ if (or $location.Rewrite.ForceSSLRedirect (and (not (empty $server.SSLCertificate)) $location.Rewrite.SSLRedirect)) }} # enforce ssl on server side - if ($redirect_to_https) { + if ($redirect_to_https) { {{ if $location.UsePortInRedirects }} # using custom ports require a different rewrite directive - {{ $redirect_port := (printf ":%v" $all.ListenPorts.HTTPS) }} + {{ $redirect_port := (printf ":%v" $all.ListenPorts.HTTPS) }} error_page 497 ={{ $all.Cfg.HTTPRedirectCode }} https://$host{{ $redirect_port }}$request_uri; return 497; From 07a34bfaf8cae779a6a4fc1a9d5dd5599039d8c5 Mon Sep 17 00:00:00 2001 From: Jian Qiu Date: Mon, 22 Jan 2018 13:45:58 +0800 Subject: [PATCH 2/2] Add e2d test case --- test/e2e/annotations/websocket.go | 141 ++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 test/e2e/annotations/websocket.go diff --git a/test/e2e/annotations/websocket.go b/test/e2e/annotations/websocket.go new file mode 100644 index 000000000..e54c2bf69 --- /dev/null +++ b/test/e2e/annotations/websocket.go @@ -0,0 +1,141 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package annotations + +import ( + "net/http" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/parnurzeal/gorequest" + + v1beta1 "k8s.io/api/extensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.IngressNginxDescribe("Annotations - EnableWebSocket", func() { + f := framework.NewDefaultFramework("websocket") + + BeforeEach(func() { + err := f.NewEchoDeployment() + Expect(err).NotTo(HaveOccurred()) + }) + + AfterEach(func() { + }) + + It("should set 'Close' in connection header for host 'foo'", func() { + host := "foo" + + ing, err := f.EnsureIngress(&v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: host, + Namespace: f.Namespace.Name, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: host, + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Path: "/", + Backend: v1beta1.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + }, + }, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(ing).NotTo(BeNil()) + + err = f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("'' close")) + }) + Expect(err).NotTo(HaveOccurred()) + + resp, _, errs := gorequest.New(). + Get(f.NginxHTTPURL). + Set("Host", host). + End() + + Expect(len(errs)).Should(BeNumerically("==", 0)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + }) + + It("should set 'keep-alive' in connection header for host 'foo'", func() { + host := "foo" + ing, err := f.EnsureIngress(&v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: host, + Namespace: f.Namespace.Name, + Annotations: map[string]string{ + "nginx.ingress.kubernetes.io/enable-websocket": "false", + }, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: host, + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Path: "/", + Backend: v1beta1.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + }, + }, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(ing).NotTo(BeNil()) + + err = f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("'' keep-alive")) + }) + Expect(err).NotTo(HaveOccurred()) + + resp, _, errs := gorequest.New(). + Get(f.NginxHTTPURL). + Set("Host", host). + End() + + Expect(len(errs)).Should(BeNumerically("==", 0)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + }) +})