Skip to content

fix(ci): golangci-lint + feat(bench): 500k-request harness & report#2

Merged
dedeez14 merged 4 commits intomainfrom
devin/1777057948-benchmark-ci-fix
Apr 25, 2026
Merged

fix(ci): golangci-lint + feat(bench): 500k-request harness & report#2
dedeez14 merged 4 commits intomainfrom
devin/1777057948-benchmark-ci-fix

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Apr 24, 2026

Summary

Follow-up to #1. Two things:

1. Fix the failing golangci-lint check on main.

Root cause: go.mod was pinned to go 1.25, but golangci-lint v1.61.0 (the pinned CI version) is built against go1.23 and refuses to analyse modules targeting a newer language version. That forced a cascade: every transitive dep had also drifted to a go1.25-required release.

  • Pin go.modgo 1.23, drop the toolchain directive so the module stays buildable with Go 1.23 too.
  • Pin direct deps to their last Go-1.23-compatible minor: pgx v5.7.2, validator v10.22.1, zerolog v1.33.0, viper v1.19.0, jwt/v5 v5.2.1, x/crypto v0.31.0, x/text v0.21.0, x/sync v0.10.0.
  • Switch two .(*T) type assertions on error to errors.As so errorlint stops flagging them:
    • internal/infrastructure/server/server.govar fe *fiber.Error; errors.As(err, &fe)
    • pkg/validatorx/validator.goerrors.As(err, &validator.ValidationErrors)
  • Run gofmt -s -w and goimports -local github.com/dedeez14/goforge -w over the tree to satisfy gofmt/goimports linters.
  • Drop the unused *inMemoryUserRepo return from auth_test.newAuthFixture (unparam).
  • Dockerfile: base image switched to golang:1.23-alpine to match go.mod.

2. Expose Argon2id cost via config + ship a 500 000-request benchmark harness + publish the report.

Argon2id parameters are now read from config.Security.ArgonMemoryKiB / ArgonIters / ArgonParallel (with OWASP-2023 defaults 65 536 / 3 / 2). This allows lowering the hash cost for load-testing profiles without touching code — the feature is useful in its own right (e.g. tuning cost up over time or lowering it for integration tests).

cmd/bench/main.go is a Go harness that:

  • fires -total=500000 requests across -concurrency=N goroutines,
  • generates a distinct payload per request (unique email/name on register, individual tokens on /me, etc.),
  • writes successful register tokens to a fixtures file so login/me can replay them,
  • reports throughput, p50/p90/p95/p99/max, status-code breakdown, and a client heap snapshot.

Ran all four scenarios × 500 000 requests on the same 2 vCPU / 8 GiB VM (client, API, and Postgres all co-located):

scenario duration rps p50 p95 p99 success
GET /healthz 27.4 s 18 276 12 ms 31 ms 43 ms 500 000 × 200
POST /register 13 m 1 s 640 66 ms 155 ms 230 ms 500 000 × 201
POST /login 9 m 45 s 855 46 ms 137 ms 194 ms 500 000 × 200
GET /me 3 m 5 s 2 708 45 ms 85 ms 112 ms 500 000 × 200
  • API process RSS stayed at 33 MiB after serving 2 000 000 requests (zerolog + fasthttp + pgxpool all keep allocations low).
  • Zero transport errors, zero 5xx, 100 % success across all 2 M requests.

Full report: docs/benchmark-500k.md.

Review & Testing Checklist for Human

  • Confirm the Go downgrade to go 1.23 is acceptable — it widens the audience of Go toolchains that can build the module without changing runtime behaviour, but it does pin some deps to older minors.
  • Re-run the four benchmark scenarios on your own hardware if you want numbers for your deployment shape: make up then the four /tmp/bench -scenario=… commands from docs/benchmark-500k.md.
  • Check that make test && golangci-lint run ./... is clean locally. CI should now be all green on main.

Notes

  • The benchmark was deliberately run with reduced Argon2id cost (mem=1024, t=1, p=1) so the run fits in a reasonable wall-clock window; production defaults remain OWASP-recommended and unchanged. The report explicitly calls this out and explains why register/login p50 scales with Argon2 cost, not framework overhead.
  • No changes to the public HTTP contract. All added code is additive (cmd/bench) or config-surface (security.argon_*).

Link to Devin session: https://app.devin.ai/sessions/8fdfc20358514c97a766adca630a2527
Requested by: @dedeez14


Open in Devin Review

- Downgrade go.mod directive to go 1.23 so golangci-lint v1.61.0 can parse
- Pin pgx/v5, validator, zerolog, viper, jwt, x/crypto to 1.23-compatible
  minor versions (behavior-identical for our use)
- Switch fiber.Error type-assert to errors.As (errorlint)
- Switch validator.ValidationErrors type-assert to errors.As (errorlint)
- Run gofmt -s + goimports with -local github.com/dedeez14/goforge
- Simplify auth_test helper to not return unused repo value (unparam)
- Dockerfile: use golang:1.23-alpine
- Add cmd/bench with goroutine-based harness supporting
  healthz/register/login/login-refresh/me scenarios.
  Each scenario issues total=500_000 unique requests by default,
  reports rps, p50/p90/p95/p99/max and status breakdown.
- Expose Argon2id parameters via config (security.argon_*) so
  load-test profiles can lower the cost without touching code.
- Add docs/benchmark-500k.md with full run results:
    healthz  : 18 276 rps (p50=12ms)   — 500 000 × 200
    register :    640 rps (p50=66ms)   — 500 000 × 201
    login    :    855 rps (p50=46ms)   — 500 000 × 200
    me       :  2 708 rps (p50=45ms)   — 500 000 × 200
  All on 2 vCPU/8 GiB VM, API RSS stayed at 33 MiB.
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

…urity docs + 200k benchmark

- README rewritten with clear quickstart, architecture diagram, 200k bench table, HTTP reference, configuration table, scaffolding, production checklist, doc index
- Split detailed reference into docs/{architecture,configuration,scaffolding,security}.md
- Renamed docs/benchmark-500k.md to docs/benchmark.md and added the 200k smoke run alongside the 500k run with a unified methodology section

Co-Authored-By: dede febriansyah <febriansyahd65@gmail.com>
Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 6 additional findings in Devin Review.

Open in Devin Review

Comment thread internal/config/config.go
Comment on lines +74 to +76
ArgonMemoryKiB uint32 `mapstructure:"argon_memory_kib"`
ArgonIters uint32 `mapstructure:"argon_iters"`
ArgonParallel uint8 `mapstructure:"argon_parallel"`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Missing validation on Argon2id config allows zero iterations/parallelism, causing runtime panic

The new config fields ArgonIters and ArgonParallel (internal/config/config.go:74-76) have no minimum-value validation. If a user sets GOFORGE_SECURITY_ARGON_ITERS=0 or GOFORGE_SECURITY_ARGON_PARALLEL=0, these zero values pass through to argon2.IDKey at internal/infrastructure/security/hasher.go:60, which panics with "argon2: number of rounds too small" or "argon2: parallelism degree too low" respectively. The NewPasswordHasher guard at internal/infrastructure/security/hasher.go:49 only falls back to defaults when Memory == 0, so a non-zero memory with zero iterations/parallelism bypasses the safety net. The panic is caught by the recover middleware, but every register/login call returns a 500 — authentication is completely broken. Other config fields like JWT.Secret and HTTP.Port already carry validate:"min=..." tags that are checked at startup; these new fields should follow the same pattern.

Suggested change
ArgonMemoryKiB uint32 `mapstructure:"argon_memory_kib"`
ArgonIters uint32 `mapstructure:"argon_iters"`
ArgonParallel uint8 `mapstructure:"argon_parallel"`
ArgonMemoryKiB uint32 `mapstructure:"argon_memory_kib" validate:"min=1"`
ArgonIters uint32 `mapstructure:"argon_iters" validate:"min=1"`
ArgonParallel uint8 `mapstructure:"argon_parallel" validate:"min=1"`
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner

@dedeez14 dedeez14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bagus

@dedeez14 dedeez14 merged commit 1b0e67e into main Apr 25, 2026
3 checks passed
@devin-ai-integration devin-ai-integration Bot deleted the devin/1777057948-benchmark-ci-fix branch April 25, 2026 02:29
dedeez14 added a commit that referenced this pull request Apr 25, 2026
feat(forge): rbac sync — auto-register permission codes from RequirePermission scans (Tier-A #2)
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