Singularizes and expands godocs of HTTPRoute hostname #43

danehans · 2020-01-15T20:06:43Z

Previously the hostname of a HTTPRouteHost was plural. This PR singularizes the field and adds more detailed godocs.

Note: The RFC 3986 requirement is carried over from IngressRule v1beta1.

k8s-ci-robot · 2020-01-15T20:06:46Z

@danehans: GitHub didn't allow me to request PR reviews from the following users: ironcladlou, Miciah.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Previously the hostname of a HTTPRouteHost was plural. This PR singularizes the field and adds more detailed godocs.

Note: The RFC 3986 requirement is carried over from IngressRule v1beta1.

/assign @bowei
/cc @ironcladlou @Miciah

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/test-infra repository.

bowei · 2020-01-15T20:16:48Z

/approve

bowei · 2020-01-15T20:18:30Z

Referenced conversation: https://kubernetes.slack.com/archives/CR0H13KGA/p1578958692052500

Miciah

What is the motivation for singularizing Hostnames? Is it because Hosts in HTTPRouteSpec is already plural?

Miciah · 2020-01-15T20:19:16Z

api/v1alpha1/httproute_types.go

-	// TODO: RFC link
-	Hostnames []string `json:"hostnames"`
+	// 1. IPs are not allowed.
+	// 2. The `:` delimiter is not respected because ports are not allowed.


port is in authority but not in host.

Miciah · 2020-01-15T20:19:38Z

api/v1alpha1/httproute_types.go

 	//
-	// TODO: RFC link
-	Hostnames []string `json:"hostnames"`
+	// 1. IPs are not allowed.


Would it be simpler to say that the value must be a reg-name per RFC 3986?

I think stating "IPs are not allowed" will make sense to most, if not all, users. I just spent a few minutes reviewing https://tools.ietf.org/html/rfc3986 and I still don't understand what a reg-name is. Maybe we can iterate on the godoc for this field to include reg-name?

It is useful to state explicitly that IP addresses are prohibited, fair enough. I think referencing the specific syntax item is also useful. From the RFC:

host = IP-literal / IPv4address / reg-name reg-name = *( unreserved / pct-encoded / sub-delims ) pct-encoded = "%" HEXDIG HEXDIG unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" sub-delims = "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "="

Miciah · 2020-01-15T20:23:59Z

api/v1alpha1/httproute_types.go

+	//
+	// Incoming requests are matched against Hostname before processing HTTPRoute
+	// rules. If Hostname is unspecified, the Gateway routes all traffic based on
+	// the specified rules.


Does Hostname need to be unique among HTTPRoute resources?

How does SNI fit in? I would guess that for a TLS connection, first some certificate must match, and then the ingress controller uses the first route that also matches?

Does Hostname need to be unique among HTTPRoute resources?

@Miciah this is a good point. @bowei do you agree that the value should be unique?. I don't see any mention of this in the ingress v1beta api docs.

How does SNI fit in? I would guess that for a TLS connection, first some certificate must match, and then the ingress controller uses the first route that also matches?

I would expect similar behavior as ingress v1beta1. From the ingress docs:

If the TLS configuration section in an Ingress specifies different hosts, they are multiplexed on the same port according to the hostname specified through the SNI TLS extension (provided the Ingress controller supports SNI).

Translating the above doc to ingress v2, I would expect the following flow for ingress https connections:

The client initiates a connection to the HTTPRoute hostname.

The client resolves the HTTPRoute hostname to an IP address of the binding Gateway.

The connection hits the binding Gateway TLS listener (default 443).

The Gateway uses the SNI hostname to match the correct HTTPRoute

The Gateway processes the HTTPRoute rules and creates a corresponding connection to the backend service or endpoint associated to the service.

@bowei feel free to chime in if ^ is incorrect. It would also be helpful to understand if the Gateway implementation would forward directly to an endpoint to bypass kube-proxy or to the Service IP. Maybe this is up to the Gateway implementation?

Why do we need to singularize the hostname?
There’s a use case for having multiple hostnames in one endpoint, for example multi language site through example.TLD.
A client can select to one or the other hostname and the developer wants not to duplicate her Rules. If we singularize we need to duplicate the Rules.
I don’t see any blockers to not support it.

I think this should be singular but no unique.
Some use-cases create multiple routes with the same hosts but different paths (or other HTTP metadata). And these routes can belong to different teams.

Translating the above doc to ingress v2, I would expect the following flow for ingress https connections:

TLS and Routing details should not be clubbed together. Although, there is leakage between the layers of the OSI model, they are largely different layers. @danehans, in your description, I think Step 2 and 3 should be changed. HTTPRoute can't be resolved until after TLS is done. One can use only the TLS config setting of a route, but shouldn't select that route. Route selection is based on other metadata in the HTTP reqeust.

Regarding, Sandor's point, I think this can be opened up to allow for prefix and suffix wildcards in future if there is a need for it.

In the case of HTTPRoutes, the gateway can terminate TLS and then use HTTP to select a HTTPRoute, but in the case of a TCPRoute, HTTP headers are unavailable. So faced with an incoming TLS request, the gateway could first use SNI to build a set of candidate routes, which must comprise either multiple HTTPRoutes or a single TCPRoute, and then the gateway could either (a) terminate TLS and use HTTP metadata to select a single HTTPRoute or (b) use the TLSRoute.

I think Step 2 and 3 should be changed

@haiyanmeng I submitted #68 to doc a typical ingress request flow, ptal.

Reference for uniqueness: #67

I'm inclined to merge this and continue conversation about the wildcard on an issue specific to having wildcards.

I extracted the use case that @szuecs raised into #80.

danehans · 2020-01-15T20:31:16Z

What is the motivation for singularizing Hostnames? Is it because Hosts in HTTPRouteSpec is already plural?

@Miciah ptal at #43 (comment) for add'l background.

bowei · 2020-02-14T20:23:49Z

I think most of the issues raised above are captured in the ongoing discussions on the issues, so we can merge this.

/lgtm
/approve

k8s-ci-robot · 2020-02-14T20:24:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bowei, danehans

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:

~~OWNERS~~ [bowei]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

bowei · 2020-02-14T20:24:09Z

Please open an issue if you don't see something that should be tracked.

…-docs-reference-manifests updating multi-cluster-gateway docs manifests

k8s-ci-robot assigned bowei Jan 15, 2020

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 15, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 15, 2020

Miciah reviewed Jan 15, 2020

View reviewed changes

danehans changed the title ~~Changes HTTPRoute hostname field to be singular~~ Singularizes and expands godocs of HTTPRoute hostname Feb 10, 2020

danehans force-pushed the gw_single_host branch 2 times, most recently from 4e3a916 to 60d480d Compare February 10, 2020 19:11

Changes HTTPRoute hostname field to be singular

0544a05

danehans force-pushed the gw_single_host branch from 60d480d to 0544a05 Compare February 12, 2020 20:24

jpeach mentioned this pull request Feb 13, 2020

Use case: succinctly expose the same service on multiple names #80

Closed

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2020

k8s-ci-robot merged commit ef7cd09 into kubernetes-sigs:master Feb 14, 2020

danehans mentioned this pull request May 12, 2020

Singularizes example HTTPRoute hostname #192

Merged

jaison-tiu pushed a commit to jaison-tiu/gateway-api that referenced this pull request Apr 14, 2022

Merge pull request kubernetes-sigs#43 from GoogleCloudPlatform/update…

1181799

…-docs-reference-manifests updating multi-cluster-gateway docs manifests

mikemorris mentioned this pull request Oct 2, 2025

GEP-2643: TLSRoute #4064

Open

Singularizes and expands godocs of HTTPRoute hostname #43

Singularizes and expands godocs of HTTPRoute hostname #43

Uh oh!

Conversation

danehans commented Jan 15, 2020

Uh oh!

k8s-ci-robot commented Jan 15, 2020

Uh oh!

bowei commented Jan 15, 2020

Uh oh!

bowei commented Jan 15, 2020

Uh oh!

Miciah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szuecs Jan 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danehans commented Jan 15, 2020

Uh oh!

bowei commented Feb 14, 2020

Uh oh!

k8s-ci-robot commented Feb 14, 2020

Uh oh!

bowei commented Feb 14, 2020

Uh oh!

Uh oh!

szuecs Jan 15, 2020 •

edited

Loading