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

Allow map[string]interface{} CRD fields #636

Closed
armsnyder opened this issue Nov 9, 2021 · 21 comments
Closed

Allow map[string]interface{} CRD fields #636

armsnyder opened this issue Nov 9, 2021 · 21 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@armsnyder
Copy link

armsnyder commented Nov 9, 2021

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

@k8s-triage-robot
Copy link

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-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 Feb 7, 2022
@armsnyder
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 7, 2022
@camilamacedo86
Copy link
Member

/remove-lifecycle stale

@k8s-triage-robot
Copy link

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-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 Jul 4, 2022
@FillZpp
Copy link
Contributor

FillZpp commented Jul 4, 2022

@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"`
}

@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 13, 2022
@alexstojda
Copy link

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Sep 9, 2022
@vsoch
Copy link

vsoch commented Jan 4, 2023

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

@JoelSpeed
Copy link
Contributor

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?

@vsoch
Copy link

vsoch commented Jan 5, 2023

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

@JoelSpeed
Copy link
Contributor

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.

@vsoch
Copy link

vsoch commented Jan 5, 2023

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!

@errordeveloper
Copy link

It seems to me that this issues can be closed?

@vsoch
Copy link

vsoch commented Jan 31, 2023

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.

@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 May 1, 2023
@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 31, 2023
@matheusfm
Copy link

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.

@JoelSpeed
Copy link
Contributor

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

@matheusfm
Copy link

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

@k8s-triage-robot
Copy link

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-ci-robot
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

10 participants