controller-tools icon indicating copy to clipboard operation
controller-tools copied to clipboard

Allow map[string]interface{} CRD fields

Open armsnyder opened this issue 3 years ago • 8 comments

Overview

I would love it if controller-gen allowed you to use map[string]interface{} types in CRD fields, specifically those marked with: // +kubebuilder:pruning:PreserveUnknownFields.

The use case is when you have a CRD that has a "template" field, similar to Deployment or Job's .spec.template field. It is desirable to have unstructured input, so that the controller can use server-side-apply to create the child object without implicitly owning zero-valued fields of the embedded type.

I have read past discussion in #294, #301, and #577. I see the recommendation at the time was to use either runtime.RawExtension or json.RawMessage. Both require manually marshalling and unmarshalling json, and I would rather use my CRD types in a consistent way without making assumptions about how the data is encoded. The former additionally does not make sense for unstructured data that is not a Kubernetes object. The latter is currently bugged (see #637). I believe that themap[string]interface{} type has a legitimate use case that controller-gen should not discourage.

Example

// BackendSpec defines the desired state of Backend
type BackendSpec struct {
	DeploymentTemplate DeploymentTemplate `json:"deploymentTemplate"`
}

type DeploymentTemplate struct {
	ObjectMeta ObjectMetaTemplate `json:"metadata,omitempty"`
	// +kubebuilder:pruning:PreserveUnknownFields
	Spec map[string]interface{} `json:"spec"`
}

type ObjectMetaTemplate struct {
	Labels      map[string]string `json:"labels,omitempty"`
	Annotations map[string]string `json:"annotations,omitempty"`
}
$ controller-gen crd
api/v1alpha1/backend_types.go:18:32: map values must be a named type, not *ast.InterfaceType
api/v1alpha1:-: name requested for invalid type: interface{}
api/v1alpha1:-: invalid map value type: interface{}

Versions

controller-gen v0.6.2 k8s.io/apimachinery v0.22.2 k8s.io/client-go v0.22.2 sigs.k8s.io/controller-runtime v0.10.2

armsnyder avatar Nov 09 '21 20:11 armsnyder

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

k8s-triage-robot avatar Feb 07 '22 21:02 k8s-triage-robot

/remove-lifecycle stale

armsnyder avatar Feb 07 '22 23:02 armsnyder

/remove-lifecycle stale

camilamacedo86 avatar Apr 05 '22 00:04 camilamacedo86

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

k8s-triage-robot avatar Jul 04 '22 05:07 k8s-triage-robot

@armsnyder PreserveUnknownFields only helps it to have more fields beyond the definition, but the error means we can't determine the type of interface in CRD properties.

You can do it like this to define a schemaless deployment spec:

type DeploymentTemplate struct {
	ObjectMeta ObjectMetaTemplate `json:"metadata,omitempty"`
	// +kubebuilder:pruning:PreserveUnknownFields
        // +kubebuilder:validation:Schemaless
	Spec interface{} `json:"spec"`
}

Or if you must define a map with its value schemaless, this will be fine:

type DeploymentTemplate struct {
	ObjectMeta ObjectMetaTemplate `json:"metadata,omitempty"`
	Spec map[string]AnyType `json:"spec"`
}

type AnyType struct {
	// +kubebuilder:pruning:PreserveUnknownFields
	// +kubebuilder:validation:Schemaless
	Field interface{} `json:",inline"`
}

And you can simply use RawExtension like this (after #701 #699 cherry-picked):

type DeploymentTemplate struct {
	ObjectMeta ObjectMetaTemplate `json:"metadata,omitempty"`
	Spec map[string]runtime.RawExtension `json:"spec"`
}

FillZpp avatar Jul 04 '22 09:07 FillZpp

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle rotten

k8s-triage-robot avatar Aug 13 '22 02:08 k8s-triage-robot

/remove-lifecycle rotten

alexstojda avatar Sep 09 '22 20:09 alexstojda

Is there a solution for this? I'm still hitting these issues - cannot use an interface in my CRD.

vsoch avatar Jan 04 '23 21:01 vsoch

Both require manually marshalling and unmarshalling json, and I would rather use my CRD types in a consistent way without making assumptions about how the data is encoded.

I'm curious about the use case here. Kubernetes objects over the wire are encoded as JSON, they are often presented as YAML, reading this, it suggests to me that you want to put some other data encoding, embedded within the spec of your object? Or have I misread that?

When your client serializes the data to send to the API server, it will convert everything to JSON, I don't think if you put some random data in, that it would understand you've changed data type and that it shouldn't convert that too (unless it's in a multi-line string?).

There have been many folks who've wanted something schemaless, or, to be able to put varying data in and work out the content at runtime that have used runtime.RawExtension, can you provide some concrete examples of where this breaks for your use cases so that we can better understand the motivation for this request?

JoelSpeed avatar Jan 05 '23 09:01 JoelSpeed

If it helps, I wrote a short thread about my use case (and the “so what” is at the bottom). https://twitter.com/vsoch/status/1610855797713670145?s=46&t=6DRoASXZ9cLoZLvjoPrJ7A

vsoch avatar Jan 05 '23 09:01 vsoch

Your example of limits as a use case for this, mimics existing Kube APIs. Except the Kube APIs use map[string]string since pretty much everything can be parsed from a string that you would need to have there. In fact, for the example you've got, there's even another specialist type, the IntOrString which will allow users to input either an integer or a string and still have a structural schema with validation.

I think your use case would have been covered by map[string]intstr.IntOrString without any additional work.

JoelSpeed avatar Jan 05 '23 09:01 JoelSpeed

Oh this is fantastic to know - I’ll try it out! I think beyond this example, the only other use cases I might imagine would allow a third type. I can’t comment on the original poster’s need, but I think the solution I figured out might be helpful. Thanks for the help!

vsoch avatar Jan 05 '23 09:01 vsoch

It seems to me that this issues can be closed?

errordeveloper avatar Jan 31 '23 12:01 errordeveloper

Yes! Sorry forgot to do so. For future readers, it was actually really important to use IntOrString because although my hack appeared to work, the variables in the CRD for that field (with the hack) were always null and it never took. Changing to IntOrString fixed the bug / the CRD worked. Thanks for your help here @JoelSpeed ! I can't speak for @armsnyder but it's good to close for me.

vsoch avatar Jan 31 '23 17:01 vsoch

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-triage-robot avatar May 01 '23 17:05 k8s-triage-robot

The Kubernetes project currently lacks enough active 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 rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

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

/lifecycle rotten

k8s-triage-robot avatar May 31 '23 18:05 k8s-triage-robot

I think it's still relevant.

The solution that worked for me was to use the JSON type from the package k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.

	Params *apiextensionsv1.JSON `json:"params,omitempty"`

It would be more convenient to use a map.

matheusfm avatar Jun 02 '23 21:06 matheusfm

Given Kubernetes guidelines discourage the use of maps, have you considered using a list type with a discriminator key instead of a map here? I suspect this isn't something we really want to support in controller-tools since it goes against the kube conventions in a few ways

JoelSpeed avatar Jun 07 '23 10:06 JoelSpeed

Thanks for sharing this convention. Good reference.

My case is similar to the flux helm-controller that has a field to represent the values of the HelmRelease API. It seems to me that a list is not the best option.

Any suggestion?

See the helm-controller example: https://github.com/fluxcd/helm-controller/blob/main/api/v2beta1/helmrelease_types.go#L181

I have used helm-controller as an example because Helm is a common tool in the Kubernetes ecosystem. But my case is this: https://github.com/undistro/zora/blob/main/api/zora/v1alpha1/customcheck_types.go#L35

matheusfm avatar Jun 08 '23 22:06 matheusfm

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

This bot triages 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:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

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

/close not-planned

k8s-triage-robot avatar Jul 08 '23 22:07 k8s-triage-robot

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

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

This bot triages 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:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

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

/close not-planned

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-ci-robot avatar Jul 08 '23 22:07 k8s-ci-robot