Unified Queue: tentative DB schema, start refactoring scripts #25215
Conversation
There was a problem hiding this comment.
This is a small starting point for the unified queue story:
- Tentative upcoming activities table, only supports scripts for now
- Creates scripts in this new table
- Handles some lookups for pending scripts
- Handles some deletions
A couple of tests are passing that verify the new code, but most of the tests will not pass, and won't pass for quite a while due to the nature of this story.
|
|
||
| func Up_20250106162751(tx *sql.Tx) error { | ||
| _, err := tx.Exec(` | ||
| CREATE TABLE upcoming_activities ( |
There was a problem hiding this comment.
Keep in mind that this will still evolve - it only handles scripts for now, as I try to take small bites at that huge story. Be ready to reset your DB with future changes to this same migration if you try that out.
| @@ -64,199 +63,237 @@ func testHostScriptResult(t *testing.T, ds *Datastore) { | |||
|
|
|||
| // create a createdScript execution request (with a user) | |||
| u := test.NewUser(t, ds, "Bob", "bob@example.com", true) | |||
| createdScript, err := ds.NewHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{ | |||
There was a problem hiding this comment.
I adapted this test so that my new code is lightly tested and I'm not totally in the blind until the end of the story.
server/datastore/mysql/migrations/tables/20250106162751_AddUnifiedQueueTable.go
Show resolved
Hide resolved
| -- user_id is the user that triggered the activity, it may be null if the | ||
| -- activity is fleet-initiated or the user was deleted. Additional user | ||
| -- information (name, email, etc.) is stored in the JSON payload. | ||
| user_id INT UNSIGNED NULL, |
There was a problem hiding this comment.
If user was deleted, the id should point to the new deleted_users table (or whatever we call it).
There was a problem hiding this comment.
I'm not sure I'll be able to add that feature as part of this already-huge story. I'm trying to keep the scope in check as there are still tons of unknowns, storing the deleted user's info in the JSON for now.
| script_id INT UNSIGNED NULL, | ||
| script_content_id INT UNSIGNED NULL, | ||
| policy_id INT UNSIGNED NULL, | ||
| setup_experience_script_id INT UNSIGNED NULL, | ||
|
|
||
| payload JSON NOT NULL, |
There was a problem hiding this comment.
This doesn't feel very clean -- a bunch of columns specific to one type of activity. What if we need to add another column -- will we keep growing this table?
What do you think about normalizing and having a separate table for this script-specific data?
We can still have FKs by having the JOIN column like script_activity_id. If we have this column, then maybe we also don't need activity_type/execution_id columns. Or at least the activity_type can be autogenerated by simply checking if script_activity_id is null or not.
There was a problem hiding this comment.
Yeah I think a join table makes sense for specific activity stuff that is outside of the JSON payload. I think I'll keep the activity type as a) it cleanly describes the supported types and b) we may have some activity types that don't require a join table at all.
As mentioned, I'll address those things by going forward in the next PR, please approve if there's nothing else.
#23913 and #23918 , both only partially addressed by this PR.
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).