From c74e59fa4c001c7d75d6f1f307ccc8cf6817c1ea Mon Sep 17 00:00:00 2001 From: liuwei Date: Fri, 2 Nov 2018 17:05:38 +0800 Subject: [PATCH] Use second as cookie expires unit --- .../annotations/sessionaffinity/main.go | 14 ++- .../annotations/sessionaffinity/main_test.go | 4 +- rootfs/etc/nginx/lua/balancer/sticky.lua | 33 +------ .../nginx/lua/test/balancer/sticky_test.lua | 87 +------------------ test/e2e/annotations/affinity.go | 33 ++++++- test/e2e/lua/dynamic_configuration.go | 25 +----- 6 files changed, 45 insertions(+), 151 deletions(-) diff --git a/internal/ingress/annotations/sessionaffinity/main.go b/internal/ingress/annotations/sessionaffinity/main.go index 35ae09335..09599b6b5 100644 --- a/internal/ingress/annotations/sessionaffinity/main.go +++ b/internal/ingress/annotations/sessionaffinity/main.go @@ -40,9 +40,8 @@ const ( annotationAffinityCookieHash = "session-cookie-hash" defaultAffinityCookieHash = "md5" - // This is used to control the cookie expires, its value is a time period, the cookie will - // not expire during the time period. e.g. 1h (1 hour) the validated time unit includes: - // y, M, w, d, h, m and s + // This is used to control the cookie expires, its value is a number of seconds until the + // cookie expires annotationAffinityCookieExpires = "session-cookie-expires" // This is used to control the cookie expires, its value is a number of seconds until the @@ -52,8 +51,7 @@ const ( var ( affinityCookieHashRegex = regexp.MustCompile(`^(index|md5|sha1)$`) - affinityCookieExpiresRegex = regexp.MustCompile(`^(\d+(y|M|w|d|h|m|s))$`) - affinityCookieMaxAgeRegex = regexp.MustCompile(`(^0|-?[1-9]\d*$)`) + affinityCookieExpiresRegex = regexp.MustCompile(`(^0|-?[1-9]\d*$)`) ) // Config describes the per ingress session affinity config @@ -69,7 +67,7 @@ type Cookie struct { Name string `json:"name"` // The hash that will be used to encode the cookie in case of cookie affinity type Hash string `json:"hash"` - // The time period to control cookie expires + // The time duration to control cookie expires Expires string `json:"expires"` // The number of seconds until the cookie expires MaxAge string `json:"maxage"` @@ -96,14 +94,14 @@ func (a affinity) cookieAffinityParse(ing *extensions.Ingress) *Cookie { cookie.Hash = sh se, err := parser.GetStringAnnotation(annotationAffinityCookieExpires, ing) - if err != nil && !affinityCookieExpiresRegex.MatchString(se) { + if err != nil || !affinityCookieExpiresRegex.MatchString(se) { glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieExpires) se = "" } cookie.Expires = se sm, err := parser.GetStringAnnotation(annotationAffinityCookieMaxAge, ing) - if err != nil && !affinityCookieMaxAgeRegex.MatchString(sm) { + if err != nil || !affinityCookieExpiresRegex.MatchString(sm) { glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Ignoring it", ing.Name, annotationAffinityCookieMaxAge) sm = "" } diff --git a/internal/ingress/annotations/sessionaffinity/main_test.go b/internal/ingress/annotations/sessionaffinity/main_test.go index d00e41c20..a3e89d662 100644 --- a/internal/ingress/annotations/sessionaffinity/main_test.go +++ b/internal/ingress/annotations/sessionaffinity/main_test.go @@ -69,7 +69,7 @@ func TestIngressAffinityCookieConfig(t *testing.T) { data[parser.GetAnnotationWithPrefix(annotationAffinityType)] = "cookie" data[parser.GetAnnotationWithPrefix(annotationAffinityCookieHash)] = "sha123" data[parser.GetAnnotationWithPrefix(annotationAffinityCookieName)] = "INGRESSCOOKIE" - data[parser.GetAnnotationWithPrefix(annotationAffinityCookieExpires)] = "1h" + data[parser.GetAnnotationWithPrefix(annotationAffinityCookieExpires)] = "4500" data[parser.GetAnnotationWithPrefix(annotationAffinityCookieMaxAge)] = "3000" ing.SetAnnotations(data) @@ -91,7 +91,7 @@ func TestIngressAffinityCookieConfig(t *testing.T) { t.Errorf("expected route as sticky-name but returned %v", nginxAffinity.Cookie.Name) } - if nginxAffinity.Cookie.Expires != "1h" { + if nginxAffinity.Cookie.Expires != "4500" { t.Errorf("expected 1h as sticky-expires but returned %v", nginxAffinity.Cookie.Expires) } diff --git a/rootfs/etc/nginx/lua/balancer/sticky.lua b/rootfs/etc/nginx/lua/balancer/sticky.lua index fd1af8ed3..15f3f3971 100644 --- a/rootfs/etc/nginx/lua/balancer/sticky.lua +++ b/rootfs/etc/nginx/lua/balancer/sticky.lua @@ -33,32 +33,6 @@ local function encrypted_endpoint_string(self, endpoint_string) return encrypted end -local function parse_cookie_expires(expires) - local time = tonumber(string.sub(expires, 0, string.len(expires) - 1)) - if time == nil then - return nil, string.format("the time of expires (%s) is wrong", expires) - end - - local unit = string.sub(expires, -1) - if unit == "y" then - return time * 60 * 60 * 24 * 365 - elseif unit == "M" then - return time * 60 * 60 * 24 * 30 - elseif unit == "w" then - return time * 60 * 60 * 24 * 7 - elseif unit == "d" then - return time * 60 * 60 * 24 - elseif unit == "h" then - return time * 60 * 60 - elseif unit == "m" then - return time * 60 - elseif unit == "s" then - return time - else - return nil, string.format("the unit of expires (%s) is wrong", expires) - end -end - local function set_cookie(self, value) local cookie, err = ck:new() if not cookie then @@ -75,12 +49,7 @@ local function set_cookie(self, value) local expires if self.cookie_expires and self.cookie_expires ~= "" then - expires, err = parse_cookie_expires(self.cookie_expires) - if err then - ngx.log(ngx.WARN, string.format("error when parsing cookie expires: %s, ignoring it", tostring(err))) - else - cookie_data.expires = ngx.cookie_time(expires) - end + cookie_data.expires = ngx.cookie_time(tonumber(self.cookie_expires)) end if self.cookie_max_age and self.cookie_max_age ~= "" then diff --git a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua index a6b4652ad..c38773885 100644 --- a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua @@ -132,91 +132,6 @@ describe("Sticky", function() assert.has_no.errors(function() sticky_balancer_instance:balance() end) assert.spy(s).was_called() end) - - it("sets an expires cookie (with expires) on the client", function() - local s = {} - local expected_expires - cookie.new = function(self) - local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash - local cookie_instance = { - set = function(self, payload) - assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name) - assert.equal(payload.value, util[test_backend_hash_fn .. "_digest"](test_backend_endpoint)) - assert.equal(payload.path, ngx.var.location_path) - assert.equal(payload.domain, nil) - assert.equal(payload.httponly, true) - expected_expires = payload.expires - return true, nil - end, - get = function(k) return false end, - } - s = spy.on(cookie_instance, "set") - return cookie_instance, false - end - local temp_backend = util.deepcopy(get_test_backend()) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "1y" - local sticky_balancer_instance = sticky:new(temp_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - assert.equal(expected_expires, ngx.cookie_time(1 * 60 * 60 * 24 * 365)) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "10M" - sticky_balancer_instance = sticky:new(temp_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - assert.equal(expected_expires, ngx.cookie_time(10 * 60 * 60 * 24 * 30)) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "8w" - sticky_balancer_instance = sticky:new(temp_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - assert.equal(expected_expires, ngx.cookie_time(8 * 60 * 60 * 24 * 7)) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "200d" - sticky_balancer_instance = sticky:new(temp_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - assert.equal(expected_expires, ngx.cookie_time(200 * 60 * 60 * 24)) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "24h" - sticky_balancer_instance = sticky:new(temp_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - assert.equal(expected_expires, ngx.cookie_time(24 * 60 * 60)) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "30m" - sticky_balancer_instance = sticky:new(temp_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - assert.equal(expected_expires, ngx.cookie_time(30 * 60)) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.expires = "300s" - sticky_balancer_instance = sticky:new(temp_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - assert.equal(expected_expires, ngx.cookie_time(300)) - end) - - it("sets an expires cookie (with max-age) on the client", function() - local s = {} - local cookie_payload = {} - cookie.new = function(self) - local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash - local cookie_instance = { - set = function(self, payload) - assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name) - assert.equal(payload.value, util[test_backend_hash_fn .. "_digest"](test_backend_endpoint)) - assert.equal(payload.path, ngx.var.location_path) - assert.equal(payload.domain, nil) - assert.equal(payload.httponly, true) - assert.equal(payload.max_age, 1000) - return true, nil - end, - get = function(k) return false end, - } - s = spy.on(cookie_instance, "set") - return cookie_instance, false - end - local temp_backend = util.deepcopy(get_test_backend()) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.maxage = "1000" - local sticky_balancer_instance = sticky:new(temp_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - end) end) context("when client has a cookie set", function() @@ -242,4 +157,4 @@ describe("Sticky", function() end) end) end) -end) +end) \ No newline at end of file diff --git a/test/e2e/annotations/affinity.go b/test/e2e/annotations/affinity.go index 5903f7e15..4b8c028db 100644 --- a/test/e2e/annotations/affinity.go +++ b/test/e2e/annotations/affinity.go @@ -20,6 +20,7 @@ import ( "fmt" "net/http" "strings" + "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -28,7 +29,6 @@ import ( "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" ) @@ -157,4 +157,35 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity/Sticky Sessions", Expect(resp.StatusCode).Should(Equal(http.StatusOK)) Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Path=/somewhereelese;")) }) + + It("should set sticky cookie with expires", func() { + host := "sticky.foo.com" + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/affinity": "cookie", + "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", + "nginx.ingress.kubernetes.io/session-cookie-expires": "172800", + "nginx.ingress.kubernetes.io/session-cookie-max-age": "259200", + } + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) + }) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + + Expect(len(errs)).Should(BeNumerically("==", 0)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + local, _ := time.LoadLocation("GMT") + duration, _ := time.ParseDuration("48h") + want := time.Date(1970, time.January, 1, 0, 0, 0, 0, local).Add(duration).Format("Mon, 02-Jan-06 15:04:05 MST") + Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring(fmt.Sprintf("Expires=%s", want))) + Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Max-Age=259200")) + }) }) diff --git a/test/e2e/lua/dynamic_configuration.go b/test/e2e/lua/dynamic_configuration.go index 74ddf428b..38bb9ffd2 100644 --- a/test/e2e/lua/dynamic_configuration.go +++ b/test/e2e/lua/dynamic_configuration.go @@ -149,31 +149,12 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() { }) Expect(nginxConfig).ShouldNot(Equal(newNginxConfig)) }) - - It("should set sticky cookie correctly", func() { - resp, _, errs := gorequest.New(). - Get(f.IngressController.HTTPURL). - Set("Host", "foo.com"). - End() - - Expect(len(errs)).Should(BeNumerically("==", 0)) - Expect(resp.StatusCode).Should(Equal(http.StatusOK)) - Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("SERVERID=")) - Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Expires=")) - Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("Max-Age=172800")) - }) }) func ensureIngress(f *framework.Framework, host string) *extensions.Ingress { - ing, err := f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, - &map[string]string{ - "nginx.ingress.kubernetes.io/load-balance": "ewma", - "nginx.ingress.kubernetes.io/affinity": "cookie", - "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", - "nginx.ingress.kubernetes.io/session-cookie-expires": "2d", - "nginx.ingress.kubernetes.io/session-cookie-max-age": "172800"})) - Expect(err).NotTo(HaveOccurred()) - Expect(ing).NotTo(BeNil()) + ing := f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, + &map[string]string{"nginx.ingress.kubernetes.io/load-balance": "ewma"})) + f.WaitForNginxServer(host, func(server string) bool { return strings.Contains(server, fmt.Sprintf("server_name %s ;", host)) &&