Skip to content

Commit

Permalink
pkg/crd: fix type casting panic with new default *types.Alias
Browse files Browse the repository at this point in the history
In the latest version of go, a change was made to the generation of
Alias types by default. From the release notes:

> By default, go/types now produces Alias type nodes for type aliases.
> This behavior can be controlled by the GODEBUG gotypesalias flag. Its
> default has changed from 0 in Go 1.22 to 1 in Go 1.23.

This provoked a panic in the localNamedToSchema function when processing
any type alias becaused it was expecting only a *types.Named and not a
*types.Alias. This can be fixed by using an anonymous interface to more
broadly "ask" if the object supports the Obj() function instead of
asking it to be a specific type. Types *types.Named and *types.Alias
share this method so it can be used directly.

Note that it would be better (and would have made this easier to debug)
to retrieve the "ok" second return value and print some error on failure
to cast but this is a minimal working patch.

For example you can reproduce a panic, switching the project to use Go
1.23 (and thus gotypesalias to 1), putting the following file in
repro/repro.go:

	// +groupName=repro.io
	package repro

	import (
		metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	)

	type Repro struct {
		metav1.TypeMeta   `json:",inline"`
		metav1.ObjectMeta `json:"metadata"`

		Reproducer StringAlias `json:"reproducer"`
	}

	type StringAlias = string

Then run:

	go run ./cmd/controller-gen/ crd paths=./repro

You should see something similar to:

	panic: interface conversion: types.Type is *types.Alias, not *types.Named

	goroutine 1 [running]:
	sigs.k8s.io/controller-tools/pkg/crd.localNamedToSchema(0x4001315f50, 0x40003918e0)
		/home/mtardy.linux/controller-tools/pkg/crd/schema.go:258 +0x3bc
	sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0x4001315f50, {0xa391e8, 0x40003918e0})
		/home/mtardy.linux/controller-tools/pkg/crd/schema.go:197 +0xd0
	sigs.k8s.io/controller-tools/pkg/crd.structToSchema(0x40009ee5f8, 0x400000e300)
		/home/mtardy.linux/controller-tools/pkg/crd/schema.go:436 +0x7d8
	sigs.k8s.io/controller-tools/pkg/crd.typeToSchema(0x40009ee5f8, {0xa391b8, 0x400000e300})
		/home/mtardy.linux/controller-tools/pkg/crd/schema.go:207 +0x90
	sigs.k8s.io/controller-tools/pkg/crd.infoToSchema(0x40000505f8)
		/home/mtardy.linux/controller-tools/pkg/crd/schema.go:125 +0xcc
	sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedSchemaFor(0x400009a120, {0x4000207b80, {0x40002c2540, 0x5}})
		/home/mtardy.linux/controller-tools/pkg/crd/parser.go:193 +0x1e8
	sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedFlattenedSchemaFor(0x400009a120, {0x4000207b80, {0x40002c2540, 0x5}})
		/home/mtardy.linux/controller-tools/pkg/crd/parser.go:205 +0x9c
	sigs.k8s.io/controller-tools/pkg/crd.(*Parser).NeedCRDFor(0x400009a120, {{0x4000185a06, 0x8}, {0x40002c2540, 0x5}}, 0x0)
		/home/mtardy.linux/controller-tools/pkg/crd/spec.go:93 +0x3d8
	sigs.k8s.io/controller-tools/pkg/crd.Generator.Generate({0x0, 0x0, 0x0, {0x0, 0x0, 0x0}, 0x0, {0x0, 0x0}, {0x0, ...}, ...}, ...)
		/home/mtardy.linux/controller-tools/pkg/crd/gen.go:182 +0x464
	sigs.k8s.io/controller-tools/pkg/genall.(*Runtime).Run(0x400015cbd0)
		/home/mtardy.linux/controller-tools/pkg/genall/genall.go:272 +0x21c
	main.main.func1(0x40002a2200?, {0x40002d9ad0?, 0x4?, 0x8e519c?})
		/home/mtardy.linux/controller-tools/cmd/controller-gen/main.go:176 +0x64
	github.com/spf13/cobra.(*Command).execute(0x40002aec08, {0x4000136090, 0x3, 0x3})
		/home/mtardy.linux/go/pkg/mod/github.com/spf13/[email protected]/command.go:985 +0x834
	github.com/spf13/cobra.(*Command).ExecuteC(0x40002aec08)
		/home/mtardy.linux/go/pkg/mod/github.com/spf13/[email protected]/command.go:1117 +0x344
	github.com/spf13/cobra.(*Command).Execute(...)
		/home/mtardy.linux/go/pkg/mod/github.com/spf13/[email protected]/command.go:1041
	main.main()
		/home/mtardy.linux/controller-tools/cmd/controller-gen/main.go:200 +0x290
	exit status 2

Also added a test case to reproduce the error thanks to sbueringer.

Signed-off-by: Mahe Tardy <[email protected]>
  • Loading branch information
mtardy committed Sep 23, 2024
1 parent 0035369 commit 0a52475
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/crd/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiext.JSONSchema
}
// NB(directxman12): if there are dot imports, this might be an external reference,
// so use typechecking info to get the actual object
typeNameInfo := typeInfo.(*types.Named).Obj()
typeNameInfo := typeInfo.(interface{ Obj() *types.TypeName }).Obj()
pkg := typeNameInfo.Pkg()
pkgPath := loader.NonVendorPath(pkg.Path())
if pkg == ctx.pkg.Types {
Expand Down
5 changes: 5 additions & 0 deletions pkg/crd/testdata/cronjob_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,9 @@ type CronJobSpec struct {

HostsAlias Hosts `json:"hostsAlias,omitempty"`

// This tests that string alias is handled correctly.
StringAlias StringAlias `json:"stringAlias,omitempty"`

// This tests string slice validation.
// +kubebuilder:validation:MinItems=2
// +kubebuilder:validation:MaxItems=2
Expand All @@ -351,6 +354,8 @@ type CronJobSpec struct {
Protocol corev1.Protocol `json:"protocol,omitempty" protobuf:"bytes,4,opt,name=protocol,casttype=Protocol"`
}

type StringAlias = string

type ContainsNestedMap struct {
InnerMap map[string]string `json:"innerMap,omitempty"`
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8972,6 +8972,9 @@ spec:
time for any reason. Missed jobs executions will be counted as failed ones.
format: int64
type: integer
stringAlias:
description: This tests that string alias is handled correctly.
type: string
stringPair:
description: This tests string slice validation.
items:
Expand Down

0 comments on commit 0a52475

Please sign in to comment.