Skip to content

RHINENG-24787: remove system platform sql objects - #2161

Merged
TenSt merged 3 commits into
RedHatInsights:masterfrom
TenSt:stepan/RHINENG-24787-remove-system-platform-sql-objects
Apr 20, 2026
Merged

RHINENG-24787: remove system platform sql objects#2161
TenSt merged 3 commits into
RedHatInsights:masterfrom
TenSt:stepan/RHINENG-24787-remove-system-platform-sql-objects

Conversation

@TenSt

@TenSt TenSt commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

This PR:

  • removes system_platform related sql objects
  • updates script to generate data
  • updates docs
  • narrows manager db privileges how it was intended

Summary by Sourcery

Remove legacy system_platform SQL objects and restore least-privilege manager access on system tables.

Bug Fixes:

  • Correct manager database privileges to restrict updates on system_inventory and system_patch to the intended columns only.

Enhancements:

  • Simplify the schema by dropping the system_platform compatibility view, triggers, and related grants in favor of direct system_inventory/system_patch usage.
  • Adjust the test data generation script to create systems directly in system_inventory and system_patch and to use robust progress logging based on total counts.
  • Clarify database and architecture documentation to describe system_inventory and system_patch as the canonical system tables without referencing system_platform.

Documentation:

  • Update database and architecture docs to remove system_platform references and explain the split between system_inventory and system_patch more clearly.

Chores:

  • Add forward and backward database migrations for dropping the system_platform view and narrowing/restoring manager table privileges.
  • Bump the schema migration version to reflect the new migrations.

@TenSt
TenSt requested a review from a team as a code owner April 17, 2026 20:05
@sourcery-ai

sourcery-ai Bot commented Apr 17, 2026

Copy link
Copy Markdown

Reviewer's Guide

Removes the legacy system_platform SQL view and its triggers, tightens manager DB privileges back to least-privilege on system_inventory and system_patch, updates test data generation to work directly with system_inventory/system_patch and large-table stats, and refreshes docs to describe only the split system_inventory/system_patch design.

File-Level Changes

Change Details Files
Drop legacy system_platform view layer and its triggers while keeping a reversible migration path.
  • Remove system_platform view and all related trigger functions and GRANTs from the main schema SQL
  • Add migration that drops system_platform triggers, functions, and view in the forward (up) direction
  • Add reverse migration that recreates system_platform view, its insert/update/delete trigger functions, triggers, and GRANTs for rollback
database_admin/schema/create_schema.sql
database_admin/migrations/151_drop_system_platform_view.up.sql
database_admin/migrations/151_drop_system_platform_view.down.sql
Tighten manager role privileges on system_inventory and system_patch to only the intended columns.
  • Remove broad GRANT UPDATE privileges for manager on system_inventory and system_patch from the base schema
  • Add migration that revokes generic UPDATE from manager and grants UPDATE only on stale and selected cache/template columns
  • Add down migration that restores broad UPDATE privileges on system_inventory and system_patch for manager
database_admin/schema/create_schema.sql
database_admin/migrations/152_narrow_manager_inventory_patch_privileges.up.sql
database_admin/migrations/152_narrow_manager_inventory_patch_privileges.down.sql
Update test data generation script to work directly with system_inventory/system_patch and improve progress reporting.
  • Change system count constant semantics and reduce default test size, now counting system_inventory rows with matching system_patch
  • Generate systems by inserting into system_inventory and system_patch separately instead of using system_platform
  • Update advisory and repo assignment loops to iterate over system_inventory instead of system_platform
  • Adjust progress-notice calculations to avoid division-by-zero and use greatest/ceil-based step sizes
  • Update partition size query to report partitions for system_inventory, system_patch, and system_package2 instead of system_platform
dev/test_generate_data.sql
Align documentation with the removal of system_platform and clarify system_inventory/system_patch responsibilities.
  • Remove references to system_platform as a compatibility view from database documentation
  • Clarify ERD caveat text to describe the split between system_inventory and system_patch without mentioning system_platform
  • Update architecture docs for the listener to state it only touches system_inventory and system_patch, not system_platform
docs/md/database.md
docs/md/architecture.md
Clean up code comments now that manager privileges are narrowed via migrations.
  • Remove obsolete TODO about revisiting manager privileges tied to system_platform view/trigger migration
manager/controllers/template_systems_update.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions

Copy link
Copy Markdown

SC Environment Impact Assessment

Overall Impact:NONE

No SC Environment-specific impacts detected in this PR.

What was checked

This PR was automatically scanned for:

  • Database migrations
  • ClowdApp configuration changes
  • Kessel integration changes
  • AWS service integrations (S3, RDS, ElastiCache)
  • Kafka topic changes
  • Secrets management changes
  • External dependencies

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@TenSt
TenSt force-pushed the stepan/RHINENG-24787-remove-system-platform-sql-objects branch from 9435571 to 48b2802 Compare April 17, 2026 20:09
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.13%. Comparing base (acc25ae) to head (48b2802).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2161   +/-   ##
=======================================
  Coverage   59.13%   59.13%           
=======================================
  Files         134      134           
  Lines        8738     8738           
=======================================
  Hits         5167     5167           
  Misses       3028     3028           
  Partials      543      543           
Flag Coverage Δ
unittests 59.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

GRANT UPDATE ON system_patch TO manager;

REVOKE UPDATE ON system_inventory FROM manager;
GRANT UPDATE ON system_inventory TO manager;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of revoking and granting UPDATE back?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The column-wide and table-wide privileges are distinct in postgres. So just granting privileges on the table level will leave privileges on the column level. REVOKE on the table level however also removes the column level privileges. So this is done to be consistent - remove all update privileges (column and table) and then assign just table privileges. Same approach is done in the "up" migration.

@TenSt
TenSt merged commit b21cf43 into RedHatInsights:master Apr 20, 2026
8 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.

3 participants