kubebuilder generates blank manifests if there is no empty line between kubebuilder markers and user comments

[anemade]

I am trying to create the manifests using the kubebuilder markers. This is working fine but I noticed one tiny issue which might drive you crazy.

If we add the user comment or TODO comments immediately on the next line of the markers then manifests are not getting generated

For example,

//+kubebuilder:webhook:path=/mutate,mutating=true,failurePolicy=fail,groups=test.prac.io,resources=foo;bar,verbs=create;update,versions=v1alpha1,name=mapplication.kb.io
//+kubebuilder:webhook:verbs=create;update,path=/validate,mutating=false,failurePolicy=fail,groups=test.prac.io,resources=foo;bar,versions=v1alpha1,name=vapplication.kb.io

// TODO: Handle all the cases

Please notice that there is an empty line after the kubebuilder markers. This is working fine and manifests are getting generated.

Issue:
If I try the following,

//+kubebuilder:webhook:path=/mutate,mutating=true,failurePolicy=fail,groups=test.prac.io,resources=foo;bar,verbs=create;update,versions=v1alpha1,name=mapplication.kb.io
//+kubebuilder:webhook:verbs=create;update,path=/validate,mutating=false,failurePolicy=fail,groups=test.prac.io,resources=foo;bar,versions=v1alpha1,name=vapplication.kb.io
// TODO: Handle all the cases

Please notice that there is no blank line after the markers. because of this, blank manifests are getting generated. I am not sure if it's expected behavior. I think this is an issue as we are expecting the markers to start with //+kubebuilder

Versions:
Golang version: 1.12.9
kubebuilder version: 2.2.0
Kubernetes version: 1.16.3

/kind bug

[camilamacedo86]

HI @estroz @mengqiy,

WDYT about this one? Is not this the expected behaviour? Is it something that we should address?
Also, this implementation in the controller-tools. Am I right?

[ryanzhang-oss]

FWIW, for someone like me, that has no previous in-depth Kubernetes experience, this is pretty much a hack. I don't know what makes people think that "an empty line is significant" is acceptable. Again, this is an absolute anti-pattern in my 10+ years of professional software development. I know I will take this type of convention for granted after a few more months working in this eco-system but I would never assume that this is the right thing to do.

[mengqiy]

An empty line should not matter here. There may be a bug somewhere in controller-gen.

/help

[k8s-ci-robot]

@mengqiy:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

An empty line should not matter here. There may be a bug somewhere in controller-gen.

/help

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.

[dmvolod]

Yes, it's a problem with the last marker comment block inside the controller-tools/pkg/markers/collect.go implementation

/assign @dmvolod

[camilamacedo86]

HI @dmvolod,

The issue should be raised and fix in the controller-tools. Then, when it has a newly released then when it be released we can get it in kubebuilder. since you already move forward by raising the issue in the project and shows working with, could we agree to close this one here?

[dmvolod]

Hi @camilamacedo86,
Yes, you are right. But we can keep it open until the new version controller-tools has been released.
Just for tracking purpose only.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[camilamacedo86]

The issue shows that was closed as well in the controller-tools. So, we can close this one.
However, if someone faces the same feel free to raise an issue against https://github.com/kubernetes-sigs/controller-tools/pull/437#issuecomment-681178740 and working on that. Your help will be very welcome

[RiRa12621]

@camilamacedo86 I see that the PR you linked to has also been closed due to inactivity.
Are you planning to re-open this one, or should a new issue be opened under the controller-tools? Because picking up someone's abandoned work feels weird, particularly when it didn't seem to actually focus around the correct root cause according to https://github.com/kubernetes-sigs/controller-tools/pull/437#issuecomment-635011788

edit: i think i found the correct issue https://github.com/kubernetes-sigs/controller-tools/issues/436 but that is closed too, sadlt