notaryproject / notation

A CLI tool to sign and verify artifacts
https://notaryproject.dev/
Apache License 2.0
340 stars 84 forks source link

feat: notation plugin install command #827

Closed Two-Hearts closed 9 months ago

Two-Hearts commented 10 months ago

This PR is based on the new spec: https://github.com/notaryproject/notation/pull/809

It supports following notation plugin installations:

  1. Install from .tar.gz/.zip file in file system
  2. Install from HTTPS URL, SHA256 checksum is required
  3. Install a plugin executable file in file system

Test: Corresponding E2E test cases are added in this PR as well.

Note: This PR depends on the notation-go PR: https://github.com/notaryproject/notation-go/pull/364

The notation-go PR: https://github.com/notaryproject/notation-go/pull/364 has been Merged.

codecov-commenter commented 10 months ago

Codecov Report

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

Comparison is base (df8596d) 64.39% compared to head (45788ae) 63.59%.

Files Patch % Lines
cmd/notation/plugin/install.go 58.37% 78 Missing and 14 partials :warning:
internal/osutil/file.go 47.91% 18 Missing and 7 partials :warning:
cmd/notation/internal/plugin/plugin.go 57.14% 8 Missing and 4 partials :warning:

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #827 +/- ## ========================================== - Coverage 64.39% 63.59% -0.80% ========================================== Files 42 45 +3 Lines 2390 2692 +302 ========================================== + Hits 1539 1712 +173 - Misses 720 824 +104 - Partials 131 156 +25 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Two-Hearts commented 10 months ago

Are we missing functionality to copy/install plugin executable not zip or tar.gz ?

@priteshbandi This is a good question, it is tracked by this issue created by @FeynmanZhou : https://github.com/notaryproject/specifications/issues/284. Basically, we need to be clear in the notaryproject spec on how should a vendor release their plugins in what kind of format.

Therefore, in this iteration, I think we could just support zip/tar.gz as a first step. We can add the support for executable in the future once we have a decision on the spec.

priteshbandi commented 10 months ago

Are we missing functionality to copy/install plugin executable not zip or tar.gz ?

@priteshbandi This is a good question, it is tracked by this issue created by @FeynmanZhou : notaryproject/specifications#284. Basically, we need to be clear in the notaryproject spec on how should a vendor release their plugins in what kind of format.

Therefore, in this iteration, I think we could just support zip/tar.gz as a first step. We can add the support for executable in the future once we have a decision on the spec.

I mean the functionality to copy plugin from a location on disk. This should be easy and straightforward.

FeynmanZhou commented 10 months ago

I mean the functionality to copy plugin from a location on disk. This should be easy and straightforward. Hi @priteshbandi ,

Just want to clarify it: Do you mean support installing a plugin from an executable file (like .exe)?

If so, this has not been included in the plugin management CLI spec. Notation v1.1.0 will only support .zip and .tar.gz format of the plugin file as we aligned in the CLI spec.

IMO, if there is an executable plugin file, users should be able to install the plugin without notation plugin install. But in that case it is not secured by Notation plugin system.

Have you seen any plugin vendor or customer needs relying on executable file format? How about we put the executable file format in the notation plugin spec as its roadmap first and considering it until received vendor/customer needs on this format?

priteshbandi commented 10 months ago

I mean the functionality to copy plugin from a location on disk. This should be easy and straightforward. Hi @priteshbandi ,

Just want to clarify it: Do you mean support installing a plugin from an executable file (like .exe)? If so, this has not been included in the plugin management CLI spec. Notation v1.1.0 will only support .zip and .tar.gz format of the plugin file as we aligned in the CLI spec.

yes, install command would just need to copy file from user provided location to notation's location. It is part of the spec with following flag --file install plugin from a file in file system

Two-Hearts commented 9 months ago

yes, install command would just need to copy file from user provided location to notation's location. It is part of the spec with following flag --file install plugin from a file in file system

@priteshbandi Added this logic.

Two-Hearts commented 9 months ago

@priteshbandi As discussed, issue created: https://github.com/notaryproject/notation/issues/853