Skip to content

conformance: add TLSRoute rejection conformance tests#4433

Merged
k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Thealisyed:tlsroutestarter
Jan 29, 2026
Merged

conformance: add TLSRoute rejection conformance tests#4433
k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Thealisyed:tlsroutestarter

Conversation

@Thealisyed
Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind test
/area conformance-test

What this PR does / why we need it:
Add two new TLSRoute conformance tests that validate proper rejection
behavior and user feedback when a TLSRoute cannot attach to a Gateway:

1. TLSRouteInvalidNoMatchingListener: Verifies that a TLSRoute
referencing a port that doesn't exist on the Gateway receives
Accepted=False with Reason=NoMatchingParent.

2. TLSRouteInvalidNoMatchingListenerHostname: Verifies that a TLSRoute
with a hostname that doesn't match the Gateway listener hostname
receives Accepted=False with Reason=NoMatchingListenerHostname.

Both tests validate:
 - Route has correct rejection condition with appropriate reason
 - Route has no accepted parents in status
 - Gateway shows 0 attached routes

Also adds TLSRouteMustHaveNoAcceptedParents helper function to support
validation of TLSRoute rejection scenarios.

Does this PR introduce a user-facing change?:

@k8s-ci-robot k8s-ci-robot added kind/test do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. area/conformance-test Issues or PRs related to Conformance tests. labels Jan 16, 2026
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Jan 16, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: Thealisyed / name: Ali Syed (ae912b6)

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 16, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Hi @Thealisyed. 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.

Details

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-sigs/prow repository.

@Thealisyed Thealisyed force-pushed the tlsroutestarter branch 2 times, most recently from 35ff3ec to dfdb179 Compare January 16, 2026 12:43
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 16, 2026
@rikatz
Copy link
Copy Markdown
Member

rikatz commented Jan 16, 2026

/ok-to-test
/release-note-none

overall lgtm, I have tested locally with Istio and works as expected

/cc @rostislavbobo @snorwin @kl52752

@k8s-ci-robot k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Jan 16, 2026
@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 16, 2026
@k8s-ci-robot k8s-ci-robot requested a review from snorwin January 16, 2026 15:58
@k8s-ci-robot k8s-ci-robot removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 16, 2026
@rikatz
Copy link
Copy Markdown
Member

rikatz commented Jan 16, 2026

(as we work for the same company, I will leave the lgtm for someone else and then we can proceed with approval)

Copy link
Copy Markdown
Member

@rostislavbobo rostislavbobo left a comment

Choose a reason for hiding this comment

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

Thank you @Thealisyed for the conformance tests for rejection! This helps us to move TLSRoutes forward. Overall, looks good, just a couple of minot comments

Test: func(t *testing.T, suite *suite.ConformanceTestSuite) {
routeNN := types.NamespacedName{Name: "tlsroute-hostname-mismatch", Namespace: "gateway-conformance-infra"}
gwNN := types.NamespacedName{Name: "gateway-tls-hostname", Namespace: "gateway-conformance-infra"}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

don't we need to add kubernetes.NamespacesMustBeReady() here because we're we're creating an additional Gateway in this test and we need to wait for it to be ready

listeners:
- name: tls
port: 443
protocol: TLS
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

make this HTTP, otherwise it will fail early. Usually controllers will drop this kind of configuration on listener already as you are enforcing a kind HTTPRoute, but a type listener.

Let's take a look into Istio example: https://github.com/istio/istio/blob/eab5fb0686458842f328ff53f0d985ec1fa6df5e/pilot/pkg/config/kube/gateway/conditions.go#L377

It already gets the listener type protocol to define what kind of routes can be added on kinds. If you enforce TLS and HTTPRoute, it may fail early

}
return true, nil
}
if len(actual) > 1 {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

actually this validation is broken.

You are passing 2 parentRefs here: https://github.com/kubernetes-sigs/gateway-api/pull/4433/files#diff-7d16187df452dee9c420da3b314b29038f0335dca12e33e125057fb69fa9a824R48-R52

So this will never work :) Because your controller is reporting 2 parentRefs.

Add TLSRoute conformance tests for rejection scenarios:

  TLSRouteInvalidNoMatchingListener:
  - No matching listener port
  - allowedRoutes.kinds excludes TLSRoute
  - Protocol mismatch (HTTPS vs TLS)
  - Invalid sectionName

  TLSRouteInvalidNoMatchingListenerHostname:
  - Two Gateways (exact + wildcard hostnames)
  - Two TLSRoutes with non-matching exact + wildcard hostnames

  All tests verify:
  - Correct Accepted=False condition with appropriate reason
  - No accepted parents in route status
  - Gateway shows 0 attached routes

  Also adds generic RouteMustHaveNoAcceptedParents helper using
  reflection, reducing duplication per kubernetes-sigs#1728.
@rikatz
Copy link
Copy Markdown
Member

rikatz commented Jan 28, 2026

changes lgtm, I will defer for @rostislavbobo and @robscott for approval.

I think there are some conflicting scenarios on TLSRouteInvalidNoMatchingListener that we may need to take care, but given that the author of the PR will be out for the next week and we are willing to have all the TLSRoute conformance merged, probably me or @rostislavbobo can fix some things as followups

Copy link
Copy Markdown
Member

@rostislavbobo rostislavbobo left a comment

Choose a reason for hiding this comment

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

Thanks @Thealisyed for covering more cases and addressing comments! LGTM pending minor comments from my side

Comment on lines +35 to +49
kind: Gateway
metadata:
name: gateway-tls-httproute-only
namespace: gateway-conformance-infra
spec:
gatewayClassName: "{GATEWAY_CLASS_NAME}"
listeners:
- name: http
port: 80
protocol: HTTP
allowedRoutes:
namespaces:
from: Same
kinds:
- kind: HTTPRoute
Copy link
Copy Markdown
Member

@rostislavbobo rostislavbobo Jan 28, 2026

Choose a reason for hiding this comment

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

If I understand correctly, this Gateway is used by "tlsroute-not-allowed-kind" to verify that the TLSRoute is not attached when allowedRoutes.kinds doesn't list it.

In that case, wouldn't it be better to configure the listener with protocol: TLS instead of HTTP? So that we have a valid attachment that is only prevented by the allowedRoutes.kinds.

namespace: gateway-conformance-infra
spec:
parentRefs:
- name: gateway-http-only
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again, I'd use a valid Gateway here for attachment to check that the wrong sectionName is the only preventing the attachment

tls:
mode: Passthrough
---
apiVersion: gateway.networking.k8s.io/v1alpha2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mind using v1alpha3 here and everywhere below?

@rostislavbobo
Copy link
Copy Markdown
Member

there are some conflicting scenarios on TLSRouteInvalidNoMatchingListener that we may need to take care
probably me or @rostislavbobo can fix some things as followups

SG @rikatz , let's sync on that tmr

@rostislavbobo
Copy link
Copy Markdown
Member

/lgtm

and taking my comments on me as the author of the PR is out

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 29, 2026
@rikatz
Copy link
Copy Markdown
Member

rikatz commented Jan 29, 2026

@rostislavbobo I can fix them as a followup if you are out of time :D just let me know!

@rikatz
Copy link
Copy Markdown
Member

rikatz commented Jan 29, 2026

/approve
/hold
Wait for some time and then we merge

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 29, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rikatz, rostislavbobo, Thealisyed

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

The pull request process is described here

Details 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 29, 2026
@rikatz
Copy link
Copy Markdown
Member

rikatz commented Jan 29, 2026

/hold cancel
Given @rostislavbobo reviewed as a 2nd person (out of Red Hat) and we are ok with this approach, merging.

Rosti will followup on his own comments that are non-blocking for TLSRoute

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 29, 2026
@k8s-ci-robot k8s-ci-robot merged commit 2fae1d5 into kubernetes-sigs:main Jan 29, 2026
8 checks passed
danehans pushed a commit to danehans/gateway-api that referenced this pull request Mar 10, 2026
…s#4433)

Add TLSRoute conformance tests for rejection scenarios:

  TLSRouteInvalidNoMatchingListener:
  - No matching listener port
  - allowedRoutes.kinds excludes TLSRoute
  - Protocol mismatch (HTTPS vs TLS)
  - Invalid sectionName

  TLSRouteInvalidNoMatchingListenerHostname:
  - Two Gateways (exact + wildcard hostnames)
  - Two TLSRoutes with non-matching exact + wildcard hostnames

  All tests verify:
  - Correct Accepted=False condition with appropriate reason
  - No accepted parents in route status
  - Gateway shows 0 attached routes

  Also adds generic RouteMustHaveNoAcceptedParents helper using
  reflection, reducing duplication per kubernetes-sigs#1728.
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. area/conformance-test Issues or PRs related to Conformance tests. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/test 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. release-note-none Denotes a PR that doesn't merit a release note. 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.

5 participants