Small code improvements (#634)

Several small improvement:

* Add an option to not cleanup the wix build directories. This aids in debugging MSI builds
* Improve the method to find osqueryd binaries. This was hardcoded to an prior products install directory
* Simplify the subcommand logic in main
* Fix the makefile's code generation target

Makefile

@@ -128,7 +128,7 @@ deps: deps-go generate
 
 .PHONY: generate
 generate:
-	go generate ./pkg/packagekit ./pkg/packaging
+	go generate ./pkg/packagekit/... ./pkg/packaging/...
 	go run cmd/make/make.go -targets=generate-tuf
 
 .PHONY: proto

cmd/launcher/launcher.go

@@ -28,9 +28,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-// defaultOsquerydPath is the default path to the bundled osqueryd binary
-const defaultOsquerydPath = "/usr/local/kolide/bin/osqueryd"
-
 // runLauncher is the entry point into running launcher. It creates a
 // rungroups with the various options, and goes! If autoupdate is
 // enabled, the finalizers will trigger various restarts.

cmd/launcher/main.go

@@ -5,6 +5,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"strings"
 	"text/tabwriter"
 
 	"github.com/go-kit/kit/log/level"
@@ -68,10 +69,9 @@ func main() {
 		}
 	}
 
-	// if the launcher is being ran with a positional argument, handle that
-	// argument. If a known positional argument is not supplied, fall-back to
-	// running an osquery instance.
-	if isSubCommand() {
+	// if the launcher is being ran with a positional argument,
+	// handle that argument. Fall-back to running launcher
+	if len(os.Args) > 1 && !strings.HasPrefix(os.Args[1], `-`) {
 		if err := runSubcommands(); err != nil {
 			logutil.Fatal(logger, "err", errors.Wrap(err, "run with positional args"))
 		}
@@ -94,29 +94,6 @@ func main() {
 	}
 }
 
-func isSubCommand() bool {
-	if len(os.Args) < 2 {
-		return false
-	}
-
-	subCommands := []string{
-		"socket",
-		"query",
-		"flare",
-		"svc",
-		"svc-fg",
-		"version",
-	}
-
-	for _, sc := range subCommands {
-		if sc == os.Args[1] {
-			return true
-		}
-	}
-
-	return false
-}
-
 func runSubcommands() error {
 	var run func([]string) error
 	switch os.Args[1] {
@@ -132,7 +109,10 @@ func runSubcommands() error {
 		run = runWindowsSvcForeground
 	case "version":
 		run = runVersion
+	default:
+		return errors.Errorf("Unknown subcommand %s", os.Args[1])
 	}
+
 	err := run(os.Args[2:])
 	return errors.Wrapf(err, "running subcommand %s", os.Args[1])
 }

cmd/launcher/options.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"time"
@@ -113,11 +114,8 @@ func parseOptions(args []string) (*launcher.Options, error) {
 	// osqueryd found in the path
 	osquerydPath := *flOsquerydPath
 	if *flOsquerydPath == "" {
-		if _, err := os.Stat(defaultOsquerydPath); err == nil {
-			osquerydPath = defaultOsquerydPath
-		} else if path, err := exec.LookPath("osqueryd"); err == nil {
-			osquerydPath = path
-		} else {
+		*flOsquerydPath = findOsquery()
+		if *flOsquerydPath == "" {
 			return nil, errors.New("Could not find osqueryd binary")
 		}
 	}
@@ -290,3 +288,27 @@ func parseCertPins(pins string) ([][]byte, error) {
 	}
 	return certPins, nil
 }
+
+// findOsquery will attempt to find osquery. We don't much care about
+// errors here, either we find it, or we don't.
+func findOsquery() string {
+	osqBinaryName := "osqueryd"
+	if runtime.GOOS == "windows" {
+		osqBinaryName = osqBinaryName + ".exe"
+	}
+
+	if exPath, err := os.Executable(); err != nil {
+		maybeOsq := filepath.Join(filepath.Base(exPath), osqBinaryName)
+
+		info, err := os.Stat(maybeOsq)
+		if err != nil && !info.IsDir() {
+			return maybeOsq
+		}
+	}
+
+	if osqPath, err := exec.LookPath(osqBinaryName); err == nil {
+		return osqPath
+	}
+
+	return ""
+}

cmd/package-builder/package-builder.go

@@ -175,6 +175,11 @@ func runMake(args []string) error {
 			defaultWixPath,
 			fmt.Sprintf(`Location of wix binaries (default: "%s")`, defaultWixPath),
 		)
+		flWixSkipCleanup = flagset.Bool(
+			"wix_skip_cleanup",
+			false,
+			"Keep wix temp files",
+		)
 		flOsqueryFlags arrayFlags // set below with flagset.Var
 	)
 	flagset.Var(&flOsqueryFlags, "osquery_flag", "Flags to pass to osquery (possibly overriding Launcher defaults)")
@@ -244,6 +249,7 @@ func runMake(args []string) error {
 		MirrorURL:         *flMirrorURL,
 		NotaryPrefix:      *flNotaryPrefix,
 		WixPath:           *flWixPath,
+		WixSkipCleanup:    *flWixSkipCleanup,
 	}
 
 	outputDir := *flOutputDir

pkg/packagekit/package.go

@@ -14,6 +14,7 @@ type PackageOptions struct {
 	WindowsUseSigntool  bool     // whether to use signtool.exe on windows
 	WindowsSigntoolArgs []string // Extra args for signtool. May be needed for finding a key
 
-	WixPath string // path to wix installation
-	WixUI   bool   //include the wix ui or not
+	WixPath        string // path to wix installation
+	WixUI          bool   //include the wix ui or not
+	WixSkipCleanup bool   // keep the temp dirs
 }

pkg/packagekit/package_wix.go

@@ -85,6 +85,10 @@ func PackageWixMSI(ctx context.Context, w io.Writer, po *PackageOptions, include
 
 	wixArgs := []wix.WixOpt{}
 
+	if po.WixSkipCleanup {
+		wixArgs = append(wixArgs, wix.SkipCleanup())
+	}
+
 	if po.WixPath != "" {
 		wixArgs = append(wixArgs, wix.WithWix(po.WixPath))
 	}

pkg/packagekit/wix/wix.go

@@ -24,6 +24,7 @@ type wixTool struct {
 	services       []*Service // array of services.
 	dockerImage    string     // If in docker, what image?
 	skipValidation bool       // Skip light validation. Seems to be needed for running in 32bit wine environments.
+	skipCleanup    bool       // Skip cleaning temp dirs. Useful when debugging wix generation
 	cleanDirs      []string   // directories to rm on cleanup
 	ui             bool       // whether or not to include a ui
 	extraFiles     []extraFile
@@ -96,6 +97,12 @@ func WithFile(name string, content []byte) WixOpt {
 	}
 }
 
+func SkipCleanup() WixOpt {
+	return func(wo *wixTool) {
+		wo.skipCleanup = true
+	}
+}
+
 // New takes a packageRoot of files, and a wxsContent of xml wix
 // configuration, and will return a struct with methods for building
 // packages with.
@@ -154,6 +161,10 @@ func New(packageRoot string, mainWxsContent []byte, wixOpts ...WixOpt) (*wixTool
 
 // Cleanup removes temp directories. Meant to be called in a defer.
 func (wo *wixTool) Cleanup() {
+	if wo.skipCleanup {
+		return
+	}
+
 	for _, d := range wo.cleanDirs {
 		os.RemoveAll(d)
 	}

pkg/packagekit/wix/wix_test.go

@@ -16,7 +16,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-//go:generate go-bindata -pkg testdata -o testdata/assets.go testdata/assets/
+//go:generate go-bindata  -nometadata -nocompress -pkg testdata -o testdata/assets.go testdata/assets/
 
 func TestWixPackage(t *testing.T) {
 	t.Parallel()

pkg/packaging/packaging.go

@@ -54,6 +54,7 @@ type PackageOptions struct {
 	NotaryPrefix      string
 	WixPath           string
 	MSIUI             bool
+	WixSkipCleanup    bool
 
 	AppleSigningKey     string   // apple signing key
 	WindowsUseSigntool  bool     // whether to use signtool.exe on windows
@@ -293,6 +294,7 @@ func (p *PackageOptions) Build(ctx context.Context, packageWriter io.Writer, tar
 		FlagFile:            p.canonicalizePath(flagFilePath),
 		WixPath:             p.WixPath,
 		WixUI:               p.MSIUI,
+		WixSkipCleanup:      p.WixSkipCleanup,
 	}
 
 	if err := p.makePackage(ctx); err != nil {