Skip to content

fix(cli): Prune was not classifying multi-namespaced RRs properly.#3488

Merged
c-r33d merged 3 commits into
mainfrom
multi-namespace-rr
May 19, 2026
Merged

fix(cli): Prune was not classifying multi-namespaced RRs properly.#3488
c-r33d merged 3 commits into
mainfrom
multi-namespace-rr

Conversation

@c-r33d
Copy link
Copy Markdown
Contributor

@c-r33d c-r33d commented May 19, 2026

1.) RRs with multi-namespaces should be classified as Blocked and require manual deletion.

Note

While we could attempt to locate all the pieces of a split RR across different namespaces. I think it's better to just
have the customer manually delete / handle multi-namespace pruning since trying to do this for customers could
result in a brittle workflow.

Summary by CodeRabbit

  • Bug Fixes

    • Registered resources spanning multiple namespaces are now properly marked for manual deletion instead of reporting source mismatches.
  • Chores

    • Simplified prune planning logic and removed the interactive reviewer option.
    • Streamlined prune plan details and source state verification.

Review Change Stack

@c-r33d c-r33d requested a review from a team as a code owner May 19, 2026 12:43
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Warning

Rate limit exceeded

@c-r33d has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 57 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1198369f-da14-4cb2-8941-d304171514c6

📥 Commits

Reviewing files that changed from the base of the PR and between bbbaec2 and fdecfcb.

📒 Files selected for processing (2)
  • otdfctl/docs/man/migrate/prune/namespaced-policy.md
  • otdfctl/migrations/namespacedpolicy/prune_summary_test.go
📝 Walkthrough

Walkthrough

The PR refactors registered resource pruning in the namespaced-policy planner to replace a source-mismatch verification flow with simpler multi-namespace conflict detection. It removes the FullSource field, eliminates separate source reloading, and blocks resources spanning multiple target namespaces via a new MultiNamespaceManualDelete status reason.

Changes

Registered Resource Prune Refactoring

Layer / File(s) Summary
Prune status reasons and PruneRegisteredResourcePlan structure
otdfctl/migrations/namespacedpolicy/prune_plan.go
RegisteredResourceSourceMismatch reason removed and replaced with MultiNamespaceManualDelete. The PruneRegisteredResourcePlan struct loses its FullSource field; Source now represents the legacy RR source being deleted.
PrunePlanner API and configuration simplification
otdfctl/migrations/namespacedpolicy/prune_planner.go
Removed ErrInvalidPruneResolvedSource error, interactive-reviewer configuration, and WithPruneInteractiveReviewer export. NewPrunePlanner now constructs with only WithPageSize; interactive review differences in scope documentation removed.
Plan building logic refactoring
otdfctl/migrations/namespacedpolicy/prune_planner.go
Plan method simplified to call buildPrunePlanFromResolved directly without extra source loading. buildPrunePlanFromResolved and builder refactored to remove sourceRegisteredResources parameter. registeredResources now detects multi-namespace conflicts via new pruneManualDeleteRequired helper and blocks with MultiNamespaceManualDelete reason. Removed helpers: sourceRegisteredResources, sourceRegisteredResourcesByID, newPruneReasonf.
Prune details output simplification
otdfctl/migrations/namespacedpolicy/prune_details.go
pruneDetails now always returns a fixed set of three fields: source_id, source, found_migrated_target. Removed conditional append of full_source detail.
Planner unit tests
otdfctl/migrations/namespacedpolicy/prune_planner_test.go
Updated API calls to match new buildPrunePlanFromResolved signature. Replaced filtered-source test with new TestPrunePlannerPlanBlocksUnmigratedMultiNamespaceRegisteredResourceForManualDelete covering multi-namespace blocking. Removed FullSource assertions and deleted registeredResourceFilterReviewer helper.
Execute and summary test fixtures
otdfctl/migrations/namespacedpolicy/prune_execute_test.go, otdfctl/migrations/namespacedpolicy/prune_summary_test.go
Simplified execute fixture by removing FullSource from delete entries. Replaced source-mismatch test case with new multi-namespace manual-delete test; asserts MultiNamespaceManualDelete reason and absence of full_source= in rendered output.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • opentdf/platform#3458: Both PRs directly modify the namespaced-policy prune planning/data model (e.g., migrations/namespacedpolicy/prune_plan.go changes to PruneRegisteredResourcePlan/reason handling in the main PR and adding sourceID()/execution-result wiring to prunePlanItem in the retrieved PR), which is required for the new execute/confirm flow.
  • opentdf/platform#3411: The main PR refines the newly added namespaced-policy prune planner/models (e.g., PruneRegisteredResourcePlan.pruneDetails, PrunePlanner registered-resource handling, and prune reason types), directly building on the functionality introduced by the "feat(cli): Add prune planner" PR.
  • opentdf/platform#3469: Both PRs modify the namespaced-policy prune summary behavior/tests around rendered per-item reasons—main PR swaps registered-resource mismatch/full_source handling for a MultiNamespaceManualDelete reason, while the retrieved PR refactors RenderNamespacedPolicyPruneSummary to use typed results and updates summary tests (including registered-resource cases).

Suggested labels

size/l

Suggested reviewers

  • alkalescent
  • elizabethhealy

Poem

🐰 Hops through the code with glee,
Removes what once did cross-check be,
Multi-namespace conflicts now block with care,
No source-mismatch, no full-source affair,
Simpler plans for the policy to share!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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.
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.
Title check ✅ Passed The title accurately describes the main change: fixing how the prune functionality classifies registered resources that span multiple namespaces, which is the core objective of this PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch multi-namespace-rr

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 and usage tips.

@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 186.364228ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 421.441166ms
Throughput 237.28 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 44.401215947s
Average Latency 442.685503ms
Throughput 112.61 requests/second

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 updates the CLI prune logic to handle multi-namespaced Registered Resources more safely. Rather than attempting to automatically resolve and prune these resources, which could lead to inconsistent states, the system now flags them as blocked and prompts for manual deletion. This change also simplifies the internal prune planning infrastructure by removing unnecessary verification steps and interactive review dependencies.

Highlights

  • Multi-namespace RR Handling: Registered Resources (RRs) that span multiple namespaces are now classified as 'Blocked' during the prune process, requiring manual intervention instead of attempting an automated, potentially brittle, migration.
  • Code Cleanup: Removed the 'FullSource' field from 'PruneRegisteredResourcePlan' and associated verification logic, as the prune planner now relies directly on the resolved migration state.
  • Planner Simplification: Removed 'InteractiveReviewer' from the prune planner configuration and simplified the 'buildPrunePlanFromResolved' logic to streamline the planning process.
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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.


In namespaces deep where resources reside, We found a split that we could not hide. No automated path for this messy state, Manual deletion is now the fate.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the prune planner by removing the FullSource field from PruneRegisteredResourcePlan and eliminating the need for interactive review during planning. It introduces a MultiNamespaceManualDelete status to handle multi-namespace legacy registered resources that cannot be automatically deleted. The reviewer identified a discrepancy in the test expectation for the new manual delete reason message, which needs to be corrected to match the defined constant.

Comment thread otdfctl/migrations/namespacedpolicy/prune_summary_test.go
@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 175.05847ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 415.151782ms
Throughput 240.88 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 42.05104539s
Average Latency 419.491212ms
Throughput 118.90 requests/second

@c-r33d c-r33d changed the title bug(cli): Prune was not classifying multi-namespaced RRs properly. feat(cli): Prune was not classifying multi-namespaced RRs properly. May 19, 2026
@c-r33d c-r33d changed the title feat(cli): Prune was not classifying multi-namespaced RRs properly. fix(cli): Prune was not classifying multi-namespaced RRs properly. May 19, 2026
@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 172.331054ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

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

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 457.093058ms
Throughput 218.77 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.106301821s
Average Latency 429.133417ms
Throughput 115.99 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.

@c-r33d
Copy link
Copy Markdown
Contributor Author

c-r33d commented May 19, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@c-r33d c-r33d added this pull request to the merge queue May 19, 2026
Merged via the queue into main with commit eae8645 May 19, 2026
39 checks passed
@c-r33d c-r33d deleted the multi-namespace-rr branch May 19, 2026 14:55
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants