diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 3799e33..c5d4851 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -27,9 +27,10 @@ jobs: release: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 + persist-credentials: false # If the event that triggered the build was an annotated tag (which our # tags are supposed to be), actions/checkout has a bug where the tag in @@ -40,15 +41,16 @@ jobs: run: git fetch --tags --force - name: Docker Login - uses: docker/login-action@v2 + uses: docker/login-action@5e57cd118135c172c3672efd75eb46360885c0ef # v3.6.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - - uses: actions/setup-go@v3 + - uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6.1.0 with: - go-version: "~1.20" + go-version: "~1.23" + cache: false - name: Build tunneld and Docker images id: build @@ -65,14 +67,16 @@ jobs: exit 1 fi - echo "docker_tag=${image_tag}" >> $GITHUB_OUTPUT + echo "docker_tag=${image_tag}" >> "$GITHUB_OUTPUT" - name: Push Docker image if: ${{ !github.event.inputs.dry_run && !github.event.inputs.snapshot }} + env: + DOCKER_TAG: ${{ steps.build.outputs.docker_tag }} run: | set -euxo pipefail - image_tag="${{ steps.build.outputs.docker_tag }}" + image_tag="$DOCKER_TAG" docker push "$image_tag" latest_tag="ghcr.io/coder/wgtunnel/tunneld:latest" @@ -84,7 +88,7 @@ jobs: - name: Publish release if: ${{ !github.event.inputs.dry_run && !github.event.inputs.snapshot }} - uses: ncipollo/release-action@v1 + uses: ncipollo/release-action@b7eabc95ff50cbeeedec83973935c8f306dfcd0b # v1.20.0 with: artifacts: "build/tunneld" body: "Docker image: `${{ steps.build.outputs.docker_tag }}`" @@ -92,7 +96,7 @@ jobs: - name: Upload artifacts to actions (if dry-run or snapshot) if: ${{ github.event.inputs.dry_run || github.event.inputs.snapshot }} - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: release-artifacts path: | diff --git a/.github/workflows/wgtunnel.yaml b/.github/workflows/wgtunnel.yaml index c1406dc..bc2df77 100644 --- a/.github/workflows/wgtunnel.yaml +++ b/.github/workflows/wgtunnel.yaml @@ -30,11 +30,13 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false - name: Setup Go - uses: actions/setup-go@v3 + uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6.1.0 with: - go-version: "~1.20" + go-version: "~1.23" - name: Check for unstaged files run: ./scripts/check_unstaged.sh @@ -42,31 +44,33 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false - name: Setup Go - uses: actions/setup-go@v3 + uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6.1.0 with: - go-version: "~1.20" + go-version: "~1.23" - name: golangci-lint - uses: golangci/golangci-lint-action@v3.2.0 - with: - version: v1.51.0 + run: go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8 run test: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false - name: Setup Go - uses: actions/setup-go@v3 + uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6.1.0 with: - go-version: "~1.20" + go-version: "~1.23" - name: Install gotestsum - uses: jaxxstorm/action-install-gh-release@v1.7.1 + uses: jaxxstorm/action-install-gh-release@6096f2a2bbfee498ced520b6922ac2c06e990ed2 # v2.1.0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: repo: gotestyourself/gotestsum - tag: v1.9.0 + tag: v1.12.1 - name: Test run: make test diff --git a/.golangci.yaml b/.golangci.yaml index 20bdc5e..fa70ea5 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,11 +1,14 @@ -# This is copied from github.com/coder/coder. -# -# Changes: -# - removed ruleguard +# See https://golangci-lint.run/usage/configuration/ +# Over time we should try tightening some of these. linters-settings: + dupl: + # goal: 100 + threshold: 412 + + gocognit: - min-complexity: 46 # Min code complexity (def 30). + min-complexity: 300 goconst: min-len: 4 # Min length of string consts (def 3). @@ -15,30 +18,19 @@ linters-settings: enabled-checks: # - appendAssign # - appendCombine - - argOrder # - assignOp # - badCall - - badCond - badLock - badRegexp - boolExprSimplify # - builtinShadow - builtinShadowDecl - - captLocal - - caseOrder - - codegenComment # - commentedOutCode - commentedOutImport - - commentFormatting - - defaultCaseOrder - deferUnlambda # - deprecatedComment # - docStub - - dupArg - - dupBranchBody - - dupCase - dupImport - - dupSubExpr # - elseif - emptyFallthrough # - emptyStringTest @@ -47,8 +39,6 @@ linters-settings: # - exitAfterDefer # - exposedSyncMutex # - filepathJoin - - flagDeref - - flagName - hexLiteral # - httpNoBody # - hugeParam @@ -56,53 +46,42 @@ linters-settings: # - importShadow - indexAlloc - initClause - - ioutilDeprecated - - mapKey - methodExprCall # - nestingReduce - - newDeref - nilValReturn # - octalLiteral - - offBy1 # - paramTypeCombine # - preferStringWriter # - preferWriteByte # - ptrToRefParam # - rangeExprCopy # - rangeValCopy - - regexpMust - regexpPattern # - regexpSimplify # - ruleguard - - singleCaseSwitch - - sloppyLen # - sloppyReassign - - sloppyTypeAssert - sortSlice - sprintfQuotedString - sqlQuery # - stringConcatSimplify # - stringXbytes # - suspiciousSorting - - switchTrue - truncateCmp - typeAssertChain # - typeDefFirst - - typeSwitchVar # - typeUnparen - - underef # - unlabelStmt # - unlambda # - unnamedResult # - unnecessaryBlock # - unnecessaryDefer # - unslice - - valSwap - weakCond # - whyNoLint # - wrapperFunc # - yodaStyleExpr + staticcheck: # https://staticcheck.io/docs/options#checks # We disable SA1019 because it gets angry about our usage of xerrors. We @@ -113,17 +92,17 @@ linters-settings: goimports: local-prefixes: coder.com,cdr.dev,go.coder.com,github.com/cdr,github.com/coder - gocyclo: - min-complexity: 50 - importas: no-unaliased: true misspell: locale: US + ignore-words: + - trialer nestif: - min-complexity: 4 # Min complexity of if statements (def 5, goal 4) + # goal: 10 + min-complexity: 20 revive: # see https://github.com/mgechev/revive#available-rules for details. @@ -163,8 +142,6 @@ linters-settings: - name: modifies-value-receiver - name: package-comments - name: range - - name: range-val-address - - name: range-val-in-closure - name: receiver-naming - name: redefines-builtin-id - name: string-of-int @@ -178,30 +155,58 @@ linters-settings: - name: unnecessary-stmt - name: unreachable-code - name: unused-parameter + exclude: "**/*_test.go" - name: unused-receiver - name: var-declaration - name: var-naming - name: waitgroup-by-value + usetesting: + # Only os-setenv is enabled because we migrated to usetesting from another linter that + # only covered os-setenv. + os-setenv: true + os-create-temp: false + os-mkdir-temp: false + os-temp-dir: false + os-chdir: false + context-background: false + context-todo: false + + # irrelevant as of Go v1.22: https://go.dev/blog/loopvar-preview + govet: + disable: + - loopclosure + gosec: + excludes: + # Implicit memory aliasing of items from a range statement (irrelevant as of Go v1.22) + - G601 issues: + exclude-dirs: + - node_modules + - .git + + exclude-files: + - scripts/rules.go + # Rules listed here: https://github.com/securego/gosec#available-rules exclude-rules: - path: _test\.go linters: # We use assertions rather than explicitly checking errors in tests - errcheck + - forcetypeassert + # - exhaustruct # This is unhelpful in tests. + + - path: scripts/rules.go + linters: + - ALL fix: true max-issues-per-linter: 0 max-same-issues: 0 run: - concurrency: 4 - skip-dirs: - - node_modules - skip-files: - - scripts/rules.go - timeout: 5m + timeout: 10m # Over time, add more and more linters from # https://golangci-lint.run/usage/linters/ as the code improves. @@ -215,10 +220,15 @@ linters: - errcheck - errname - errorlint - - exportloopref + # - exhaustruct - forcetypeassert - gocritic - - gocyclo + # gocyclo is may be useful in the future when we start caring + # about testing complexity, but for the time being we should + # create a good culture around cognitive complexity. + # - gocyclo + - gocognit + - nestif - goimports - gomodguard - gosec @@ -241,7 +251,6 @@ linters: # - wastedassign - staticcheck - - tenv # In Go, it's possible for a package to test it's internal functionality # without testing any exported functions. This is enabled to promote # decomposing a package before testing it's internals. A function caller @@ -254,3 +263,5 @@ linters: - typecheck - unconvert - unused + - usetesting + - dupl diff --git a/cmd/tunneld/main.go b/cmd/tunneld/main.go index f179069..526d04b 100644 --- a/cmd/tunneld/main.go +++ b/cmd/tunneld/main.go @@ -249,7 +249,7 @@ func runApp(ctx *cli.Context) error { options := &tunneld.Options{ BaseURL: baseURLParsed, WireguardEndpoint: wireguardEndpoint, - WireguardPort: uint16(wireguardPort), + WireguardPort: uint16(wireguardPort), //nolint:gosec // validated earlier WireguardKey: wireguardKeyParsed, WireguardMTU: wireguardMTU, WireguardServerIP: wireguardServerIPParsed, diff --git a/tunneld/api.go b/tunneld/api.go index dd1e49c..fd7300d 100644 --- a/tunneld/api.go +++ b/tunneld/api.go @@ -54,7 +54,7 @@ func (api *API) Router() http.Handler { }), ) - apiRouter.Get("/", func(w http.ResponseWriter, r *http.Request) { + apiRouter.Get("/", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte("https://coder.com")) }) @@ -254,7 +254,7 @@ func (api *API) handleTunnel(rw http.ResponseWriter, r *http.Request) { rp := httputil.ReverseProxy{ // This can only happen when it fails to dial. - ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) { + ErrorHandler: func(_ http.ResponseWriter, _ *http.Request, err error) { httpapi.Write(ctx, rw, http.StatusBadGateway, tunnelsdk.Response{ Message: "Failed to dial peer.", Detail: err.Error(), diff --git a/tunneld/options.go b/tunneld/options.go index e67ac20..579c4eb 100644 --- a/tunneld/options.go +++ b/tunneld/options.go @@ -243,6 +243,6 @@ func (options *Options) HostnameToWireguardIP(hostname string) (netip.Addr, erro } addrBytes := options.WireguardNetworkPrefix.Addr().As16() - copy(addrBytes[8:], addrLast8Bytes[:]) + copy(addrBytes[8:], addrLast8Bytes) return netip.AddrFrom16(addrBytes), nil } diff --git a/tunneld/tunneld.go b/tunneld/tunneld.go index 7a4e2cb..6b1560a 100644 --- a/tunneld/tunneld.go +++ b/tunneld/tunneld.go @@ -86,7 +86,7 @@ listen_port=%d`, wgDevice: dev, pkeyCache: make(map[netip.Addr]cachedPeer), transport: &http.Transport{ - DialContext: func(ctx context.Context, network, addr string) (nc net.Conn, err error) { + DialContext: func(ctx context.Context, _, _ string) (nc net.Conn, err error) { ctx, span := otel.GetTracerProvider().Tracer("").Start(ctx, "(http.Transport).DialContext") defer span.End() defer func() {