⚠️ Add support for encoding.TextMarshaler (#1015)

* ⚠️ Add support for encoding.TextMarshaler

Whenever a type is encountered that implements encoding.TextMarshaler
but not encoding/json.Marshaler, assume that it will be encoded as
a string.

This is a breaking change, as types that implement TextMarshaler are now
handled differently. On the other hand, this probably restores reality
for all but the most customized setups, as Go's JSON package will also
check for the presence of this interface, and output a JSON string.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>

* 🌱 More tests for encoding.TextMarshaler

Test that json.Marshaler has precedence over encoding.TextMarshaler.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>

---------

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>

Author: twz123

pkg/crd/schema.go

@@ -113,12 +113,26 @@ func (c *schemaContext) requestSchema(pkgPath, typeName string) {
 
 // infoToSchema creates a schema for the type in the given set of type information.
 func infoToSchema(ctx *schemaContext) *apiext.JSONSchemaProps {
-	// If the obj implements a JSON marshaler and has a marker, use the markers value and do not traverse as
-	// the marshaler could be doing anything. If there is no marker, fall back to traversing.
-	if obj := ctx.pkg.Types.Scope().Lookup(ctx.info.Name); obj != nil && implementsJSONMarshaler(obj.Type()) {
-		schema := &apiext.JSONSchemaProps{}
-		applyMarkers(ctx, ctx.info.Markers, schema, ctx.info.RawSpec.Type)
-		if schema.Type != "" {
+	if obj := ctx.pkg.Types.Scope().Lookup(ctx.info.Name); obj != nil {
+		switch {
+		// If the obj implements a JSON marshaler and has a marker, use the
+		// markers value and do not traverse as the marshaler could be doing
+		// anything. If there is no marker, fall back to traversing.
+		case implements(obj.Type(), jsonMarshaler):
+			schema := &apiext.JSONSchemaProps{}
+			applyMarkers(ctx, ctx.info.Markers, schema, ctx.info.RawSpec.Type)
+			if schema.Type != "" {
+				return schema
+			}
+
+		// If the obj implements a text marshaler, encode it as a string.
+		case implements(obj.Type(), textMarshaler):
+			schema := &apiext.JSONSchemaProps{Type: "string"}
+			applyMarkers(ctx, ctx.info.Markers, schema, ctx.info.RawSpec.Type)
+			if schema.Type != "string" {
+				err := fmt.Errorf("%q implements encoding.TextMarshaler but schema type is not string: %q", ctx.info.RawSpec.Name, schema.Type)
+				ctx.pkg.AddError(loader.ErrFromNode(err, ctx.info.RawSpec.Type))
+			}
 			return schema
 		}
 	}
@@ -521,6 +535,15 @@ var jsonMarshaler = types.NewInterfaceType([]*types.Func{
 				types.NewVar(token.NoPos, nil, "", types.Universe.Lookup("error").Type())), false)),
 }, nil).Complete()
 
-func implementsJSONMarshaler(typ types.Type) bool {
-	return types.Implements(typ, jsonMarshaler) || types.Implements(types.NewPointer(typ), jsonMarshaler)
+// Open coded go/types representation of encoding.TextMarshaler
+var textMarshaler = types.NewInterfaceType([]*types.Func{
+	types.NewFunc(token.NoPos, nil, "MarshalText",
+		types.NewSignatureType(nil, nil, nil, nil,
+			types.NewTuple(
+				types.NewVar(token.NoPos, nil, "text", types.NewSlice(types.Universe.Lookup("byte").Type())),
+				types.NewVar(token.NoPos, nil, "err", types.Universe.Lookup("error").Type())), false)),
+}, nil).Complete()
+
+func implements(typ types.Type, iface *types.Interface) bool {
+	return types.Implements(typ, iface) || types.Implements(types.NewPointer(typ), iface)
 }

pkg/crd/testdata/cronjob_types.go

@@ -23,9 +23,11 @@ limitations under the License.
 package cronjob
 
 import (
+	"encoding"
 	"encoding/json"
 	"fmt"
 	"net/url"
+	"strconv"
 	"time"
 
 	batchv1beta1 "k8s.io/api/batch/v1beta1"
@@ -592,6 +594,52 @@ func (u *URL2) String() string {
 	return (*url.URL)(u).String()
 }
 
+// URL3 wraps [net/url.URL]. It implements [encoding.TextMarshaler] so that it
+// can be used in K8s CRDs such that the CRD resource will have the URL but
+// operator code can can work with the URL struct.
+type URL3 struct{ url.URL }
+
+var _ encoding.TextMarshaler = (*URL3)(nil)
+
+// MarshalText implements [encoding.TextMarshaler].
+func (u *URL3) MarshalText() (text []byte, err error) {
+	return u.MarshalBinary()
+}
+
+// URL4 is newtype around [net/url.URL]. It implements [encoding.TextMarshaler]
+// so that it can be used in K8s CRDs such that the CRD resource will have the
+// URL but operator code can can work with the URL struct.
+type URL4 url.URL
+
+var _ encoding.TextMarshaler = (*URL4)(nil)
+
+// MarshalText implements [encoding.TextMarshaler].
+func (u *URL4) MarshalText() (text []byte, err error) {
+	return (*url.URL)(u).MarshalBinary()
+}
+
+// +kubebuilder:validation:Type=integer
+// +kubebuilder:validation:Format=int64
+// Time2 is a newtype around [metav1.Time].
+// It implements both [encoding.TextMarshaler] and [json.Marshaler].
+// The latter is authoritative for the CRD generation.
+type Time2 time.Time
+
+var _ interface {
+	encoding.TextMarshaler
+	json.Marshaler
+} = (*Time2)(nil)
+
+// MarshalText implements [encoding.TextMarshaler].
+func (t *Time2) MarshalText() (text []byte, err error) {
+	return []byte((*time.Time)(t).String()), nil
+}
+
+// MarshalJSON implements [json.Marshaler].
+func (t *Time2) MarshalJSON() ([]byte, error) {
+	return strconv.AppendInt(nil, (*time.Time)(t).UnixMilli(), 10), nil
+}
+
 // Duration has a custom Marshaler but no markers.
 // We want the CRD generation to infer type information
 // from the go types and ignore the presense of the Marshaler.
@@ -642,6 +690,10 @@ type CronJobStatus struct {
 	// +optional
 	LastScheduleTime *metav1.Time `json:"lastScheduleTime,omitempty"`
 
+	// Information when was the last time the job was successfully scheduled.
+	// +optional
+	LastScheduleTime2 Time2 `json:"lastScheduleTime2,omitempty"`
+
 	// Information about the last time the job was successfully scheduled,
 	// with microsecond precision.
 	// +optional
@@ -655,6 +707,14 @@ type CronJobStatus struct {
 	// +optional
 	LastActiveLogURL2 *URL2 `json:"lastActiveLogURL2,omitempty"`
 
+	// LastActiveLogURL3 specifies the logging url for the last started job
+	// +optional
+	LastActiveLogURL3 *URL3 `json:"lastActiveLogURL3,omitempty"`
+
+	// LastActiveLogURL4 specifies the logging url for the last started job
+	// +optional
+	LastActiveLogURL4 *URL4 `json:"lastActiveLogURL4,omitempty"`
+
 	Runtime *Duration `json:"duration,omitempty"`
 }
 

pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml

@@ -9221,6 +9221,14 @@ spec:
                 description: LastActiveLogURL2 specifies the logging url for the last
                   started job
                 type: string
+              lastActiveLogURL3:
+                description: LastActiveLogURL3 specifies the logging url for the last
+                  started job
+                type: string
+              lastActiveLogURL4:
+                description: LastActiveLogURL4 specifies the logging url for the last
+                  started job
+                type: string
               lastScheduleMicroTime:
                 description: |-
                   Information about the last time the job was successfully scheduled,
@@ -9232,6 +9240,11 @@ spec:
                   scheduled.
                 format: date-time
                 type: string
+              lastScheduleTime2:
+                description: Information when was the last time the job was successfully
+                  scheduled.
+                format: int64
+                type: integer
             type: object
         type: object
     selectableFields: