Skip to content
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

Please add proxy_cookie_flags #6368

Closed
fengerzh opened this issue Oct 25, 2020 · 11 comments
Closed

Please add proxy_cookie_flags #6368

fengerzh opened this issue Oct 25, 2020 · 11 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@fengerzh
Copy link

As Nginx 1.19.3 have proxy_cookie_flags support:

Syntax: | proxy_cookie_flags off | cookie [flag ...];
Default: proxy_cookie_flags off;
Context: http, server, location

Now we can change cookie flags in Nginx:

Sets one or more flags for the cookie. The cookie can contain text, variables, and their combinations. The secure, httponly, samesite=strict, samesite=lax, samesite=none parameters add the corresponding flags. The nosecure, nohttponly, nosamesite parameters remove the corresponding flags.

When will Ingress also support this feature? Thanks!

@fengerzh fengerzh added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 25, 2020
@aledbf
Copy link
Member

aledbf commented Oct 25, 2020

You can use the directive with the annotation configuration-snippet

@fengerzh
Copy link
Author

You can use the directive with the annotation configuration-snippet

There seems no effect. I even tried to put proxy_cookie_path in configurtion snippet, but have no effect too:

    nginx.ingress.kubernetes.io/configuration-snippet: |
      proxy_cookie_path /two/ /;

Yes, I know we can use nginx.ingress.kubernetes.io/proxy-cookie-path to change, but I don't know why if I put it in configuration-snippet, it cannot work?

I also tried below, but has no effect too:

    nginx.ingress.kubernetes.io/configuration-snippet: |
      proxy_cookie_flags something nohttponly;

@Tyrion85
Copy link

Tyrion85 commented Jan 5, 2021

@fengerzh Have you solved this issue?

What I've noticed (installing via Helm chart 3.19.0, controller version 0.43.0) is that, if I put proxy_cookie_path via configuration-snippet annotation (and not use nginx.ingress.kubernetes.io/proxy-cookie-path at all), the underlying nginx.conf file will contain two proxy_cookie_path directives, i.e.:

proxy_cookie_path off; ..... proxy_cookie_path ~^(.+)$ "$1; domain=something.com";

Presumably, proxy_cookie_path off comes from the default value of nginx.ingress.kubernetes.io/proxy-cookie-path annotation (which is "off"). I don't think this is a correct behaviour, as there shouldn't be both "off" and some value for this directive at the same time.

@aledbf
Copy link
Member

aledbf commented Jan 5, 2021

I don't think this is a correct behaviour, as there shouldn't be both "off" and some value for this directive at the same time.

This is the default NGINX value http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cookie_path
When we add some feature from NGINX, in most of the cases we respect the default value.

@Tyrion85
Copy link

Tyrion85 commented Jan 5, 2021

Of course - fully understood. On the other hand, I don't understand why there are two entries for proxy_cookie_path though, shouldn't there only be one? In fact, my nginx pods won't even perform successful reload in this case (no nginx.ingress.kubernetes.io/proxy-cookie-path annotation and with configuration-snippet used), because of duplicate entries.

I haven't fully tested the hypothesis that duplicate proxy_cookie_path directive (or rather, the additional "off" value that is set by default) is what's causing the problem @fengerzh describes, so I will stop speculating here. I can just confirm that it doesn't work, unless you use nginx.ingress.kubernetes.io/proxy-cookie-path annotation explicitly (i.e. just using nginx.ingress.kubernetes.io/configuration-snippet: | proxy_cookie_path /two/ / doesn't work for me)

@aledbf
Copy link
Member

aledbf commented Jan 5, 2021

(i.e. just using nginx.ingress.kubernetes.io/configuration-snippet: | proxy_cookie_path /two/ / doesn't work for me)

Because proxy_cookie_path already exists in the template. If you want to set a value, please use the annotation.

@Tyrion85
Copy link

Tyrion85 commented Jan 5, 2021

I do, thanks. I couldn't find in documentation that using configuration-snippet for proxy_cookie_path won't work, and neither did engineers in my company who set this value in their services (and presumably, neither did @fengerzh), so hopefully discussion in this issue will help someone who might search for this in the future.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 5, 2021
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 5, 2021
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

5 participants