Add golangci-lint, fix warnings

- Simplify the monitor test, it had an unused variable warning from
  `functionReturned`
- The job lister didn't need to add a namespace indexer to the job
  informer, it comes with one by default. When I added error handling to
  the `AddIndexers` call, we were missing an "indexer conflict" error.

.buildkite/pipeline.yml

@@ -1,4 +1,14 @@
 steps:
+  - label: ":lint-roller: lint"
+    agents:
+      queue: kubernetes
+    plugins:
+      - kubernetes:
+          podSpec:
+            containers:
+              - image: golang:latest
+                command: [.buildkite/steps/lint.sh]
+
   - label: ":buildkite: integration tests"
     agents:
       queue: kubernetes

.buildkite/steps/lint.sh

@@ -0,0 +1,11 @@
+#!/bin/bash
+set -euxo pipefail
+
+apt update && apt install -y lsb-release
+curl -q 'https://proget.makedeb.org/debian-feeds/prebuilt-mpr.pub' | gpg --dearmor | tee /usr/share/keyrings/prebuilt-mpr-archive-keyring.gpg 1>/dev/null
+echo "deb [signed-by=/usr/share/keyrings/prebuilt-mpr-archive-keyring.gpg] https://proget.makedeb.org prebuilt-mpr $(lsb_release -cs)" | tee /etc/apt/sources.list.d/prebuilt-mpr.list
+apt update && apt install -y just
+
+curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.50.1
+
+just lint

.golangci.yaml

@@ -0,0 +1,2 @@
+run:
+  timeout: 5m

Brewfile

@@ -3,3 +3,4 @@ brew "go"
 brew "ko"
 brew "helm"
 brew "goreleaser"
+brew "golangci-lint"

api/api.go

@@ -50,6 +50,5 @@ func (c Config) MarshalLogObject(enc zapcore.ObjectEncoder) error {
 	enc.AddInt("max-in-flight", c.MaxInFlight)
 	enc.AddString("namespace", c.Namespace)
 	enc.AddString("org", c.Org)
-	enc.AddReflected("tags", c.Tags)
-	return nil
+	return enc.AddReflected("tags", c.Tags)
 }

api/client.go

@@ -43,7 +43,6 @@ func (t *logTransport) RoundTrip(in *http.Request) (out *http.Response, err erro
 	// Inspired by: github.com/motemen/go-loghttp
 	if _, ok := os.LookupEnv("DEBUG"); !ok {
 		return t.inner.RoundTrip(in)
-
 	}
 
 	log.Printf("--> %s %s", in.Method, in.URL)

api/genqlient.graphql

@@ -10,7 +10,6 @@ fragment CommandJob on JobTypeCommand {
   scheduledAt
   agentQueryRules
   command
-  label
 }
 
 # @genqlient(omitempty: true)

integration/integration_test.go

@@ -74,6 +74,7 @@ func TestSSHRepoClone(t *testing.T) {
 }
 
 func basicTest(t *testing.T, fixture, repo string) {
+	t.Helper()
 	// create pipeline
 	ctx := context.Background()
 	graphqlClient := api.NewClient(cfg.BuildkiteToken)
@@ -138,9 +139,11 @@ func basicTest(t *testing.T, fixture, repo string) {
 	})
 	require.NoError(t, err)
 	EnsureCleanup(t, func() {
-		api.BuildCancel(ctx, graphqlClient, api.BuildCancelInput{
+		if _, err := api.BuildCancel(ctx, graphqlClient, api.BuildCancelInput{
 			Id: createBuild.BuildCreate.Build.Id,
-		})
+		}); err != nil {
+			t.Logf("failed to cancel build: %v", err)
+		}
 	})
 	build := createBuild.BuildCreate.Build
 	require.Len(t, build.Jobs.Edges, 1)

justfile

@@ -1,7 +1,4 @@
-default: lint generate build test
-
-build:
-  echo Building…
+default: lint generate test
 
 run *FLAGS:
   go run ./... {{FLAGS}}
@@ -10,6 +7,7 @@ test *FLAGS:
   go test {{FLAGS}} ./...
 
 lint: gomod
+  golangci-lint run
 
 generate:
     go run github.com/Khan/genqlient api/genqlient.yaml

main.go

@@ -3,8 +3,6 @@ package main
 import (
 	"log"
 
-	_ "net/http/pprof"
-
 	"github.com/buildkite/agent-stack-k8s/cmd/controller"
 	"k8s.io/client-go/kubernetes"
 

monitor/lister.go

@@ -22,17 +22,15 @@ func NewJobLister(ctx context.Context, log *zap.Logger, clientset kubernetes.Int
 	if err != nil {
 		return nil, fmt.Errorf("failed to build tag label selector for job manager: %w", err)
 	}
-	hasUuid, err := labels.NewRequirement(api.UUIDLabel, selection.Exists, nil)
+	hasUUID, err := labels.NewRequirement(api.UUIDLabel, selection.Exists, nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to build uuid label selector for job manager: %w", err)
 	}
 	factory := informers.NewSharedInformerFactoryWithOptions(clientset, 0, informers.WithTweakListOptions(func(opt *metav1.ListOptions) {
-		opt.LabelSelector = labels.NewSelector().Add(*hasTag, *hasUuid).String()
+		opt.LabelSelector = labels.NewSelector().Add(*hasTag, *hasUUID).String()
 	}))
 	informer := factory.Batch().V1().Jobs()
 	jobInformer := informer.Informer()
-	indexer := cache.NewIndexer(MetaJobLabelKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
-	jobInformer.AddIndexers(indexer.GetIndexers())
 
 	go factory.Start(ctx.Done())
 
@@ -49,7 +47,7 @@ func MetaJobLabelKeyFunc(obj interface{}) (string, error) {
 	}
 	meta, err := meta.Accessor(obj)
 	if err != nil {
-		return "", fmt.Errorf("object has no meta: %v", err)
+		return "", fmt.Errorf("object has no meta: %w", err)
 	}
 	labels := meta.GetLabels()
 	if v, ok := labels[api.UUIDLabel]; ok {

monitor/monitor.go

@@ -95,7 +95,6 @@ func (m *Monitor) start() {
 					cmdJ := builds[j].Node.(*api.JobJobTypeCommand)
 
 					return cmdI.ScheduledAt.Before(cmdJ.ScheduledAt)
-
 				})
 
 				for _, job := range builds {

monitor/monitor_test.go

@@ -89,38 +89,30 @@ func TestScheduleBuild(t *testing.T) {
 		shouldSchedule: true,
 		shouldError:    false,
 	}}
+	m.jobs = make(chan Job, 1)
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			var errResult error
-			jobCreated := 0
-			functionReturned := 0
-
-			returned := make(chan error)
-			go func() {
-				err := m.scheduleBuild(test.job, test.tag)
-				returned <- err
-			}()
+			err := m.scheduleBuild(test.job, test.tag)
+			var job *Job
 			select {
-			case <-m.jobs:
-				jobCreated++
-			case errResult = <-returned:
-				functionReturned++
+			case j := <-m.jobs:
+				job = &j
+			default:
 			}
 			if test.shouldSchedule {
-				if jobCreated == 0 {
-					if errResult != nil {
-						t.Fatalf("Test should schedule job, but error was returned: %s", errResult)
+				if job == nil {
+					if err != nil {
+						t.Fatalf("Test should schedule job, but error was returned: %s", err)
 					} else {
 						t.Fatalf("Test should schedule job, but nil error was returned")
 					}
 				}
 			}
 			if test.shouldError {
-				if errResult == nil {
+				if err == nil {
 					t.Fatalf("Test should error, but no error was returned")
 				}
 			}
 		})
 	}
-
 }

scheduler/scheduler.go

@@ -66,11 +66,11 @@ func (w *worker) k8sify(
 			return nil, fmt.Errorf("err parsing plugins: %w", err)
 		}
 		for _, plugin := range plugins {
-			asJson, err := json.Marshal(plugin.Configuration)
+			asJSON, err := json.Marshal(plugin.Configuration)
 			if err != nil {
 				return nil, fmt.Errorf("failed to marshal config: %w", err)
 			}
-			if err := json.Unmarshal(asJson, &pluginConfig); err != nil {
+			if err := json.Unmarshal(asJSON, &pluginConfig); err != nil {
 				return nil, fmt.Errorf("failed to unmarshal config: %w", err)
 			}
 		}

scheduler/scheduler_test.go

@@ -29,7 +29,7 @@ func TestJobPluginConversion(t *testing.T) {
 			},
 		},
 	}
-	pluginsJson, err := json.Marshal([]map[string]interface{}{
+	pluginsJSON, err := json.Marshal([]map[string]interface{}{
 		{
 			"github.com/buildkite-plugins/kubernetes-buildkite-plugin": pluginConfig,
 		},
@@ -39,7 +39,7 @@ func TestJobPluginConversion(t *testing.T) {
 	input := &monitor.Job{
 		CommandJob: api.CommandJob{
 			Uuid: "abc",
-			Env:  []string{fmt.Sprintf("BUILDKITE_PLUGINS=%s", string(pluginsJson))},
+			Env:  []string{fmt.Sprintf("BUILDKITE_PLUGINS=%s", string(pluginsJSON))},
 		},
 		Tag: "queue=kubernetes",
 	}