feat(cli): Sort parameters.#3478
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds ChangesPolicy List Command Sorting Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a standardized sorting mechanism for otdfctl policy list commands. By implementing a new --sort flag, users gain the ability to organize output by relevant fields in either ascending or descending order. The changes include a robust shared parsing utility, validation logic, updated manual pages, and extensive end-to-end test coverage to ensure consistent and reliable sorting across the entire policy suite. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. A CLI tool that needed to sort, With a flag that was quite the support. "Field:direction" you type, With no need for the hype, Now the lists are all tidy, in short. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@otdfctl/docs/man/policy/subject-condition-sets/list.md`:
- Around line 50-56: The examples at the bottom use the wrong command path
(singular) — update the Example block to use the correct plural command name
subject-condition-sets; replace occurrences of `otdfctl policy
subject-condition-set list` with `otdfctl policy subject-condition-sets list`
(and likewise for the namespaced variant) so the examples match the actual
command `subject-condition-sets`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 86886afb-e764-48d3-87b9-fc33c760632b
📒 Files selected for processing (40)
otdfctl/cmd/policy/attributes.gootdfctl/cmd/policy/kasKeys.gootdfctl/cmd/policy/kasRegistry.gootdfctl/cmd/policy/namespaces.gootdfctl/cmd/policy/obligations.gootdfctl/cmd/policy/policy.gootdfctl/cmd/policy/registeredResources.gootdfctl/cmd/policy/subjectConditionSets.gootdfctl/cmd/policy/subjectMappings.gootdfctl/docs/man/policy/attributes/list.mdotdfctl/docs/man/policy/kas-registry/key/list.mdotdfctl/docs/man/policy/kas-registry/list.mdotdfctl/docs/man/policy/namespaces/list.mdotdfctl/docs/man/policy/obligations/list.mdotdfctl/docs/man/policy/registered-resources/list.mdotdfctl/docs/man/policy/subject-condition-sets/list.mdotdfctl/docs/man/policy/subject-mappings/list.mdotdfctl/e2e/attributes.batsotdfctl/e2e/kas-keys.batsotdfctl/e2e/kas-registry.batsotdfctl/e2e/namespaces.batsotdfctl/e2e/obligations.batsotdfctl/e2e/registered-resources.batsotdfctl/e2e/subject-condition-sets.batsotdfctl/e2e/subject-mapping.batsotdfctl/migrations/namespacedpolicy/planner.gootdfctl/migrations/namespacedpolicy/planner_test.gootdfctl/migrations/namespacedpolicy/retrieve.gootdfctl/migrations/namespacedpolicy/retrieve_test.gootdfctl/pkg/handlers/attribute.gootdfctl/pkg/handlers/kas-keys.gootdfctl/pkg/handlers/kas-registry.gootdfctl/pkg/handlers/namespaces.gootdfctl/pkg/handlers/obligations.gootdfctl/pkg/handlers/registeredResources.gootdfctl/pkg/handlers/sort.gootdfctl/pkg/handlers/sort_test.gootdfctl/pkg/handlers/subjectConditionSets.gootdfctl/pkg/handlers/subjectmappings.gootdfctl/tui/attributeList.go
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces sorting functionality across multiple resource list commands in otdfctl, including attributes, namespaces, and KAS keys. It implements a new SortOption handler to parse field:direction strings and updates the CLI, documentation, and end-to-end tests accordingly. Feedback suggests enhancing the sort string parser to allow field names without a colon for better usability and refactoring the repetitive sort-mapping logic across different handlers into a shared helper function to improve maintainability.
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
jakedoublev
reviewed
May 14, 2026
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
jakedoublev
reviewed
May 14, 2026
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@otdfctl/e2e/registered-resources.bats`:
- Around line 196-201: The test's ordering assertions are flaky because the
three successive run_otdfctl_reg_res create calls (which populate rr_a_id,
rr_b_id, rr_c_id) can get identical timestamps; modify the test to ensure
distinct write times by adding a short pause between creations (e.g., sleep 1)
or, if the CLI/API supports it, supply distinct explicit timestamps/created_at
values when calling run_otdfctl_reg_res create so created_at/updated_at are
guaranteed different; apply the same change to the other similar blocks that
create/update resources (the blocks referenced around rr_* assignments at the
other noted ranges).
In `@otdfctl/e2e/subject-condition-sets.bats`:
- Around line 142-145: Tests assert strict ordering by created_at/updated_at but
writes can share timestamps causing flakes; update the test to avoid relying on
timestamp tie-breaking by using the created scs ids (scs_a_id, scs_b_id,
scs_c_id) as a deterministic tie-breaker or by sorting API results by id before
asserting order (or assert set membership rather than strict sequence). Locate
the assertions that compare created_at/updated_at ordering in
subject-condition-sets.bats and change them to either sort the returned items by
id (or use the captured scs_*_id values to order expected results) or relax the
timestamp check to allow equal timestamps instead of failing on ties; apply the
same change to the other affected assertion blocks referenced around the scs
creation lines.
In `@otdfctl/e2e/subject-mapping.bats`:
- Around line 178-180: The subject-mapping test is flaky because multiple sm
create calls (the commands that set sm_a_id, sm_b_id, sm_c_id using ./otdfctl
and sort_val_a_id/ sort_val_b_id/ sort_val_c_id) run so fast that
created_at/updated_at can collide; fix by making the creates produce
deterministic ordering — either inject a short pause between each create (e.g.,
add a sleep between the ./otdfctl policy sm create invocations) or, better, rely
on the explicit unique sort values (sort_val_a_id, sort_val_b_id, sort_val_c_id)
in your assertions instead of created_at/updated_at so the test asserts on those
stable keys rather than timestamps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a7745ed7-d903-4bf5-bf49-0516dd797761
📒 Files selected for processing (35)
otdfctl/cmd/policy/attributes.gootdfctl/cmd/policy/kasKeys.gootdfctl/cmd/policy/kasRegistry.gootdfctl/cmd/policy/namespaces.gootdfctl/cmd/policy/obligations.gootdfctl/cmd/policy/policy.gootdfctl/cmd/policy/registeredResources.gootdfctl/cmd/policy/subjectConditionSets.gootdfctl/cmd/policy/subjectMappings.gootdfctl/docs/man/policy/attributes/list.mdotdfctl/docs/man/policy/kas-registry/key/list.mdotdfctl/docs/man/policy/kas-registry/list.mdotdfctl/docs/man/policy/namespaces/list.mdotdfctl/docs/man/policy/obligations/list.mdotdfctl/docs/man/policy/registered-resources/list.mdotdfctl/docs/man/policy/subject-condition-sets/list.mdotdfctl/docs/man/policy/subject-mappings/list.mdotdfctl/e2e/attributes.batsotdfctl/e2e/kas-keys.batsotdfctl/e2e/kas-registry.batsotdfctl/e2e/namespaces.batsotdfctl/e2e/obligations.batsotdfctl/e2e/registered-resources.batsotdfctl/e2e/subject-condition-sets.batsotdfctl/e2e/subject-mapping.batsotdfctl/pkg/handlers/attribute.gootdfctl/pkg/handlers/kas-keys.gootdfctl/pkg/handlers/kas-registry.gootdfctl/pkg/handlers/namespaces.gootdfctl/pkg/handlers/obligations.gootdfctl/pkg/handlers/registeredResources.gootdfctl/pkg/handlers/sort.gootdfctl/pkg/handlers/sort_test.gootdfctl/pkg/handlers/subjectConditionSets.gootdfctl/pkg/handlers/subjectmappings.go
Contributor
|
Contributor
jakedoublev
approved these changes
May 15, 2026
JBCongdon
pushed a commit
to JBCongdon/platform
that referenced
this pull request
May 24, 2026
🤖 I have created a release *beep* *boop* --- ## [0.32.0](opentdf/platform@otdfctl/v0.31.0...otdfctl/v0.32.0) (2026-05-19) ### Features * **cli:** Add better unit testing. ([opentdf#3378](opentdf#3378)) ([3ad33dc](opentdf@3ad33dc)) * **cli:** Add interactive review for prune plans ([opentdf#3421](opentdf#3421)) ([c11680b](opentdf@c11680b)) * **cli:** Add prune confirmation. ([opentdf#3469](opentdf#3469)) ([c6d47ec](opentdf@c6d47ec)) * **cli:** Add prune planner. ([opentdf#3411](opentdf#3411)) ([3e294e6](opentdf@3e294e6)) * **cli:** Add prune summary information ([opentdf#3456](opentdf#3456)) ([c900c53](opentdf@c900c53)) * **cli:** add sensitive flag annotation to DocFlag ([opentdf#3457](opentdf#3457)) ([98f48d2](opentdf@98f48d2)) * **cli:** Confirm and execute pruning of legacy objects ([opentdf#3458](opentdf#3458)) ([24c09dd](opentdf@24c09dd)) * **cli:** Print report on failure ([opentdf#3365](opentdf#3365)) ([05a4473](opentdf@05a4473)) * **cli:** Sort parameters. ([opentdf#3478](opentdf#3478)) ([73ad878](opentdf@73ad878)) * **policy:** Add FQN to RegisteredResourceValues ([opentdf#3446](opentdf#3446)) ([3199583](opentdf@3199583)) * **policy:** Add resource mapping group FQNs ([opentdf#3447](opentdf#3447)) ([6a0b3c6](opentdf@6a0b3c6)) ### Bug Fixes * **cli:** Prune was not classifying multi-namespaced RRs properly. ([opentdf#3488](opentdf#3488)) ([eae8645](opentdf@eae8645)) * **cli:** support json profile output ([opentdf#3448](opentdf#3448)) ([61f194c](opentdf@61f194c)) * **deps:** bump github.com/opentdf/platform/lib/identifier from 0.3.0 to 0.4.0 in /otdfctl ([opentdf#3367](opentdf#3367)) ([aa23179](opentdf@aa23179)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.27.0 to 0.28.0 in /otdfctl ([opentdf#3419](opentdf#3419)) ([c80374f](opentdf@c80374f)) * **deps:** bump github.com/opentdf/platform/sdk from 0.16.0 to 0.17.0 in /otdfctl ([opentdf#3397](opentdf#3397)) ([bb9fcd6](opentdf@bb9fcd6)) * **deps:** bump go.opentelemetry.io/otel from 1.40.0 to 1.41.0 in /otdfctl ([opentdf#3400](opentdf#3400)) ([5631c37](opentdf@5631c37)) * **deps:** bump module protocol/go to v0.30.0 throughout ([opentdf#3459](opentdf#3459)) ([8eaa502](opentdf@8eaa502)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com> Co-authored-by: Chris Reed <87077975+c-r33d@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--sortsupport to otdfctl policy list commands for namespaces, attributes, KAS registry entries, KAS keys, obligations, registered resources, subject condition sets, and subject mappings.<field>:<direction>syntax, including partial forms likename:and:asc.Testing
go test ./pkg/handlers ./cmd/policygo test ./...golangci-lint run ./pkg/handlers ./cmd/policySummary by CodeRabbit
New Features
Documentation
Tests