Variety of fixes in response to feedback on kubernetes-sigs#780

Author: robscott

apis/v1alpha2/gateway_types.go

@@ -30,13 +30,8 @@ import (
 // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 
-// Gateway represents an instantiation of a service-traffic handling
-// infrastructure by binding Listeners to a set of IP addresses.
-//
-// Implementations should add the `gateway-exists-finalizer.gateway.networking.k8s.io`
-// finalizer on the associated GatewayClass whenever Gateway(s) is running.
-// This ensures that a GatewayClass associated with a Gateway(s) is not
-// deleted while in use.
+// Gateway represents an instance of a service-traffic handling infrastructure
+// by binding Listeners to a set of IP addresses.
 type Gateway struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -52,7 +47,7 @@ type Gateway struct {
 
 // +kubebuilder:object:root=true
 
-// GatewayList contains a list of Gateway.
+// GatewayList contains a list of Gateways.
 type GatewayList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`
@@ -118,18 +113,17 @@ type GatewaySpec struct {
 	// +kubebuilder:validation:MaxItems=64
 	Listeners []Listener `json:"listeners"`
 
-	// Addresses requested for this gateway. This is optional and
-	// behavior can depend on the GatewayClass. If a value is set
-	// in the spec and the requested address is invalid, the
-	// GatewayClass MUST indicate this in the associated entry in
-	// GatewayStatus.Addresses.
+	// Addresses requested for this Gateway. This is optional and behavior can
+	// depend on the implementation. If a value is set in the spec and the
+	// requested address is invalid or unavailable, the implementation MUST
+	// indicate this in the associated entry in GatewayStatus.Addresses.
 	//
-	// If no Addresses are specified, the GatewayClass may
-	// schedule the Gateway in an implementation-defined manner,
-	// assigning an appropriate set of Addresses.
+	// If no Addresses are specified, the implementation MAY schedule the
+	// Gateway in an implementation-specific manner, assigning an appropriate
+	// set of Addresses.
 	//
-	// The GatewayClass MUST bind all Listeners to every
-	// GatewayAddress that it assigns to the Gateway.
+	// The implementation MUST bind all Listeners to every GatewayAddress that
+	// it assigns to the Gateway.
 	//
 	// Support: Core
 	//
@@ -151,18 +145,18 @@ type Listener struct {
 	Name SectionName `json:"name"`
 
 	// Hostname specifies the virtual hostname to match for protocol types that
-	// define this concept. When unspecified, "", or `*`, all hostnames are
-	// matched. This field can be omitted for protocols that don't require
-	// hostname based matching.
-	//
-	// For HTTPRoute objects, there is an interaction with the
-	// `spec.hostnames` array. When both listener and route specify hostnames,
-	// there must be an intersection between the values for a Route to be admitted.
-	// For example, a Gateway with `*.example.com` would admit a Route that included
+	// define this concept. When unspecified or `*`, all hostnames are matched.
+	// This field can be omitted for protocols that don't require hostname based
+	// matching.
+	//
+	// For HTTPRoute objects, there is an interaction with the `spec.hostnames`
+	// array. When both listener and route specify hostnames, there must be an
+	// intersection between the values for a Route to be admitted. For example,
+	// a Gateway with `*.example.com` would admit a Route that included
 	// `foo.example.com` as a hostname, but not a Route that *only* included
-	// `foo.acme.io` as a hostname. A Route that included both `foo.example.com`
-	// and `foo.acme.io` would be admitted, but the `foo.acme.io` hostname would
-	// be silently ignored.
+	// `foo.example.net` as a hostname. A Route that included both
+	// `foo.example.com` and `foo.example.net` would be admitted, but the
+	// `foo.example.net` hostname would be silently ignored.
 	//
 	// Support: Core
 	//
@@ -207,7 +201,7 @@ type Listener struct {
 	// +optional
 	TLS *GatewayTLSConfig `json:"tls,omitempty"`
 
-	// Routes specifies which Routes may be attached to this Listener.
+	// AllowedRoutes specifies which Routes may be attached to this Listener.
 	//
 	// Although a client request may technically match multiple route rules,
 	// only one rule may ultimately receive the request. Matching precedence
@@ -232,7 +226,7 @@ type Listener struct {
 	// Support: Core
 	// +kubebuilder:default={namespaces:{from: Same}}
 	// +optional
-	Routes *ListenerRoutes `json:"routes,omitempty"`
+	AllowedRoutes *AllowedRoutes `json:"allowedRoutes,omitempty"`
 }
 
 // ProtocolType defines the application protocol accepted by a Listener.
@@ -293,7 +287,7 @@ type GatewayTLSConfig struct {
 	// References to a resource in different namespace are invalid UNLESS there
 	// is a ReferencePolicy in the target namespace that allows the certificate
 	// to be attached. If a ReferencePolicy does not allow this reference, the
-	// "ResolvedRefs" condition MUST be set to false for this listener with the
+	// "ResolvedRefs" condition MUST be set to False for this listener with the
 	// "InvalidCertificateRef" reason.
 	//
 	// This field is required when mode is set to "Terminate" (default) and
@@ -339,8 +333,8 @@ const (
 	TLSModePassthrough TLSModeType = "Passthrough"
 )
 
-// ListenerRoutes defines which Routes may be attached to this Listener.
-type ListenerRoutes struct {
+// AllowedRoutes defines which Routes may be attached to this Listener.
+type AllowedRoutes struct {
 	// Namespaces indicates which namespaces Routes may be attached to this
 	// Listener from. This is restricted to the namespace of this Gateway by
 	// default.
@@ -358,7 +352,7 @@ type ListenerRoutes struct {
 	// A RouteGroupKind MUST correspond to kinds of Routes that are compatible
 	// with the application protocol specified in the Listener's Protocol field.
 	// If an implementation does not support or recognize this resource type, it
-	// MUST set the "ResolvedRefs" condition to false for this Listener with the
+	// MUST set the "ResolvedRefs" condition to False for this Listener with the
 	// "InvalidRoutesRef" reason.
 	//
 	// Support: Core
@@ -532,10 +526,9 @@ const (
 	//
 	// * "Scheduled"
 	//
-	// Possible reasons for this condition to be false are:
+	// Possible reasons for this condition to be False are:
 	//
 	// * "NotReconciled"
-	// * "NoSuchGatewayClass"
 	// * "NoResources"
 	//
 	// Controllers may raise this condition with other reasons,
@@ -551,13 +544,6 @@ const (
 	// been recently created and no controller has reconciled it yet.
 	GatewayReasonNotReconciled GatewayConditionReason = "NotReconciled"
 
-	// This reason is used with the "Scheduled" condition when the Gateway is
-	// not scheduled because there is no controller that recognizes the
-	// GatewayClassName. This reason has been deprecated and will be removed in
-	// a future release.
-	// +deprecated
-	GatewayReasonNoSuchGatewayClass GatewayConditionReason = "NoSuchGatewayClass"
-
 	// This reason is used with the "Scheduled" condition when the
 	// Gateway is not scheduled because insufficient infrastructure
 	// resources are available.
@@ -579,7 +565,7 @@ const (
 	//
 	// * "Ready"
 	//
-	// Possible reasons for this condition to be false are:
+	// Possible reasons for this condition to be False are:
 	//
 	// * "ListenersNotValid"
 	// * "ListenersNotReady"
@@ -614,9 +600,7 @@ const (
 
 // ListenerStatus is the status associated with a Listener.
 type ListenerStatus struct {
-	// Name is the name of the Listener. If the Gateway has more than one
-	// Listener present, each ListenerStatus MUST specify a name. The names of
-	// ListenerStatus objects MUST be unique within a Gateway.
+	// Name is the name of the Listener that this status corresponds to.
 	Name SectionName `json:"name"`
 
 	// SupportedKinds is the list indicating the Kinds supported by this
@@ -662,7 +646,7 @@ const (
 	// * "ProtocolConflict"
 	// * "RouteConflict"
 	//
-	// Possible reasons for this condition to be false are:
+	// Possible reasons for this condition to be False are:
 	//
 	// * "NoConflicts"
 	//
@@ -690,7 +674,7 @@ const (
 	ListenerReasonRouteConflict ListenerConditionReason = "RouteConflict"
 
 	// This reason is used with the "Conflicted" condition when the condition
-	// is false.
+	// is False.
 	ListenerReasonNoConflicts ListenerConditionReason = "NoConflicts"
 )
 
@@ -712,7 +696,7 @@ const (
 	// * "UnsupportedProtocol"
 	// * "UnsupportedAddress"
 	//
-	// Possible reasons for this condition to be false are:
+	// Possible reasons for this condition to be False are:
 	//
 	// * "Attached"
 	//
@@ -742,7 +726,7 @@ const (
 	ListenerReasonUnsupportedAddress ListenerConditionReason = "UnsupportedAddress"
 
 	// This reason is used with the "Detached" condition when the condition is
-	// false.
+	// False.
 	ListenerReasonAttached ListenerConditionReason = "Attached"
 )
 
@@ -754,7 +738,7 @@ const (
 	//
 	// * "ResolvedRefs"
 	//
-	// Possible reasons for this condition to be false are:
+	// Possible reasons for this condition to be False are:
 	//
 	// * "DegradedRoutes"
 	// * "InvalidCertificateRef"
@@ -770,10 +754,10 @@ const (
 	// is true.
 	ListenerReasonResolvedRefs ListenerConditionReason = "ResolvedRefs"
 
-	// This reason is used with the "ResolvedRefs" condition
-	// when not all of the routes selected by this Listener could be
-	// configured. The specific reason for the degraded route should
-	// be indicated in the route's .Status.Conditions field.
+	// This reason is used with the "ResolvedRefs" condition when not all of the
+	// routes selected by this Listener could be configured. The specific reason
+	// for the degraded route should be indicated in the Conditions field of the
+	// appropriate entry of the route's .Status.Parents.
 	ListenerReasonDegradedRoutes ListenerConditionReason = "DegradedRoutes"
 
 	// This reason is used with the "ResolvedRefs" condition when the
@@ -803,7 +787,7 @@ const (
 	//
 	// * "Ready"
 	//
-	// Possible reasons for this condition to be false are:
+	// Possible reasons for this condition to be False are:
 	//
 	// * "Invalid"
 	// * "Pending"

apis/v1alpha2/gatewayclass_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.