8000 TLSRoute: Require hostnames and bump version to v1alpha3 by rostislavbobo · Pull Request #3872 · kubernetes-sigs/gateway-api · GitHub
[go: up one dir, main page]
Skip to content

TLSRoute: Require hostnames and bump version to v1alpha3 #3872

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jul 14, 2025
Merged

TLSRoute: Require hostnames and bump version to v1alpha3 #3872

merged 8 commits into from
Jul 14, 2025

Conversation

rostislavbobo
Copy link
Member
@rostislavbobo rostislavbobo commented Jun 20, 2025 •
edited
8000 Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR makes the hostnames field in TLSRoute non-optional. Hostnames is the only available match for TLSRoutes and, per the TLSRoute spec, at least one hostname match is required. If SNI hostname matching is not needed, TCPRoute should be used instead.

Which issue(s) this PR fixes:

Fixes #3871

Does this PR introduce a user-facing change?:

- Adds v1alpha3 experimental TLSRoute with the hostnames field being required.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 20, 2025
@k8s-ci-robot
Copy link
Contributor

Welcome @rostislavbobo!

It looks like this is your first PR to kubernetes-sigs/gateway-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/gateway-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot
Copy link
Contributor

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

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 20, 2025
@rostislavbobo rostislavbobo force-pushed the tlsroute-hostnames-required branch from 8a842f0 to 5630d40 Compare June 27, 2025 12:43
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 27, 2025
@rostislavbobo rostislavbobo force-pushed the tlsroute-hostnames-required branch from 75ab19e to 563794e Compare June 27, 2025 13:01
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 27, 2025
@rostislavbobo rostislavbobo force-pushed the tlsroute-hostnames-required branch 4 times, most recently from 2cc1661 to bf124db Compare June 27, 2025 14:04
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 27, 2025
@rostislavbobo rostislavbobo force-pushed the tlsroute-hostnames-required branch from bf124db to 1c19f20 Compare June 27, 2025 14:53
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 27, 2025
@rostislavbobo rostislavbobo marked this pull request as ready for review June 27, 2025 16:47
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 27, 2025
@robscott
Copy link
Member

Thanks @rostislavbobo!

/ok-to-test

@rostislavbobo rostislavbobo force-pushed the tlsroute-hostnames-required branch from 07a0313 to afb02dc Compare July 10, 2025 21:48
@rostislavbobo rostislavbobo force-pushed the tlsroute-hostnames-required branch from 9dc3faf to 2ba0374 Compare July 10, 2025 22:07
robscott
robscott reviewed Jul 11, 2025
View reviewed changes
Copy link
Member
@robscott robscott left a comment

Choose a reason for hiding this comment

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

Thanks @rostislavbobo! Some small nits, but otherwise LGTM.

apis/v1alpha3/tlsroute_types.go
}

// TLSRouteSpec defines the desired state of a TLSRoute resource.
type TLSRouteSpec struct {
Copy link
Member

Choose a reason for hiding this comment

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

Going back in time to one of the recent times we did something similar, we used aliases in the old API version that pointed to the new one. I'd recommend doing that here as well. So in this case v1alpha3 would be the source of truth, and v1alpha2 would point at that. Take a look at this for an example:

gateway-api/apis/v1beta1/gateway_types.go

Lines 48 to 55 in 093f653

// GatewaySpec defines the desired state of Gateway.
//
// Not all possible combinations of options specified in the Spec are
// valid. Some invalid configurations can be caught synchronously via a
// webhook, but there are many cases that will require asynchronous
// signaling via the GatewayStatus block.
// +k8s:deepcopy-gen=false
type GatewaySpec = v1.GatewaySpec

Copy link
Member Author
@rostislavbobo rostislavbobo Jul 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

When promoting TLSRoutes to v1, we'll use aliases in v1alpha3 to reference the v1 version.

TLSRoute promotion from from v1alpha2 to v1alpha3 is different, let me summarize the conversation above:

  1. @youngnick asked to keep old v1alpha2 TLSRoute while introducing new v1alpha3 TLSRoute. This is to avoid breaking existing implementations that use v1alpha2 in prod.
    • As a result, we can only aliases unchanged parts of TLSRoute – TLSRouteStatus and TLSRouteRule. TLSRouteSpec has changed and can't be aliased.
  2. We can't reference v1alpha2 from v1alpha3.

robscott
robscott reviewed Jul 11, 2025
View reviewed changes
@rostislavbobo rostislavbobo force-pushed the tlsroute-hostnames-required branch 2 times, most recently from 7b67116 to 892a84d Compare July 13, 2025 16:15
@rostislavbobo rostislavbobo force-pushed the tlsroute-hostnames-required branch from 892a84d to cf5c16d Compare July 13, 2025 16:36
@robscott
Copy link
Member

/retitle TLSRoute: Require hostnames and bump version to v1alpha3

@k8s-ci-robot k8s-ci-robot changed the title TLSRoute: Require hostnames TLSRoute: Require hostnames and bump version to v1alpha3 Jul 14, 2025
@robscott
Copy link
Member

Thanks @rostislavbobo!

/approve
/cc @mlavacca @shaneutt @youngnick

@k8s-ci-robot k8s-ci-robot requested a review from shaneutt July 14, 2025 17:51
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2025
shaneutt
shaneutt approved these changes Jul 14, 2025
View reviewed changes
Copy link
Member
@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 14, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: robscott, rostislavbobo, shaneutt

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

The pull request process is described here

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 merged commit 6d76ec6 into kubernetes-sigs:main Jul 14, 2025
13 checks passed
rikatz
rikatz reviewed Jul 14, 2025
View reviewed changes
@rostislavbobo rostislavbobo deleted the tlsroute-hostnames-required branch July 15, 2025 11:28
@rostislavbobo rostislavbobo mentioned this pull request Jul 15, 2025
Merged
@shaneutt shaneutt moved this to Done in Release v1.4.0 Jul 15, 2025
@shaneutt shaneutt added this to the v1.4.0 milestone Jul 15, 2025
@rostislavbobo rostislavbobo mentioned this pull request Aug 18, 2025
Closed
11 tasks
@rikatz rikatz mentioned this pull request Sep 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robscott robscott robscott left review comments

@shaneutt shaneutt shaneutt approved these changes

@mlavacca mlavacca Awaiting requested review from mlavacca

@sunjayBhatia sunjayBhatia Awaiting requested review from sunjayBhatia

@youngnick youngnick Awaiting requested review from youngnick

+1 more reviewer

@rikatz rikatz rikatz left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@shaneutt shaneutt

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. 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 Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
Release v1.4.0
Status: Done
Milestone
v1.4.0
Development

Successfully merging this pull request may close these issues.

Require hostnames for TLSRoutes
7 participants
@rostislavbobo @k8s-ci-robot @robscott @youngnick @shaneutt @Rycieos @rikatz
0