From 5462dfc22cfc8fab22a6930b0ad780c3ba2c4f8b Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Tue, 30 Jul 2024 10:55:45 +0200 Subject: [PATCH] :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 2eaadb675..7656387b5 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 } } @@ -493,6 +507,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 50cd689b6..f39d83f5f 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" @@ -552,6 +553,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. @@ -611,6 +636,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 95a5d870b..2785b7750 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml @@ -7018,6 +7018,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,