From 7c2c393d745659bcf4748b04ca78f2a9868fbd49 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Wed, 16 Sep 2026 20:38:08 +0200 Subject: [PATCH] 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 --- .github/workflows/ci.yml | 2 ++ Makefile | 10 +++++- docker-bake.hcl | 18 +++++++++- relay/.golangci.yml | 77 ++++++++++++++++++++++++++++++++++++++++ relay/Dockerfile | 28 ++++++++++++++- relay/main.go | 23 ++++++++---- relay/main_test.go | 6 ++-- 7 files changed, 152 insertions(+), 12 deletions(-) create mode 100644 relay/.golangci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5ec6c1d55..489f3f9ab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,6 +33,8 @@ jobs: - validate-headers - validate-docs - validate-mocks + - relay-lint + - relay-test steps: - name: Checkout diff --git a/Makefile b/Makefile index db04afa0a..539843955 100644 --- a/Makefile +++ b/Makefile @@ -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: diff --git a/docker-bake.hcl b/docker-bake.hcl index 04aa7a3c5..733df6e5f 100644 --- a/docker-bake.hcl +++ b/docker-bake.hcl @@ -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). diff --git a/relay/.golangci.yml b/relay/.golangci.yml new file mode 100644 index 000000000..4554a187e --- /dev/null +++ b/relay/.golangci.yml @@ -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 diff --git a/relay/Dockerfile b/relay/Dockerfile index a878279d0..3827271f1 100644 --- a/relay/Dockerfile +++ b/relay/Dockerfile @@ -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 . diff --git a/relay/main.go b/relay/main.go index 8abfc1f41..686234b32 100644 --- a/relay/main.go +++ b/relay/main.go @@ -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} diff --git a/relay/main_test.go b/relay/main_test.go index e8c0f4156..c959e0fc7 100644 --- a/relay/main_test.go +++ b/relay/main_test.go @@ -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)