Publish bundles and Allow Install From Tag #379

jeremyrickard · 2019-06-04T23:05:09Z

This PR starts down the road of implementing a bundle cache per #322. It doesn't fully implement that issue, but lays some groundwork.

The major addition here is that users can now porter publish a bundle to an OCI registry using cnab-to-oci and install a bundle from an OCI registry using a new --tag flag on porter install:

porter install --tag jeremyrickard/porter-kube-bundle:1.0 -c kool-kred
installing HELLO...
executing porter install configuration from /cnab/app/porter.yaml
Install Hello World App

When the --tag flag is provided, Porter will attempt to locate the specified bundle in the new ~/.porter/cache directory using the hashed tag name as a directory key. If it is not found, it is pulled with cnab-to-oci, stored in the cache, and the new path in the cache is returned and used to update the install opts. If it is found, the cached path is returned.

Sorry this is rather large.

carolynvs · 2019-06-05T15:23:58Z

cmd/porter/bundle.go

+	f.StringVarP(&opts.Tag, "tag", "t", "",
+		"Install from a bundle in an OCI registry specified by the given tag")
+	f.BoolVar(&opts.InsecureRegistry, "insecure-registry", false,
+		"Treat the given bundle registry as insecure")


Can we improve this error message a bit? Based on what you told me, I think it means "Allow connecting to a registry that isn't secured with TLS certificates" or something like that, no?

carolynvs · 2019-06-05T15:25:32Z

cmd/porter/bundle.go

@@ -288,7 +291,7 @@ func buildBundlePublishCommand(p *porter.Porter) *cobra.Command {

 	f := cmd.Flags()
 	f.StringVarP(&opts.File, "file", "f", "", "Path to the Porter manifest. Defaults to `porter.yaml` in the current directory.")
-	f.StringVarP(&opts.Tag, "tag", "t", "", "Tag to apply to the CNAB bundle. Defaults to the `Tag` value currently in the Porter manifest.")
+	f.BoolVar(&opts.InsecureRegistry, "insecure", false, "Treat the registry as insecure.")


Just a note that whatever we change the flag description to above, let's change this one as well.

carolynvs · 2019-06-05T15:26:49Z

docs/content/distributing-bundles.md


-* This needs to wait until bundle registries is figured out
+Before you can publish your bundle, you must first run a `porter build` command. This will create the invocation image so it can be pushed to an OCI (Docker) registry along with your CNAB bundle manifest. It's a good idea to work with your bundle and test it locally before you decide to publish it to a registry.


nit: "before you decide to publish" -> "before you publish"

carolynvs · 2019-06-05T15:27:16Z

docs/content/distributing-bundles.md

+## Bundle Publish
+
+Once you are satisfied with the bundle, the next step is to publish the bundle! Bundle publishing involves pushing the both the invocation image and the CNAB bundle manifest to an OCI registry. Porter uses [docker tags](https://docs.docker.com/engine/reference/commandline/tag/) for both nvocation images and CNAB bundle manifests. These are defined in your `porter.yaml` file:


spelling: "nvocation images" -> "invocation images"

carolynvs · 2019-06-05T15:28:46Z

docs/content/distributing-bundles.md

+```
+
+This YAML snippet indicates that the invocation image will be built and tagged as `deislabs/porter-kubernetes:latest`. The first part of this reference, `deislabs` indicates the registry that the invocation image should eventually be published to. The `porter-kubernetes` segment identifies the iamge, while the `:latest` portion denotes a specific version. Much like the `invocationImage` attribute is used to control the name of resulting Docker invocation image, the `tag` attribute is used to specify the name and location of the resulting CNAB bundle. In both cases, when you are ready to publish your bundle, it would be a good idea to provide specific versions for both of these, such as `v1.0.0`. We recommend using [semantic versioning](https://semver.org/) for both the invocation image and the bundle. We also recommend specifying the same registry for both, in order to simplify access to your bundle and invocation image by end users.


spelling: "iamge"

carolynvs · 2019-06-05T15:34:36Z

pkg/cache/cache.go

+	bid := getBundleID(bundleTag)
+	bundleCnabDir, err := c.getCachedBundleCNABDir(bid)
+	cachedBundlePath := filepath.Join(bundleCnabDir, "bundle.json")
+	c.FileSystem.MkdirAll(bundleCnabDir, os.ModePerm)


This isn't checking for an error

carolynvs · 2019-06-05T15:37:23Z

pkg/cache/cache.go

+	// hash the tag, tags have characters that won't work as part of a path
+	// so hashing here to get a path friendly name
+	bid := md5.Sum([]byte(bundleTag))
+	return fmt.Sprintf("%x", bid)


You can also use hex.EncodeToString. Not sure if that's better or not, or if fmt uses that under the hood. It may be more clear though.

carolynvs · 2019-06-05T15:43:02Z

pkg/porter/install.go

@@ -46,6 +64,21 @@ func (o *InstallOptions) ToDuffleArgs() cnabprovider.InstallArguments {
 // InstallBundle accepts a set of pre-validated InstallOptions and uses
 // them to install a bundle.
 func (p *Porter) InstallBundle(opts InstallOptions) error {
+	// If opts.Tag is set, fetch the bundle


I see you had to put this here because there's a bug in applyDefaultOptions. It assumes there there is always a porter.yaml to load. Heads up that I'm fixing that as part of the work I'm doing today on rebuilding before actions when the hash is out of date.

carolynvs · 2019-06-05T15:48:48Z

pkg/porter/install_test.go

@@ -89,6 +90,7 @@ func TestInstallOptions_validateParams(t *testing.T) {
 		sharedOptions{
 			Params: []string{"A=1", "B=2"},
 		},
+		BundlePullOptions{},


Since you declared InstallOptions with BundlePullOptions as an embedded type (and not a pointer), you don't need to have this line here. Same for the other tests below. I

The only time you need stuff like this line here is if you defined it like this instead

type InstallOptions struct { *BundlePullOptions }

huh, the compiler complained until I added this.

carolynvs · 2019-06-05T15:55:55Z

pkg/porter/publish.go

 		pwd, err := os.Getwd()
 		if err != nil {
 			return errors.Wrap(err, "could not get current working directory")
 		}
+		f = filepath.Join(pwd, config.Name)


I notice that we are duplicating validation between sharedOptions and here. As part of my --cnab-file work, I'm making another struct that is going to extract out logic for --file and --cnab-file that will do the common validation for these flags across all the commands. Just a heads up.

carolynvs

YES! Can't wait to have this in Porter. SHIP. IT. ⛵️

jeremyrickard requested review from carolynvs and vdice June 4, 2019 23:05

jeremyrickard self-assigned this Jun 4, 2019

jeremyrickard added 6 commits June 5, 2019 09:45

WIP pull on install

9d8ca53

Push, Pull with a little bit of cache thrown in

e8c2586

Updated docs

f5b9692

Fix dep

d11567f

Dep again

6c66118

Fixing test, didn't re-run unit tests after rebasing, hash changed since

8671c52

jeremyrickard force-pushed the publish-bundles branch from feaac0a to 8671c52 Compare June 5, 2019 15:58

carolynvs requested changes Jun 5, 2019

View reviewed changes

jeremyrickard added 2 commits June 5, 2019 10:16

Resolve code review comments

ab50525

make verify-vendor is now happy

2bb702e

jeremyrickard requested a review from carolynvs June 5, 2019 16:24

Change the flag/help text for insecure-registry on publish

a374ef1

carolynvs approved these changes Jun 5, 2019

View reviewed changes

jeremyrickard merged commit 1a9c6e8 into getporter:master Jun 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Publish bundles and Allow Install From Tag #379

Publish bundles and Allow Install From Tag #379

jeremyrickard commented Jun 4, 2019

carolynvs Jun 5, 2019

carolynvs Jun 5, 2019

carolynvs Jun 5, 2019

carolynvs Jun 5, 2019

carolynvs Jun 5, 2019

carolynvs Jun 5, 2019

carolynvs Jun 5, 2019

carolynvs Jun 5, 2019

carolynvs Jun 5, 2019

jeremyrickard Jun 5, 2019

carolynvs Jun 5, 2019

carolynvs left a comment


		* This needs to wait until bundle registries is figured out
		Before you can publish your bundle, you must first run a `porter build` command. This will create the invocation image so it can be pushed to an OCI (Docker) registry along with your CNAB bundle manifest. It's a good idea to work with your bundle and test it locally before you decide to publish it to a registry.

		## Bundle Publish

		Once you are satisfied with the bundle, the next step is to publish the bundle! Bundle publishing involves pushing the both the invocation image and the CNAB bundle manifest to an OCI registry. Porter uses [docker tags](https://docs.docker.com/engine/reference/commandline/tag/) for both nvocation images and CNAB bundle manifests. These are defined in your `porter.yaml` file:

		```

		This YAML snippet indicates that the invocation image will be built and tagged as `deislabs/porter-kubernetes:latest`. The first part of this reference, `deislabs` indicates the registry that the invocation image should eventually be published to. The `porter-kubernetes` segment identifies the iamge, while the `:latest` portion denotes a specific version. Much like the `invocationImage` attribute is used to control the name of resulting Docker invocation image, the `tag` attribute is used to specify the name and location of the resulting CNAB bundle. In both cases, when you are ready to publish your bundle, it would be a good idea to provide specific versions for both of these, such as `v1.0.0`. We recommend using [semantic versioning](https://semver.org/) for both the invocation image and the bundle. We also recommend specifying the same registry for both, in order to simplify access to your bundle and invocation image by end users.

Publish bundles and Allow Install From Tag #379

Publish bundles and Allow Install From Tag #379

Conversation

jeremyrickard commented Jun 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carolynvs left a comment

Choose a reason for hiding this comment