Add launcher and publishing support to the mirror command #93

murphybytes · 2017-08-25T00:15:27Z

This PR includes @groob #69. It adds the following capabilities to the package-builder mirror command.

Create Launcher tarballs.
Push Launcher and Osquery tarballs to mirror.
Publish updates and additions to targets to Notary.

marpaia · 2017-08-25T01:45:48Z

cmd/package-builder/README.md

@@ -53,6 +53,22 @@ To authenticate to GCloud, use the following:
 ```
 gcloud auth application-default login
 ```
+#### Notary Setup
+Notary Client must be properly installed and **be in your search path** in order to publish
+binaries. Notary Client can be found [here](https://github.com/docker/notary).  Prepare


nit: don't wrap your lines to 80 in this file

marpaia · 2017-08-25T01:47:41Z

cmd/package-builder/README.md

+    osqueryd-stable.tar.gz
+    osqueryd-2.6.0.tar.gz
+    ```
+The tarballs are stored in the `gs://binaries-for-launcher` GCS bucket, and exposed at the `https://dl.kolide.com/kolide/<binary>/<platform>/<tarball>` url.


in which GCP organization?

marpaia · 2017-08-25T01:48:46Z

cmd/package-builder/README.md

@@ -67,6 +83,46 @@ To use the tool to generate Kolide internal development packages, run:

 To use the tool to generate Kolide production packages, run:


it looks like the next section of the docs got weirdly inserted in the production packages section

marpaia · 2017-08-25T01:50:58Z

cmd/package-builder/README.md

+may be used to produce tar archives for both Launcher and Osquery, upload them
+to the mirror site, and register them with Notary so that they can be validated as
+part of the Launcher autoupdate process. The following commands would download and
+publish the latest version of Osquery, and would publish version 1.2 of Launcher to


I don't think you need a VERSION env for this as the package builder is already built with github.com/kolide/kit/version metadata.

marpaia · 2017-08-25T01:56:47Z

cmd/package-builder/mirror.go

+	mirror
+}
+
+func (m mirror) upload(binary, source string) error {


these two mirror methods are operating on a non-pointer mirror here. is that intentional?

marpaia · 2017-08-25T01:57:21Z

cmd/package-builder/mirror.go

+}
+
+// implements methods to post launcher to mirror
+type launcher struct {


maybe launcherMirror?

marpaia · 2017-08-25T01:57:52Z

cmd/package-builder/mirror.go

+		return errors.Wrap(err, "preparing osquery mirror upload")
+	}
+	bkt := client.Bucket(mirrorBucketname)
+	objectName := fmt.Sprintf("kolide/%s/%s/%s", binary, m.platform, filepath.Base(source))


Would path.join be useful here?

marpaia · 2017-08-25T01:59:45Z

cmd/package-builder/mirror.go

+	}
+	// we assume we are running from project root
+	// but just in case, check for binary
+	launcherPath := filepath.Join("build", runtime.GOOS, "launcher")


you can do filepath.Join(packaging.LauncherSource(), "build", "runtime.GOOS, "launcher") here for the absolute path.

marpaia · 2017-08-25T02:00:21Z

cmd/package-builder/mirror.go

+	if _, err := os.Stat(launcherPath); err != nil {
+		return "", errors.Wrap(err, "getting launcher version")
+	}
+	cmd := exec.Command(launcherPath, "-version")


Package builder is compiled with github.com/kolide/kit/version metadata, so executing the binary is not required to get the version.

marpaia · 2017-08-25T02:00:56Z

cmd/package-builder/mirror.go

+		"output", tarFilePath,
+	)
+	sources := []string{
+		filepath.Join("build", l.platform, "launcher"),


use LauncherSource() here for the absolute path

marpaia

This is looking a lot better. My main concern now, like I said in one of the comments, is that all of the options complicate the control flow. Ideally, I could just run package-builder mirror and (assuming I had the keys and notary configured locally) the mirror would be created, binaries uploaded, etc.

marpaia · 2017-08-25T18:11:57Z

cmd/package-builder/mirror.go

+// name of GCS bucket where tarballs are saved to
+const (
+	mirrorBucketname = "binaries-for-launcher"
+	stagingPath      = "/tmp/osquery_mirror"


use ioutil.TempDir instead of a hardcoded temp deirectory

marpaia · 2017-08-25T18:13:43Z

cmd/package-builder/mirror.go

+		)
+		flPlatform = flagset.String(
+			"platform",
+			"darwin",


Perhaps just do all platforms and remove this flag?

marpaia · 2017-08-25T18:14:31Z

cmd/package-builder/mirror.go

+		flTar = flagset.Bool(
+			"tar",
+			true,
+			"create osqueryd.tar.gz archive from binary",


why wouldn't I want to do this when running the mirror command?

marpaia · 2017-08-25T18:14:52Z

cmd/package-builder/mirror.go

+		flExtract = flagset.Bool(
+			"extract",
+			true,
+			"extract binary from downloaded archive",


why wouldn't I want to do this when running the mirror command?

marpaia · 2017-08-25T18:15:30Z

cmd/package-builder/mirror.go

+		flDownload = flagset.Bool(
+			"download",
+			true,
+			"download a fresh copy of osquery from s3",


package-builder dev and prod don't give you the option on whether or not you should download anything, I don't think this should either.

marpaia · 2017-08-25T18:16:21Z

cmd/package-builder/mirror.go

+		flUpdateChannel = flagset.String(
+			"update_channel",
+			"stable",
+			"create a tarball for a specific autoupdate channel. Valid values: stable,beta,nightly",


Similar to platform, I think this should not be an option and it should just loop through all of them.

Well, here I disagree. You can't mark a beta binary as stable for example.

You have to make that decision before running the utility.

I think that all of this conditional logic should be flattened out and

for each update channel

for each platform

the binaries should be

downloaded

extracted

tar'd

uploaded

published

@marpaia I agree for the most part, but @groob is correct, you'll have to choose the channel that you'll be publishing.

why can't you make sure that all of the update channels are appropriately published?

we resolved this one in chat with regards to update channel.

marpaia · 2017-08-25T18:18:49Z

cmd/package-builder/mirror.go

+			return errors.Wrap(err, "uploading")
+		}
+	}
+	if m.wantUpload && m.wantPublish {


I think all of the flags that are allowed to be used in this tool make the control flow very complex. It's likely that there are several combinations of options that don't work together, for example. I think we should remove most of the command-line options from this tool and have the default behavior just do all of the things you want.

groob · 2017-08-30T00:19:29Z

tools/packaging/mirror/mirror.go

+		"msg", "starting",
+	)
+	ctx := context.Background()
+	client, err := storage.NewClient(ctx)


groob · 2017-08-30T00:20:09Z

tools/packaging/download/Makefile

@@ -0,0 +1,33 @@
+all: clean-darwin dl-darwin extract-darwin tar-darwin dl-linux tar-linux


Is this makefile still necessary?

groob · 2017-08-30T00:21:43Z

tools/packaging/mirror/mirror.go

+		target,
+		archive,
+		"-p",
+	)


Probably want to add stdout/stderr to the publish command in case it fails.

groob · 2017-08-30T00:23:45Z

tools/packaging/mirror/mirror.go

+	buildDir := filepath.Join(packaging.LauncherSource(), "build")
+	sources := []string{
+		filepath.Join(buildDir, platform, "launcher"),
+		filepath.Join(buildDir, platform, "osquery-extension.ext"),


You're publishing the .ext file as well. I am not sure it's necessary. It's unlikely to change, it's already shipped with the package.

I also don't think the autoupdate code supports tarballs with multiple binaries.
If you include the launcher, definitely double-check the autoupdate code.

marpaia · 2017-08-30T00:05:48Z

cmd/package-builder/README.md

+```
+
+
+### Version info


It looks like this got a little out of place as the next section here isn't about version info as far as I can tell.

murphybytes requested review from marpaia and groob August 25, 2017 00:15

marpaia mentioned this pull request Aug 25, 2017

download current releases from the osquery site #69

Closed

marpaia reviewed Aug 25, 2017

View reviewed changes

marpaia mentioned this pull request Aug 25, 2017

Permissions changed on binaries for launcher #99

Closed

marpaia added the zzHistoric:In Progress label Aug 29, 2017

Use dependency graph to enforce order of mirror build steps

92bd57c

murphybytes removed the zzHistoric:In Progress label Aug 30, 2017

groob reviewed Aug 30, 2017

View reviewed changes

tools/packaging/mirror/mirror.go

"msg", "starting",

)

ctx := context.Background()

client, err := storage.NewClient(ctx)

Copy link

Contributor

groob Aug 30, 2017

Choose a reason for hiding this comment

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

Neat!

groob reviewed Aug 30, 2017

View reviewed changes

marpaia reviewed Aug 30, 2017

View reviewed changes

Code review changes

2c96cc5

murphybytes merged commit 874e302 into kolide:master Aug 30, 2017

murphybytes deleted the launcher_download branch August 30, 2017 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add launcher and publishing support to the mirror command #93

Add launcher and publishing support to the mirror command #93

murphybytes commented Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia left a comment

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

marpaia Aug 25, 2017

groob Aug 25, 2017

marpaia Aug 25, 2017

murphybytes Aug 28, 2017

marpaia Aug 28, 2017

marpaia Aug 28, 2017

marpaia Aug 25, 2017

groob Aug 30, 2017

groob Aug 30, 2017

groob Aug 30, 2017

groob Aug 30, 2017

marpaia Aug 30, 2017

		@@ -67,6 +83,46 @@ To use the tool to generate Kolide internal development packages, run:

		To use the tool to generate Kolide production packages, run:

		@@ -0,0 +1,33 @@
		all: clean-darwin dl-darwin extract-darwin tar-darwin dl-linux tar-linux

Add launcher and publishing support to the mirror command #93

Add launcher and publishing support to the mirror command #93

Conversation

murphybytes commented Aug 25, 2017

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

marpaia left a 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

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