Provide metadata to ClusterClass variables

[Danil-Grigorev]

What would you like to be added (User Story)?

As a developer/operator of cluster classes I would like to have additional metadata to the ClusterClass template variables. This would make possible automated selection of variable groups for UI purposes, and improve overall UX.

Detailed Description

type ClusterClassVariable struct {
	// Name of the variable.
	Name string `json:"name"`

        // Arbitrary metadata related to the variable
        Metadata map[string]string `json:"metadata"`

        … // All other fields
}

Cluster class will get an ability to assign common groups where the variable belongs, or pass any kind of information wich will not be visible in documentation, as would happen while using Example field.

Anything else you would like to add?

No response

Label(s) to be applied

/kind feature

[k8s-ci-robot]

This issue is currently awaiting triage.

If CAPI contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

[richardcase]

I think having the ability to add metadata/annotate the variables is a good idea. As you point out, this would be especially useful for integrating with ClusterClass where the additional metadata can be used for purposes such as grouping or providing hints to a UI.

[fabriziopandini]

I like the idea, even if Metadata might be confusing because in Kubernetes it usually implies labels and annotations (and probably more); what you are proposing could be just Annotations.
Let's see if others have opinions as well

[fabriziopandini]

/area clusterclass

[sbueringer]

Or maybe we should have metadata.labels & metadata.annotations

[Danil-Grigorev]

I think that annotations is better choice here, as they don’t have any specific regex expectations. When specifying labels, it may seem like label selectors could be used. But this field is just a sub-resource, so this is impossible. I implemented this to match the following instead:

type ClusterClassVariable struct {
	// Name of the variable.
	Name string `json:"name"`

        // Arbitrary metadata related to the variable
        Annotations map[string]string `json:"annotations,omitempty"`

        … // All other fields
}

/assign

[Danil-Grigorev]

/assign

[sbueringer]

This would make possible automated selection of variable groups for UI purposes

This sounds like labels would be more appropriate for your use case.

I think nobody thinks that labels somewhere in the spec can be used to select on ClusterClass (see the labels fields we have deeper down in MachineDeployment / MachineSet / Cluster / ClusterClass today)

[fabriziopandini]

@vincepri @JoelSpeed we could use an API reviewer opinion on this one

[JoelSpeed]

I would lean away from allowing arbitrary data into the API. Having validation of the content of the labels/annotations is a good thing in general, gives users an understanding of what they can put into the field, and prevents unexpected breakages from, say, someone trying to write an entire JPG into the metadata as a URL encoded string.

I'd be tempted to follow K8s and other patterns we see elsewhere. Create a metadata struct ClusterClassVariableObjectMeta and add to that a subset of fields from metav1.ObjectMeta. This will then feel familiar as an API and provide future extensibility if you so desire.

In general map[T]V in Kube APIs is forbidden. We prefer slices with listType=map and listMapKey annotations. I think on this occasion, if you are mimicking ObjectMeta, then it's probably ok.

I would like to explore the use case a little further though, @Danil-Grigorev (and others), can you please provide real world examples of metadata you expect to be stored on these variables? And even better, examples of things you wouldn't expect to be stored

[sbueringer]

In general map[T]V in Kube APIs is forbidden. We prefer slices with listType=map and listMapKey annotations. I think on this occasion, if you are mimicking ObjectMeta, then it's probably ok.

I think it's okay. Also mentioned here in the API conventions

The only exceptions are pure maps in the API (currently, labels, selectors, annotations, data), as opposed to sets of subobjects.

We can consider adding the same validation we have on other nested labels / annotations fields (which should match what Kubernetes is doing)

[Danil-Grigorev]

@JoelSpeed I suspect I don't have real world examples yet. @richardcase do you have any?
What I'm seeing is just this:

annotations:
    provider: aws
    type: air-gapped
    size: small

I'm ok with anything embedding mapping of values, whether annotations or labels or ObjectMeta subset with both of them.
From the UI workflow, such values can be used to separate set of fields which should be filled together from a single tab view for example.
If this suites everyone, I'll go ahead and implement this.

[fabriziopandini]

/triage accepted
I'm +1 to go ahead and implement it, the proposed API is consistent and the example makes sense to me