ci: lint, vet and test relay/ as its own module

relay/ (github.com/docker/compose-relay) is a separate go.mod: root `go vet`/`golangci-lint`/`go test` never covered it, and the local pre-commit hook explicitly excludes any directory with its own go.mod for the same reason. Only the image build (relay-image-cross) was ever validated in CI -- the Go source itself had no quality gate. Adds relay-lint and relay-test bake targets, mirroring the root lint/ test Dockerfile stages: a build-base downloading go.mod's (currently empty) dependency set, a lint stage running the same pinned golangci-lint version as the root, and a test stage running go vet then go test. relay-lint joins the "validate" bake group; both are new matrix entries in the CI validate job. relay/ gets its own .golangci.yml (the lint stage's build context is "./relay" only, so it can't see the root config) with the same rule set as root, minus what doesn't apply to a dependency-free module (the e2e build tag, testify/gotest.tools-specific depguard entries). Turning lint on for the first time surfaced 8 real findings in relay/main.go and main_test.go (unchecked Close() errors, %v instead of %w on a wrapped error, a redundant embedded-field selector, and log.Fatalf after a defer that would never run) -- fixed here so the new job is green from the start, not merged red. Signed-off-by: Nicolas De Loof <[email protected]>
2026-10-12 02:13:08 +00:00 · 2026-09-17 09:23:54 +02:00
parent 8d266c871d
commit 7c2c393d74
7 changed files with 152 additions and 12 deletions
@@ -33,6 +33,8 @@ jobs:
          - validate-headers
          - validate-docs
          - validate-mocks
+          - relay-lint
+          - relay-test
    steps:
      -
        name: Checkout
@@ -118,6 +118,10 @@ cross: ## Compile the CLI for linux, darwin and windows
 test: ## Run unit tests
 	$(BUILDX_CMD) bake test

+.PHONY: relay-test
+relay-test: ## Run unit tests for the relay/ module
+	$(BUILDX_CMD) bake relay-test
+
 .PHONY: cache-clear
 cache-clear: ## Clear the builder cache
 	$(BUILDX_CMD) prune --force --filter type=exec.cachemount --filter=unused-for=24h
@@ -126,6 +130,10 @@ cache-clear: ## Clear the builder cache
 lint: ## run linter(s)
 	$(BUILDX_CMD) bake lint

+.PHONY: relay-lint
+relay-lint: ## run linter(s) for the relay/ module
+	$(BUILDX_CMD) bake relay-lint
+
 .PHONY: fmt
 fmt:
 	gofumpt --version >/dev/null 2>&1 || go install mvdan.cc/gofumpt@latest
@@ -165,7 +173,7 @@ validate-mocks: ## Validate pkg/mocks is up-to-date with the mocked interfaces

 validate: validate-go-mod validate-headers validate-docs validate-mocks  ## Validate sources

-pre-commit: validate check-dependencies lint build test e2e-compose
+pre-commit: validate check-dependencies lint relay-lint build test relay-test e2e-compose

 help: ## Show help
 	@echo Please specify a build target. The choices are:
@@ -53,7 +53,7 @@ group "default" {
 }

 group "validate" {
-  targets = ["lint", "vendor-validate", "license-validate", "mocks-validate"]
+  targets = ["lint", "vendor-validate", "license-validate", "mocks-validate", "relay-lint"]
 }

 target "lint" {
@@ -167,6 +167,22 @@ target "image-module-cross" {
  ]
 }

+// relay-lint and relay-test validate relay/'s own module (a separate go.mod:
+// go vet/lint/test from the repo root don't cover it). Self-contained build
+// context, so relay/ carries its own .golangci.yml rather than sharing the
+// root one, which isn't in scope for a "./relay" context.
+target "relay-lint" {
+  context = "./relay"
+  target  = "lint"
+  output  = ["type=cacheonly"]
+}
+
+target "relay-test" {
+  context = "./relay"
+  target  = "test"
+  output  = ["type=cacheonly"]
+}
+
 // relay-image is the local/dev build of the network relay compose deploys in
 // place of a provider service that published endpoints (see relay/). The tag
 // matches the runtime default (COMPOSE_RELAY_IMAGE overrides it).
@@ -0,0 +1,77 @@
+version: "2"
+run:
+  concurrency: 2
+linters:
+  default: none
+  disable:
+    # gocognit deliberately replaced gocyclo — do not re-enable. Cyclomatic
+    # complexity charges a flat guard clause the same as a nested branch, so
+    # our fail-fast style accumulated //nolint suppressions on functions that
+    # are long but flat. Cognitive complexity penalizes nesting and barely
+    # charges early returns, keeping the signal on genuinely tangled code.
+    - gocyclo
+  enable:
+    - copyloopvar
+    - errcheck
+    - errorlint
+    - forbidigo
+    - gocognit
+    - gocritic
+    - govet
+    - ineffassign
+    - lll
+    - misspell
+    - nakedret
+    - nolintlint
+    - perfsprint
+    - revive
+    - staticcheck
+    - unconvert
+    - unparam
+    - unused
+  settings:
+    forbidigo:
+      analyze-types: true
+      forbid:
+        - pattern: 'context\.Background'
+          pkg: '^context$'
+          msg: "in tests, use t.Context() instead of context.Background()"
+        - pattern: 'context\.TODO'
+          pkg: '^context$'
+          msg: "in tests, use t.Context() instead of context.TODO()"
+    gocritic:
+      disabled-checks:
+        - paramTypeCombine
+        - unnamedResult
+        - whyNoLint
+      enabled-tags:
+        - diagnostic
+        - opinionated
+        - style
+    gocognit:
+      min-complexity: 30
+    lll:
+      line-length: 200
+    revive:
+      rules:
+        - name: package-comments
+          disabled: true
+  exclusions:
+    rules:
+      - path-except: '_test\.go'
+        linters:
+          - forbidigo
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+formatters:
+  enable:
+    - gci
+    - gofumpt
+  settings:
+    gci:
+      sections:
+        - standard
+        - default
+        - localmodule
+      custom-order: true
@@ -16,9 +16,35 @@
 #   limitations under the License.

 ARG GO_VERSION=1.26.8
+ARG GOLANGCI_LINT_VERSION=v2.13.2

-FROM golang:${GO_VERSION}-alpine AS build
+FROM golangci/golangci-lint:${GOLANGCI_LINT_VERSION}-alpine AS golangci-lint
+
+FROM golang:${GO_VERSION}-alpine AS base
 WORKDIR /src
+
+FROM base AS build-base
+COPY go.mod ./
+RUN --mount=type=cache,target=/go/pkg/mod \
+    go mod download
+
+FROM build-base AS lint
+ENV GOLANGCI_LINT_CACHE=/cache/golangci-lint
+RUN --mount=type=bind,target=. \
+    --mount=type=cache,target=/root/.cache \
+    --mount=type=cache,target=/go/pkg/mod \
+    --mount=type=cache,target=/cache/golangci-lint \
+    --mount=from=golangci-lint,source=/usr/bin/golangci-lint,target=/usr/bin/golangci-lint \
+    golangci-lint run ./...
+
+FROM build-base AS test
+RUN --mount=type=bind,target=. \
+    --mount=type=cache,target=/root/.cache \
+    --mount=type=cache,target=/go/pkg/mod \
+    go vet ./... && \
+    go test -v ./...
+
+FROM base AS build
 COPY go.mod main.go ./
 RUN CGO_ENABLED=0 go build -trimpath -ldflags="-w -s" -o /compose-relay .

@@ -46,9 +46,19 @@ import (
 )

 func main() {
+	if err := run(); err != nil {
+		log.Fatal(err)
+	}
+}
+
+// run is separate from main so that a listen failure returns instead of
+// exiting mid-setup: os.Exit from within main would skip the deferred
+// stop(), which is harmless in practice (the process is dying either way)
+// but flagged by the linter, so this shape avoids it for real.
+func run() error {
 	routes, err := parseRoutes(os.Getenv("RELAY_ROUTES"))
 	if err != nil {
-		log.Fatalf("RELAY_ROUTES: %v", err)
+		return fmt.Errorf("RELAY_ROUTES: %w", err)
 	}

 	ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT)
@@ -58,7 +68,7 @@ func main() {
 	for port, upstream := range routes {
 		listener, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
 		if err != nil {
-			log.Fatalf("listen :%d: %v", port, err)
+			return fmt.Errorf("listen :%d: %w", port, err)
 		}
 		log.Printf("relaying :%d -> %s", port, upstream)
 		wg.Add(1)
@@ -72,6 +82,7 @@ func main() {
 		}()
 	}
 	wg.Wait()
+	return nil
 }

 // parseRoutes decodes "port=host:port[,port=host:port...]".
@@ -98,7 +109,7 @@ func parseRoutes(spec string) (map[int]string, error) {
 		}
 		host, hostPort, err := net.SplitHostPort(upstream)
 		if err != nil {
-			return nil, fmt.Errorf("invalid upstream in route %q: %v", entry, err)
+			return nil, fmt.Errorf("invalid upstream in route %q: %w", entry, err)
 		}
 		if hostIsContainerLocal(host) {
 			upstream = net.JoinHostPort("host.docker.internal", hostPort)
@@ -167,7 +178,7 @@ type idleConn struct {

 func (c *idleConn) Read(p []byte) (int, error) {
 	if c.armed.Load() {
-		_ = c.Conn.SetReadDeadline(time.Now().Add(halfCloseIdleTimeout))
+		_ = c.SetReadDeadline(time.Now().Add(halfCloseIdleTimeout))
 	}
 	return c.Conn.Read(p)
 }
@@ -177,14 +188,14 @@ func (c *idleConn) Read(p []byte) (int, error) {
 // be served — that is the drain contract — and a cancelled context would make
 // DialContext fail instantly. The dialer timeout bounds the dial instead.
 func forward(downstream net.Conn, upstream string) {
-	defer downstream.Close()
+	defer func() { _ = downstream.Close() }()
 	dialer := net.Dialer{Timeout: 10 * time.Second}
 	up, err := dialer.Dial("tcp", upstream)
 	if err != nil {
 		log.Printf("dial %s: %v", upstream, err)
 		return
 	}
-	defer up.Close()
+	defer func() { _ = up.Close() }()

 	down := &idleConn{Conn: downstream}
 	upc := &idleConn{Conn: up}
@@ -59,7 +59,7 @@ func tcpPair(t *testing.T) (net.Conn, net.Conn) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer ln.Close()
+	defer func() { _ = ln.Close() }()
 	type accepted struct {
 		conn net.Conn
 		err  error
@@ -94,7 +94,7 @@ func silentUpstream(t *testing.T) net.Listener {
 		if err != nil {
 			return
 		}
-		defer conn.Close()
+		defer func() { _ = conn.Close() }()
 		_, _ = io.Copy(io.Discard, conn) // read the FIN, never answer, never close
 		time.Sleep(5 * time.Second)
 	}()
@@ -150,7 +150,7 @@ func TestForwardKeepsStreamingAfterHalfClose(t *testing.T) {
 		if err != nil {
 			return
 		}
-		defer conn.Close()
+		defer func() { _ = conn.Close() }()
 		// stream chunks for well past the idle grace, each within it
 		for range chunks {
 			time.Sleep(150 * time.Millisecond)