apimachinery icon indicating copy to clipboard operation
apimachinery copied to clipboard

Condition type has developer-focused godoc that ends up in CRDs that use the type

Open Miciah opened this issue 1 year ago • 1 comments

The metav1.Condition type definition has the following godoc:

// Condition contains details for one aspect of the current state of this API Resource.
// + ---
// + This struct is intended for direct use as an array at the field path .status.conditions.  For example,
// +
// + 	type FooStatus struct{
// + 	    // Represents the observations of a foo's current state.
// + 	    // Known .status.conditions.type are: "Available", "Progressing", and "Degraded"
// + 	    // +patchMergeKey=type
// + 	    // +patchStrategy=merge
// + 	    // +listType=map
// + 	    // +listMapKey=type
// + 	    Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
// +
// + 	    // other fields
// + 	}
type Condition struct {

The Type field has the following godoc:

	// type of condition in CamelCase or in foo.example.com/CamelCase.
	// ---
	// Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
	// useful (see .node.status.conditions), the ability to deconflict is important.
	// The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
	// +required
	// +kubebuilder:validation:Required
	// +kubebuilder:validation:Pattern=`^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$`
	// +kubebuilder:validation:MaxLength=316
	Type string `json:"type" protobuf:"bytes,1,opt,name=type"`

This information ends up in CRDs that use the Condition type. For example, it can be observed in Gateway API's HTTPRoute API, which uses the Condition type (observe the last part of the output before "FIELDS:"), as well as the output after type <string>; note that I omitted the output for other fields, as denoted by [...]):

% kubectl create -f https://raw.githubusercontent.com/kubernetes-sigs/gateway-api/main/config/crd/standard/gateway.networking.k8s.io_httproutes.yaml
customresourcedefinition.apiextensions.k8s.io/httproutes.gateway.networking.k8s.io created
% kubectl explain httproutes.status.parents.conditions
KIND:     HTTPRoute
VERSION:  gateway.networking.k8s.io/v1

RESOURCE: conditions <[]Object>

DESCRIPTION:
     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 "Accepted" condition on the Route, to
     indicate whether the route has been accepted or rejected by the Gateway,
     and why.


     A Route MUST be considered "Accepted" if at least one of the Route's rules
     is implemented by the Gateway.


     There are a number of cases where the "Accepted" 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 controller does not have access to.

     Condition contains details for one aspect of the current state of this API
     Resource. --- This struct is intended for direct use as an array at the
     field path .status.conditions. For example,


     type FooStatus struct{ // Represents the observations of a foo's current
     state. // Known .status.conditions.type are: "Available", "Progressing",
     and "Degraded" // +patchMergeKey=type // +patchStrategy=merge //
     +listType=map // +listMapKey=type Conditions []metav1.Condition
     `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"
     protobuf:"bytes,1,rep,name=conditions"`


     // other fields }

FIELDS:
[...]
   type <string> -required-
     type of condition in CamelCase or in foo.example.com/CamelCase. --- Many
     .condition.type values are consistent across resources like Available, but
     because arbitrary conditions can be useful (see .node.status.conditions),
     the ability to deconflict is important. The regex it matches is
     (dns1123SubdomainFmt/)?(qualifiedNameFmt)

Ideally, the godoc should use kubebuilder syntax to omit the developer-focused portions from generated CRDs:

diff --git a/pkg/apis/meta/v1/types.go b/pkg/apis/meta/v1/types.go
index 9695ba50..395990e0 100644
--- a/pkg/apis/meta/v1/types.go
+++ b/pkg/apis/meta/v1/types.go
@@ -1515,26 +1515,26 @@ type PartialObjectMetadataList struct {
 }
 
 // Condition contains details for one aspect of the current state of this API Resource.
-// ---
-// This struct is intended for direct use as an array at the field path .status.conditions.  For example,
-//
-//	type FooStatus struct{
-//	    // Represents the observations of a foo's current state.
-//	    // Known .status.conditions.type are: "Available", "Progressing", and "Degraded"
-//	    // +patchMergeKey=type
-//	    // +patchStrategy=merge
-//	    // +listType=map
-//	    // +listMapKey=type
-//	    Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
-//
-//	    // other fields
-//	}
+// + ---
+// + This struct is intended for direct use as an array at the field path .status.conditions.  For example,
+// +
+// + 	type FooStatus struct{
+// + 	    // Represents the observations of a foo's current state.
+// + 	    // Known .status.conditions.type are: "Available", "Progressing", and "Degraded"
+// + 	    // +patchMergeKey=type
+// + 	    // +patchStrategy=merge
+// + 	    // +listType=map
+// + 	    // +listMapKey=type
+// + 	    Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
+// +
+// + 	    // other fields
+// + 	}
 type Condition struct {
 	// type of condition in CamelCase or in foo.example.com/CamelCase.
-	// ---
-	// Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
-	// useful (see .node.status.conditions), the ability to deconflict is important.
-	// The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
+	// + ---
+	// + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
+	// + useful (see .node.status.conditions), the ability to deconflict is important.
+	// + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
 	// +required
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:Pattern=`^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$`

If this suggestion is acceptable, please let me know, and I will submit a PR. If I am reporting the issue in the wrong place, please let me know where the correct place to report the issue and suggest a change would be.

Miciah avatar Mar 19 '24 17:03 Miciah