[WIP] GEP-1619: Define session name conflict resolution rules

[gcs278]

🚧 WIP (Depends on #4870, #4873, #4876).

What type of PR is this?

/kind gep

What this PR does / why we need it:
This PR updates GEP-1619 to define session name conflict resolution rules, resolving ambiguities documented in #4268.

Context: Design Doc

Require unique cookie and header names

Cookie and header names MUST be unique across all session persistence configurations within a Gateway. This includes across route rules, across BackendTrafficPolicies, and between route-level and BackendTrafficPolicy configurations. Within the same xRoute, uniqueness is enforced by CEL validation at admission time. Across xRoutes, the oldest route wins (PartiallyInvalid/UnsupportedValue). Across BackendTrafficPolicies, the oldest policy wins (Accepted: False/Conflicted).

Which issue(s) this PR fixes:

Fixes #4268

Does this PR introduce a user-facing change?:

Cookie and header names are now required to be unique across route rules (enforced by CEL validation).

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[mikemorris]

I see the note about controllers rejecting and setting a PartiallyInvalid status below, but it this something that can/should be checked earlier in a ValidatingAdmissionPolicy? Should this be considered now that we're actually distributing a VAP alongside the Gateway API CRDs?

[youngnick]

Even if we did do that, controllers will still need to check for it, because users could delete that VAP. I probably don't favor auto-including a VAP for that reason, although we could allow it (via a MAY or similar).

[gcs278]

I'm assuming you mean that implementations can implement their own VAP (rather than including it in a standard VAP that we ship). Added a MAY line about this for clarity.

[mikemorris]

Looks like a good direction and useful clarifications so far to me!

[youngnick]

Is this a standard way to do this? Any prior art on this kind of thing?

Feels a bit suboptimal that we don't have the computed path shown anywhere, a bit too magical for me.

[gcs278]

Doing some more review: no, I don't think there is prior art for computed cookie path. I did a survey about how existing implementations and dataplanes handle cookie path (prior to GEP-1619), and here's what I found:

Implementation	API	Cookie Path	Source
Istio	DestinationRule httpCookie.path	User-configured (default: omitted)	source (L1487)
Contour	HTTPProxy loadBalancerPolicy	Hardcoded /	source (L717)
HAProxy Ingress	Annotations	Hardcoded /	template (L740); #528 requests it
Traefik	IngressRoute sticky.cookie	User-configured (default: /)	source (L58)
Kong	upstream hash_on_cookie_path	User-configured (default: /)	source (L194)
NGINX (dataplane)	sticky cookie path=	User-configured (default: omitted)	docs
HAProxy (dataplane)	cookie directive	Hardcoded /	docs
Envoy (dataplane)	hash_policy cookie.path	User-configured (default: omitted)	API ref

So this is a good point. We are paving a new path for...path. There are pros and cons to each option:

Computed path: Scopes cookies more precisely, avoids cookie proliferation, but no prior art, can break when edge proxies rewrite the request path (#4713), and is a bit "magical".

Default /: Matches what most implementations already do (5 of 8), predictable, universally implementable, but broader scope than needed.

Computed path is a more elegant solution, but this is one of those complexity tradeoffs and we can re-evaluate.

Not defaulting and requiring an explicit cookieConfig.path field isn't a great option for users who just want to configure the bare minimum. Alternatively, omitting path entirely (letting the browser set it) feels like bad UX because the browser scopes the cookie to the parent path of the request URL, which potentially breaks shared sessions inside a multi-path route rule.

We can also take this conversation to #4713 - it is very relevant there. Thoughts on these options?

[zhaohuabing]

+1 for defaulting to /
It would also be helpful to allow an optional cookieConfig.path for explicit configuration.

[youngnick]

Unfortunately, we need to explicitly specify these rules. I assume they are something like:

• Within the same resource, all conflicting rules must be rejected, with the whole Route being rejected if that means there are no valid rules left. Otherwise, a PartiallyInvalid or similar Condition must be set on the Route in reference to that parentref. (I can't remember what the Reason should be, but that should be specified as well).
• Across multiple resources, conflicting rules mean that the Route with the oldest creation time wins for that rule only. For all other Routes with conflicting rules, those rules must be rejected, and the Routes with conflicting, rejected rules MUST have a PartiallyInvalid Condition added. (This really sucks for implementors, because it means that you have to do per-rule comparisons across the entire Gateway set of attached routes as part of conflict resolution. However, we already specify this for path, so it's not completely unprecedented).

The second one also means that it's possible to have Routes with multiple Rules where some are valid in the older one, and some in the newer one, because the newer one contains non-conflicting rules.

[gcs278]

Good point - I see now that saying "just reject" isn't enough.

However, I realize we can check session name uniqueness within the same xRoute via CEL validation (I see precedent for rule name uniqueness is enforced). That's a win-win - removes the foot gun of accidentally breaking two rules by adding a duplicate cookie name, and implementations don't have to handle this special logic at all. Let me know if you know why CEL won't work.

But we can't use CEL across multiple resources, so I will add those details.

[gcs278]

@youngnick @mikemorris Thank you for the reviews - my apologies for the gap in responses, but I'm allocating more time to getting this moving again.

Changes are here: 1e39b5d

[gcs278]

I'm assuming you mean that implementations can implement their own VAP (rather than including it in a standard VAP that we ship). Added a MAY line about this for clarity.

[gcs278]

Doing some more review: no, I don't think there is prior art for computed cookie path. I did a survey about how existing implementations and dataplanes handle cookie path (prior to GEP-1619), and here's what I found:

Implementation	API	Cookie Path	Source
Istio	DestinationRule httpCookie.path	User-configured (default: omitted)	source (L1487)
Contour	HTTPProxy loadBalancerPolicy	Hardcoded /	source (L717)
HAProxy Ingress	Annotations	Hardcoded /	template (L740); #528 requests it
Traefik	IngressRoute sticky.cookie	User-configured (default: /)	source (L58)
Kong	upstream hash_on_cookie_path	User-configured (default: /)	source (L194)
NGINX (dataplane)	sticky cookie path=	User-configured (default: omitted)	docs
HAProxy (dataplane)	cookie directive	Hardcoded /	docs
Envoy (dataplane)	hash_policy cookie.path	User-configured (default: omitted)	API ref

So this is a good point. We are paving a new path for...path. There are pros and cons to each option:

Computed path: Scopes cookies more precisely, avoids cookie proliferation, but no prior art, can break when edge proxies rewrite the request path (#4713), and is a bit "magical".

Default /: Matches what most implementations already do (5 of 8), predictable, universally implementable, but broader scope than needed.

Computed path is a more elegant solution, but this is one of those complexity tradeoffs and we can re-evaluate.

Not defaulting and requiring an explicit cookieConfig.path field isn't a great option for users who just want to configure the bare minimum. Alternatively, omitting path entirely (letting the browser set it) feels like bad UX because the browser scopes the cookie to the parent path of the request URL, which potentially breaks shared sessions inside a multi-path route rule.

We can also take this conversation to #4713 - it is very relevant there. Thoughts on these options?

[gcs278]

Good point - I see now that saying "just reject" isn't enough.

However, I realize we can check session name uniqueness within the same xRoute via CEL validation (I see precedent for rule name uniqueness is enforced). That's a win-win - removes the foot gun of accidentally breaking two rules by adding a duplicate cookie name, and implementations don't have to handle this special logic at all. Let me know if you know why CEL won't work.

But we can't use CEL across multiple resources, so I will add those details.

[zhaohuabing]

Nit: This uniqueness CEL rule compares header names with ==. should we normalize the headers to avoid mixed-case duplicates?

[gcs278]

Yes - good catch - I'll fix on my next round of updates.

[zhaohuabing]

I’m not sure cookie.name / header.name uniqueness should be a hard MUST across all route rules. This seems to prohibit intentional session sharing between routes, for example two route rules that deliberately use the same cookie or header name so clients keep the same backend affinity across related paths. For cookies, name alone is also not the full browser cookie identity because path/domain affect whether sessions are actually shared.

Would it make sense to soften this to a recommendation, or limit it to cases where duplicate names introduce ambiguity or conflicting behavior, rather than applying it to all duplicates? This would let Gateway users decide how cookies should be shared, without constraining how Routes and RouteRules are structured.

[zhaohuabing]

#4268 (comment) suggested to use the following for shared session.

kind: HTTPRoute
spec:
  rules:
    - matches:
        - path:
            type: PathPrefix
            value: /cart
        - path:
            type: PathPrefix
            value: /checkout
      backendRefs:
        - name: svc-a
          port: 80
      sessionPersistence:
        name: shop-session

However, this may unnecessarily limit the flexibility of how Routes / RouteRules can be constructed. A single rule with multiple matches works when all matched paths have identical routing behavior, but it does not cover some cases, for example, where paths need different rule-level behavior while intentionally sharing stickiness. For example:

kind: HTTPRoute
spec:
  hostnames: ["shop.example.com"]
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /cart
    filters:
    - type: RequestHeaderModifier
      requestHeaderModifier:
        set:
        - name: X-Flow
          value: cart
    backendRefs:
    - name: shop-app
      port: 80
    sessionPersistence:
      type: Cookie
      cookie:
        name: shop-session

  - matches:
    - path:
        type: PathPrefix
        value: /checkout
    filters:
    - type: RequestHeaderModifier
      requestHeaderModifier:
        set:
        - name: X-Flow
          value: checkout
    backendRefs:
    - name: shop-app
      port: 80
    sessionPersistence:
      type: Cookie
      cookie:
        name: shop-session

Or where different HTTPRoutes are needed for separate policy attachment.

apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: shop-public
spec:
  hostnames: ["shop.example.com"]
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /products
    backendRefs:
    - name: shop-app
      port: 80
    sessionPersistence:
      type: Cookie
      cookie:
        name: shop-session
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: shop-account
spec:
  hostnames: ["shop.example.com"]
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /account
    backendRefs:
    - name: shop-app
      port: 80
    sessionPersistence:
      type: Cookie
      cookie:
        name: shop-session
---
apiVersion: gateway.envoyproxy.io/v1alpha1
kind: SecurityPolicy
metadata:
  name: account-oidc
spec:
  targetRef:
    kind: HTTPRoute
    name: shop-account
  oidc:
    provider:
      issuer: https://idp.example.com/realms/shop
    clientID: shop-web
    clientSecret:
      name: shop-oidc-client-secret

[gcs278]

Right, the cookie tuple is (name, domain, path). I agree that enforcing unique session names limits the ability to share sessions for route-inline session persistence.

For softening, I suppose you are suggesting that we allow duplicate session names for route rules that:

1. Target the same service
2. Have the same session persistence configuration

If you don't target the same service (# 1), then you have a collision with potential breakage (encoded pod IP is not common between services). If you don't have the same configuration (# 2), then you get a thrashing cookie configuration (set-cookies fighting).

It's valid - but the tradeoff is:

1. It feels a bit magical with implicit behavior - applying two same name sessions shares them
2. Complexity increase for implemenations that have to compare API configurations for conflicts.

Alternatively, I suppose we could go hands off - and don't do any validation. That'd be the most simple for implementation, while allowing sharing. The tradeoff there, is that users now have foot guns for setting conflicting configuration.

For context - originally, I designed the uniqueness requirement with the assumption that session sharing would be handled by BackendTrafficPolicy, where it works naturally without nearly as much collision concerns (sharing sessions across services doesn't make as much sense like sharing across routes to the same service). However, supporting both attachment points simultaneously in the API is becoming hard to justify.

There's a broader discussion about this in #4462 - would love your input there if you get a chance. If BackendTrafficPolicy is the right end solution - then our debates on conflicts get much simpler.

Did I capture your suggestion correctly?

[gcs278]

@zhaohuabing thanks for the comments, really appreciated! Feel free to keep the discussion going - I'm going to hold on any major changes until I get more input from the community on the direction we are taking.

[gcs278]

Right, the cookie tuple is (name, domain, path). I agree that enforcing unique session names limits the ability to share sessions for route-inline session persistence.

For softening, I suppose you are suggesting that we allow duplicate session names for route rules that:

1. Target the same service
2. Have the same session persistence configuration

If you don't target the same service (# 1), then you have a collision with potential breakage (encoded pod IP is not common between services). If you don't have the same configuration (# 2), then you get a thrashing cookie configuration (set-cookies fighting).

It's valid - but the tradeoff is:

1. It feels a bit magical with implicit behavior - applying two same name sessions shares them
2. Complexity increase for implemenations that have to compare API configurations for conflicts.

Alternatively, I suppose we could go hands off - and don't do any validation. That'd be the most simple for implementation, while allowing sharing. The tradeoff there, is that users now have foot guns for setting conflicting configuration.

For context - originally, I designed the uniqueness requirement with the assumption that session sharing would be handled by BackendTrafficPolicy, where it works naturally without nearly as much collision concerns (sharing sessions across services doesn't make as much sense like sharing across routes to the same service). However, supporting both attachment points simultaneously in the API is becoming hard to justify.

There's a broader discussion about this in #4462 - would love your input there if you get a chance. If BackendTrafficPolicy is the right end solution - then our debates on conflicts get much simpler.

Did I capture your suggestion correctly?

[gcs278]

Yes - good catch - I'll fix on my next round of updates.

[zhaohuabing]

For context - originally, I designed the uniqueness requirement with the assumption that session sharing would be handled by BackendTrafficPolicy, where it works naturally without nearly as much collision concerns (sharing sessions across services doesn't make as much sense like sharing across routes to the same service). However, supporting both attachment points simultaneously in the API is becoming hard to justify.

If we clarify that sessionPersistence.cookie is meant to be used per route, and BackendTrafficPolicy should be used for session sharing, then requiring session name uniqueness across all routes sounds reasonable to me.

[zhaohuabing]

Hi @gcs278, just checking in — is there anything blocking this PR?

We’re hoping the default cookie path / in this PR can help unblock the edge proxy rewrite use case in Envoy Gateway.

[gcs278]

Hey @zhaohuabing - yea recently there has been discussions about Backend (#4488) - which is being considered as an alternative to policy attachment. I was waiting for those discussions to finalize before making too many more modifications to this GEP.

It sounds like there's general agreement on Session Persistence going experimental in Backend and also agreement that it shouldn't live in multiple places (e.g. route inline, backend, and BackendTrafficPolicy). I'm going to try to finalize the direction on that soon. I'll be advocating for Backend, but I need to discuss with the implementations that already shipped it as experimental in route-inline first.

I'm planning to refactor this draft GEP soon - slim it down and address only a couple things at a time. Stay tuned.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gcs278
Once this PR has been reviewed and has the lgtm label, please assign danwinship for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• geps/OWNERS

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