Skip to content

fix: keep additive mode from rewriting brownfield roles - #95

Merged
hardbyte merged 3 commits into
mainfrom
fix/additive-role-attrs
Apr 30, 2026
Merged

fix: keep additive mode from rewriting brownfield roles#95
hardbyte merged 3 commits into
mainfrom
fix/additive-role-attrs

Conversation

@hardbyte

@hardbyte hardbyte commented Apr 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • stop additive mode from rewriting attributes and comments on pre-existing roles
  • add profile-level generated-role inherit support and thread it through the operator CRD
  • make CLI execution stick to a single backend and include backend identity in SQL execution failures
  • update user-facing docs for additive brownfield behavior and generated-role attributes

Why

A brownfield role like accounts_editor LOGIN NOINHERIT could cause additive mode to plan ALTER ROLE ... NOLOGIN INHERIT, which is not consistent with additive adoption semantics.

In environments where one hostname can resolve to multiple PostgreSQL servers, one-shot CLI commands could also inspect one backend and execute mutations against another. This change reduces that risk by keeping CLI execution on a single connection and improves diagnosis by surfacing backend identity when SQL execution fails.

Validation

  • cargo test --workspace
  • SQLX_OFFLINE=true cargo clippy --all-targets --all-features -- -D warnings
  • scripts/check-crd-drift.sh
  • DATABASE_URL=postgres://postgres:testpassword@localhost:5432/pgroles_test cargo test -p pgroles-cli --test cli additive_mode -- --ignored --test-threads=1
  • DATABASE_URL=postgres://postgres:testpassword@localhost:5432/pgroles_test cargo test --workspace -- --include-ignored

Fixes #94

@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This pull request extends profile-generated roles to support the inherit attribute, implements filtering of destructive changes in additive reconciliation mode to preserve pre-existing role attributes and comments, and enhances error reporting in the CLI. Changes span CRD schema extensions, core manifest expansion and change filtering logic, comprehensive documentation clarifications, and integration tests validating the new behaviors.

Changes

Cohort / File(s) Summary
Documentation: Additive Mode & Profiles
README.md, docs/src/pages/docs/adoption.md, docs/src/pages/docs/cli.md, docs/src/pages/docs/profiles.md, docs/src/pages/docs/operator.md
Updated documentation to clarify that additive mode preserves pre-existing role attributes and comments, and that profile-generated roles support the inherit attribute with defaults of login: false (NOLOGIN) and inherit: true (INHERIT).
Documentation: Manifest & Features
docs/src/pages/docs/manifest-format.md
New comprehensive section introducing profiles as reusable templates with field reference (login, inherit, grants, default_privileges) and callout describing additive mode preservation semantics for existing roles.
CRD & Schema Extensions
charts/pgroles-operator/crds/postgrespolicies.pgroles.io.yaml, k8s/crd.yaml
Extended CRD schema to add optional nullable inherit boolean property to profile object definitions in spec.profiles.
Core Manifest & Profile Support
crates/pgroles-core/src/manifest.rs, crates/pgroles-operator/src/crd.rs
Added inherit: Option<bool> field to Profile struct; implemented propagation of profile-level inherit to generated role's inherit field during manifest expansion and CRD-to-manifest conversion; updated and added tests verifying default/explicit inherit handling.
Additive Mode Filtering Logic
crates/pgroles-core/src/diff.rs
Modified filter_additive_changes to treat AlterRole and SetComment as destructive and remove them from applied change set; updated test expectations to verify absence of these changes.
Test Infrastructure & Integration Tests
crates/pgroles-cli/tests/cli.rs, crates/pgroles-core/src/composition.rs
Added database URL construction utilities and role attribute inspection helpers; introduced integration tests validating additive mode does not rewrite pre-existing role attributes and that generated roles preserve profile-level inherit settings; updated test fixture to include new inherit field.
CLI: Error Handling & Transaction Refactoring
crates/pgroles-cli/src/main.rs
Refactored apply logic: centralized statement execution into execute_changes helper; enhanced database connectivity with PgPoolOptions and max_connections(1); added backend context fetching and improved error reporting to include SQLSTATE and backend details on statement failure; introduced unit tests for render_execution_failure with database and non-database error cases.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 With whiskers twitching bright and proud,
Profiles now inherit without cloud!
Additive mode keeps what's already there,
No pre-existing roles need repair,
Schema-bound roles dance without a care! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preventing additive reconciliation mode from rewriting attributes on pre-existing brownfield roles.
Linked Issues check ✅ Passed The PR successfully implements all linked issue #94 objectives: additive mode filters AlterRole/SetComment for pre-existing roles, inherit attribute support is added to profiles and propagated through the CRD, SQL error classification is improved in execute_changes, and comprehensive regression tests are included.
Out of Scope Changes check ✅ Passed All changes are directly related to issue #94: additive mode filtering, profile inherit support, CRD updates, test coverage, documentation updates, and CLI error handling improvements. No unrelated changes detected.
Docstring Coverage ✅ Passed Docstring coverage is 81.63% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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 fix/additive-role-attrs

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.

@hardbyte
hardbyte merged commit 430858d into main Apr 30, 2026
10 checks passed
@hardbyte
hardbyte deleted the fix/additive-role-attrs branch April 30, 2026 00:27
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.

Additive mode should not converge pre-existing role attributes

1 participant