Skip to content

RHINENG-25147: refactor workspaces#2176

Draft
Dugowitch wants to merge 34 commits intoRedHatInsights:masterfrom
Dugowitch:workspaces
Draft

RHINENG-25147: refactor workspaces#2176
Dugowitch wants to merge 34 commits intoRedHatInsights:masterfrom
Dugowitch:workspaces

Conversation

@Dugowitch
Copy link
Copy Markdown
Contributor

Secure Coding Practices Checklist GitHub Link

Secure Coding Checklist

  • Input Validation
  • Output Encoding
  • Authentication and Password Management
  • Session Management
  • Access Control
  • Cryptographic Practices
  • Error Handling and Logging
  • Data Protection
  • Communication Security
  • System Configuration
  • Database Security
  • File Management
  • Memory Management
  • General Coding Practices

…ystemVmaasJSONHandler, SystemYumUpdatesHandler
…dler, TemplateSystemsListIDsHandler, TemplateSystemsExportHandler
…andler, TemplateSystemsDeleteHandler, TemplateSubscribedSystemsUpdateHandler
… AdvisoriesListIDsHandler, AdvisoriesExportHandler
…dler, AdvisorySystemsListIDsHandler, AdvisorySystemsExportHandler
…r, SystemAdvisoriesIDsHandler, SystemAdvisoriesExportHandler
…ler, PackageSystemsListIDsHandler, PackageSystemsExportHandler
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @Dugowitch, your pull request is larger than the review limit of 150000 diff characters

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.84848% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.03%. Comparing base (6d0becb) to head (d19ec59).
⚠️ Report is 19 commits behind head on master.

Files with missing lines Patch % Lines
base/database/utils.go 18.18% 8 Missing and 1 partial ⚠️
listener/upload.go 66.66% 4 Missing and 1 partial ⚠️
manager/controllers/system_detail.go 50.00% 2 Missing ⚠️
manager/controllers/template_systems.go 71.42% 2 Missing ⚠️
manager/middlewares/kessel.go 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2176      +/-   ##
==========================================
- Coverage   59.13%   59.03%   -0.11%     
==========================================
  Files         134      136       +2     
  Lines        8738     8697      -41     
==========================================
- Hits         5167     5134      -33     
+ Misses       3028     3023       -5     
+ Partials      543      540       -3     
Flag Coverage Δ
unittests 59.03% <84.84%> (-0.11%) ⬇️

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.

@@ -0,0 +1,10 @@
ALTER TABLE system_inventory
ADD COLUMN workspace_id TEXT CHECK (NOT empty(workspace_id)),
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.

workspace_id is uuid so we should use UUID type here - 2 reasons (at least):

  • you won't need empty() check,
  • and mostly UUID takes 16 bytes, while as text it takes 36 bytes .

}

type SystemWorkspace struct {
WorkspaceID *string `json:"workspace_id" csv:"workspace_id" query:"si.workspace_id" gorm:"column:workspace_id"`
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.

*uuid.UUID

Comment thread base/models/models.go Outdated
Tags []byte `gorm:"column:tags"`
Created time.Time // set by trigger system_platform_insert_trigger
Workspaces *inventory.Groups `gorm:"column:workspaces"`
WorkspaceID *string `gorm:"column:workspace_id"`
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.

*uuid.UUID

OSAttributes
SystemTimestamps
SystemTags
SystemGroups
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.

Removal of SystemGroups is actually trickier...
Since group column is part of API removal will break API v3 (contract). So we have 2 options:

  • keep groups there as "virtual" column made from workspace_id and workspace_name,
  • or create new API v4 without this column.

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.

So as a part of this PR / ticket I propose keep groups as is. And I created new epic for V4 API - we have many such columns over the time so it make sense to clean things a bit.

@MichaelMraka MichaelMraka self-assigned this May 4, 2026
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