Skip to content

Fix grants for former owners after owner change#174

Merged
nettrash merged 3 commits intonettrash:v1.0.18from
iBarBuba:173-missing-grants-when-object-owner-changes
Apr 25, 2026
Merged

Fix grants for former owners after owner change#174
nettrash merged 3 commits intonettrash:v1.0.18from
iBarBuba:173-missing-grants-when-object-owner-changes

Conversation

@iBarBuba
Copy link
Copy Markdown

@iBarBuba iBarBuba commented Apr 25, 2026

Summary

  • Fix ACL diffing so object owners are filtered per side instead of globally across FROM and TO owners.
  • Preserve explicit TO grants for a former owner after ownership changes
  • Update grant comparison for schemas, tables, sequences, views, foreign tables, routines, types, and column ACLs to pass side-specific owner context.

Root cause

diff_acls previously removed FROM and TO owners from the merged grantee set. When ownership changed, this incorrectly suppressed explicit grants to the former owner in the TO database.

Closes #173

Copilot AI review requested due to automatic review settings April 25, 2026 08:22
@iBarBuba iBarBuba requested a review from nettrash as a code owner April 25, 2026 08:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes grant/ACL diffing when object ownership changes by filtering implicit-owner grantees per side (FROM vs TO) rather than removing both owners globally, ensuring explicit TO grants for a former owner are preserved.

Changes:

  • Update ACL diffing to accept from_owners and to_owners and skip owner grantees only on the side where they are owners.
  • Thread side-specific owner context through grant generation in Comparer::compare_grants for multiple object types (schemas, relations, routines, types, columns).
  • Add regression tests covering explicit grants to a former owner after ownership changes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
app/src/dump/acl.rs Adds side-specific owner filtering to ACL diffing and extends unit tests for former-owner grants.
app/src/comparer/core.rs Passes from_owners/to_owners into grant generation across object types.
app/src/comparer/core_tests.rs Adds an integration-style regression test reproducing issue #173 behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/src/dump/acl.rs
Comment thread app/src/comparer/core.rs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/src/dump/acl.rs
Comment thread app/src/comparer/core.rs Outdated
@iBarBuba iBarBuba force-pushed the 173-missing-grants-when-object-owner-changes branch from 669324d to d96ed88 Compare April 25, 2026 08:52
Copilot AI review requested due to automatic review settings April 25, 2026 09:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nettrash nettrash changed the base branch from master to v1.0.18 April 25, 2026 17:17
Copy link
Copy Markdown
Owner

@nettrash nettrash left a comment

Choose a reason for hiding this comment

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

Bug is real.
Fix is correct.

@nettrash nettrash merged commit 2d61121 into nettrash:v1.0.18 Apr 25, 2026
6 of 7 checks passed
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.

Missing grants when object owner changes and former owner has an explicit ACL entry

3 participants