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

GEP-1619 Session Persistence and Session Affinity #1643

Merged
merged 1 commit into from
Mar 22, 2023

Conversation

gcs278
Copy link
Contributor

@gcs278 gcs278 commented Jan 12, 2023

What type of PR is this?
/kind gep

What this PR does / why we need it:
Add a GEP for session persistence that establishes the foundation for describing it and establishing a common language.

Which issue(s) this PR fixes:

Doesn't fix, but supports #1619

Does this PR introduce a user-facing change?:
N/A

Markdown Preview: https://github.com/gcs278/gateway-api/blob/gep-1619/site-src/geps/gep-1619.md

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/gep PRs related to Gateway Enhancement Proposal(GEP) labels Jan 12, 2023
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 12, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: gcs278 / name: Grant Spence (803d1b6)

@k8s-ci-robot
Copy link
Contributor

Welcome @gcs278!

It looks like this is your first PR to kubernetes-sigs/gateway-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/gateway-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 12, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @gcs278. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 12, 2023

Some of the concerns of session persistence are the duration and expiration of the session, security of the transaction stream, and storage of the context or state. Application developers use session persistence to improve the performance and reduce overall storage needs by aligning each transaction and its cached data with the same server.

Session persistence is not to be confused with session affinity, which uses information below the application layer to maintain a client request to a single server. Session affinity can be considered a weaker form of session persistence: it is not guaranteed to persist a connection to the same backend server.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be saying the difference between Affinity and Persistence is L7 vs L4. Its not clear to me that is an important distinction

In my view, the primary difference between session types is whether the client directly specifies the backend they want OR if some attribute of the request is just used to consistently send to a backend.

  • Type 1 example: A cookie with Backend=1.2.3.4
  • Type 2 example: Send to backend based on backend[hash(SomeHeader) % BackendCount]

In Envoy terms, 1 is "stick sessions" and 2 is "consistent hash".

To me, that distinction seems more relevant than L7 vs L4. I would agree that "affinity" does tend to be L4 based on source IP, as you noted in your examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I agree. I reworded Persistence and Affinity verbiage with focus on what information/attributes are being used to achieve the connection consistency, let me know what you think.

@gcs278 gcs278 force-pushed the gep-1619 branch 3 times, most recently from c4d0e4a to b1bddb0 Compare January 19, 2023 15:55
| Acnodal EPIC | Envoy | Cookie-Based, Header-Based | Consistent Hashing via Maglev and Ring Hash | Envoy proxy implements both [cookie-based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto) and [header-based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) stateful sessions. Envoy offers [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash) load balancing algorithms that provide consistent hashing. |
| Apache APISIX | Nginx | ? | ? | ? |
| Cilium | eBPF | No | Source IP Address | Cilium does not have a true session persistence feature, but does implement [session affinity by default](https://docs.cilium.io/en/v1.12/gettingstarted/kubeproxy-free/#id2) using source IP address. |
| Contour | Envoy | Cookie-Based | Consistent Hashing via Maglev and Ring Hash | Envoy proxy implements both [cookie-based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto) and [header-based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) stateful sessions. Envoy offers [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash) load balancing algorithms that provide consistent hashing. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contour also supports using other request attributes other than cookies to route requests to a consistent backend: https://projectcontour.io/docs/v1.23.2/config/request-routing/#load-balancing-strategy (the RequestHash load balancing strategy)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the info - I added a note about this in the Contour row.

@gcs278 gcs278 force-pushed the gep-1619 branch 2 times, most recently from ce198f8 to 803d1b6 Compare January 19, 2023 19:52
@gcs278 gcs278 marked this pull request as ready for review January 19, 2023 19:53
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 19, 2023
@k8s-ci-robot k8s-ci-robot requested a review from bowei January 19, 2023 19:53
Copy link
Member

@robscott robscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gcs278!

- Identify differences in session persistence functionality between implementations

## Non-Goals
- Create proposals for adding session persistence configuration to Gateway API object specification
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this may be a non-goal for this particular "provisional" stage of the GEP, I think it would need to be a goal of the GEP as a whole. Or maybe I'm misunderstanding the purpose of writing this up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, it seems that this should be an eventual goal of this KEP, however it's reasonable for the first iteration to just be the "what and the why" without the how and still merge.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Create proposals for adding session persistence configuration to Gateway API object specification

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair, I'll remove it to avoid confusion.

## Non-Goals
- Create proposals for adding session persistence configuration to Gateway API object specification
- Mandate a default or expected session persistence functionality for implementations
- Require implementations to support session persistence or a specific form of it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are values that are broadly implementable, we may want to consider this "core" conformance which would conflict with this non-goal. (I don't actually know that this is broadly implementable, but it may be safer to leave this non-goal out).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definitely take an inventory, in time, of implementations ability to support this. But since we don't fully know what "this" is yet I don't think we need to worry too much about it. I would just advise dropping this for now, and we can suss out support levels in a follow-up PR:

Suggested change
- Require implementations to support session persistence or a specific form of it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, will drop this non-goal for now to defer this decision to later PRs.

Comment on lines 23 to 25
At the November 28th, 2022 SIG Gateway API meeting, we discussed the idea of standardizing and documenting session persistence (also known as session stickiness). It was recommended that we first generate discussion for how we describe session persistence, agree on how it is achieved, and find out which implementers support session persistence.

Conformance test for session persistence will be considered if deemed appropriate. Furthermore, this information can be used for developing a Gateway API spec to configure session persistence in the future.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of providing a history, I think I'd just explain what session affinity means and why it should be configurable within Gateway API (assuming that's the goal). Edit: Looks like you've already done that well below, so maybe just leave this part out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So dropping these paragraphs? Sure, it doesn't really add much value to the GEP. Done.

Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I really like the approach of talking about the "what" and "why" first, rather than digging right into details.

I'm approving this GEP based on the fact that it is provisional, and I see no reason to block merging it in this provisional state as I expect we can iteratively update it afterwards. That said, I do have a few comments that I would like to see resolved before we merge please see what you think of them.

- Identify differences in session persistence functionality between implementations

## Non-Goals
- Create proposals for adding session persistence configuration to Gateway API object specification
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, it seems that this should be an eventual goal of this KEP, however it's reasonable for the first iteration to just be the "what and the why" without the how and still merge.

## Non-Goals
- Create proposals for adding session persistence configuration to Gateway API object specification
- Mandate a default or expected session persistence functionality for implementations
- Require implementations to support session persistence or a specific form of it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definitely take an inventory, in time, of implementations ability to support this. But since we don't fully know what "this" is yet I don't think we need to worry too much about it. I would just advise dropping this for now, and we can suss out support levels in a follow-up PR:

Suggested change
- Require implementations to support session persistence or a specific form of it


At the November 28th, 2022 SIG Gateway API meeting, we discussed the idea of standardizing and documenting session persistence (also known as session stickiness). It was recommended that we first generate discussion for how we describe session persistence, agree on how it is achieved, and find out which implementers support session persistence.

Conformance test for session persistence will be considered if deemed appropriate. Furthermore, this information can be used for developing a Gateway API spec to configure session persistence in the future.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conformance test for session persistence will be considered if deemed appropriate.

I'm a little confused about this statement. We intend to provide conformance for all API specification in time, why would we not with session persistence?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've dropped this statement per rob's last comment. I originally was coming at this from the angle of just documenting session persistence and not adding an API field (yet). At that instance in time, with no API fields for session persistence, a conformance test would be to establish default functionality, i.e. an expectation of default session persistence. I should have worded it more like:

Conformance test for default session persistence functionality will be considered if deemed appropriate.

Though, I think this was confusing because I don't think there is precedent of Gateway API conformance testing something that is not a feature controllable by an API field. After more discussion, it sounded like we wanted the API fields established before doing a conformance test. Sounds like we should have conformance test for confirming an API field is functional, not necessarily for prescribing default functionality.

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2023
| Traefik | Traefik Proxy | Cookie-Based | No | Traefik implements [cookied-based](https://doc.traefik.io/traefik/routing/services/#sticky-sessions) stateful sessions. [Docs](https://traefik.io/glossary/what-are-sticky-sessions/) mention consistent hashing, but Traefik [only offers round robin](https://doc.traefik.io/traefik/routing/services/#load-balancing) (not consistent). |

### Open Questions
- What are the needs and use cases from end users for specifying session persistence?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a section on Java ( and tomcat/jetty/etc), which has standardised the API and behavior long ago, including more advanced features like session migration to a different host, activating session on demand.

Also missing is a section on privacy and maybe a small not on compliance - k8s gateway is used not only for service to service but may also be used for web server workloads with browsers as clients.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An doc I wrote for Istio: https://docs.google.com/document/d/1s_eQCOukYlR8wMW0sHIi7hGyfGsTPV6Eyc-oFwvxsxQ/edit#heading=h.xw1gqgyqs5b

( the example uses K8S Gateway API only, only an istio-specific envoyfilter used)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean a new section dedicated to talking about Java? Or table entries? I started a section, but will probably need your advice on expanding on the content.

Thanks for the doc, it's very thorough. I'll leverage that.

For privacy, I think I addressed that in the new Security and Privacy Implications section. For compliance, could you expand? What compliance requirements are we referring to? Are you saying we should create requirements in this GEP?

| Google Kubernetes Engine | ? | ? | ? | ? |
| HAProxy Ingress | HAProxy | Cookie-Based | Stick Tables | HAProxy implements cookie-based stateful sessions via the [forwardfor option](https://docs.haproxy.org/2.6/configuration.html#4-option%20forwardfor). [Stick tables](https://cbonte.github.io/haproxy-dconv/2.4/configuration.html#4-stick-table) implement stickiness via client attributes. |
| HashiCorp Consul | Envoy | Cookie-Based, Header-Based | Consistent Hashing via Maglev and Ring Hash | ? |
| Istio | Envoy | Cookie-Based, Header-Based | Consistent Hashing via Maglev and Ring Hash | Envoy proxy implements both [cookie-based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto) and [header-based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) stateful sessions.Envoy offers [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash) load balancing algorithms that provide consistent hashing. |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would note that envoy current implementation is not suitable for use in gateways - session/header value is plain text ip/port of backend ( base64 for some weird reasons). Inside a mesh - GAMMA - backend IP is exposed and any pod can directly hit any backend (unless blocked by a policy), but on the internet I have not seen any provider allowing clients to connect to specific internal IPs.

| **Implementation** | **Proxy Type** | **Session Persistence** | **Session Affinity** | **Notes** |
| --- | --- | --- | --- | --- |
| Acnodal EPIC | Envoy | Cookie-Based, Header-Based | Consistent Hashing via Maglev and Ring Hash | Envoy proxy implements both [cookie-based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto) and [header-based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) stateful sessions. Envoy offers [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash) load balancing algorithms that provide consistent hashing. |
| Apache APISIX | Nginx | ? | ? | ? |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add Apache httpd - supports Java standard sessions ( or did - mod_jk ) for multiple java implementations. Back in the days other long-dead load balancers had
java support as well ( IIS, Netscape, etc ) - and I would guess any enterprise using Java servlets in last 20 years had a load balancer with session support.

Nginx has some support for at least tomcat sessions in a pretty convoluted way: https://docs.nginx.com/nginx/deployment-guides/load-balance-third-party/apache-tomcat/

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( not sure mod_jk is still alive and what replaced it - I implemented the persistent sessions in ~2000 )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the nginx docs were very helpful. Looks like you have to pay for full session persistence.

Added Apache httpd to the list.


Session persistence is when a client request is directed to the same backend server for the duration of a "session". It is achieved when a client directly provides information, such as a header, that a proxy uses as a reference to direct traffic to a specific server. Persistence is an exception to load balancing: a persistent client request bypasses the proxy's load balancing algorithm, going directly to a backend server it has previously established a session with.

Some of the concerns of session persistence are the duration and expiration of the session, security of the transaction stream, and storage of the context or state. Application developers use session persistence to improve the performance and reduce overall storage needs by aligning each transaction and its cached data with the same server.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Privacy too. The information used for establishing the session on the internet ( the main use of the gateway ), in particular in browser context is very sensitive, used for tracking and may include user-specific info.

Copy link

@costinm costinm Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also expand on the 'security' - maybe in separate section. Many systems use session persistence as part of authentication - holding user info in a 'server side session' and using the session cookie as a bearer token ( Java but not only)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I added a ### Security and Privacy Implications section. Let me know if this addresses this.


**Header-Based Session Persistence**

Header-based stateful sessions are achieved by a backend providing an arbitrarily-named HTTP response header and the client using the same arbitrarily-named header in subsequent HTTP requests. Proxies can use this arbitrarily-named header to maintain a persistent connection to a single backend server on behalf of the client.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure 'arbitrarily named' is right. We may support custom names, but I think Java made the right choice to define a 'default' name ( while allowing implementations to customize using their own settings).

I would also use plural - for example in envoy-based persistence it is critical to use a second header to guarantee the same cluster ( aka Service/subset ) is used.

I'm also not sure outside of envoy how many Internet proxies use header based persistence - would be nice having a survey.

Using request parameter ( jsessionid=xxx in java ) is common, using the auth info is even more common. Until envoy - I haven't seen arbitrary header-based persistence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see. I wrote this using Envoy as my "source", but that's a good point. I made it more generic by removing "arbitrarily named"

| Google Kubernetes Engine | ? | ? | ? | ? |
| HAProxy Ingress | HAProxy | Cookie-Based | Stick Tables | HAProxy implements cookie-based stateful sessions via the [forwardfor option](https://docs.haproxy.org/2.6/configuration.html#4-option%20forwardfor). [Stick tables](https://cbonte.github.io/haproxy-dconv/2.4/configuration.html#4-stick-table) implement session affinity via client attributes. |
| HashiCorp Consul | Envoy | [Cookie-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto), [Header-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) | Consistent Hashing via [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash)| N/A |
| Istio | Envoy | [Cookie-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto), [Header-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) | Consistent Hashing via [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash)| N/A |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/istio/istio/blob/master/pilot/pkg/features/pilot.go#L123

Current implementation is based on a Service label - since sessions are associated with a Service ( can't have sessions cross service due to current envoy impl ).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay added a note, let me know if that makes sense or not.

@shaneutt
Copy link
Member

Thanks for the update @gcs278 we appreciate that. We'll mark this one in draft for the moment to indicate that active review is not ready just yet, flip the switch when you're ready.

@shaneutt shaneutt marked this pull request as draft March 20, 2023 12:27
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 20, 2023
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 20, 2023
Copy link
Contributor Author

@gcs278 gcs278 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed reviews. Sorry for the delay.

- Identify differences in session persistence functionality between implementations

## Non-Goals
- Create proposals for adding session persistence configuration to Gateway API object specification
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair, I'll remove it to avoid confusion.

## Non-Goals
- Create proposals for adding session persistence configuration to Gateway API object specification
- Mandate a default or expected session persistence functionality for implementations
- Require implementations to support session persistence or a specific form of it
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, will drop this non-goal for now to defer this decision to later PRs.

Comment on lines 23 to 25
At the November 28th, 2022 SIG Gateway API meeting, we discussed the idea of standardizing and documenting session persistence (also known as session stickiness). It was recommended that we first generate discussion for how we describe session persistence, agree on how it is achieved, and find out which implementers support session persistence.

Conformance test for session persistence will be considered if deemed appropriate. Furthermore, this information can be used for developing a Gateway API spec to configure session persistence in the future.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So dropping these paragraphs? Sure, it doesn't really add much value to the GEP. Done.


At the November 28th, 2022 SIG Gateway API meeting, we discussed the idea of standardizing and documenting session persistence (also known as session stickiness). It was recommended that we first generate discussion for how we describe session persistence, agree on how it is achieved, and find out which implementers support session persistence.

Conformance test for session persistence will be considered if deemed appropriate. Furthermore, this information can be used for developing a Gateway API spec to configure session persistence in the future.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've dropped this statement per rob's last comment. I originally was coming at this from the angle of just documenting session persistence and not adding an API field (yet). At that instance in time, with no API fields for session persistence, a conformance test would be to establish default functionality, i.e. an expectation of default session persistence. I should have worded it more like:

Conformance test for default session persistence functionality will be considered if deemed appropriate.

Though, I think this was confusing because I don't think there is precedent of Gateway API conformance testing something that is not a feature controllable by an API field. After more discussion, it sounded like we wanted the API fields established before doing a conformance test. Sounds like we should have conformance test for confirming an API field is functional, not necessarily for prescribing default functionality.

| Envoy Gateway | Envoy | [Cookie-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto), [Header-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) | Consistent Hashing via [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash)| N/A |
| Flomesh Service Mesh | Pipy | ? | ? | ? |
| Gloo Edge 2.0 | Envoy | [Cookie-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto), [Header-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) | Consistent Hashing via [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash)| N/A |
| Google Kubernetes Engine | ? | ? | ? | ? |
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice thanks. Added. Another naming collision though - Google calls what we call session persistence (with cookies) session affinity. Seems like there isn't a common language between implementation, I worry about this a little. I added a section on naming in alternatives.

| Google Kubernetes Engine | ? | ? | ? | ? |
| HAProxy Ingress | HAProxy | Cookie-Based | Stick Tables | HAProxy implements cookie-based stateful sessions via the [forwardfor option](https://docs.haproxy.org/2.6/configuration.html#4-option%20forwardfor). [Stick tables](https://cbonte.github.io/haproxy-dconv/2.4/configuration.html#4-stick-table) implement session affinity via client attributes. |
| HashiCorp Consul | Envoy | [Cookie-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto), [Header-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) | Consistent Hashing via [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash)| N/A |
| Istio | Envoy | [Cookie-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto), [Header-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) | Consistent Hashing via [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash)| N/A |
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay added a note, let me know if that makes sense or not.

| Kong | Kong | Cookie-Based | ? | Kong has the [session plugin](https://docs.konghq.com/hub/kong-inc/session/) which manages persistent sessions with cookie-based stateful sessions. |
| Kuma | Envoy | [Cookie-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/cookie/v3/cookie.proto), [Header-Based](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/stateful_session/header/v3/header.proto) | Consistent Hashing via [Maglev](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#maglev) and [Ring Hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash)| N/A |
| LiteSpeed Ingress Controller | ? | ? | ? | ? |
| NGINX Kubernetes Gateway | Nginx | ? | ? | ? |
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, tweaked a little bit. Let me know if it looks okay.

| Traefik | Traefik Proxy | Cookie-Based | No | Traefik implements [cookied-based](https://doc.traefik.io/traefik/routing/services/#sticky-sessions) stateful sessions. [Docs](https://traefik.io/glossary/what-are-sticky-sessions/) mention consistent hashing, but Traefik [only offers round robin](https://doc.traefik.io/traefik/routing/services/#load-balancing) (not consistent). |

### Open Questions
- What are the needs and use cases from end users for specifying session persistence?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean a new section dedicated to talking about Java? Or table entries? I started a section, but will probably need your advice on expanding on the content.

Thanks for the doc, it's very thorough. I'll leverage that.

For privacy, I think I addressed that in the new Security and Privacy Implications section. For compliance, could you expand? What compliance requirements are we referring to? Are you saying we should create requirements in this GEP?

@gcs278 gcs278 marked this pull request as ready for review March 20, 2023 17:27
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 20, 2023
@k8s-ci-robot k8s-ci-robot requested a review from howardjohn March 20, 2023 17:27
@gcs278
Copy link
Contributor Author

gcs278 commented Mar 20, 2023

@shaneutt Thanks - marked back as ready for review, posted responses, and updates to the GEP.

Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great starting point, albeit we are already starting to get quite big for a first provisional GEP. So my suggestion is to wrap up any unresolved comments or thoughts into a TODO list (similar to https://gateway-api.sigs.k8s.io/geps/gep-1709/#todo) to mark them as things that can be picked up for follow-up PRs and that we want to resolve before implementable. Once that's done I think we're in a fine place to merge this as Provisional and start thinking about the next iterative steps.

Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm given the stated goals

@gcs278
Copy link
Contributor Author

gcs278 commented Mar 22, 2023

I've created a TODO section and added TODOs from the comments from the review, mostly from @costinm's comments. Let me know if this looks good.

Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the TODO list in place for things we need to resolve before implementable, the time this has been up for review, the fact that it's provisional, and that nobody has any blocking comments it seems like now is a good time to get this first pass landed and we can build off from here.

Thanks @gcs278!

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 22, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gcs278, shaneutt, sunjayBhatia

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@shaneutt shaneutt added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 22, 2023
@shaneutt
Copy link
Member

/test all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/gep PRs related to Gateway Enhancement Proposal(GEP) lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants