🐛 Make topology markers also valid for type definitions

[chrischdi]

This PR allows setting listMapKey, listType, mapType and structType at both type and field level to allow setting them at typed slices too.

Currently the markers are only valid at the field level.

The markers are important for defining CRDs which should get used in combination with Server-Side Apply (xref SSA).

Fixes: #435

Example

When a struct references a typed slice it is currently not possible to set the markers.
Example:

type BarSpec struct {
	Foos Foos `json:"foos,omitempty"`
}

type Foos []Foo

• When setting it at field level, controller-gen will throw errors because the detected type is empty / ""

  /tmp/test-ct/api/v1beta1/bar_types.go:32:2: must apply listType to an array, found
  /tmp/test-ct/api/v1beta1/bar_types.go:32:2: must apply listMapKey to an array, found
  

• At type level the markers are currently ignored.

After this PR, setting the markers will be possible at type level too, where the type information will be available:

type BarSpec struct {
	Foos Foos `json:"foos,omitempty"`
}

// +listType=map
// +listMapKey=id
type Foos []Foo

[FillZpp]

@chrischdi Thanks. Please add some test cases in integration test, and you may also have to test the case that the markers defined on both type and field, like this:

type BarSpec struct {
        // +listType=map
        // +listMapKey=id
	Foos Foos `json:"foos,omitempty"`
}

// +listType=map
// +listMapKey=id
type Foos []Foo

[chrischdi]

Hi @FillZpp , thank you for the feedback 👍

I added a test case similar to

// +listType=map
// +listMapKey=id
type Foos []Foo

However it is not possible to add:

type BarSpec struct {
	// +listType=map
	// +listMapKey=name
	// +listMapKey=secondary
	Foos Foos `json:"foos,omitempty"`
}

Because this will fail at the following marker checks:

• controller-tools/pkg/crd/markers/topology.go

  Line 112 in 9e2b90e

  return fmt.Errorf("must apply listType to an array, found %s", schema.Type)

• controller-tools/pkg/crd/markers/topology.go

  Line 126 in 9e2b90e

  return fmt.Errorf("must apply listMapKey to an array, found %s", schema.Type)

The information if it is an array or not is not available there (it is empty string in this case), which is why this PR introduces the possibility to set the markers at the type definition instead.

[FillZpp]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, chrischdi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [alvaroaleman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[alvaroaleman]

/cherrypick release-0.9

[k8s-infra-cherrypick-robot]

@alvaroaleman: once the present PR merges, I will cherry-pick it on top of release-0.9 in a new PR and assign it to you.

In response to this:

/cherrypick release-0.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-infra-cherrypick-robot]

@alvaroaleman: new pull request created: #693

In response to this:

/cherrypick release-0.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.