ci: add linters, run linters, make linters happy

.github/workflows/ci.yaml

@@ -39,6 +39,9 @@ jobs:
         with:
           go-version: "~1.22"
 
+      - name: Lint
+        run: make -j lint
+
       - name: Test
         run: make test
   docs:

Makefile

@@ -4,10 +4,25 @@ PWD=$(shell pwd)
 GO_SRC_FILES := $(shell find . -type f -name '*.go' -not -name '*_test.go')
 GO_TEST_FILES := $(shell find . -type f -not -name '*.go' -name '*_test.go')
 GOLDEN_FILES := $(shell find . -type f -name '*.golden')
+SHELL_SRC_FILES := $(shell find . -type f -name '*.sh')
+GOLANGCI_LINT_VERSION := v1.59.1
 
 fmt: $(shell find . -type f -name '*.go')
 	go run mvdan.cc/[email protected] -l -w .
 
+.PHONY: lint
+lint: lint/go lint/shellcheck
+
+.PHONY: lint/go
+lint/go: $(GO_SRC_FILES)
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)
+	golangci-lint run
+
+.PHONY: lint/shellcheck
+lint/shellcheck: $(SHELL_SRC_FILES)
+	echo "--- shellcheck"
+	shellcheck --external-sources $(SHELL_SRC_FILES)
+
 develop:
 	./scripts/develop.sh
 

cmd/envbuilder/main.go

@@ -57,28 +57,33 @@ func envbuilderCmd() serpent.Command {
 					},
 				}
 				var flushAndClose func(ctx context.Context) error
+				// nolint: staticcheck // FIXME: https://github.com/coder/envbuilder/issues/260
 				sendLogs, flushAndClose = notcodersdk.LogsSender(notcodersdk.ExternalLogSourceID, client.PatchLogs, slog.Logger{})
-				defer flushAndClose(inv.Context())
+				defer func() {
+					_ = flushAndClose(inv.Context())
+				}()
 
 				// This adds the envbuilder subsystem.
 				// If telemetry is enabled in a Coder deployment,
 				// this will be reported and help us understand
 				// envbuilder usage.
 				if !slices.Contains(options.CoderAgentSubsystem, string(notcodersdk.AgentSubsystemEnvbuilder)) {
 					options.CoderAgentSubsystem = append(options.CoderAgentSubsystem, string(notcodersdk.AgentSubsystemEnvbuilder))
-					os.Setenv("CODER_AGENT_SUBSYSTEM", strings.Join(options.CoderAgentSubsystem, ","))
+					_ = os.Setenv("CODER_AGENT_SUBSYSTEM", strings.Join(options.CoderAgentSubsystem, ","))
 				}
 			}
 
 			options.Logger = func(level notcodersdk.LogLevel, format string, args ...interface{}) {
 				output := fmt.Sprintf(format, args...)
-				fmt.Fprintln(inv.Stderr, output)
+				_, _ = fmt.Fprintln(inv.Stderr, output)
 				if sendLogs != nil {
-					sendLogs(inv.Context(), notcodersdk.Log{
+					if err := sendLogs(inv.Context(), notcodersdk.Log{
 						CreatedAt: time.Now(),
 						Output:    output,
 						Level:     level,
-					})
+					}); err != nil {
+						_, _ = fmt.Fprintf(inv.Stderr, "failed to send logs: %s\n", err.Error())
+					}
 				}
 			}
 

envbuilder_internal_test.go

@@ -52,7 +52,8 @@ func TestFindDevcontainerJSON(t *testing.T) {
 		fs := memfs.New()
 		err := fs.MkdirAll("/workspace/.devcontainer", 0o600)
 		require.NoError(t, err)
-		fs.Create("/workspace/.devcontainer/devcontainer.json")
+		_, err = fs.Create("/workspace/.devcontainer/devcontainer.json")
+		require.NoError(t, err)
 
 		// when
 		devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{
@@ -73,7 +74,8 @@ func TestFindDevcontainerJSON(t *testing.T) {
 		fs := memfs.New()
 		err := fs.MkdirAll("/workspace/experimental-devcontainer", 0o600)
 		require.NoError(t, err)
-		fs.Create("/workspace/experimental-devcontainer/devcontainer.json")
+		_, err = fs.Create("/workspace/experimental-devcontainer/devcontainer.json")
+		require.NoError(t, err)
 
 		// when
 		devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{
@@ -95,7 +97,8 @@ func TestFindDevcontainerJSON(t *testing.T) {
 		fs := memfs.New()
 		err := fs.MkdirAll("/workspace/.devcontainer", 0o600)
 		require.NoError(t, err)
-		fs.Create("/workspace/.devcontainer/experimental.json")
+		_, err = fs.Create("/workspace/.devcontainer/experimental.json")
+		require.NoError(t, err)
 
 		// when
 		devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{
@@ -117,7 +120,8 @@ func TestFindDevcontainerJSON(t *testing.T) {
 		fs := memfs.New()
 		err := fs.MkdirAll("/workspace", 0o600)
 		require.NoError(t, err)
-		fs.Create("/workspace/devcontainer.json")
+		_, err = fs.Create("/workspace/devcontainer.json")
+		require.NoError(t, err)
 
 		// when
 		devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{
@@ -138,7 +142,8 @@ func TestFindDevcontainerJSON(t *testing.T) {
 		fs := memfs.New()
 		err := fs.MkdirAll("/workspace/.devcontainer/sample", 0o600)
 		require.NoError(t, err)
-		fs.Create("/workspace/.devcontainer/sample/devcontainer.json")
+		_, err = fs.Create("/workspace/.devcontainer/sample/devcontainer.json")
+		require.NoError(t, err)
 
 		// when
 		devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{

init.sh

@@ -3,5 +3,5 @@
 echo hey there
 sleep 1
 
-echo INIT_COMMAND=/bin/sh >> $ENVBUILDER_ENV
-echo INIT_ARGS="-c /bin/bash" >> $ENVBUILDER_ENV
+echo INIT_COMMAND=/bin/sh >> "${ENVBUILDER_ENV}"
+echo INIT_ARGS="-c /bin/bash" >> "${ENVBUILDER_ENV}"

integration/integration_test.go

@@ -1612,9 +1612,11 @@ func cleanOldEnvbuilders() {
 		panic(err)
 	}
 	for _, ctr := range ctrs {
-		cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{
+		if err := cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{
 			Force: true,
-		})
+		}); err != nil {
+			_, _ = fmt.Fprintf(os.Stderr, "failed to remove old test container: %s\n", err.Error())
+		}
 	}
 }
 

scripts/build.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-cd $(dirname "${BASH_SOURCE[0]}")
+cd "$(dirname "${BASH_SOURCE[0]}")"
 set -euo pipefail
 
 archs=()
@@ -48,24 +48,24 @@ docker buildx inspect --bootstrap &> /dev/null
 
 for arch in "${archs[@]}"; do
   echo "Building for $arch..."
-  GOARCH=$arch CGO_ENABLED=0 go build -o ./envbuilder-$arch ../cmd/envbuilder &
+  GOARCH=$arch CGO_ENABLED=0 go build -o "./envbuilder-${arch}" ../cmd/envbuilder &
 done
 wait
 
 args=()
 for arch in "${archs[@]}"; do
-  args+=( --platform linux/$arch )
+  args+=( --platform "linux/${arch}" )
 done
 if [ "$push" = true ]; then
   args+=( --push )
 else
   args+=( --load )
 fi
 
-docker buildx build --builder $BUILDER_NAME "${args[@]}" -t $base:$tag -t $base:latest -f Dockerfile .
+docker buildx build --builder $BUILDER_NAME "${args[@]}" -t "${base}:${tag}" -t "${base}:latest" -f Dockerfile .
 
 # Check if archs contains the current. If so, then output a message!
-if [[ -z "${CI:-}" ]] && [[ " ${archs[@]} " =~ " ${current} " ]]; then
-  docker tag $base:$tag envbuilder:latest
+if [[ -z "${CI:-}" ]] && [[ " ${archs[*]} " =~ ${current} ]]; then
+  docker tag "${base}:${tag}" envbuilder:latest
   echo "Tagged $current as envbuilder:latest!"
 fi

scripts/develop.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-cd $(dirname "${BASH_SOURCE[0]}")
+cd "$(dirname "${BASH_SOURCE[0]}")"
 set -euxo pipefail
 
 ./build.sh

scripts/diagram.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-cd $(dirname "${BASH_SOURCE[0]}")
+cd "$(dirname "${BASH_SOURCE[0]}")"
 set -euxo pipefail
 
 d2 ./diagram.d2 --pad=32 -t 1 ./diagram-light.svg

scripts/version.sh

@@ -1,7 +1,7 @@
 #!/usr/bin/env bash
 
 set -euo pipefail
-cd $(dirname "${BASH_SOURCE[0]}")
+cd "$(dirname "${BASH_SOURCE[0]}")"
 
 last_tag="$(git describe --tags --abbrev=0)"
 version="$last_tag"

testutil/registrytest/registrytest.go

@@ -44,16 +44,6 @@ func New(t *testing.T) string {
 	return srv.URL
 }
 
-type logrusFormatter struct {
-	callback func(entry *logrus.Entry)
-	empty    []byte
-}
-
-func (f *logrusFormatter) Format(entry *logrus.Entry) ([]byte, error) {
-	f.callback(entry)
-	return f.empty, nil
-}
-
 // WriteContainer uploads a container to the registry server.
 // It returns the reference to the uploaded container.
 func WriteContainer(t *testing.T, serverURL, containerRef, mediaType string, files map[string]any) string {