vmware-tanzu / tanzu-framework

Tanzu Framework provides a set of building blocks to build atop of the Tanzu platform and leverages Carvel packaging and plugins to provide users with a much stronger, more integrated experience than the loose coupling and stand-alone commands of the previous generation of tools.
Apache License 2.0
194 stars 192 forks source link

Allow capability API discover resources when version is unknown #1294

Closed danniel1205 closed 2 years ago

danniel1205 commented 2 years ago

(This is used to request new product features)

Describe the feature request

As a capability API user, I would like to have an API allows me to discover a resource without specifying the version, because I do not know what version of that resource is currently running in the cluster.

Example: Package plugin would like to use capdiscovery.Group("packageInstallAPIQuery", "packaging.carvel.dev").WithVersions("xxxx").WithResource("packageinstalls") to check if packageInstall CRD has been deployed. However, the cli code does not know what version of that CRD is current deployed/served. We could put v1alpha1 as of today and fully rely on the conversion webhook from carvel in the future once they have newer version, but it does not scale if v1alpha1 gets deprecated (We have to fix it passively and ask user to upgrade their CLI plugin).

A related discussion could be found here: https://github.com/vmware-tanzu/tanzu-framework/pull/1242/files#r761528548

Describe alternatives you've considered

Using other APIs(client) to get packageInstall CRD, if the result is non-empty then we know packgeInstall is available.

Affected product area (please put an X in all that apply)

Additional context

danniel1205 commented 2 years ago

Loop in @rajathagasthya and @yharish991.

rajathagasthya commented 2 years ago

We could add a WithAnyVersion option. I can see that being useful, although anyone caring about a resource usually also cares about its version.

codegold79 commented 2 years ago

While I was working on this issue, I found some interesting logic/behavior.

BUG: Non-existing resource when empty string version is provided returns Found: true

When these are provided,

I expect the result of the findings to be false because the resource doesn't exist. Actual result:true.

Line 79 of cluster_gvr.go allows empty string version to continue the search for unmatched loop and in effect does not actually check to see if the non-existing resource should be in the unmatched collection.

Example: https://github.com/codegold79/tanzu-playground/blob/b37be2187a14ea628c8547bc4a41fa1ccfc5f308/basic_capability/main.go


Not a bug: Using empty string resource returns Found:true if group and versions are found

When these are provided,

I expect the result of the findings to be false because there should be no resource that is an empty string. Actual result: true.

The reason for the current result is when an empty string resource is encountered, it continues the search for unmatched loop that checks for unmatched gvrs. It effectively counts the resource as being a match even though no such (empty string) resource was found.

UPDATE Feb 17, 2022:

After speaking with @rajathagasthya, he agreed the first case I mentioned above is a bug. However, the second case is not a bug. He says that WithResource method is optional. By omitting WithResource method, the user intends to find all groups and versions regardless of resource.

By extension, I believe using the WithResource method with an empty string argument, in other words, WithResource("") is the same. So, the result should be the same as omitting WithResource.

Which brings me to my next question related to this issue (#1294): Why is WithVersions method required when resource and group are provided? Shouldn't that be optional as well to be consistent with how WithResource works?

codegold79 commented 2 years ago

Plan: Make WithVersion method optional and not error out

In this design, I would like to make the WithVersion method optional and not error out. Omitting WithVersion in this way will mean, "I don't care which version is found, just return Found: true if the provided group and/or resource matches."

By extension, because WithVersion("") intuitively seems like it should be the same as omitting the WithVersion method, I want to make sure those two have the same results. Passing in an empty string argument for WithVersions or WithResource methods, for example, using WithVersions("") or WithResource("") will return an error.

As it is in the current PR, when the WithResource and WithVersions are omitted, it will still be like saying "any resource", or "any versions", respectively.

When the following are provided,

The current result is an error:

check package availability: cannot check for group and resource existence without version info; use WithVersion method

I would like the change the behavior such that omitting WithVersion method will not return an error. Instead, it will find matching group and resource (if provided) regardless of version.

I also would like to change the WithVersion("") behavior such that it returns an errorthe matching groups and resources regardless of version. Currently, using empty string version results in a bug describe in the previous comment.

Desired result when WithVersion is omitted (given the provided resource and/or version exist):

discovery.QueryResult{Found:true, NotFoundReason:""}