From 5b0e340ddb81b1eb996dcfe4b6e06a5f164b2a24 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Tue, 30 Jul 2024 10:55:45 +0200 Subject: [PATCH 1/2] :warning: 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 --- pkg/crd/schema.go | 39 +++++++++++++++---- pkg/crd/testdata/cronjob_types.go | 33 ++++++++++++++++ .../testdata.kubebuilder.io_cronjobs.yaml | 8 ++++ 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go index 219d0dbb6..5f30ee855 100644 --- a/pkg/crd/schema.go +++ b/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) } diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go index e81a28acb..3429e8239 100644 --- a/pkg/crd/testdata/cronjob_types.go +++ b/pkg/crd/testdata/cronjob_types.go @@ -23,6 +23,7 @@ limitations under the License. package cronjob import ( + "encoding" "encoding/json" "fmt" "net/url" @@ -592,6 +593,30 @@ 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 an alias of [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() +} + // 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. @@ -655,6 +680,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"` } diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index cdc5190b2..f7c032e05 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/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, From 768681bf3bc970f039ec9585e6e389ed2f08008b Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Fri, 15 Nov 2024 10:21:10 +0100 Subject: [PATCH 2/2] :seedling: More tests for encoding.TextMarshaler Test that json.Marshaler has precedence over encoding.TextMarshaler. Signed-off-by: Tom Wieczorek --- pkg/crd/testdata/cronjob_types.go | 33 +++++++++++++++++-- .../testdata.kubebuilder.io_cronjobs.yaml | 5 +++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go index 3429e8239..4569d2f7d 100644 --- a/pkg/crd/testdata/cronjob_types.go +++ b/pkg/crd/testdata/cronjob_types.go @@ -27,6 +27,7 @@ import ( "encoding/json" "fmt" "net/url" + "strconv" "time" batchv1beta1 "k8s.io/api/batch/v1beta1" @@ -605,9 +606,9 @@ func (u *URL3) MarshalText() (text []byte, err error) { return u.MarshalBinary() } -// URL4 is an alias of [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. +// 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) @@ -617,6 +618,28 @@ 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. @@ -667,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 diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index f7c032e05..e99637a39 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml @@ -9240,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: