Skip to content

chore(core): DSPX-4607 drop stale lib/fixtures nolint directives and reuse keycloakBoolTrue - #3971

Open
dmihalcik-virtru wants to merge 1 commit into
mainfrom
fix/lint-fixtures-nolint
Open

chore(core): DSPX-4607 drop stale lib/fixtures nolint directives and reuse keycloakBoolTrue#3971
dmihalcik-virtru wants to merge 1 commit into
mainfrom
fix/lint-fixtures-nolint

Conversation

@dmihalcik-virtru

@dmihalcik-virtru dmihalcik-virtru commented Sep 3, 2026

Copy link
Copy Markdown
Member

Part of the DSPX-4607 lint burndown, following the golangci-lint v2.13.2 bump (#3965, merged). Branched from main, independent of the other burndown PRs.

What

lib/fixtures has 26 findings, all in keycloak.go:

25 × nolintlint//nolint:sloglint // allow existing emojis directives that no longer suppress anything:

lib/fixtures/keycloak.go:168:3: directive `//nolint:sloglint // allow existing emojis` is unused for linter "sloglint" (nolintlint)

sloglint's msg-style: lowercased no longer objects to messages that begin with an emoji ("✅ realm created", "⏭️ realm already exists"), so every one of these directives is now dead. Deleted rather than kept, since nolintlint is what flags them and a stale directive is worse than none — it silently suppresses future real findings.

1 × goconst — six raw "true" map values duplicating the keycloakBoolTrue constant already declared at line 25. Replaced those six, plus the one gocloak.StringP("true") at line 274 that goconst skips (ignore-calls is on by default) — leaving a lone raw literal next to six converted ones would just be untidy.

Net: 32 deletions, 7 insertions. No behaviour change — the emitted log messages and the Keycloak config values are byte-identical.

Testing

$ cd lib/fixtures && golangci-lint run -c ../../.golangci.yaml
0 issues.

Pre-existing test failures (not from this PR)

go test ./... fails two tests, identically on unmodified main:

--- FAIL: TestTokenManager_InitialLogin
    keycloak_token_manager_test.go:43: Expected default token buffer of 120s, got 30s
--- FAIL: TestTokenManager_CustomTokenBuffer
    keycloak_token_manager_test.go:73: Expected token buffer of 1m0s, got 30s

Verified by jj restore --from main lib/fixtures/keycloak.go and re-running — same two failures. They're in keycloak_token_manager.go, which this PR doesn't touch; the "adjusted token buffer for short token lifetime" clamp is overriding the configured buffer. Out of scope here, but worth a separate look.

DSPX-4607 burndown index

@dmihalcik-virtru
dmihalcik-virtru requested review from a team as code owners September 3, 2026 19:52
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 6b3b8616-68ce-4e3a-9301-19f1fca4d93e

📥 Commits

Reviewing files that changed from the base of the PR and between d5933ed and 485c233.

📒 Files selected for processing (1)
  • lib/fixtures/keycloak.go

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 214.1179ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 130.383676ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 443.228674ms
Throughput 225.62 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 55.202374424s
Average Latency 550.818795ms
Throughput 90.58 requests/second

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 247.222642ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 143.022956ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 431.432547ms
Throughput 231.79 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 1m1.777698747s
Average Latency 616.311433ms
Throughput 80.94 requests/second

jakedoublev
jakedoublev previously approved these changes Sep 3, 2026
…reuse keycloakBoolTrue

golangci-lint v2.13.2 reports 26 findings in lib/fixtures:

  - 25 //nolint:sloglint directives that no longer suppress anything. sloglint
    stopped flagging the emoji log messages they were added for, so nolintlint
    now reports each as unused.
  - 6 raw "true" map values that duplicate the existing keycloakBoolTrue
    constant.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
@dmihalcik-virtru dmihalcik-virtru changed the title chore(fixtures): DSPX-4607 drop stale nolint directives and reuse keycloakBoolTrue chore(core): DSPX-4607 drop stale lib/fixtures nolint directives and reuse keycloakBoolTrue Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 226.893322ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 126.59439ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 458.977658ms
Throughput 217.88 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 55.694042893s
Average Latency 555.5879ms
Throughput 89.78 requests/second

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • otdfctl
  • service
  • tests-bdd

See the workflow run for details.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants