Skip to content

chore(nats-auth-callout): fix golangci-lint findings#231

Open
mesutoezdil wants to merge 2 commits into
NVIDIA:mainfrom
mesutoezdil:mesutoezdil/chore/nats-auth-callout-lint
Open

chore(nats-auth-callout): fix golangci-lint findings#231
mesutoezdil wants to merge 2 commits into
NVIDIA:mainfrom
mesutoezdil:mesutoezdil/chore/nats-auth-callout-lint

Conversation

@mesutoezdil

@mesutoezdil mesutoezdil commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Cleared most golangci-lint findings for nats-auth-callout (50 to 34). Deleted an unused parallel Service/Authorizer/JWTGenerator/TokenValidator implementation in internal/nats/auth_types.go that nothing imports (clears 8 unused findings too). Rest is mechanical: import grouping, one WriteString to Fprintf, stray blank lines, a repeated test string pulled into a const.

Left out of scope: goheader (repo-wide convention question), dupl (real refactor, not lint-driven), one accepted errcheck idiom, one gosec pattern already used elsewhere in the repo, one long doc-comment line.

Tested: go build, go vet, go test ./... pass. golangci-lint fmt --diff empty.

Refs: NO-REF

- Remove internal/nats/auth_types.go: the whole package (Service,
  Authorizer, JWTGenerator, TokenValidator, AuthRequest) was never
  imported anywhere, had no tests, and its fields were never read.
  It predates the internal/service + internal/plugins implementation
  that does the actual work now. This clears all 8 golangci-lint
  "unused" findings at once.
- Fix import grouping (goimports) across 8 files.
- Use fmt.Fprintf instead of WriteString(fmt.Sprintf(...)) in
  internal/config/config.go (staticcheck QF1012).
- Drop two stray blank lines (whitespace).
- Extract the repeated "test-key-1" JWKS key ID into a testKeyID
  const in test_helpers.go (goconst).

Left goheader (pre-existing template/header mismatch across many
files, needs a repo-wide decision), dupl, errcheck on a Sync()-style
call, gosec G114 (matches the bare http.ListenAndServe pattern already
used elsewhere in this repo for internal-only endpoints), and one
long example command in a doc comment out of scope for this pass.

Verified with go build, go vet, go test ./... (all packages pass),
and golangci-lint fmt --diff (empty).

NO-REF

Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
@mesutoezdil
mesutoezdil requested a review from a team as a code owner July 17, 2026 09:27
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The NATS auth callout service adds a Swagger endpoint, centralizes OAuth test JWT key identifiers, updates configuration formatting, reorders imports, and removes the internal NATS auth types and Bazel targets.

Changes

NATS auth callout updates

Layer / File(s) Summary
Swagger route registration
src/control-plane-services/nats-auth-callout/internal/router/router.go
setupRoutes() registers GET /swagger/*any with the Swagger handler.
OAuth test key consistency
src/control-plane-services/nats-auth-callout/internal/plugins/oauth/test_helpers/test_helpers.go, src/control-plane-services/nats-auth-callout/internal/plugins/oauth/oauth_test.go
A shared testKeyID is used by the mock JWKS response and generated JWT headers.
Configuration and source cleanup
src/control-plane-services/nats-auth-callout/internal/config/config.go, src/control-plane-services/nats-auth-callout/cmd/.../cli/*, src/control-plane-services/nats-auth-callout/internal/plugins/nkey/nkey_test.go, src/control-plane-services/nats-auth-callout/internal/service/test_e2e/*
Configuration output is written directly to the builder, and Go imports are reordered.
NATS auth type removal
src/control-plane-services/nats-auth-callout/internal/nats/auth_types.go, src/control-plane-services/nats-auth-callout/internal/nats/BUILD.bazel
The internal NATS auth data types, service constants, Bazel library target, and alias are removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: resolving golangci-lint findings in nats-auth-callout.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant