Add author ID to labels #27055
Add author ID to labels #27055
Conversation
| func (ds *Datastore) ApplyLabelSpecs(ctx context.Context, specs []*fleet.LabelSpec) (err error) { | ||
| return ds.ApplyLabelSpecsWithAuthor(ctx, specs, 0) | ||
| } | ||
|
|
||
| func (ds *Datastore) ApplyLabelSpecsWithAuthor(ctx context.Context, specs []*fleet.LabelSpec, authorID uint) (err error) { |
There was a problem hiding this comment.
There's no ApplyLabels method that takes Label types as an argument, and there's no reason to add AuthorID to LabelSpec since it should never be set manually. So the cleanest way to do this was to update ApplyLabelSpecs to ApplyLabelSpecsWithAuthor and then create a new ApplyLabelSpecs wrapper with a default. This way we don't have to update the many calls to ApplyLabelSpecs in the codebase.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #27055 +/- ##
==========================================
- Coverage 63.98% 63.97% -0.02%
==========================================
Files 1706 1708 +2
Lines 162797 163100 +303
Branches 4326 4326
==========================================
+ Hits 104172 104346 +174
- Misses 50513 50617 +104
- Partials 8112 8137 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Left a few comments.
A product question: Should Labels also have team_id?
With the new idea of team admins/maintainers being able to create labels, a team admin/maintainer can now create a "global" label query that will run on all hosts from all teams. This is different to how Policies and Queries work [*].
[*] Policies and Queries that have author_id also have team_id.
|
|
||
| func Up_20250311132525(tx *sql.Tx) error { | ||
| _, err := tx.Exec( | ||
| "ALTER TABLE `labels` " + |
There was a problem hiding this comment.
Similar to author_id in the policies and queries tables, we need a foreign constraint and ON DELETE SET NULL:
CONSTRAINT `labels_users_fk_1` FOREIGN KEY (`author_id`) REFERENCES `users` (`id`) ON DELETE SET NULL
| @@ -15,6 +15,10 @@ import ( | |||
| ) | |||
|
|
|||
| func (ds *Datastore) ApplyLabelSpecs(ctx context.Context, specs []*fleet.LabelSpec) (err error) { | |||
| return ds.ApplyLabelSpecsWithAuthor(ctx, specs, 0) | |||
There was a problem hiding this comment.
Should the argument be authorID *uint and the default nil?
|
|
||
| // Get the user from the context. | ||
| user, ok := viewer.FromContext(ctx) | ||
| // If we have a user, mark them as the label's author. |
There was a problem hiding this comment.
Is it possible to not have a user set at this point?
For #27035
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements)COLLATE utf8mb4_unicode_ci).Details
This PR adds an
author_idcolumn to thelabelstable, and adds the associated properties to theLabelandLabelSpectypes. When a new label is created via the UI or API, an author ID is set on the label if one can be inferred from the context. Otherwise, the author ID is set tonull.Authz and Automated testing
Additional backend authorization logic is introduced in a follow-on PR, #27089, because rconciling all of the test updates between this PR and #27038 was getting complicated.
Manual Testing
fleetctl gitopswith a global config withlabels:in it.