feat(cli): add dynamic value mapping CRUD commands and e2e tests - #3768
Conversation
Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds SDK-backed handlers and a Cobra command group for creating, retrieving, listing, updating, and deleting dynamic value mappings, with documentation and end-to-end coverage for validation and lifecycle behavior. ChangesDynamic Value Mapping CLI
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CobraCommands
participant Handler
participant DynamicValueMappingSDK
User->>CobraCommands: invoke dynamic value mapping command
CobraCommands->>Handler: validate inputs and call operation
Handler->>DynamicValueMappingSDK: send CRUD request
DynamicValueMappingSDK-->>Handler: return mapping response
Handler-->>CobraCommands: return mapping data
CobraCommands-->>User: render table or JSON
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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: 6
🤖 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/cmd/policy/dynamicValueMappings.go`:
- Around line 145-166: Update the validation block in the command handler to
reject requests when both existingSCSID and newScsJSON are provided, using the
same mutual-exclusivity pattern and cli.ExitWithError as the
attribute-definition checks. Preserve requests where either value is provided
alone or both are omitted.
In `@otdfctl/docs/man/policy/dynamic-value-mappings/create.md`:
- Around line 59-60: Insert a blank line immediately before each of the three
opening fenced code examples in the dynamic value mappings documentation,
including the examples near the referenced sections, to satisfy Markdownlint
MD031.
- Around line 24-25: Update the --action option description in the create
dynamic value mapping documentation to state that at least one action is
required. Keep the existing action examples and align the wording with the
validation enforced by policyCreateDynamicValueMapping.
In `@otdfctl/e2e/dynamic-value-mapping.bats`:
- Line 128: Update the dynamic value mapping create invocation in the affected
test to use the singular dynamic-value-mapping command alias instead of dvm,
while preserving all existing arguments and assertions so the advertised alias
is exercised.
- Around line 223-229: Extend the “Delete dynamic value mapping” test after the
existing successful delete assertions by calling run_otdfctl_dvm get --id
"$to_delete" and asserting failure. Keep the current deletion-success and ID
checks, and verify the deleted mapping is no longer retrievable.
- Around line 11-19: Update every fixture-provisioning command in the test,
including the namespace and dynamic-value mapping setup sites, to run the
otdfctl command separately from jq. Use run and assert_success before parsing
output, then extract the returned ID and validate it is non-empty before
continuing, so failed CLI commands cannot be masked by jq or produce misleading
assertions.
🪄 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: 3caca32b-e590-45b2-9832-44215199dd47
📒 Files selected for processing (10)
otdfctl/cmd/policy/dynamicValueMappings.gootdfctl/cmd/policy/policy.gootdfctl/docs/man/policy/dynamic-value-mappings/_index.mdotdfctl/docs/man/policy/dynamic-value-mappings/create.mdotdfctl/docs/man/policy/dynamic-value-mappings/delete.mdotdfctl/docs/man/policy/dynamic-value-mappings/get.mdotdfctl/docs/man/policy/dynamic-value-mappings/list.mdotdfctl/docs/man/policy/dynamic-value-mappings/update.mdotdfctl/e2e/dynamic-value-mapping.batsotdfctl/pkg/handlers/dynamicvaluemappings.go
Signed-off-by: Krish Suchak <suchak.krish@gmail.com>
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
🤖 I have created a release *beep* *boop* --- ## [0.36.0](opentdf/platform@otdfctl/v0.35.0...otdfctl/v0.36.0) (2026-08-04) ### ⚠ BREAKING CHANGES * **policy:** GetPC does not respect manager field. ([opentdf#3752](opentdf#3752)) ### Features * **authz:** dynamic value mapping and direct entitlement e2e coverage ([opentdf#3774](opentdf#3774)) ([49c80a0](opentdf@49c80a0)) * **cli:** add an sdkfactory for optional override in otdfctl for extension ([opentdf#3793](opentdf#3793)) ([c2f2afe](opentdf@c2f2afe)) * **cli:** add dynamic value mapping CRUD commands and e2e tests ([opentdf#3768](opentdf#3768)) ([aecd265](opentdf@aecd265)) * **cli:** expose handler hook for injecting SDK options ([opentdf#3755](opentdf#3755)) ([76305b0](opentdf@76305b0)) * **cli:** Unsafe update key cli ([opentdf#3733](opentdf#3733)) ([65d888e](opentdf@65d888e)) ### Bug Fixes * **deps:** bump github.com/opentdf/platform/protocol/go from 0.36.0 to 0.39.0 in /otdfctl ([opentdf#3765](opentdf#3765)) ([a3346f8](opentdf@a3346f8)) * **deps:** bump github.com/opentdf/platform/sdk from 0.25.0 to 0.27.0 in /otdfctl ([opentdf#3773](opentdf#3773)) ([490c990](opentdf@490c990)) * **policy:** GetPC does not respect manager field. ([opentdf#3752](opentdf#3752)) ([0d3c0a0](opentdf@0d3c0a0)) --- 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: Elizabeth Healy <35498075+elizabethhealy@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [0.36.0](opentdf/platform@otdfctl/v0.35.0...otdfctl/v0.36.0) (2026-08-04) ### ⚠ BREAKING CHANGES * **policy:** GetPC does not respect manager field. ([opentdf#3752](opentdf#3752)) ### Features * **authz:** dynamic value mapping and direct entitlement e2e coverage ([opentdf#3774](opentdf#3774)) ([49c80a0](opentdf@49c80a0)) * **cli:** add an sdkfactory for optional override in otdfctl for extension ([opentdf#3793](opentdf#3793)) ([c2f2afe](opentdf@c2f2afe)) * **cli:** add dynamic value mapping CRUD commands and e2e tests ([opentdf#3768](opentdf#3768)) ([aecd265](opentdf@aecd265)) * **cli:** expose handler hook for injecting SDK options ([opentdf#3755](opentdf#3755)) ([76305b0](opentdf@76305b0)) * **cli:** Unsafe update key cli ([opentdf#3733](opentdf#3733)) ([65d888e](opentdf@65d888e)) ### Bug Fixes * **deps:** bump github.com/opentdf/platform/protocol/go from 0.36.0 to 0.39.0 in /otdfctl ([opentdf#3765](opentdf#3765)) ([a3346f8](opentdf@a3346f8)) * **deps:** bump github.com/opentdf/platform/sdk from 0.25.0 to 0.27.0 in /otdfctl ([opentdf#3773](opentdf#3773)) ([490c990](opentdf@490c990)) * **policy:** GetPC does not respect manager field. ([opentdf#3752](opentdf#3752)) ([0d3c0a0](opentdf@0d3c0a0)) --- 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>
Proposed Changes
policy dynamic-value-mappingscommand group tootdfctl(aliasesdvm,dynamic-value-mapping) withcreate,get,list,update, anddelete, mirroring thesubject-mappingspattern. Wraps theDynamicValueMappingSDK client with a new handler, cobra commands, and man docs.IN/IN_CONTAINS; rejectNOT_INand unspecified), require exactly one of--attribute-definition-id/--attribute-definition-fqn, and require--selectorand--operatortogether on update.otdfctl/e2e/dynamic-value-mapping.bats) covering CRUD (table +--json), static-gate create, namespace id/fqn, sort/filter, and error/rejection cases (NOT_IN, missing operator/definition/action, HIERARCHY definition, value-subject-mapping coexistence).Checklist
Testing Instructions
cd otdfctl && golangci-lint run ./... && go test ./...cd otdfctl && make test-bats(orbats ./e2e/dynamic-value-mapping.bats).Verified locally: all 13 tests pass against a live platform.
Summary by CodeRabbit