Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

controller-gen: Type aliases to array types don't work with +listType/+listMapKey markers #988

Open
ahmetb opened this issue Jun 12, 2024 · 2 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@ahmetb
Copy link
Member

ahmetb commented Jun 12, 2024

Summary

If Go type of a CRD has a field that uses +listType or +listMapKey markers, but the field (struct member) is a typedef to a slice type, the field is not detected as an "array" field, and these markers cannot be applied to it.

Example

// +k8s:deepcopy-gen=true
type Conditions []Condition

type FooStatus struct{
	// +patchMergeKey=type
	// +patchStrategy=merge
	// +listType=map
	// +listMapKey=type
	Conditions Conditions `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
}

Fails with error:

controller-gen crd paths=./api/... [...]

[...]/status_types.go:35:2: must apply listType to an array, found
[...]/status_types.go:35:2: must apply listMapKey to an array, found

Details

This reproes in HEAD (as well as v0.14, v0.15).

If we do not use the declaration for the slice type (type Conditions []Condition) in the root object and instead use []Conditions as the struct member type, it works as expected.

I think this is happening probably because the tool uses AST parsing, and Conditions struct member doesn't appear like an array in this snippet, it calls structToSchema instead of arrayToSchema.

/kind bug

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 12, 2024
@JoelSpeed
Copy link
Contributor

Have you tried applying the list type directly to the type declaration for Conditions instead of the field itself?

@ahmetb
Copy link
Member Author

ahmetb commented Jun 12, 2024

@JoelSpeed that seems to work (thanks!), albeit a bit awkward:

// +k8s:deepcopy-gen=true
// +listType=map
// +listMapKey=type
type Conditions []Condition

type FooStatus struct{
	Conditions Conditions `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
}

It correctly generates:

#...
              conditions:
                type: array
                x-kubernetes-list-map-keys:
                - type
                x-kubernetes-list-type: map

In my opinion, let's keep the bug open. It seems like it would be possible to evaluate the parsed AST to resolve the specified type as a typedef (ast.Ident) and realize the underlying type is of an array type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants