Add support for OCI based plugin discovery and distribution by anujc25 · Pull Request #1043 · vmware-tanzu/tanzu-framework

anujc25 · 2021-11-02T21:10:55Z

What this PR does / why we need it

Implement plugin artifacts publish to OCI image with 'builder' plugin
- This change adds support for oci discovery/distribution with
  tanzu builder publish command
- Adds a new Makefile section for "Building and publishing CLIPlugin
  Discovery resources and binaries to OCI image
Implement OCI based plugin discovery and distribution
- Adds support for OCI based plugin discovery and artifact downloads
- Adds support to use carvel package as an OCI discovery endpoint. This
  change uses ytt and kbld library to read the carvel package
Add unit tests

Which issue(s) this PR fixes

Fixes #946

Describe testing done for PR

Added unit tests
Manually tested the publishing script to publish plugin discovery and artifacts to remote registry repo
Use the published discovery source as part of the tanzu configuration and use it to fetch available plugins as well as download plugins

Release note

Added support for OCI based plugin discovery and distribution for CLIPlugin discovery

PR Checklist

Squash the commits into one or a small number of logical commits
Use good commit messages
Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

go.mod

github-actions · 2021-11-02T21:22:01Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211102212200/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-11-02T22:57:28Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211102225726/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-11-03T17:46:01Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211103174559/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-11-03T18:54:29Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211103185426/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-11-03T19:47:04Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211103194703/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-11-03T21:34:06Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211103213405/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

zjs

I appreciate the quick progress on this! Overall, it looks like things are coming together as planned. (Which means good job on the design as well 🙂)

I left a few minor comments. Let me know if anything is unclear!

Makefile

go.mod

pkg/v1/builder/command/publish/local.go

pkg/v1/builder/command/publish/oci.go

pkg/v1/cli/artifact/oci.go

pkg/v1/cli/carvelhelpers/package.go

zjs · 2021-11-03T23:19:40Z

pkg/v1/tkr/pkg/registry/client.go

+func (r *registry) GetFile(imageWithTag, filename string) ([]byte, error) {
+	ref, err := regname.ParseReference(imageWithTag, regname.WeakValidation)


question: What's driving this change? (Naively, passing in the image and tag and handling the concatenation within the method seems better.)

The main reason behind this change is to support sha256 based images as well. For example, User can also provide the image as projects.registry.vmware.com/tanzu/tanzu-plugins/standalone-cliplugins@sha256:e78f0c57a68737561b6217dcbe4976b46393e1ee6615f7c957f162ef428d5ac8.

For this type of images, providing image as projects.registry.vmware.com/tanzu/tanzu-plugins/standalone-cliplugins@sha256 and tag as e78f0c57a68737561b6217dcbe4976b46393e1ee6615f7c957f162ef428d5ac8 seems odd and hence the refactoring.

github-actions · 2021-11-04T07:03:26Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211104070324/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-11-04T07:11:30Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211104071128/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

vuil

Thanks for the changes! I have some minor comments and questions.

pkg/v1/builder/command/publish/oci.go

vuil · 2021-11-04T07:16:21Z

pkg/v1/cli/artifact/oci.go

+
+	for path, fileData := range filesMap {
+		// Skip any testing related directory paths if bundled
+		if utils.ContainsString(filepath.SplitList(path), "test") {


Is there some convention where these test related directories are often bundled in the image that we have to explicitly filter them? If so are they always named 'test'?

The publish script takes the artifacts directory as an input. And for publishing the artifacts, It traverse to the specific plugin/version artifact directory at bundles the plugins from there. Sometimes, if the test plugin directory exists the bundle also includes test plugin directory, and this change just trying to skip that if it exists.

To improve around this, we can make some changes in the publish script to make sure it only publishes plugin binary and skip the publishing of the directory. But this check has been added as a safeguard for now.

pkg/v1/cli/carvelhelpers/registry.go

pkg/v1/cli/discovery/oci.go

- This change adds support for `oci` discovery/distribution with `tanzu builder publish` command - Adds a new `Makefile` section for "Building and publishing CLIPlugin Discovery resources and binaries to OCI image

- Adds support for OCI based plugin discovery and artifact downloads - Adds support to use carvel package as an OCI discovery endpoint. This change uses ytt and kbld library to read the carvel package

github-actions · 2021-11-04T18:19:24Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211104181923/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-11-04T18:25:27Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211104182526/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

github-actions · 2021-11-04T21:21:24Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1043/20211104212122/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

vuil

lgtm, thanks!

anujc25 requested review from a team, miclettej and prkalle as code owners November 2, 2021 21:10

anujc25 requested a review from a team November 2, 2021 21:10

vmwclabot added the cla-not-required label Nov 2, 2021

anujc25 commented Nov 2, 2021

View reviewed changes

go.mod Show resolved Hide resolved

anujc25 force-pushed the cli-plugin-oci-support-1 branch from 1390a65 to 047853f Compare November 2, 2021 22:42

anujc25 force-pushed the cli-plugin-oci-support-1 branch from 047853f to c8a28f3 Compare November 3, 2021 17:34

anujc25 requested a review from zjs November 3, 2021 17:55

anujc25 force-pushed the cli-plugin-oci-support-1 branch from c8a28f3 to d824e90 Compare November 3, 2021 18:41

anujc25 force-pushed the cli-plugin-oci-support-1 branch from d824e90 to 3a7c874 Compare November 3, 2021 19:36

anujc25 force-pushed the cli-plugin-oci-support-1 branch from 3a7c874 to f82f837 Compare November 3, 2021 21:23

zjs reviewed Nov 3, 2021

View reviewed changes

anujc25 force-pushed the cli-plugin-oci-support-1 branch 2 times, most recently from fdd21e3 to d20be10 Compare November 4, 2021 06:57

anujc25 added area/cli area/plugin area/release kind/feature Categorizes issue or PR as related to a new feature labels Nov 4, 2021

vuil reviewed Nov 4, 2021

View reviewed changes

vuil added the ok-to-merge PRs should be labelled with this before merging label Nov 4, 2021

Implement plugin artifacts publish to OCI image with 'builder' plugin

6e161f5

- This change adds support for `oci` discovery/distribution with `tanzu builder publish` command - Adds a new `Makefile` section for "Building and publishing CLIPlugin Discovery resources and binaries to OCI image

Implement OCI based plugin discovery and distribution

6a2b881

- Adds support for OCI based plugin discovery and artifact downloads - Adds support to use carvel package as an OCI discovery endpoint. This change uses ytt and kbld library to read the carvel package

anujc25 force-pushed the cli-plugin-oci-support-1 branch from d20be10 to 9cc9029 Compare November 4, 2021 18:06

Update GetFile, GetFiles in registry pkg to take full image as param

cc71669

anujc25 force-pushed the cli-plugin-oci-support-1 branch from 9cc9029 to cc71669 Compare November 4, 2021 18:10

Refactor registery helpers to its clientconfighelper packages

c4068b2

vuil approved these changes Nov 5, 2021

View reviewed changes

anujc25 merged commit fe23cf4 into vmware-tanzu:main Nov 5, 2021

rajaskakodkar mentioned this pull request Nov 22, 2021

Update BoM retrieval version to 0.11.1 rajaskakodkar/tanzu-framework#1

Closed

		func (r *registry) GetFile(imageWithTag, filename string) ([]byte, error) {
		ref, err := regname.ParseReference(imageWithTag, regname.WeakValidation)

Conversation

anujc25 commented Nov 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it

Which issue(s) this PR fixes

Describe testing done for PR

Release note

PR Checklist

Additional information

Special notes for your reviewer

Uh oh!

Uh oh!

github-actions bot commented Nov 2, 2021

Uh oh!

github-actions bot commented Nov 2, 2021

Uh oh!

github-actions bot commented Nov 3, 2021

Uh oh!

github-actions bot commented Nov 3, 2021

Uh oh!

github-actions bot commented Nov 3, 2021

Uh oh!

github-actions bot commented Nov 3, 2021

Uh oh!

zjs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zjs Nov 3, 2021

Choose a reason for hiding this comment

Uh oh!

anujc25 Nov 4, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 4, 2021

Uh oh!

github-actions bot commented Nov 4, 2021

Uh oh!

vuil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vuil Nov 4, 2021

Choose a reason for hiding this comment

Uh oh!

anujc25 Nov 4, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 4, 2021

Uh oh!

github-actions bot commented Nov 4, 2021

Uh oh!

github-actions bot commented Nov 4, 2021

Uh oh!

vuil left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anujc25 commented Nov 2, 2021 •

edited

Loading