-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Session affinity using cookie does not work when "path" is not set in the Ingress - Wrong generated Nginx configuration #1980
Comments
Hitting the same issue, seems to be related to TLS. My two TLS ingresses work with sticky sessions, a new one without doesn't. |
Nevermind, works on 0.10.2, just be sure to actually have more than one endpoint for testing (it doesn't send out cookies if you don't). |
It does not work with 0.10.2 too. Verification : IngressController has been updated to v0.10.2:
Verification : 2 endpoints are set for the service:
Verification : Nginx configuration is still wrong. Upstream configuration "sticky-default-echo-server-8080" is never referenced in the server "echo-server" block.
|
Hello, It works for me from 0.10.2. Jeff |
@jfpucheu : Jeff, does it work when you apply my sample configuration (just change If it works, what is your environment (Cloud provider VS hardware configuration ? OS ? Install tools) and Kubernetes version? |
I too am consistently running into this issue. In my scenario it appears the Though all configurations seem correct, a cookie is no longer returned. To ensure the cookie was not simply being dropped by an intermediary, I've evaluated a At this point I'm running out of ideas, maybe a possible load order?, maybe even a compilation/build specific issue with the plugin? Thoughts? nginx configuration http {
...
upstream sticky-stage-1-s1cas-cas-8080 {
sticky hash=md5 name=INGRESSCOOKIE httponly;
keepalive 32;
server 10.44.69.2:80 max_fails=0 fail_timeout=0;
}
...
server {
...
set $proxy_upstream_name "-";
...
location / {
port_in_redirect off;
set $proxy_upstream_name "sticky-stage-1-s1cas-cas-8080";
set $namespace "stage-1";
set $ingress_name "s1cas-cas";
set $service_name "s1cas-cas";
... console logs $ kubectl --namespace stage-1 logs -f --tail 100 s1cashttps-nginx-ingress-controller-759b6c89cc-glcjm`
73.xxx.xxx.xxx - [73.xxx.xxx.xxx] - - [31/Jan/2018:23:11:59 +0000] "GET /login HTTP/2.0" 200 3424 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.119 Safari/537.36" 281 0.008 [sticky-stage-1-s1cas-cas-8080] 10.44.69.2:80 3424 0.008 200
73.xxx.xxx.xxx - [73.xxx.xxx.xxx] - - [31/Jan/2018:23:12:01 +0000] "GET /login HTTP/2.0" 200 3410 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.119 Safari/537.36" 22 0.007 [sticky-stage-1-s1cas-cas-8080] 10.44.69.2:80 3410 0.007 200 curl from within the pod $ kubectl --namespace stage-1 exec -it s1cashttps-nginx-ingress-controller-759b6c89cc-glcjm bash
$ curl -o /dev/null --http1.1 --resolve accounts.domain.tld:443:127.0.0.1:443 https://accounts.domain.tld/login -v
* Server certificate:
...
* issuer: C=GB; ST=Greater Manchester; L=Salford; O=COMODO CA Limited; CN=COMODO RSA Domain Validation Secure Server CA
* SSL certificate verify ok.
} [5 bytes data]
> GET /login HTTP/1.1
> Host: accounts.domain.tld
> User-Agent: curl/7.52.1
> Accept: */*
>
{ [5 bytes data]
< HTTP/1.1 200 OK
< Date: Wed, 31 Jan 2018 23:40:07 GMT
< Content-Type: text/html;charset=utf-8
< Content-Length: 8023
< Connection: keep-alive
< Vary: Accept-Encoding
< Pragma: no-cache
< Expires: Thu, 01 Jan 1970 00:00:00 GMT
< Cache-Control: no-cache
< Cache-Control: no-store
< Set-Cookie: JSESSIONID=mpw7r01ef0y4i03jzon87mt;Path=/;Secure;HttpOnly
< Vary: Accept-Encoding
< Strict-Transport-Security: max-age=15724800;
<
{ [3684 bytes data]
* Curl_http_done: called premature == 0
100 8023 100 8023 0 0 144k 0 --:--:-- --:--:-- --:--:-- 145k
* Connection #0 to host accounts.domain.tld left intact |
@icereval The cookie is only being sent if you have more than 1 endpoint. You only have one. |
@lorenz, that makes sense! I've retested w/ the replicas back at their normal size, and its working as expected now. I had initially turned the replicas down to simplify troubleshooting, well apparently too far, oops... Thanks for the quick response, and I can confirm 0.10.2 is working w/ my simple test setup. $ curl -o /dev/null -s --location -D - https://account.domain.tls/login
HTTP/2 200
server: nginx
date: Thu, 01 Feb 2018 02:32:55 GMT
content-type: text/html;charset=utf-8
content-length: 8043
vary: Accept-Encoding
set-cookie: INGRESSCOOKIE=26b7af4429c0e7f7b19058dfb72886d0; Path=/; HttpOnly
pragma: no-cache
expires: Thu, 01 Jan 1970 00:00:00 GMT
cache-control: no-cache
cache-control: no-store
set-cookie: JSESSIONID=1584kdt4d4867sea3d6d7uhnl;Path=/;Secure;HttpOnly
vary: Accept-Encoding
strict-transport-security: max-age=15724800; |
Finally, I find why my sample configuration does not work! I confirm there is a bug with the Nginx IngressController. The issue is related to the definition of the Ingress and the absence of the directive "path:". Hereby 2 Ingresses that illustrate the different behaviors : ingress-without-path.yml (sticky session does not work):
ingress-without-path.yml (sticky session works):
As you can see, the only difference is the presence of the directive 'path: /' in the second one:
First, let's use the Ingress without the "path" directive:
As we can see, there is no "Set-Cookie" header in the response:
Now, let's use the Ingress with the "path" directive:
The "Set-Cookie" header is correcly set!
Referring to the official Kubernetes documentation (https://kubernetes.io/docs/concepts/services-networking/ingress/#name-based-virtual-hosting), it should be possible to define an Ingress without the "path" directive :
That's why my Ingress ingress-without-path.yml is valid. And so, the bug is located in the "ingress-nginx" project. |
If the origin ingress rule has no field `path`, the default value will be an empty string which will cause issues when rendering template as other place will use `/` as the default value. Set the default value of path to `/` when retrieve ingress rules from api-server. Thie will fix kubernetes#1980
If the origin ingress rule has no field `path`, the default value will be an empty string which will cause issues when rendering template as other place will use `/` as the default value. Set the default value of path to `/` when retrieve ingress rules from api-server. Thie will fix #1980
Thank you for finding this bug! I can finally run a multi-pod cluster with meteor node framework without having problems with image uploads. Images where being uploaded to all nodes but now with session affinity I am without problems. |
* Correct typo (kubernetes#2238) * correct spelling * correct typo * fix-link (kubernetes#2239) * Add missing configuration in kubernetes#2235 (kubernetes#2236) * to kubernetes (kubernetes#2240) to kubernetes * fix: cannot set $service_name if use rewrite (kubernetes#2220) $path here is the regular expression formatted nginx location not the origin path in ingress rules. Fix kubernetes#2131 * Revert "Get file max from fs/file-max. (kubernetes#2050)" (kubernetes#2241) This reverts commit d8efd39. * add http/2 * fix: empty ingress path (kubernetes#2244) If the origin ingress rule has no field `path`, the default value will be an empty string which will cause issues when rendering template as other place will use `/` as the default value. Set the default value of path to `/` when retrieve ingress rules from api-server. Thie will fix kubernetes#1980 * Fix grpc json tag name (kubernetes#2246) * Add EWMA as configurable load balancing algorithm (kubernetes#2229) * Update go dependencies (kubernetes#2234) * Add deployment docs for AWS NLB (kubernetes#1785) * Update annotations.md (kubernetes#2255) a typo fix * Update README.md (kubernetes#2267) It should be "your Ingress targets" in line 7. * Managing a whitelist for _/nginx_status (kubernetes#2187) Signed-off-by: Sylvain Rabot <s.rabot@lectra.com> * Revert deleted assignment in kubernetes#2146 (kubernetes#2270) * Use SharedIndexInformers in place of Informers (kubernetes#2271) * clean up tmpl (kubernetes#2263) The nginx.conf generated now is too messy remove some section only useful when dynamic configure enabled and headers only useful for https. * Disable opentracing for nginx internal urls (kubernetes#2272) * Typo fixes in modsecurity.md (kubernetes#2274) * Update modsecurity.md Some typo fixes * Update modsecurity.md * Update go to 1.10.1 (kubernetes#2273) * Update README.md (kubernetes#2276) Small typo fix . * Fix bug when auth req is enabled(external authentication) (kubernetes#2280) * set proxy_upstream_name correctly when auth_req module is used * log a more meaningful message when backend is not found * Fix nlb instructions (kubernetes#2282) * e2e tests for dynamic configuration and Lua features and a bug fix (kubernetes#2254) * e2e tests for dynamic configuration and Lua features * do not rely on force reload to dynamically configure when reload is needed * fix misspelling * skip dynamic configuration in the first template rendering * dont error on first sync * Fix flaky e2e tests by always waiting after redeploying the ingress controller (kubernetes#2283) * Add NoAuthLocations and default it to "/.well-known/acme-challenge" (kubernetes#2243) * Add NoAuthLocations and default it to "/.well-known/acme-challenge" * Add e2e tests for no-auth-location * Improve wording of no-auth-location tests * Update controller.go (kubernetes#2285) * Fix custom-error-pages image publication script (kubernetes#2289) * Update nginx to 1.13.11 (kubernetes#2290) * Fix HSTS without preload (kubernetes#2294) * Disable dynamic configuration in s390x and ppc64le (kubernetes#2298) * Improve indentation of generated nginx.conf (kubernetes#2296) * Escape variables in add-base-url annotation * Fix race condition when Ingress does not contains a secret (kubernetes#2300) * include lua-resty-waf and its dependencies in the base Nginx image (kubernetes#2301) * install lua-resty-waf * bump version * include Kubernetes header * include the rest of lua-resty-waf dependencies (kubernetes#2303) * Fix issues building nginx image in different platforms (kubernetes#2305) * Disable lua waf where luajit is not available (kubernetes#2306) * Add verification of lua load balancer to health check (kubernetes#2308) * Configure upload limits for setup of lua load balancer (kubernetes#2309) * lua-resty-waf controller (kubernetes#2304) * annotation to ignore given list of WAF rulesets (kubernetes#2314) * extra waf rules per ingress (kubernetes#2315) * extra waf rules per ingress * document annotation nginx.ingress.kubernetes.io/lua-resty-waf-extra-rules * regenerate internal/file/bindata.go * run lua-resty-waf in different modes (kubernetes#2317) * run lua-resty-waf in different modes * update docs * Add ingress-nginx survey (kubernetes#2319) * Fix survey link (kubernetes#2321) * Update nginx to 1.13.12 (kubernetes#2327) * Update nginx image (kubernetes#2328) * Update nginx image * Update minikube start script * fix nil pointer when ssl with ca.crt (kubernetes#2331) * disable lua for arch s390x and ppc64le LuaJIT is not available for s390x and ppc64le, disable the lua part in nginx.tmpl on these platform. * Fix buildupstream name to work with dynamic session affinity * fix make verify-all failures * Add session affinity to custom load balancing * Fix nginx template * Fixed tests * Sync secrets (SSL certificates) on events Remove scheduled check for missing secrets. * Include missing secrets in secretIngressMap Update secretIngressMap independently from stored annotations, which may miss some secret references. * Add test for channel events with referenced secret * Release nginx ingress controller 0.13.0 * Update owners * Use same convention, curl + kubectl for GKE * Correct some returned messages in server_tokens.go should not exists->should not exist should exists->should exist * Typo fix in cli-arguments.md it's endpoints->its endpoints * Correct some info in flags.go Correct some info in flags.go * Add proxy-add-original-uri-header config flag This makes it configurable if a location adds an X-Original-Uri header to the backend request. Default is "true", the current behaviour. * Check ingress rule contains HTTP paths * Detect if header injected request_id before creating one * fix: fill missing patch yaml config. The patch-service yaml missing livenessProbe, readinessProbe and prometheus annotation parts. * Add vts-sum-key config flag * Introduce ConfigMap updating helpers into e2e/framework and retain default nginx-configuration state between tests Group sublogic * Update nginx image to fix modsecurity crs issues * Move the resetting logic into framework Stylistic fixes based on feedback * Fix leaky test * fix the default cookie name in doc * DOCS: Add clarification regarding ssl passthrough * Remove most of the time.Sleep from the e2e tests * Accept ns/name Secret reference in annotations * Document changes to annotations with Secret reference * Improve speed of e2e tests * include lua-resty-balancer in nginx image * Silence unnecessary MissingAnnotations errors * Ensure dep fix fsnotify * Update nginx image * fix flaky dynamic configuration test * shave off some more seconds * cleanup redundant code * Update go dependencies * Allow tls section without hosts in Ingress rule * Add test for store helper ListIngresses * Add tests for controller getEndpoints * Add busted unit testing framework for lua code * Add deployment instructions for Docker for Mac (Edge) * Update nginx-opentracing to 0.3.0 This version includes a new `http.host` header to make searching by vhost in zipkin or jaeger more trivial. * Fix golint installation * add balancer unit tests * Endpoint Awareness: Read backends data from tmp file as well Actually read from the file Logs probably shouldn't assume knowledge of implementation detail Typos Added integration test, and dynamic update config refactor Don't force the 8k default Minimal test case to make the configuration/backends request body write to temp file Leverage new safe config updating methods, and use 2 replicas instead of 4 Small refactor Better integration test, addresses other feedback Update bindata * Update nginx image * automate dev environment build * Remove unnecessary externalTrafficPolicy on Docker for Mac service * Apply gometalinter suggestions * Move all documentation under docs/ * Move miscellaneous tidbits from README to miscellaneous.md and other files * Fix some document titles * Move deployment documentation under docs/deploy/ * Remove empty ingress-annotations document; fix up annotations.md's layout slightly * Configure mkdocs with mkdocs-material and friends * Move "Customizing NGINX" documentation under "NGINX Configuration" * Regenerate cli-arguments.md from the actual usage of 0.13 * Remove default-ssl-certificate.md (the content is already in tls.md) * Move documents related to third-party extensions under third-party-addons * Add buffer configuration to external auth location config * make code-generator * Clean JSON before post request to update configuration * Add scripts and tasks to publish docs to github pages * Improve readme file * Fix broken links in the docs * Remove data races from tests * Check ginkgo is installed before running e2e tests * Update exposing-tcp-udp-services.md Minor tick missing for syntax highlighting which makes it look ugly on https://kubernetes.github.io/ingress-nginx/user-guide/exposing-tcp-udp-services/ * Update custom-errors.md Fix grammatical errors * Update README.md Fix broken link to `CONTRIBUTING.md`. Also update other links to `CONTRIBUTING.md` for consistency. * Add annotation to enable rewrite logs in a location * upstream-hash-by annotation support for dynamic configuraton mode * luacheck ignore subfolders too * Release nginx ingress controller 0.14.0 * Use local image name for e2e tests * Bump echoserver version used in e2e test (1.10) * Refactor e2e framework for TLS tests * Add tests for global TLS settings * improve build-dev-env.sh script * always use x-request-id * Add basic security context to deployment YAMLs * Update GitHub pull request template * Improve documentation format * Add google analytics [ci skip] * Add gRPC annotation doc * Adjust size of tables and only adjust the first column on mobile * Assert or install go-bindata before incanting * Add Getting the Code section to Quick Start * TLS.md: Move the TLS secret misc bit to the TLS document * TLS.md: Clarify how to set --default-ssl-certificate * TLS.md: Remove the frankly useless curl output in the default certificate section * TLS.md: Reformat and grammar check * TLS.md: Remove useless manual TOC * multiple-ingress.md: rework page for clarity and less repetition * Add upgrade documentation Closes kubernetes#2458 * Reformat log-format.md * Add note about changing annotation prefixes * Clean up annotations.md; extract default backend from miscellaneous * Index all examples and fix their titles * Example of using nginx-ingress with gRPC * Exclude grpc-fortune-teller from go list Deps are managed by bazel so these will fail to show up in the vendor tree, triggering false positive build fail. * Fixed broken link in deploy README * Change TrimLeft for TrimPrefix on the from-to-www redirect * use roundrobin from lua-resty-balancer library and refactor balancer.lua * upstream-hash-by should override load-balance annotation * add resty cookie * [ci skip] bump nginx baseimage version * Add some clarification around multiple ingress controller behavior * Update go version in fortune teller image * Refactor update of status removing initial check for loadbalancer * Add KubeCon Europe 2018 Video to documentation Adds Make Ingress-Nginx Work for you, and the Community Video to the documentation. * force backend sync when worker starts * Remove warning when secret is used only for authentication * Fix and simplify local dev workflow and execution of e2e tests * Release nginx ingress controller 0.15.0
I can reproduce this bug in quay.io/kubernetes-ingress-controller/nginx-ingress-controller:0.20.0 |
@sylmarch why do you need to have the default backend deployments and service? seems like a waste of resources. |
@gWOLF3 it might be useful if your Ingress handles multiple hosts and when all requests for one specific host are routed to only one webapp. So |
NGINX Ingress controller version: 0.10.0
Kubernetes version (use
kubectl version
):Client Version: version.Info{Major:"1", Minor:"8", GitVersion:"v1.8.0", GitCommit:"6e937839ac04a38cac63e6a7a306c5d035fe7b0a", GitTreeState:"clean", BuildDate:"2017-09-28T22:57:57Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"8+", GitVersion:"v1.8.3-rancher3", GitCommit:"772c4c54e1f4ae7fc6f63a8e1ecd9fe616268e16", GitTreeState:"clean", BuildDate:"2017-11-27T19:51:43Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}
Environment:
uname -a
):What happened:
Session affinity using a cookie does not work, even if these 3 annotations is set on the ingress:
I think the Nginx configuration, generated from the Ingress resource, is incorrect (see below).
What you expected to happen:
A cookie "route" should be set in response to the 1st request (that isn't the case).
Then, all other requests should provide this cookie.
The IngressController should then forward all these requests to the same backend pod.
How to reproduce it (as minimally and precisely as possible):
Hereby a complete configuration.
In your browser, open a web developer console to see request and response headers.
Access the URL
http://echo-server:30080
and check the headers :Anything else we need to know:
You can retrieve the generated Nginx configuration like this:
But no configuration uses the "sticky-default-echo-server-8080" in the server section:
I never use Nginx but I think the issue is here.
The text was updated successfully, but these errors were encountered: