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

feat: add install method to plugin CLIManager #364

Merged
merged 38 commits into from
Dec 18, 2023

Conversation

Two-Hearts
Copy link
Contributor

@Two-Hearts Two-Hearts commented Nov 24, 2023

This PR adds the Install method to plugin.CLIManager.

As suggested by @shizhMSFT:
when plugin path is a dir which contains a plugin executable file and extra files such as lib files and LICENSE files, all these files will be installed. A dir is considered a valid input if and only if it contains one and only one valid plugin executable file named with format notaiton-{plugin-name}.
Note: only regular files under the dir will be installed, sub-directories are ignored.

If the input plugin path is just a single executable file, this file will be installed.

When plugin already exists:

  1. If overwrite is set, version check is skipped. If the existing plugin is malfunctioning, directly overwrite it. If the existing plugin is valid, its plugin metadata is also returned for callers to consume.
  2. If overwrite is not set, existing plugin MUST be valid, version check requires new plugin version MUST be higher than existing plugin version.

When plugin does not exist:

  1. Directly install the new plugin from file path.

@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2023

Codecov Report

Attention: 81 lines in your changes are missing coverage. Please review.

Comparison is base (966c6b7) 74.33% compared to head (d6ab20f) 73.34%.

Files Patch % Lines
plugin/manager.go 65.85% 28 Missing and 14 partials ⚠️
internal/file/file.go 50.94% 25 Missing and 1 partial ⚠️
plugin/manager_unix.go 40.00% 4 Missing and 2 partials ⚠️
internal/semver/semver.go 72.72% 2 Missing and 1 partial ⚠️
plugin/errors.go 80.00% 2 Missing ⚠️
verifier/helpers.go 0.00% 0 Missing and 1 partial ⚠️
verifier/verifier.go 0.00% 0 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
- Coverage   74.33%   73.34%   -0.99%     
==========================================
  Files          24       27       +3     
  Lines        2260     2480     +220     
==========================================
+ Hits         1680     1819     +139     
- Misses        459      522      +63     
- Partials      121      139      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
internal/semver/semver.go Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts Two-Hearts requested a review from JeyJeyGao November 28, 2023 07:00
JeyJeyGao
JeyJeyGao previously approved these changes Nov 28, 2023
Copy link
Contributor

@JeyJeyGao JeyJeyGao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

internal/file/file.go Outdated Show resolved Hide resolved
internal/file/file.go Show resolved Hide resolved
internal/semver/semver.go Outdated Show resolved Hide resolved
plugin/errors.go Outdated Show resolved Hide resolved
plugin/errors.go Outdated Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
plugin/plugin.go Outdated Show resolved Hide resolved
plugin/plugin.go Outdated Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
JeyJeyGao
JeyJeyGao previously approved these changes Dec 7, 2023
Copy link
Contributor

@JeyJeyGao JeyJeyGao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
priteshbandi
priteshbandi previously approved these changes Dec 14, 2023
@priteshbandi priteshbandi self-requested a review December 14, 2023 11:53
@priteshbandi priteshbandi dismissed their stale review December 14, 2023 11:54

Mistakenly approved this PR

plugin/manager.go Outdated Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
internal/semver/semver.go Outdated Show resolved Hide resolved
internal/semver/semver.go Outdated Show resolved Hide resolved
internal/semver/semver.go Outdated Show resolved Hide resolved
plugin/manager_unix.go Outdated Show resolved Hide resolved
plugin/manager_windows.go Outdated Show resolved Hide resolved
internal/file/file.go Outdated Show resolved Hide resolved
internal/file/file.go Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
plugin/manager.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <[email protected]>
plugin/manager.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts Two-Hearts requested a review from shizhMSFT December 15, 2023 10:47
shizhMSFT
shizhMSFT previously approved these changes Dec 15, 2023
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

internal/file/file.go Outdated Show resolved Hide resolved
internal/file/file.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Two-Hearts Two-Hearts merged commit 85a5bb9 into notaryproject:main Dec 18, 2023
7 checks passed
@Two-Hearts Two-Hearts deleted the plugin branch December 18, 2023 13:23
@Two-Hearts Two-Hearts mentioned this pull request Jan 24, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants