CLI: it would be helpful to skip PROJECT reads for extra commands

[jmrodri]

It would be helpful to skip PROJECT file reads for extra commands. kubebuilder version will throw an error if an errant PROJECT file exists. I do not expect this for version or other extra commands.

For example,

$ kubebuilder version
2020/10/29 12:19:36 failed to read the configuration file: read PROJECT: is a directory

I expected the version and extra commands to ignore the PROJECT file/directory.

$ kubebuilder version
Version: version.Version{KubeBuilderVersion:"2.3.1", KubernetesVendor:"1.16.4", GitCommit:"8b53abeb4280186e494b726edf8f54ca7aa64a49", BuildDate:"2020-03-26T16:42:00Z", GoOs:"unknown", GoArch:"unknown"}

To reproduce:

mkdir PROJECT
kubebuilder version

This also affects the operator-sdk because we use the kubebuilder cli. I've tracked down some of the interaction. Starting from cli.New which always calls c.initialize(). This fails with the following error: FATA[0000] failed to read config: read PROJECT: is a directory because of the errant PROJECT file. With most commands it is useful and/or required to read the PROJECT file but for something like version or other extra commands I believe it isn't required.

/kind bug

[Adirio]

The CLI is fine if no project configuration file exists (PROJECT is just a name, lets not use that term) as there are some subcommands (like kubebuilder init) which obviously don't expect a configuration file to exist previously. In the initalize method we are checking if the file exists or doens't exist and proceed appropiately. But when reading the file, other errors may happen and we need to detect these. We may need to split the commands that require a project configuration file and those that don't, and only trying to read the file in the first case.

/cc @estroz

[Adirio]

@camilamacedo86 the error comes from the makedir PROJECT. We already support commands with no project configuration file (for example, kubebuilder init is one of them). What we currently do is check if the file exists, then load and read the configured version. If reading the file gives a "file not exist" error we just consider it as not-configured yet and proceed. The problem is that reading the file may yield other types of errors, in this case it says that the file exists but it is a directory so we can't read it, and that's why the bug is happening.

Now we have two solutions:

1. The same way we consider "file not exist" errors to mean that the project is not configured, we can consider "file is a dir" errors to do the same. Fast fix that will work for this specific case out of the way. But there will be other cases in the future and we will have to add more exceptions to this list ("file not exist", "file is a dir", ...) which is not a very good approach.
2. Try to make the CLI work without reading the file at all until neccesary. This will be safer for the future as we won't need to keep adding exceptions but will require to change the cli initialization process.

I was trying to evaluate how much work would the second approach need and evaluate if it was worth it.

[camilamacedo86]

Hi @Adirio,

I close my previous comment to not make it confusing. I checked with @jmrodri the scenario. Following the steps to reproduce:

cd /tmp
mkdir PROJECT
kubebuilder version
2020/11/02 11:25:41 failed to read config: read PROJECT: is a directory

It is considered the directory PROJECT as a file. We need only looking for a file with this name.

[joelanford]

Even a PROJECT file should be ignored for subcommands that don't need it.

For example, consider an empty PROJECT file instead of a PROJECT directory:

$ cd /tmp
$ touch PROJECT
$ kubebuilder version
2020/11/02 10:00:39 project version 1 is no longer supported.
See how to upgrade your project: https://book.kubebuilder.io/migration/guide.html

Or another example where PROJECT is not a YAML file:

$ cd /tmp
$ echo "This is a description of my project" > PROJECT
$ kubebuilder version
2020/11/02 10:04:39 failed to read config: error unmarshalling project configuration: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type config.Config

There's no reason at all for kubebuilder version to know anything about the PROJECT file afaict.

[Adirio]

It uses the project file to try to guess the project version and the plugin layout, which gets saved in that file. But I do agree that we may be able to move that check after we know that it is needed by the subcommand. Some validation from the CLI would need to be dropped though, like having plugins for the proper version as we won't be able to know the version of the project without reading the file.

[Adirio]

@jmrodri #1828 should solve this issue. Would you mind verifying it? You would need to clone that PR's branch and build the source code from it. If you need further help you can contact me on Slack or post a comment here.

[jmrodri]

@Adirio i will check it out today. Thanks for the PR.

[jmrodri]

With the patch from PR #1828 applied, this issue is fixed.

$ mkdir PROJECT
$ /home/jesusr/dev/kubebuilder/bin/kubebuilder version
Version: main.version{KubeBuilderVersion:"v2.3.1-391-gf260cc8e", KubernetesVendor:"unknown", GitCommit:"f260cc8ea59e64b444c4b2cf0a40da2fb61f635e", BuildDate:"2020-11-23T20:09:54Z", GoOs:"linux", GoArch:"amd64"}