Listener Isolation for hostnames occupied by other listeners

[salonichf5]

Proposed changes

Write a clear and concise description that helps reviewers understand the purpose and impact of your changes. Use the
following format:

Problem: Users want to be able to isolate listeners for routes.

Solution: Adds functionality to filter out listener hostnames from the accepted hostnames of a route which belong to another listener.

Testing:

• Unit tests added
• Conformance test added and pass successfully.

CONFORMANCE PROFILE
apiVersion: gateway.networking.k8s.io/v1
date: "2025-01-26T21:09:28Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.2.1
implementation:
  contact:
  - https://github.com/nginx/nginx-gateway-fabric/discussions/new/choose
  organization: nginxinc
  project: nginx-gateway-fabric
  url: https://github.com/nginx/nginx-gateway-fabric
  version: edge
kind: ConformanceReport
mode: default
profiles:
- core:
    result: success
    statistics:
      Failed: 0
      Passed: 11
      Skipped: 0
  name: GATEWAY-TLS
  summary: Core tests succeeded.
- core:
    result: success
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests succeeded.
- core:
    result: success
    statistics:
      Failed: 0
      Passed: 33
      Skipped: 0
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 11
      Skipped: 0
    supportedFeatures:
    - GatewayHTTPListenerIsolation
    - GatewayPort8080
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRedirect
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteDestinationPortMatching
    - HTTPRouteParentRefPort
    - HTTPRouteRequestMirror
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestTimeout
  name: GATEWAY-HTTP
  summary: Core tests succeeded. Extended tests succeeded.

• Manual testing

Listeners that are configured with the gateway

  listeners:
  - name: empty-hostname
    port: 80
    protocol: HTTP
    allowedRoutes:
      namespaces:
        from: All
  - name: wildcard-example-com
    port: 80
    protocol: HTTP
    hostname: "*.example.com"
    allowedRoutes:
      namespaces:
        from: All
  - name: wildcard-foo-example-com
    port: 80
    protocol: HTTP
    hostname: "*.foo.example.com"
    allowedRoutes:
      namespaces:
        from: All
  - name: abc-foo-example-com
    port: 80
    protocol: HTTP
    hostname: "abc.foo.example.com"
    allowedRoutes:
      namespaces:

HTTPRoutes

apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: attaches-to-empty-hostname-with-hostname-intersection
spec:
  parentRefs:
  - name: gateway
    sectionName: empty-hostname
  hostnames:
  - "bar.com"
  - "*.example.com" # request matching is prevented by the isolation wildcard-example-com listener
  - "*.foo.example.com" # request matching is prevented by the isolation wildcard-foo-example-com listener
  - "abc.foo.example.com" # request matching is prevented by the isolation of abc-foo-example-com listener
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /empty-hostname
    backendRefs:
    - name: coffee
      port: 80
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: attaches-to-wildcard-example-com-with-hostname-intersection
spec:
  parentRefs:
  - name: gateway
    sectionName: wildcard-example-com
  hostnames:
  - "bar.com" # doesn't match wildcard-example-com listener
  - "*.example.com"
  - "*.foo.example.com" # request matching is prevented by the isolation of wildcard-foo-example-com listener
  - "abc.foo.example.com" # request matching is prevented by the isolation of abc-foo-example-com listener
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /wildcard-example-com
    backendRefs:
    - name: coffee
      port: 80
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: attaches-to-wildcard-foo-example-com-with-hostname-intersection
spec:
  parentRefs:
  - name: gateway
    sectionName: wildcard-foo-example-com
  hostnames:
  - "bar.com" # doesn't match wildcard-foo-example-com listener
  - "*.example.com" # this becomes *.foo.example.com, as the hostname cannot be less specific than *.foo.example.com of the listener
  - "*.foo.example.com"
  - "abc.foo.example.com" # request matching is prevented by the isolation abc-foo-example-com listener
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /wildcard-foo-example-com
    backendRefs:
    - name: coffee
      port: 80
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: attaches-to-abc-foo-example-com-with-hostname-intersection

spec:
  parentRefs:
  - name: gateway

    sectionName: abc-foo-example-com
  hostnames:
  - "bar.com" # doesn't match abc-foo-example-com listener
  - "*.example.com" # becomes abc.foo.example.com as it cannot be less specific than abc.foo.example.com of the listener
  - "*.foo.example.com" # becomes abc.foo.example.com as it cannot be less specific than abc.foo.example.com of the listener
  - "abc.foo.example.com"
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /abc-foo-example-com
    backendRefs:
    - name: coffee
      port: 80

For listener isolation to work, we need

1. path empty-hostname to not get configured with listeners wildcard-example-com, wildcard-foo-example-com, abc-foo-example-com . When collecting accepted hostnames, the route attaches-to-empty-hostname-with-hostname-intersection attaches to all hostnames since it is attached to catch-all listener (no hostname provided), but these hostnames are part of other listeners.

• "*.example.com" # request matching is prevented by the isolation wildcard-example-com listener
• "*.foo.example.com" # request matching is prevented by the isolation wildcard-foo-example-com listener
• "abc.foo.example.com" # request matching is prevented by the isolation of abc-foo-example-com listener

So,

• bar.com/empty-hostname - 200 OK response
• bar.com/wildcard-example-com - 404 Not Found response
• bar.com/foo-wildcard-example-com - 404 Not Found response
• bar.com/abc-foo-example-com - 404 Not Found response

curl --resolve bar.com:$GW_PORT:$GW_IP http://bar.com:$GW_PORT/empty-hostname --include
Handling connection for 8080
HTTP/1.1 200 OK


curl --resolve bar.com:$GW_PORT:$GW_IP http://bar.com:$GW_PORT/wildcard-example-com --include
Handling connection for 8080
HTTP/1.1 404 Not Found

 curl --resolve bar.com:$GW_PORT:$GW_IP http://bar.com:$GW_PORT/foo-wildcard-example-com --include
Handling connection for 8080
HTTP/1.1 404 Not Found

curl --resolve bar.com:$GW_PORT:$GW_IP http://bar.com:$GW_PORT/abc-foo-example-com --include
Handling connection for 8080
HTTP/1.1 404 Not Found

2. path wildcard-example-com to not get configured with listeners wildcard-foo-example-com, abc-foo-example-com
   When collecting accepted hostnames, the route attaches-to-wildcard-example-com-with-hostname-intersection is associated to a *.example.com so it attaches to these hostnames. But these listeners need to be isolated since the hostname associated with them is part of a listener attached to another route.

• "*.foo.example.com" # request matching is prevented by the isolation wildcard-foo-example-com listener
• "abc.foo.example.com" # request matching is prevented by the isolation of abc-foo-example-com listener

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 #1175

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.

Listener isolation supported for all routes. 

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.00%. Comparing base (3b5bece) to head (df7c72a).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3061      +/-   ##
==========================================
+ Coverage   89.95%   90.00%   +0.04%     
==========================================
  Files         111      111              
  Lines       11453    11510      +57     
  Branches       50       50              
==========================================
+ Hits        10303    10360      +57     
  Misses       1089     1089              
  Partials       61       61              

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

[sjberman]

This branch should be named with feat/ prefix since it's a feature and will add a label to the PR.

[sjberman]

Can we name the l7routes map something different? Using the same name here is confusing at first glance.

[sjberman]

This isn't used anywhere.

[sjberman]

You should be able to use slices.Delete() instead, using the index of the h entry in the hostnames list.

[salonichf5]

So when using slices.Delete(arr, i, j) --> we would have to specify the index which panics if out of bounds. And wouldn't deleting elements while looping the array lead to similar issues with indexing?

[sjberman]

I don't think you would be deleting while looping, you build the list first, then delete afterwards like you do now.

[sjberman]

Let's move these out into a variable.

[sjberman]

Let's name these better to reflect the listener name.

[sjberman]

Doesn't seem like we need a list of test cases here since you only have one.

[sjberman]

There could be an issue where a dev changes the name of one of the routes in the expectedResult, and then this condition never gets checked and the test passes.

[sjberman]

Same comments with this test as the last one.

[sindhushiv]

Both functions are almost identical. Combine them into a single generic function to improve maintainability. You can refactor the logic into a single function to reduce redundancy.