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

Plugin "Hidden" internal data structure interface is confusing #259

Open
marckhouzam opened this issue May 4, 2023 · 0 comments
Open

Plugin "Hidden" internal data structure interface is confusing #259

marckhouzam opened this issue May 4, 2023 · 0 comments
Labels
kind/bug PR/issue related to a bug needs-triage

Comments

@marckhouzam
Copy link
Contributor

Bug description

The PluginInventoryEntry structure has a Hidden field as can be seen towards the end of:

type PluginInventoryEntry struct {
// Name of the plugin
Name string
// Target to which the plugin applies
Target configtypes.Target
// Description of the plugin
Description string
// Publisher is the name of the publisher of this plugin
// (e.g., a product group within a company)
Publisher string
// Vendor is the name of the vendor of this plugin (e.g., a company's name)
Vendor string
// Recommended version that Tanzu CLI should install by default.
// The value should be a valid semantic version as defined in
// https://semver.org/. E.g., 2.0.1
RecommendedVersion string
// Hidden tells whether the plugin is marked as hidden or not.
Hidden bool
// Artifacts contains an artifact list for every available version.
Artifacts distribution.Artifacts
}

A PluginInventoryEntry represents an entire plugin including all its versions.
On the other hand the concept of a "hidden" plugin applies to individual versions of a plugin and not to the entire plugin.
For example, if a team has published plugin1 versions 1.0.0 and 1.1.0 (not hidden) and then has a new version v2.0.0 they want to test, they will publish only v2.0.0 as hidden and not the entire plugin.

This is actually all working because the previously mentioned Hidden field is not really used; instead, we handle "hidden" plugins through appropriate DB queries as seen here:

if !filter.IncludeHidden {
// Unless we want to also get the hidden plugins, we only request the ones that are not hidden
whereClause = fmt.Sprintf("%s Hidden='false' AND", whereClause)
}

I find that having a Hidden field at the plugin level in the PluginInventoryEntry structure is confusing and can lead to errors in future development.

Note that the Hidden field is actually used when writing plugin entries as can be seen here:

hidden: strconv.FormatBool(pluginInventoryEntry.Hidden),

This works because we normally insert a single version at a time so we don't need to fill the plugin structure with all versions, but only with the newly published version and then the Hidden field happens to only apply to that version.

I suggest we simply remove it.
If we need to actually keep track of which version of a plugin is hidden and which is not after the DB query is completed, then we can add a different Hidden field at the level of a version of a plugin. I suggest we only do this when needed as we re-use existing types for the plugin versions.
For publishing a new plugin version, we can adapt the interface to add an hidden argument to the InsertPlugin() function call.

Note that as we introduce versions for plugin groups, we will face a similar situation.

Expected behavior

The plugin data structure properly reflects the reality of information for plugins.

Steps to reproduce the bug / Relevant debug output

N/A

Output of tanzu version

Today's main: f4963fff1

Environment where the bug was observed (cloud, OS, etc)

N/A

@marckhouzam marckhouzam added kind/bug PR/issue related to a bug needs-triage labels May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug PR/issue related to a bug needs-triage
Projects
None yet
Development

No branches or pull requests

2 participants
@marckhouzam and others