Skip to content

feat(policy): Add sort functionality on key_status - #3845

Merged
elizabethhealy merged 7 commits into
mainfrom
dspx-3798-sort-on-key-status
Aug 12, 2026
Merged

feat(policy): Add sort functionality on key_status#3845
elizabethhealy merged 7 commits into
mainfrom
dspx-3798-sort-on-key-status

Conversation

@elizabethhealy

@elizabethhealy elizabethhealy commented Aug 6, 2026

Copy link
Copy Markdown
Member

Proposed Changes

  • Adds the ability to sort on key status

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

Summary by CodeRabbit

  • New Features

    • Added sorting of KAS keys by key status.
    • Supports ascending and descending alphabetical ordering by status name.
    • Added a new key-status sorting option to the API.
    • Updated API documentation to describe the new option and status ordering.
  • Tests

    • Added coverage validating key-status sorting in ascending and descending order.
    • Verified ordering across active, rotated, and unspecified key statuses.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d90e6a0f-1534-4e1c-aedc-055c35929616

📥 Commits

Reviewing files that changed from the base of the PR and between 6072327 and a817b59.

📒 Files selected for processing (1)
  • service/integration/kas_registry_key_test.go

📝 Walkthrough

Walkthrough

Adds KEY_STATUS sorting for KAS key listings. The change updates the proto and API documentation, exposes the option in otdfctl, maps it to SQL ordering, and adds unit and integration coverage for ascending and descending results.

Changes

KAS key status sorting

Layer / File(s) Summary
Sorting contract and CLI wiring
service/policy/kasregistry/key_access_server_registry.proto, otdfctl/pkg/handlers/kas-keys.go, docs/grpc/index.html, docs/openapi/.../key_access_server_registry.openapi.yaml, docs/openapi/policy/objects.openapi.yaml, docs/openapi/policy/unsafe/unsafe.openapi.yaml, service/policy/objects.proto
Adds SORT_KAS_KEYS_TYPE_KEY_STATUS = 4, accepts key_status as a CLI sort field, and documents the required status mapping.
Database sort mapping and ordering
service/policy/db/utils.go, service/policy/db/queries/key_access_server_registry.sql, service/policy/db/key_access_server_registry.sql.go
Maps the enum to key_status and applies ascending or descending alphabetical ordering.
Sort mapping and integration validation
service/policy/db/utils_test.go, service/integration/kas_registry_key_test.go
Tests enum-to-SQL mapping and verifies ordering across ACTIVE and ROTATED keys.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI as otdfctl
  participant API as KAS key listing API
  participant DB as listKeys SQL query
  CLI->>API: Request key_status sort
  API->>DB: Apply key-status sort enum
  DB-->>API: Return ordered KAS keys
Loading

Possibly related PRs

Suggested labels: comp:db, comp:policy, size/s, docs

Suggested reviewers: c-r33d

Poem

I sort the keys by status bright,
ACTIVE first or ROTATED right.
ASC and DESC now guide the way,
Tests check the order every day.
— A diligent rabbit 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: adding sorting by key_status.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dspx-3798-sort-on-key-status

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 Aug 7, 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 208.312958ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 442.030032ms
Throughput 226.23 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.929160997s
Average Latency 438.531037ms
Throughput 113.82 requests/second

@elizabethhealy
elizabethhealy marked this pull request as ready for review August 7, 2026 14:12
@elizabethhealy
elizabethhealy requested review from a team as code owners August 7, 2026 14:12
@github-actions

github-actions Bot commented Aug 7, 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 213.88764ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 417.164803ms
Throughput 239.71 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.155860312s
Average Latency 431.061326ms
Throughput 115.86 requests/second

@github-actions

github-actions Bot commented Aug 7, 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 231.132451ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 407.646446ms
Throughput 245.31 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.124181241s
Average Latency 429.942923ms
Throughput 115.94 requests/second

Comment thread service/integration/kas_registry_key_test.go
Comment thread service/integration/kas_registry_key_test.go Outdated
Comment thread service/policy/kasregistry/key_access_server_registry.proto Outdated
@github-actions

github-actions Bot commented Aug 7, 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 281.852716ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 672.601957ms
Throughput 148.68 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 42.692762508s
Average Latency 426.224958ms
Throughput 117.12 requests/second

@github-actions

github-actions Bot commented Aug 7, 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 210.642386ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 406.827353ms
Throughput 245.80 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.371413905s
Average Latency 432.92009ms
Throughput 115.28 requests/second

@github-actions

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 192.393469ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 430.135734ms
Throughput 232.48 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 44.729974277s
Average Latency 446.495639ms
Throughput 111.78 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • examples
  • otdfctl
  • sdk
  • service
  • lib/fixtures
  • tests-bdd

See the workflow run for details.

@elizabethhealy
elizabethhealy requested a review from c-r33d August 10, 2026 17:40
@elizabethhealy
elizabethhealy added this pull request to the merge queue Aug 12, 2026
Merged via the queue into main with commit 2ca20ff Aug 12, 2026
45 checks passed
@elizabethhealy
elizabethhealy deleted the dspx-3798-sort-on-key-status branch August 12, 2026 13:51
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.

2 participants