Update session persistence design

[salonichf5]

Proposed changes

Updates the session persistence design to showcase the current design considerations

Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.

Closes #ISSUE

Checklist

Before creating a PR, run through this checklist and mark each as complete.

• I have read the CONTRIBUTING doc
• I have added tests that prove my fix is effective or that my feature works
• I have checked that all unit tests pass after adding my changes
• I have updated necessary documentation
• I have rebased my branch onto main
• I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

NONE

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.10%. Comparing base (e3e7ae1) to head (e1c3d83).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4314   +/-   ##
=======================================
  Coverage   86.10%   86.10%           
=======================================
  Files         132      132           
  Lines       14342    14342           
  Branches       35       35           
=======================================
  Hits        12349    12349           
  Misses       1790     1790           
  Partials      203      203           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sjberman]

Is this case described at all in the Gateway API? I don't think this would be unique to NGF.

[salonichf5]

no there's no rule defined on the Gateway API spec. But when I did testing, session persistence was not getting rendered properly in this scenario so I chose to handle it in the said way above

[sjberman]

I'm thinking it's not defined in the Gateway API because it's intended that different rules (even to the same backend) are treated differently.

This is actually concerning too, because what if you have one rule that defines session persistence for a backend, and another one that doesn't, but uses the same backend? Now that second rule will indirectly get session persistence.

I don't exactly know how, but it would seem that we would only need to enable sticky cookie depending on the request...

[sjberman]

Well, I just thought of one potentially complicated way we could achieve this, assuming that there's no simple way to have the cookie settings be variable.

We define an upstream block per SessionPersistence config. So if you have coffee backend, and the HTTPRoute has it referenced three times, one with no persistence, one with persistence settings X, and the last one with persistence settings Y, then we define the following:

upstream coffee_no_persistence{
     server 10.0.0.0;
}

upstream coffee_persistence_X{
     server 10.0.0.0;

     sticky cookie X;
}

upstream coffee_persistence_Y{
     server 10.0.0.0;

     sticky cookie Y;
}

And then the proxy_pass in the location block for each rule would route to the proper upstream.

[sjberman]

Think about it like this example

apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: coffee
spec:
  parentRefs:
  - name: cafe
  hostnames:
  - cafe.example.com
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /coffee
    backendRefs:
    - name: coffee
      port: 80
    sessionPersistence:
        absoluteTimeout: 60s
  - matches:
    - path:
        type: PathPrefix
        value: /coffee
      headers:
      - name: version
        value: v2
    backendRefs:
    - name: coffee
      port: 80
    sessionPersistence:
        absoluteTimeout: 30s
  - matches:
    - path:
        type: PathPrefix
        value: /coffee-two
    backendRefs:
    - name: coffee
      port: 80

So here we have 3 rules, one for path /coffee-two with no session persistence, and two for /coffee, with different matching conditions and different session persistence. All 3 point to the same backend. Today, we would get three location blocks for this.

upstream coffee_backend {
     server 10.0.0.0;
}

location /coffee-two {
    proxy_pass coffee_upstream;
}

location /coffee {
   js_content httpmatches.match;
}

location /_ngf-internal-route0-rule0 { # for the first /cofffee match
   proxy_pass coffee_upstream;
}

location /_ngf-internal-route0-rule1 { # for the second /cofffee match
   proxy_pass coffee_upstream; 
}

And then I think the change would be:

upstream coffee_backend {
     server 10.0.0.0;
}

upstream coffee_backend_persistence_0{
     server 10.0.0.0;

      sticky cookie timeout=60s; # whatever the format is
}

upstream coffee_backend_persistence_1 {
     server 10.0.0.0;

     sticky cookie timeout=30s; # whatever the format is
}

location /coffee-two {
    proxy_pass coffee_upstream;
}

location /coffee {
   js_content httpmatches.match;
}

location /_ngf-internal-route0-rule0 { # for the first /cofffee match
   proxy_pass coffee_upstream_persistence_0;
}

location /_ngf-internal-route0-rule1 { # for the second /cofffee match
   proxy_pass coffee_upstream_persistence_1; 
}

So all we do is have an upstream for each sessionPersistence config for each backend, and change the proxy_pass to the direct upstream we know that it needs.

[salonichf5]

okay this makes a lot more sense. I will get on implementing this change. Thank you for the guidance.

[sjberman]

Obviously this was all in my head, and it made sense so I'm hoping it works haha.