Remove duplicated scaffold API validation code

[Adirio]

Delegate resource validation to itself

Fixes #1145

[k8s-ci-robot]

Hi @Adirio. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[Adirio]

/assign @droot

[camilamacedo86]

Hi @Adirio,

This change cannot be performed because when one of these values be not filled properly the user will not see a proper output and know what is missing. Following an example.

$ kubebuilder create api --group webapp --version  --kind Guestbook
Create Resource [y/n]
y
Create Controller [y/n]
y
2019/11/04 23:29:26 kind cannot be empty

[Adirio]

This error has nothing to do with my code, you can check it with the base kubebuilder and you will get the same. The error message will be written differently as they are not phrased the same, but it will still fail with the Kind:

$ kubebuilder create api --group webapp --version --kind Guestbook

Create Resource [y/n]
y
Create Controller [y/n]
y
2019/11/05 08:31:42 missing kind information for resource

I don't really know what is the reason behind this. Kind is checked last, so it doesn't make any sense to me unless Guestbook is being assigned to --version and therefore --kind is left empty. The opposite operation seems to point in the same direction.

$ kubebuilder create api --group webapp --kind --version Guestbook

Create Resource [y/n]
y
Create Controller [y/n]
y
2019/11/05 08:44:55 missing version information for resource

[Adirio]

As I mentioned in #1144, the Resource inside the v1 package is being used for both v1 and v2. That's the reason I suggest to move it outside of the v1 package in that issue.

The link you provided is exactly the function I'm calling in this PR (Resource.Validate). As you can see that function does pretty much the same thing as the code I deleted in this PR. Instead of checking that the string is "", it checks that its length is 0, but those are two ways of doing the same. The messages are also a bit different, the ones I deleted may be a bit more descriptive so we could keep these instead of the ones in the file you linked. Additionally it does some extra checks which you are surely aware of as you recently modified some of them.

Resource.Validate function usage:

It can be called either directly or thorugh `input.Validate` interface. The interface is only being called inside `Scaffold.Execute` which basically calls `Validate` on any file that has a method `Validate`. The direct method is being called in 33 tests (`pkg/scaffold/v1/resource/resource_test.go`), 8 v1 file types (`pkg/scaffold/v1/resource/`) and 7 v2 file types (`pkg/scaffold/v2/`, `pkg/scaffold/v2/crd/` and `pkg/scaffold/v2/webhook/`). The 15 file types for both versions just implement the `input.Validate` interface by calling the wrapped `Resource`, so the interface usage mention above applies to them. The 33 tests check most, if not all, of the corner cases directly on `Resource`s.

As seen in the Resource.Validate function usage found above, this function is only being called at Scaffold.Execute. As seen in this PR, part of the Resource.Validate code was being duplicated to check for empty strings when issuing a kubebuilder create api ... command. This PR just delegates the validation to the already tested and more comprehensive Resource.Validate function instead of having duplicated code.

About the bug you mentioned, it is not being catched by the 33 tests as the resources are being created directly instead of though the command line. I will create a bug report for it to track it separately.

[camilamacedo86]

/assign @camilamacedo86

[camilamacedo86]

/assign @mengqiy

[camilamacedo86]

/ok-to-test

Hi @droot and @mengqiy I checked this one. It should be removed. The resource.go has its validation. We can merge this one.

/lgtm /approved

[mengqiy]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Adirio, mengqiy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mengqiy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment