Merge pull request #796 from robscott/780-small-fixes

Variety of fixes in response to feedback on #780

Author: k8s-ci-robot

apis/v1alpha2/gateway_types.go

@@ -30,8 +30,21 @@ import (
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:printcolumn:name="Description",type=string,JSONPath=`.spec.description`,priority=1
 
-// GatewayClass describes a class of Gateways available to the user
-// for creating Gateway resources.
+// GatewayClass describes a class of Gateways available to the user for creating
+// Gateway resources.
+//
+// It is recommended that this resource be used as a template for Gateways. This
+// means that a Gateway is based on the state of the GatewayClass at the time it
+// was created and changes to the GatewayClass or associated parameters are not
+// propagated down to existing Gateways. This recommendation is intended to
+// limit the blast radius of changes to GatewayClass or associated parameters.
+// If implementations choose to propagate GatewayClass changes to existing
+// Gateways, that MUST be clearly documented by the implementation.
+//
+// Whenever one or more Gateways are using a GatewayClass, implementations MUST
+// add the `gateway-exists-finalizer.gateway.networking.k8s.io` finalizer on the
+// associated GatewayClass. This ensures that a GatewayClass associated with a
+// Gateway is not deleted while in use.
 //
 // GatewayClass is a Cluster level resource.
 type GatewayClass struct {
@@ -47,12 +60,19 @@ type GatewayClass struct {
 	Status GatewayClassStatus `json:"status,omitempty"`
 }
 
+const (
+	// GatewayClassFinalizerGatewaysExist should be added as a finalizer to the
+	// GatewayClass whenever there are provisioned Gateways using a
+	// GatewayClass.
+	GatewayClassFinalizerGatewaysExist = "gateway-exists-finalizer.gateway.networking.k8s.io"
+)
+
 // GatewayClassSpec reflects the configuration of a class of Gateways.
 type GatewayClassSpec struct {
 	// Controller is a domain/path string that indicates the
 	// controller that is managing Gateways of this class.
 	//
-	// Example: "acme.io/gateway-controller".
+	// Example: "example.net/gateway-controller".
 	//
 	// This field is not mutable and cannot be empty.
 	//
@@ -133,15 +153,15 @@ const (
 	// This condition defaults to False, and MUST be set by a controller when it
 	// sees a GatewayClass using its controller string. The status of this
 	// condition MUST be set to true if the controller will support provisioning
-	// Gateways using this class. Otherwise, this status MUST be set to false.
-	// If the status is set to false, the controller SHOULD set a Message and
+	// Gateways using this class. Otherwise, this status MUST be set to False.
+	// If the status is set to False, the controller SHOULD set a Message and
 	// Reason as an explanation.
 	//
 	// Possible reasons for this condition to be true are:
 	//
 	// * "Admitted"
 	//
-	// Possible reasons for this condition to be false are:
+	// Possible reasons for this condition to be False are:
 	//
 	// * "InvalidParameters"
 	// * "Waiting"
@@ -162,12 +182,8 @@ const (
 	// This reason is used with the "Admitted" condition when the
 	// requested controller has not yet made a decision about whether
 	// to admit the GatewayClass. It is the default Reason on a new
-	// GatewayClass. It indicates
+	// GatewayClass.
 	GatewayClassReasonWaiting GatewayClassConditionReason = "Waiting"
-
-	// GatewayClassFinalizerGatewaysExist should be added as a finalizer to the
-	// GatewayClass whenever there are provisioned Gateways using a GatewayClass.
-	GatewayClassFinalizerGatewaysExist = "gateway-exists-finalizer.gateway.networking.k8s.io"
 )
 
 // GatewayClassStatus is the current status for the GatewayClass.

apis/v1alpha2/gatewayclass_types.go

@@ -62,44 +62,29 @@ type HTTPRouteSpec struct {
 	// 1. IPs are not allowed.
 	// 2. The `:` delimiter is not respected because ports are not allowed.
 	//
-	// Incoming requests are matched against the hostnames before the
-	// HTTPRoute rules. If no hostname is specified, traffic is routed
-	// based on the HTTPRouteRules.
-	//
-	// Requests will be matched against the Host field in the following order:
-	//
-	// 1. If Hostname is precise, the request matches this rule if
-	//    the HTTP Host header is equal to the Hostname.
-	// 2. If Hostname is a wildcard, then the request matches this rule if
-	//    the HTTP Host header is to equal to the suffix
-	//    (removing the first label) of the wildcard rule.
-	// 3. If Hostname is unspecified, empty, or `*`, then any request will match
-	//    this route.
-	//
-	// If a hostname is specified by the Listener that the HTTPRoute is bound
-	// to, at least one hostname specified here must match the Listener specified
-	// hostname as per the rules above. Other hostnames will not affect processing
-	// of the route in that case.
-	//
-	// If no hostname is specified by the Listener, then that value will be treated
-	// as '*', match any hostname, and so any hostname on this Route will match.
-	//
-	// If all hostnames do not match, then the HTTPRoute is not admitted, and
-	// the implementation must raise an 'Admitted' Condition with a status of
-	// `false` for that Listener.
-	//
-	// Examples:
-	// - A Listener with unspecified, empty, or `*` values for Hostname matches
-	//   any HTTPRoute hostname.
-	// - A HTTPRoute with unspecified, empty, or `*` values for Hostname matches
-	//   any Listener hostname.
-	// - A Listener with `test.foo.com` as the hostname matches *only*
-	//   `test.foo.com` or `*.foo.com`. Any other hostnames present must be ignored.
-	// - A Listener with `*.foo.com` as hostname, all hostnames in the HTTPRoute
-	//   must have any single label where the star is, and the rest of the hostname
-	//   must match exactly. So, `test.foo.com`, `*.foo.com` or `blog.foo.com` match.
-	//   `test.blog.foo.com`, `test.bar.com`, or `bar.com` do not. Hostnames that do
-	//   not match will be ignored.
+	// If a hostname is specified by both the Listener and HTTPRoute, there
+	// must be at least one intersecting hostname for the HTTPRoute to be
+	// attached to the Listener. For example:
+	//
+	// * A Listener with `test.example.com` as the hostname matches HTTPRoutes
+	//   that have either not specified any hostnames, or have specified at
+	//   least one of `test.example.com` or `*.example.com`.
+	// * A Listener with `*.example.com` as as the hostname matches HTTPRoutes
+	//   that have either not specified any hostnames or have specified at least
+	//   one hostname that matches the Listener hostname. For example,
+	//   `test.example.com` and `*.example.com` would both match. On the other
+	//   hand, `a.b.example.com`, `example.com`, and `test.example.net` would
+	//   not match.
+	//
+	// If both the Listener and HTTPRoute have specified hostnames, any
+	// HTTPRoute hostnames that do not match the Listener hostname MUST be
+	// ignored. For example, if a Listener specified `*.example.com`, and the
+	// HTTPRoute specified `test.example.com` and `test.example.net`,
+	// `test.example.net` must not be considered for a match.
+	//
+	// If hostnames do not match with the criteria above, then the HTTPRoute is
+	// not admitted, and the implementation must raise an 'Admitted' Condition
+	// with a status of `False` for the target Listener(s).
 	//
 	// Support: Core
 	//
@@ -155,8 +140,10 @@ type HTTPRouteRule struct {
 	// of the following criteria, continuing on ties:
 	//
 	// * The longest matching hostname.
+	// * The longest matching non-wildcard hostname.
 	// * The longest matching path.
 	// * The largest number of header matches.
+	// * The largest number of query param matches.
 	//
 	// If ties still exist across multiple Routes, matching precedence MUST be
 	// determined in order of the following criteria, continuing on ties:
@@ -199,11 +186,14 @@ type HTTPRouteRule struct {
 	Filters []HTTPRouteFilter `json:"filters,omitempty"`
 
 	// BackendRefs defines the backend(s) where matching requests should be
-	// sent. If unspecified or invalid (refers to a non-existent resource or a Service with no endpoints),
-	// the rule performs no forwarding; if no filters are specified that would result in a
-	// response being sent, a HTTP 503 status code is returned. 503 responses must be sent so that the overall
-	// weight is respected; if an invalid backend is requested to have 80% of requests, then 80% of requests
-	// must get a 503 instead.
+	// sent.
+
+	// If unspecified or invalid (refers to a non-existent resource or a Service
+	// with no endpoints), the rule performs no forwarding. If that are also no
+	// filters specified that would result in a response being sent, a HTTP 503
+	// status code is returned. 503 responses must be sent so that the overall
+	// weight is respected; if an invalid backend is requested to have 80% of
+	// requests, then 80% of requests must get a 503 instead.
 	//
 	// Support: Core for Kubernetes Service
 	// Support: Custom for any other resource
@@ -466,11 +456,9 @@ type HTTPRouteMatch struct {
 
 	// ExtensionRef is an optional, implementation-specific extension to the
 	// "match" behavior. For example, resource "myroutematcher" in group
-	// "networking.acme.io". If the referent cannot be found, the rule is not
-	// included in the route. The controller should raise the "ResolvedRefs"
-	// condition on the Gateway with the "DegradedRoutes" reason. The gateway
-	// status for this route should be updated with a condition that describes
-	// the error more specifically.
+	// "networking.example.net". If the referent cannot be found, the rule is
+	// not included in the route. The controller must ensure the "ResolvedRefs"
+	// condition on the Route status is set to `status: False`.
 	//
 	// Support: Custom
 	//
@@ -528,7 +516,8 @@ type HTTPRouteFilter struct {
 	// +optional
 	RequestMirror *HTTPRequestMirrorFilter `json:"requestMirror,omitempty"`
 
-	// RequestRedirect defines a schema for a filter that redirects request.
+	// RequestRedirect defines a schema for a filter that responds to the
+	// request with an HTTP redirection.
 	//
 	// Support: Core
 	//
@@ -537,7 +526,7 @@ type HTTPRouteFilter struct {
 
 	// ExtensionRef is an optional, implementation-specific extension to the
 	// "filter" behavior.  For example, resource "myroutefilter" in group
-	// "networking.acme.io"). ExtensionRef MUST NOT be used for core and
+	// "networking.example.net"). ExtensionRef MUST NOT be used for core and
 	// extended filters.
 	//
 	// Support: Implementation-specific
@@ -650,10 +639,10 @@ type HTTPRequestHeaderFilter struct {
 	// +kubebuilder:validation:MaxItems=16
 	Add []HTTPHeader `json:"add,omitempty"`
 
-	// Remove the given header(s) from the HTTP request before the
-	// action. The value of RemoveHeader is a list of HTTP header
-	// names. Note that the header names are case-insensitive
-	// (see https://datatracker.ietf.org/doc/html/rfc2616#section-4.2).
+	// Remove the given header(s) from the HTTP request before the action. The
+	// value of Remove is a list of HTTP header names. Note that the header
+	// names are case-insensitive (see
+	// https://datatracker.ietf.org/doc/html/rfc2616#section-4.2).
 	//
 	// Input:
 	//   GET /foo HTTP/1.1
@@ -717,15 +706,14 @@ type HTTPRequestRedirect struct {
 type HTTPRequestMirrorFilter struct {
 	// BackendRef references a resource where mirrored requests are sent.
 	//
-	// If the referent cannot be found, this HTTPBackendRef is invalid
-	// and must be dropped from the Gateway. The controller must ensure the
-	// "ResolvedRefs" condition on the Gateway is set to `status: true`
-	// with the "DegradedRoutes" reason, and not configure this backend in the
-	// underlying implemenation.
+	// If the referent cannot be found, this BackendRef is invalid and must be
+	// dropped from the Gateway. The controller must ensure the "ResolvedRefs"
+	// condition on the Route status is set to `status: False` and not configure
+	// this backend in the underlying implementation.
 	//
 	// If there is a cross-namespace reference to an *existing* object
 	// that is not allowed by a ReferencePolicy, the controller must ensure the
-	// "ResolvedRefs"  condition on the Gateway is set to `status: true`,
+	// "ResolvedRefs"  condition on the Route is set to `status: False`,
 	// with the "RefNotPermitted" reason and not configure this backend in the
 	// underlying implementation.
 	//
@@ -743,15 +731,14 @@ type HTTPRequestMirrorFilter struct {
 type HTTPBackendRef struct {
 	// BackendRef is a reference to a backend to forward matched requests to.
 	//
-	// If the referent cannot be found, this HTTPBackendRef is invalid
-	// and must be dropped from the Gateway. The controller must ensure the
-	// "ResolvedRefs" condition on the Gateway is set to `status: true`
-	// with the "DegradedRoutes" reason, and not configure this backend in the
-	// underlying implemenation.
+	// If the referent cannot be found, this HTTPBackendRef is invalid and must
+	// be dropped from the Gateway. The controller must ensure the
+	// "ResolvedRefs" condition on the Route is set to `status: False` and not
+	// configure this backend in the underlying implementation.
 	//
 	// If there is a cross-namespace reference to an *existing* object
 	// that is not covered by a ReferencePolicy, the controller must ensure the
-	// "ResolvedRefs"  condition on the Gateway is set to `status: true`,
+	// "ResolvedRefs"  condition on the Route is set to `status: true`,
 	// with the "RefNotPermitted" reason and not configure this backend in the
 	// underlying implementation.
 	//
@@ -763,7 +750,7 @@ type HTTPBackendRef struct {
 	// +optional
 	BackendRef `json:",inline"`
 
-	// Filters defined at this-level should be executed if and only if the
+	// Filters defined at this level should be executed if and only if the
 	// request is being forwarded to the backend defined here.
 	//
 	// Support: Custom (For broader support of filters, use the Filters field

apis/v1alpha2/httproute_types.go

@@ -141,12 +141,12 @@ type BackendRef struct {
 	// BackendObjectReference references a Kubernetes object.
 	BackendObjectReference `json:",inline"`
 
-	// Weight specifies the proportion of HTTP requests forwarded to the
-	// referenced backend. This is computed as
-	// weight/(sum of all weights in this ForwardTo list). For non-zero values,
-	// there may be some epsilon from the exact proportion defined here
-	// depending on the precision an implementation supports. Weight is not a
-	// percentage and the sum of weights does not need to equal 100.
+	// Weight specifies the proportion of requests forwarded to the referenced
+	// backend. This is computed as weight/(sum of all weights in this
+	// BackendRefs list). For non-zero values, there may be some epsilon from
+	// the exact proportion defined here depending on the precision an
+	// implementation supports. Weight is not a percentage and the sum of
+	// weights does not need to equal 100.
 	//
 	// If only one backend is specified and it has a weight greater than 0, 100%
 	// of the traffic is forwarded to that backend. If weight is set to 0, no
@@ -166,9 +166,13 @@ type BackendRef struct {
 type RouteConditionType string
 
 const (
-	// This condition indicates whether the route has been admitted
-	// or rejected by a Gateway, and why.
+	// This condition indicates whether the route has been admitted or rejected
+	// by a Gateway, and why.
 	ConditionRouteAdmitted RouteConditionType = "Admitted"
+
+	// This condition indicates whether the controller was able to resolve all
+	// the object references for the Route.
+	ConditionRouteResolvedRefs RouteConditionType = "ResolvedRefs"
 )
 
 // RouteParentStatus describes the status of a route with respect to an
@@ -182,18 +186,32 @@ type RouteParentStatus struct {
 	// wrote this status. This corresponds with the controller field on
 	// GatewayClass.
 	//
-	// Example: "acme.io/gateway-controller".
+	// Example: "example.net/gateway-controller".
 	//
 	// The format of this field is DOMAIN "/" PATH, where DOMAIN and PATH are
 	// valid Kubernetes names
 	// (https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names).
 	Controller GatewayController `json:"controller"`
 
-	// Conditions describes the status of the route with respect to the
-	// Gateway. The "Admitted" condition must always be specified by controllers
-	// to indicate whether the route has been admitted or rejected by the Gateway,
-	// and why. Note that the route's availability is also subject to the Gateway's
-	// own status conditions and listener status.
+	// Conditions describes the status of the route with respect to the Gateway.
+	// Note that the route's availability is also subject to the Gateway's own
+	// status conditions and listener status.
+	//
+	// If the Route's ParentRef specifies an existing Gateway that supports
+	// Routes of this kind AND that Gateway's controller has sufficient access,
+	// then that Gateway's controller MUST set the "Admitted" condition on the
+	// Route, to indicate whether the route has been admitted or rejected by the
+	// Gateway, and why.
+	//
+	// A Route MUST be considered "Admitted" if at least one of the Route's
+	// rules is implemented by the Gateway.
+	//
+	// There are a number of cases where the "Admitted" condition may not be set
+	// due to lack of controller visibility, that includes when:
+	//
+	// * The Route refers to a non-existent parent.
+	// * The Route is of a type that the controller does not support.
+	// * The Route is in a namespace the the controller does not have access to.
 	//
 	// +listType=map
 	// +listMapKey=type
@@ -229,8 +247,6 @@ type RouteStatus struct {
 // Hostname can be "precise" which is a domain name without the terminating
 // dot of a network host (e.g. "foo.example.com") or "wildcard", which is a
 // domain name prefixed with a single wildcard label (e.g. `*.example.com`).
-// The wildcard character `*` must appear by itself as the first DNS label
-// and matches only a single label.
 //
 // Note that as per RFC1035 and RFC1123, a *label* must consist of lower case
 // alphanumeric characters or '-', and must start and end with an alphanumeric

apis/v1alpha2/shared_types.go

@@ -89,13 +89,11 @@ type TCPRouteRule struct {
 	Matches []TCPRouteMatch `json:"matches,omitempty"`
 
 	// BackendRefs defines the backend(s) where matching requests should be
-	// sent. If unspecified or invalid (refers to a non-existent resource or
-	// a Service with no endpoints), the rule performs no forwarding; if no
-	// filters are specified that would result in a response being sent, the
-	// underlying implementation must actively reject connection attempts to
-	// this backend. Connection rejections must respect weight; if an invalid
-	// backend is requested to have 80% of connections, then 80% of connections
-	// must be rejected instead.
+	// sent. If unspecified or invalid (refers to a non-existent resource or a
+	// Service with no endpoints), the underlying implementation MUST actively
+	// reject connection attempts to this backend. Connection rejections must
+	// respect weight; if an invalid backend is requested to have 80% of
+	// connections, then 80% of connections must be rejected instead.
 	//
 	// Support: Core for Kubernetes Service
 	// Support: Custom for any other resource
@@ -111,12 +109,10 @@ type TCPRouteRule struct {
 // given action.
 type TCPRouteMatch struct {
 	// ExtensionRef is an optional, implementation-specific extension to the
-	// "match" behavior.  For example, resource "mytcproutematcher" in group
-	// "networking.acme.io". If the referent cannot be found, the rule is not
-	// included in the route. The controller should raise the "ResolvedRefs"
-	// condition on the Gateway with the "DegradedRoutes" reason. The gateway
-	// status for this route should be updated with a condition that describes
-	// the error more specifically.
+	// "match" behavior. For example, resource "mytcproutematcher" in group
+	// "networking.example.net". If the referent cannot be found, the rule MUST
+	// not be included in the route. The controller must ensure the
+	// "ResolvedRefs" condition on the Route status is set to `status: False`.
 	//
 	// Support: Custom
 	//