Code tidy (#721)

Batching several small changes

1. Remove CI builder, which is now too new for go 1.13
2. Use the comshim library for wmi
3. Update dataflatten to accept a logger, and apply the info filter internally
4. Update dataflatten callers to have fewer if blocks (dataflatten duplicates that internally)

.github/workflows/go.yml

@@ -15,7 +15,7 @@ jobs:
       matrix:
         os:
           - ubuntu-18.04
-          - ubuntu-latest
+            #- ubuntu-latest # No support for go 1.13
           - macos-latest
           - windows-latest
     steps:

pkg/dataflatten/examples/flatten/main.go

@@ -46,10 +46,7 @@ func main() {
 	opts := []dataflatten.FlattenOpts{
 		dataflatten.WithLogger(logger),
 		dataflatten.WithNestedPlist(),
-	}
-
-	if *flQuery != "" {
-		opts = append(opts, dataflatten.WithQuery(strings.Split(*flQuery, `/`)))
+		dataflatten.WithQuery(strings.Split(*flQuery, `/`)),
 	}
 
 	rows := []dataflatten.Row{}

pkg/dataflatten/flatten.go

@@ -61,14 +61,15 @@ import (
 //
 // It can optionally filtering and rewriting.
 type Flattener struct {
+	debugLogging      bool
+	expandNestedPlist bool
+	includeNestedRaw  bool
 	includeNils       bool
-	rows              []Row
 	logger            log.Logger
 	query             []string
-	queryWildcard     string
 	queryKeyDenoter   string
-	expandNestedPlist bool
-	includeNestedRaw  bool
+	queryWildcard     string
+	rows              []Row
 }
 
 type FlattenOpts func(*Flattener)
@@ -90,16 +91,30 @@ func WithNestedPlist() FlattenOpts {
 
 // WithLogger sets the logger to use
 func WithLogger(logger log.Logger) FlattenOpts {
+	if logger == nil {
+		return func(_ *Flattener) {}
+	}
+
 	return func(fl *Flattener) {
 		fl.logger = logger
 	}
 }
 
+// WithDebugLogging enables debug logging. With debug logs,
+// dataflatten is very verbose. This can overwhelm the other launcher
+// logs. As we're not generally debugging this library, the default is
+// to not enable debug logging.
+func WithDebugLogging() FlattenOpts {
+	return func(fl *Flattener) {
+		fl.debugLogging = true
+	}
+}
+
 // WithQuery Specifies a query to flatten with. This is used both for
 // re-writing arrays into maps, and for filtering. See "Query
 // Specification" for docs.
 func WithQuery(q []string) FlattenOpts {
-	if q == nil || len(q) == 0 {
+	if q == nil || len(q) == 0 || (len(q) == 1 && q[0] == "") {
 		return func(_ *Flattener) {}
 	}
 
@@ -121,6 +136,10 @@ func Flatten(data interface{}, opts ...FlattenOpts) ([]Row, error) {
 		opt(fl)
 	}
 
+	if !fl.debugLogging {
+		fl.logger = level.NewFilter(fl.logger, level.AllowInfo())
+	}
+
 	if err := fl.descend([]string{}, data, 0); err != nil {
 		return nil, err
 	}

pkg/osquery/tables/dataflattentable/exec.go

@@ -97,14 +97,9 @@ func (t *Table) exec(ctx context.Context) ([]byte, error) {
 }
 
 func (t *Table) getRowsFromOutput(dataQuery string, execOutput []byte) []map[string]string {
-	flattenOpts := []dataflatten.FlattenOpts{}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
-	}
-
-	if t.logger != nil {
-		flattenOpts = append(flattenOpts, dataflatten.WithLogger(t.logger))
+	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	data, err := t.execDataFunc(execOutput, flattenOpts...)

pkg/osquery/tables/dataflattentable/tables.go

@@ -100,16 +100,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 
 func (t *Table) generatePath(filePath string, dataQuery string) ([]map[string]string, error) {
 	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
 		dataflatten.WithNestedPlist(),
-	}
-
-	if t.logger != nil {
-		// dataflatten is noisy, so unless we're not debugging it, filter it to info
-		flattenOpts = append(flattenOpts, dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())))
-	}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	data, err := t.dataFunc(filePath, flattenOpts...)

pkg/osquery/tables/dsim_default_associations/dsim_default_associations.go

@@ -52,15 +52,10 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 
 	for _, dataQuery := range tablehelpers.GetConstraints(queryContext, "query", tablehelpers.WithDefaults("*")) {
 		flattenOpts := []dataflatten.FlattenOpts{
+			dataflatten.WithLogger(t.logger),
 			dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 		}
 
-		if t.logger != nil {
-			flattenOpts = append(flattenOpts,
-				dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())),
-			)
-		}
-
 		rows, err := dataflatten.Xml(dismResults, flattenOpts...)
 		if err != nil {
 			level.Info(t.logger).Log("msg", "flatten failed", "err", err)

pkg/osquery/tables/ioreg/ioreg.go

@@ -141,16 +141,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 }
 
 func (t *Table) flattenOutput(dataQuery string, systemOutput []byte) ([]dataflatten.Row, error) {
-	flattenOpts := []dataflatten.FlattenOpts{}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
-	}
-
-	if t.logger != nil {
-		flattenOpts = append(flattenOpts,
-			dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())),
-		)
+	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	return dataflatten.Plist(systemOutput, flattenOpts...)

pkg/osquery/tables/mdmclient/mdmclient.go

@@ -99,16 +99,9 @@ func (t *Table) flattenOutput(dataQuery string, systemOutput []byte) ([]dataflat
 		return nil, errors.Wrap(err, "converting")
 	}
 
-	flattenOpts := []dataflatten.FlattenOpts{}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
-	}
-
-	if t.logger != nil {
-		flattenOpts = append(flattenOpts,
-			dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())),
-		)
+	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	return dataflatten.Plist(converted, flattenOpts...)

pkg/osquery/tables/profiles/profiles.go

@@ -114,13 +114,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 					}
 
 					flattenOpts := []dataflatten.FlattenOpts{
+						dataflatten.WithLogger(t.logger),
 						dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 					}
-					if t.logger != nil {
-						flattenOpts = append(flattenOpts,
-							dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())),
-						)
-					}
 
 					flatData, err := dataflatten.PlistFile(outputFile, flattenOpts...)
 					if err != nil {
@@ -144,16 +140,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 }
 
 func (t *Table) flattenOutput(dataQuery string, systemOutput []byte) ([]dataflatten.Row, error) {
-	flattenOpts := []dataflatten.FlattenOpts{}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
-	}
-
-	if t.logger != nil {
-		flattenOpts = append(flattenOpts,
-			dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())),
-		)
+	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	return dataflatten.Plist(systemOutput, flattenOpts...)

pkg/osquery/tables/pwpolicy/pwpolicy.go

@@ -70,13 +70,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 			}
 
 			flattenOpts := []dataflatten.FlattenOpts{
+				dataflatten.WithLogger(t.logger),
 				dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 			}
-			if t.logger != nil {
-				flattenOpts = append(flattenOpts,
-					dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())),
-				)
-			}
 
 			flatData, err := dataflatten.Plist(pwPolicyOutput, flattenOpts...)
 			if err != nil {

pkg/osquery/tables/secedit/secedit.go

@@ -80,16 +80,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 }
 
 func (t *Table) flattenOutput(dataQuery string, systemOutput []byte) ([]dataflatten.Row, error) {
-	flattenOpts := []dataflatten.FlattenOpts{}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
-	}
-
-	if t.logger != nil {
-		flattenOpts = append(flattenOpts,
-			dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())),
-		)
+	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	return dataflatten.Ini(systemOutput, flattenOpts...)

pkg/osquery/tables/systemprofiler/systemprofiler.go

@@ -159,14 +159,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 func (t *Table) getRowsFromOutput(dataQuery, detailLevel string, systemProfilerOutput []byte) []map[string]string {
 	var results []map[string]string
 
-	flattenOpts := []dataflatten.FlattenOpts{}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
-	}
-
-	if t.logger != nil {
-		flattenOpts = append(flattenOpts, dataflatten.WithLogger(t.logger))
+	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	var systemProfilerResults []Result
@@ -198,7 +193,7 @@ func (t *Table) getRowsFromOutput(dataQuery, detailLevel string, systemProfilerO
 }
 
 func (t *Table) execSystemProfiler(ctx context.Context, detailLevel string, subcommands []string) ([]byte, error) {
-	timeout := 30 * time.Second
+	timeout := 45 * time.Second
 	if detailLevel == "full" {
 		timeout = 5 * time.Minute
 	}

pkg/osquery/tables/wifi_networks/wifi_networks.go

@@ -52,7 +52,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 	if err := t.getBytes(ctx, &output); err != nil {
 		return results, errors.Wrap(err, "getting raw data")
 	}
-	rows, err := dataflatten.Json(output.Bytes(), dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())))
+	rows, err := dataflatten.Json(output.Bytes(), dataflatten.WithLogger(t.logger))
 	if err != nil {
 		return results, errors.Wrap(err, "flattening json output")
 	}

pkg/osquery/tables/windowsupdatetable/windowsupdate.go

@@ -120,16 +120,9 @@ func (t *Table) searchLocale(locale string, queryContext table.QueryContext) ([]
 }
 
 func (t *Table) flattenOutput(dataQuery string, searchResults interface{}) ([]dataflatten.Row, error) {
-	flattenOpts := []dataflatten.FlattenOpts{}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
-	}
-
-	if t.logger != nil {
-		flattenOpts = append(flattenOpts,
-			dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())),
-		)
+	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	// dataflatten won't parse the raw searchResults. As a workaround,

pkg/osquery/tables/windowsupdatetable/windowsupdate_test.go

@@ -0,0 +1,42 @@
+// +build windows
+
+package windowsupdatetable
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/go-kit/kit/log"
+	"github.com/kolide/launcher/pkg/osquery/tables/tablehelpers"
+	"github.com/stretchr/testify/require"
+)
+
+func TestTable(t *testing.T) {
+	t.Parallel()
+
+	var tests = []struct {
+		name      string
+		queryFunc queryFuncType
+	}{
+		{name: "updates", queryFunc: queryUpdates},
+		{name: "history", queryFunc: queryHistory},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			table := Table{
+				logger:    log.NewNopLogger(),
+				queryFunc: tt.queryFunc,
+			}
+
+			ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second)
+			defer cancel()
+
+			// ci doesn;t return data, but we can, at least, check that the underlying API doesn't error.
+			_, err := table.generate(ctx, tablehelpers.MockQueryContext(nil))
+			require.NoError(t, err, "generate")
+		})
+	}
+
+}

pkg/osquery/tables/wmitable/wmitable.go

@@ -122,14 +122,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) (
 }
 
 func (t *Table) flattenRowsFromWmi(dataQuery string, wmiResults []map[string]interface{}, wmiClass, wmiProperties, wmiNamespace, whereClause string) []map[string]string {
-	flattenOpts := []dataflatten.FlattenOpts{}
-
-	if dataQuery != "" {
-		flattenOpts = append(flattenOpts, dataflatten.WithQuery(strings.Split(dataQuery, "/")))
-	}
-
-	if t.logger != nil {
-		flattenOpts = append(flattenOpts, dataflatten.WithLogger(level.NewFilter(t.logger, level.AllowInfo())))
+	flattenOpts := []dataflatten.FlattenOpts{
+		dataflatten.WithLogger(t.logger),
+		dataflatten.WithQuery(strings.Split(dataQuery, "/")),
 	}
 
 	// wmi.Query returns []map[string]interface{}, but dataflatten

pkg/wmi/wmi.go

@@ -41,6 +41,7 @@ import (
 	"github.com/go-ole/go-ole/oleutil"
 	"github.com/kolide/launcher/pkg/contexts/ctxlog"
 	"github.com/pkg/errors"
+	"github.com/scjalliance/comshim"
 )
 
 // S_FALSE is returned by CoInitializeEx if it was already called on this thread.
@@ -133,14 +134,8 @@ func Query(ctx context.Context, className string, properties []string, opts ...O
 	queryString := fmt.Sprintf("SELECT * FROM %s%s", className, whereClause)
 
 	// Initialize the COM system.
-	if err := ole.CoInitializeEx(0, ole.COINIT_MULTITHREADED); err != nil {
-		oleCode := err.(*ole.OleError).Code()
-		if oleCode != ole.S_OK && oleCode != S_FALSE {
-			return nil, errors.Wrap(err, "CoInitialize returned error")
-		}
-		level.Debug(logger).Log("msg", "The COM library is already initialized on this thread")
-	}
-	defer ole.CoUninitialize()
+	comshim.Add(1)
+	defer comshim.Done()
 
 	unknown, err := oleutil.CreateObject("WbemScripting.SWbemLocator")
 	if err != nil {