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

Support for parametrized types

Open wI2L opened this issue 2 years ago • 8 comments

Hello.

I tried to experiment with the latest version of controller-gen (v0.10.0), to use a parametrized type, like this:

type MyObjectCondition Condition[MyObjectConditionType, MyObjectConditionReason]

It appears that this generics syntax is not supported by controller-gen, because it does not handle the ast.IndexListExpr type.

The error reported looks like the following:

bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
xxx_types.go:xx:xx: unsupported AST kind *ast.IndexListExpr

The line reported by the error log correspond to the declaration of the MyObjectCondition type.


To give some more context, I was trying to factorize a Condition type, to reuse it in several CRD's status field.

The idea is to define a generic Condition type like the following:

type ConditionType interface {
	~string
}

type ConditionReason interface {
	~string
}

type Condition[T ConditionType, R ConditionReason] struct {
	Type T `json:"type"`
	Status metav1.ConditionStatus `json:"status"`
	Reason R `json:"reason"`
	Message string `json:"message"`
}

...and use it like so, with well-defined type parameters, to ensure type safety/checking:

type MyObjectConditionType string
type MyObjectConditionReason string

const (
    MyObjectConditionReady MyObjectConditionType = "Ready"
)

const (
    MyObjectReasonFooBar MyObjectConditionReason = "FooBar"
)

type MyObjectCondition Condition[MyObjectConditionType, MyObjectConditionReason]

type MyObjectStatus struct {
	Conditions []MyObjectCondition `json:"conditions,omitempty"`
}

The current alternative is to simply declare the Type and Reason fields of the Condition struct as string, at the expanse of loosing type checking.

I'm reporting it, hoping that it could be considered for future improvements towards a better support of the newer AST types introduced in go1.18.

wI2L avatar Dec 31 '22 01:12 wI2L

Hi @wI2L,

It seems happens because your custom type does not exist in the project. You would need to have its specification on the project.

  • Check the content in this PR to work with third-party APIs in kubebuilder: https://github.com/kubernetes-sigs/kubebuilder/pull/3055/files#diff-57a44718fda092759c611eb7cc5eff399804244a2b6d3a24bfefc2b6a5aa655c
  • Also, see that we would like to improve the tool to better support it: https://github.com/kubernetes-sigs/kubebuilder/issues/1999#issuecomment-778075627

However, regards conditional status see that it has an standard: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties

Therefore, I would recommend you work with as it is done via the deploy image plugin provide by kubebuilder, see: https://book.kubebuilder.io/plugins/deploy-image-plugin-v1-alpha.html

You can check in the testdata samples of its scaffold:

  • Look at the type definitons: https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v4-with-deploy-image/api/v1alpha1/busybox_types.go#L41-L52
  • Look at the controller implementation: https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v4-with-deploy-image/internal/controller/memcached_controller.go

In this way, after check the info provide above could you please let us know if you still thinking that this issue is required? Also, note that your help is very welcome. Please, feel free to contribute with PR's for both projects or indeed to propose a solution for your request in this issue.

camilamacedo86 avatar Jan 31 '23 09:01 camilamacedo86

Hello @camilamacedo86

It seems happens because your custom type does not exist in the project.

Yes it does. The custom types I mentionned were defined in a _types.go file inside the api/v1alpha1 package of my controller, and were parsed by the controller-gen binary.

I am aware of API conventions regarding Conditions, and I don't understand why you're pointing me to those various documentations, because the issue I reported lies with the fact that I'm using parametrized types, and not the design/definition of the Condition type itself.

Therefore, I would recommend you work with as it is done via the deploy image plugin provide by kubebuilder, see: https://book.kubebuilder.io/plugins/deploy-image-plugin-v1-alpha.html

Again, I'm sorry, but I don't see how that's relevant or related to the issue. This example uses the metav1.Condition type, which is generic and not a type I want to use, since I define my own Condition type.

In this way, after check the info provide above could you please let us know if you still thinking that this issue is required?

As I said, this isn't a bug report, the goal of the issue is to raise awarness about the lack of support for parametrized types, in the hope that it could be a feature improvement in the future. Without parametrized types, controller-gen is working as expected.

wI2L avatar Jan 31 '23 10:01 wI2L

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 10:05 k8s-triage-robot

Also seeing this issue with a parameterised interface such as:

type Condition[T ~string] interface {
	GetType() T
}

but producing a different error:

invalid type: interface{GetType() T}

Edit: nevermind - seems that is a generic problem with any interface, not just parameterised generics.

adammw avatar May 29 '23 02:05 adammw

/remove-lifecycle stale

iamnoah avatar Jun 14 '23 23:06 iamnoah

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 Jan 22 '24 15:01 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 Feb 21 '24 15:02 k8s-triage-robot

Could be the same issue as https://github.com/kubernetes-sigs/controller-tools/issues/844

bennesp avatar Mar 21 '24 14:03 bennesp

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 Apr 20 '24 16:04 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 Apr 20 '24 16:04 k8s-ci-robot