Skip to content

fix(core): make namespacing registered resources optional#785

Merged
alkalescent merged 2 commits intomainfrom
DSPX-2496-optional-ns-rr
Mar 31, 2026
Merged

fix(core): make namespacing registered resources optional#785
alkalescent merged 2 commits intomainfrom
DSPX-2496-optional-ns-rr

Conversation

@alkalescent
Copy link
Copy Markdown
Contributor

@alkalescent alkalescent commented Mar 26, 2026

We made namespacing registered resources required. These were breaking changes, and now namespacing registered resources is optional.

Summary by CodeRabbit

  • New Features

    • Namespace information is now shown in registered resource listings and detailed views.
  • Documentation

    • --namespace for resource creation is documented as optional.
  • Tests

    • End-to-end tests updated to cover creation with and without a namespace and to assert namespace fields in responses.
  • Chores

    • Internal dependency bump.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 03b8204a-048c-4daa-bb4d-e7e10e0b261b

📥 Commits

Reviewing files that changed from the base of the PR and between 9c6d97b and 2ba6151.

📒 Files selected for processing (1)
  • e2e/registered-resources.bats
🚧 Files skipped from review as they are similar to previous changes (1)
  • e2e/registered-resources.bats

📝 Walkthrough

Walkthrough

Make the --namespace flag optional for registered-resources create; add Namespace output column/row across registered-resources commands; update docs and e2e tests for namespaced and non-namespaced creation; bump github.com/opentdf/platform/protocol/go from v0.20.0 to v0.21.0.

Changes

Cohort / File(s) Summary
CLI Command Updates
cmd/policy/registeredResources.go
Treat namespace as optional, add Namespace field to create/get/update/delete detailed output, add namespace column to list output, and perform minor cleanup of table row formatting.
Documentation
docs/man/policy/registered-resources/create.md
Removed required: true from the namespace flag in the create command documentation.
End-to-End Tests
e2e/registered-resources.bats
Adjust tests to assert namespaced output when provided, add a successful non-namespaced create/assertion path, and remove the negative test that expected omitting --namespace to fail.
Dependencies
go.mod
Bumped github.com/opentdf/platform/protocol/go from v0.20.0 to v0.21.0.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • c-r33d

Poem

🐰
I hopped through flags both near and far,
Namespace optional — now free to spar.
Tables show homes where resources stay,
Tests cheer both paths, hip-hip-hooray! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(core): make namespacing registered resources optional' directly summarizes the main change: reverting namespacing from required to optional for registered resources.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch DSPX-2496-optional-ns-rr

Comment @coderabbitai help to get the list of available commands and usage tips.

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 makes the namespace flag optional for registered resources and updates the CLI to display the namespace FQN in the output for create, get, list, update, and delete commands. It also includes corresponding documentation updates, new E2E tests for un-namespaced resources, and a dependency update for the platform protocol. The review feedback highlights several critical instances where the code lacks nil-checks for the namespace object, which will lead to application panics when processing resources that do not have a namespace assigned.

Comment thread cmd/policy/registeredResources.go
Comment thread cmd/policy/registeredResources.go
Comment thread cmd/policy/registeredResources.go
Comment thread cmd/policy/registeredResources.go
Comment thread cmd/policy/registeredResources.go
@github-actions
Copy link
Copy Markdown

@alkalescent alkalescent marked this pull request as ready for review March 26, 2026 22:15
@alkalescent alkalescent requested review from a team as code owners March 26, 2026 22:15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@e2e/registered-resources.bats`:
- Around line 83-94: The current test uses refute_line --regexp
"Namespace.*https://$NS_NAME" which only ensures the namespace isn't that URL;
update the assertion after run_otdfctl_reg_res create (the block with
assert_output/assert_line checks and before extracting created_id) to explicitly
assert that the "Namespace" field is empty (e.g., replace the existing
refute_line with an assertion that matches a blank/empty Namespace value),
keeping the rest of the flow (run_otdfctl_reg_res create, created_id extraction,
and run_otdfctl_reg_res delete) unchanged.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b054ad25-7c5d-4da4-a5cc-2b2b067b2948

📥 Commits

Reviewing files that changed from the base of the PR and between ef637ca and 9c6d97b.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • cmd/policy/registeredResources.go
  • docs/man/policy/registered-resources/create.md
  • e2e/registered-resources.bats
  • go.mod
💤 Files with no reviewable changes (1)
  • docs/man/policy/registered-resources/create.md

Comment thread e2e/registered-resources.bats
@github-actions
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown

@alkalescent alkalescent merged commit 8e6eb31 into main Mar 31, 2026
33 of 35 checks passed
@alkalescent alkalescent deleted the DSPX-2496-optional-ns-rr branch March 31, 2026 16:49
alkalescent pushed a commit that referenced this pull request Mar 31, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.30.0](v0.29.0...v0.30.0)
(2026-03-31)


### Features

* **core:** Add optional namespace flag for subject mappings and
condtion sets ([#779](#779))
([9e849c4](9e849c4))
* **core:** add scope support for client creds
([#752](#752))
([9ca9e43](9ca9e43))
* **core:** migrate registered resources
([#772](#772))
([2b49a7d](2b49a7d))
* **core:** optional namespace in actions commands and re-enable
actions/RR tests ([#775](#775))
([29a2eb1](29a2eb1))
* **core:** support namespaced registered resources
([#767](#767))
([4d786b5](4d786b5))


### Bug Fixes

* **ci:** Temporarily skip namespaced-actions impacted BATS cases
([#773](#773))
([633728a](633728a))
* **core:** bump toolchain to go 1.24.13
([#747](#747))
([6804b93](6804b93))
* **core:** disable RR E2E tests
([#768](#768))
([0821b8c](0821b8c))
* **core:** make namespacing registered resources optional
([#785](#785))
([8e6eb31](8e6eb31))
* **core:** refactor `ListAttributesValues` to use `Get`
([#769](#769))
([a82f7b7](a82f7b7))
* **core:** unsafe update result output values order
([#759](#759))
([baeba0f](baeba0f))

---
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>
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.

3 participants