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 · 4 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

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.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 10, 2024
@sbueringer
Copy link
Member

/remove-lifecycle stale
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 10, 2024
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. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

5 participants