Skip to content

fix(policy): DSPX-4607 clear sloglint and SA1019 lint findings - #3977

Open
dmihalcik-virtru wants to merge 1 commit into
mainfrom
fix/lint-policy-sloglint
Open

fix(policy): DSPX-4607 clear sloglint and SA1019 lint findings#3977
dmihalcik-virtru wants to merge 1 commit into
mainfrom
fix/lint-policy-sloglint

Conversation

@dmihalcik-virtru

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

Copy link
Copy Markdown
Member

Part of the DSPX-4607 burndown of the 351 pre-existing findings that golangci-lint v2.13.2 (#3965) surfaced and v2.8.0 never reported. They don't fail CI today — the lint step runs with only-new-issues: true — but any future PR touching one of these lines would trip on them.

This PR covers the service/policy slice (48 findings). It is independent of the other DSPX-4607 PRs: no file overlap, and it does not touch .golangci.yaml.

Changes

sloglint (28) — operational log keys renamed to snake_case, matching the linter's configured key-naming-case:

package before → after
dynamicvaluemapping dynamicValueMappingdynamic_value_mapping
keymanagement keyManagementServicekey_management_service (×5)
namespaces namespaceKasnamespace_kas, namespaceKeynamespace_key
obligations obligation valueobligation_value, obligation triggerobligation_trigger
registeredresources registered resourceregistered_resource, registered resource valueregistered_resource_value
resourcemapping resourceMappingGroupresource_mapping_group, resourceMappingresource_mapping
subjectmapping subjectMappingsubject_mapping, subjectPropertiessubject_properties, subjectConditionSetsubject_condition_set, subjectConditionSet fieldssubject_condition_set_fields

These are all diagnostic keys — none is an audit event field (those live in service/logger/audit and are deliberately left alone, see the core PR) and no test asserts on any of these strings.

staticcheck SA1019 (20)//nolint:staticcheck with a per-site reason rather than migrating off the deprecated proto fields, so this PR stays a lint-only change:

  • db/grant_mappings.go, db/key_access_server_registry.go — responses still populate the deprecated PublicKey for clients that have not moved to multiple key pairs, plus the compatibility path for the deprecated ListKeyAccessServerGrants RPC.
  • attributes/attributes_test.go, namespaces/namespaces_test.go, kasregistry/key_access_server_registry_test.go, keymanagement/key_management_test.go — these cases exist specifically to validate the deprecated Id / Name identifiers, so they have to keep setting them.

Testing

Related

#3965 (linter bump) · #3968 (config tuning) · #3969 · #3970 · #3971 · #3973 · #3974 · #3975

DSPX-4607 burndown index

@dmihalcik-virtru
dmihalcik-virtru requested review from a team as code owners September 4, 2026 12:59
@coderabbitai

coderabbitai Bot commented Sep 4, 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: 40d2fc40-027b-4286-a327-3f9506756209

📥 Commits

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

📒 Files selected for processing (14)
  • service/policy/attributes/attributes_test.go
  • service/policy/db/attribute_fqn_test.go
  • service/policy/db/grant_mappings.go
  • service/policy/db/key_access_server_registry.go
  • service/policy/dynamicvaluemapping/dynamic_value_mapping.go
  • service/policy/kasregistry/key_access_server_registry_test.go
  • service/policy/keymanagement/key_management.go
  • service/policy/keymanagement/key_management_test.go
  • service/policy/namespaces/namespaces.go
  • service/policy/namespaces/namespaces_test.go
  • service/policy/obligations/obligations.go
  • service/policy/registeredresources/registered_resources.go
  • service/policy/resourcemapping/resource_mapping.go
  • service/policy/subjectmapping/subject_mapping.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 added comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) size/s labels 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 191.015128ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 368.642316ms
Throughput 271.27 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 49.427115643s
Average Latency 493.234621ms
Throughput 101.16 requests/second

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
@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 244.052015ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 414.743189ms
Throughput 241.11 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 58.400012365s
Average Latency 582.006557ms
Throughput 85.62 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

Labels

comp:db DB component comp:policy Policy Configuration ( attributes, subject mappings, resource mappings, kas registry) size/s

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant