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

Support for parametrized types #756

Closed
wI2L opened this issue Dec 31, 2022 · 10 comments
Closed

Support for parametrized types #756

wI2L opened this issue Dec 31, 2022 · 10 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@wI2L
Copy link

wI2L commented Dec 31, 2022

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.

@camilamacedo86
Copy link
Member

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.

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:

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.

@wI2L
Copy link
Author

wI2L commented Jan 31, 2023

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.

@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
@adammw
Copy link

adammw commented May 29, 2023

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.

@iamnoah
Copy link

iamnoah commented Jun 14, 2023

/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 Jun 14, 2023
@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 Jan 22, 2024
@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 Feb 21, 2024
@bennesp
Copy link

bennesp commented Mar 21, 2024

Could be the same issue as #844

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

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 20, 2024
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

7 participants