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

Conformance Tests: TLSRoute Passthrough #1579

Open
1 of 16 tasks
Tracked by #3165
candita opened this issue Dec 5, 2022 · 14 comments
Open
1 of 16 tasks
Tracked by #3165

Conformance Tests: TLSRoute Passthrough #1579

candita opened this issue Dec 5, 2022 · 14 comments
Labels
area/conformance priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@candita
Copy link
Contributor

candita commented Dec 5, 2022

This issue is intended to track conformance test development for TLSRoute Passthrough. Comment below if you're interested in working on covering any of these areas.

Core Capabilities:

  • Valid TLSRoute with 1 backendRef/rule
  • Valid TLSRoute with 1 backendRef/Rule and matching SNI hostname
  • Valid TLSRoute with 1 backendRef/Rule and matching SNI hostname with wildcard prefix
  • Valid TLSRoute with 1 backendRef/Rule and matching SNI hostname for both Listener and TLSRoute with intersecting hostname
    - [ ] Invalid TLSRoute - no backendRef/rule doesn't become Ready
    - [ ] Invalid TLSRoute - invalid backendRef/rule doesn't become Ready (some or all from sublist?)
    - [ ] Missing Rule
    - [ ] Missing Port
    - [ ] Kind, if not nil, is also not Service
    - [ ] Multi ParentRef sections names not set
    - [ ] Multi ParentRef sections names not unique
    - [ ] SNI hostname invalid - IP address
    • TLSRoute must not be Accepted for any invalid hostnames; ‘Accepted’ condition False in RouteParentStatus
      - [ ] SNI hostname invalid - not RFC1123, not wildcard prefix
    • Matching SNI hostname for both Listener and TLSRoute without intersecting hostname
    • TLSRoute hostname that doesn’t match the Listener hostname is ignored
      - [ ] More than 16 hostnames
      - [ ] More than 16 rules
      - [ ] More than 16 backendRef
      -[ ] Invalid TLSRoute with 1 backendRef/Rule that doesn’t exist performs no default forwarding
  • Invalid TLSRoute with invalid BackendObjectReference performs no default forwarding (some or all from sublist?)
    - [ ] Name not set
    - [ ] Kind not set
    • Namespace (of backend) not set
    • Namespace (of backend) missing ReferenceGrant
      - [ ] Port missing when Kind is Service
  • Invalid TLSRoute with filters that provide response must REJECT connection or return 500 status (some or all from sublist?)
    • Rejections must observe weight, e.g. if 50% might have been delivered, then 50% should be rejected instead
  • For a Listener setting mode: "terminate", attempting to set TLSRoute in AllowedRoutes.kinds would yield a ListenerConditionType Accepted with status: false and ListenerConditionReason InvalidRouteKinds
    • this could probably be validated by webhook too
  • For a Listener setting mode: "terminate", TLSRoute should not be present in ListenerStatus.SupportedKinds
  • Attempting to attach a TLSRoute to a Listener setting mode: "terminate" should yield RouteConditionType Accepted with status: false and RouteConditionReason NotAllowedByListeners on the TLSRoute
@robscott
Copy link
Member

robscott commented Dec 5, 2022

Thanks @candita! I think it would be worth differentiating between tests of our API validation and tests of the actual underlying implementation. At least so far, I've been working with the assumption that conformance tests are only intended to cover implementation details, not our CRD or webhook validation. #1514 has a bit more context on that discussion though.

@youngnick
Copy link
Contributor

I agree that for now, if one of these cases is covered by CRD validation, webhook validation, or both, we can check it off, and then come back to testing it as part of sorting out #1514.

candita added a commit to candita/gateway-api that referenced this issue Dec 8, 2022
candita added a commit to candita/gateway-api that referenced this issue Dec 8, 2022
@candita
Copy link
Contributor Author

candita commented Dec 8, 2022

Does this adjusted list make more sense @robscott @youngnick ?

@youngnick
Copy link
Contributor

Yep, that looks great, thanks @candita!

@mikemorris
Copy link
Contributor

Suggested additions to the list of conformance tests since we've discussed disallowing mode: "terminate" with TLSRoute:

@youngnick
Copy link
Contributor

Those additions look great, thanks @mikemorris.

candita added a commit to candita/gateway-api that referenced this issue Dec 19, 2022
candita added a commit to candita/gateway-api that referenced this issue Dec 20, 2022
candita added a commit to candita/gateway-api that referenced this issue Dec 20, 2022
candita added a commit to candita/gateway-api that referenced this issue Dec 20, 2022
candita added a commit to candita/gateway-api that referenced this issue Dec 20, 2022
candita added a commit to candita/gateway-api that referenced this issue Dec 20, 2022
candita added a commit to candita/gateway-api that referenced this issue Dec 20, 2022
candita added a commit to candita/gateway-api that referenced this issue Jan 3, 2023
candita added a commit to candita/gateway-api that referenced this issue Jan 3, 2023
candita added a commit to candita/gateway-api that referenced this issue Jan 4, 2023
candita added a commit to candita/gateway-api that referenced this issue Jan 4, 2023
candita added a commit to candita/gateway-api that referenced this issue Jan 4, 2023
candita added a commit to candita/gateway-api that referenced this issue Jan 4, 2023
candita added a commit to candita/gateway-api that referenced this issue Jan 18, 2023
@candita
Copy link
Contributor Author

candita commented Jan 18, 2023

@mikemorris

Suggested additions to the list of conformance tests since we've discussed disallowing mode: "terminate" with TLSRoute:

I added these to the description so they wouldn't get lost. Can you provide more detail about this could probably be validated by webhook too? It's just a comment, not a test, right?

k8s-ci-robot pushed a commit that referenced this issue Jan 23, 2023
* Issue #1579 TLSRoute Passthrough Conformance Test (normative) rebase

* Issue #1579 TLSRoute Passthrough - PR review update - rebase

* Issue #1579 TLSRoute Passthrough - PR review update - latest
@shaneutt shaneutt modified the milestones: v0.6.1, v0.6.2 Jan 31, 2023
shaneutt pushed a commit that referenced this issue Feb 7, 2023
* Issue #1579 TLSRoute Passthrough Conformance Test (normative) rebase

* Issue #1579 TLSRoute Passthrough - PR review update - rebase

* Issue #1579 TLSRoute Passthrough - PR review update - latest
@shaneutt shaneutt added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Feb 21, 2023
@shaneutt shaneutt modified the milestones: v0.6.2, v0.7.0, v1.0.0 Feb 21, 2023
@shaneutt shaneutt removed this from the v1.0.0 milestone May 18, 2023
@sunjayBhatia
Copy link
Member

Added an issue here #2153 which is a follow up for #2076

Looks relevant to include as part of the checklist here

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please 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 Jan 23, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please 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 Feb 22, 2024
@youngnick
Copy link
Contributor

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Feb 29, 2024
@youngnick
Copy link
Contributor

This is a blocker for #2643.

@candita
Copy link
Contributor Author

candita commented May 22, 2024

@sunjayBhatia

Added an issue here #2153 which is a follow up for #2076

Looks relevant to include as part of the checklist here

That is this one, correct?

  • Invalid TLSRoute with invalid BackendObjectReference performs no default forwarding (some or all from sublist?)

@mikemorris
Copy link
Contributor

mikemorris commented Jun 17, 2024

Updating my prior comment about passthrough and terminate - there's not a conclusive determination yet on what the expected behavior should be for this, and we'll probably need to figure that out as part of encoding anything in conformance.

Refs Slack thread, #1474 and #2111

/cc @kate-osborn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/conformance priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

No branches or pull requests

8 participants