47713 auld software update assets migration - #50036
Conversation
…pdates tables and target-OS fleet_variables
WalkthroughAdds MySQL migrations for Apple software-update asset metadata and per-host target updates, including keys, defaults, timestamps, and constraints. Adds migrations and tests for host target OS version and deadline fleet variables. Regenerates schema seed data for the new tables, fleet variables, and migration status entries. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates_test.go (1)
25-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the timestamp update contract.
This only asserts that
first_seen_atis initialized. Add an update/upsert-path assertion thatfirst_seen_atremains unchanged whileupdated_atadvances. Based on PR objective: “Preserveupdated_atbehavior for asset upserts andfirst_seen_atbehavior for initial inserts.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates_test.go` around lines 25 - 36, Extend the migration test’s asset update/upsert flow after the initial insert to capture the original first_seen_at and updated_at values, perform the upsert, then assert first_seen_at is unchanged while updated_at advances. Keep the existing nullable expiration_date and initial first_seen_at assertions, and use the asset upsert/update symbols already defined in the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@server/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates_test.go`:
- Around line 25-36: Extend the migration test’s asset update/upsert flow after
the initial insert to capture the original first_seen_at and updated_at values,
perform the upsert, then assert first_seen_at is unchanged while updated_at
advances. Keep the existing nullable expiration_date and initial first_seen_at
assertions, and use the asset upsert/update symbols already defined in the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9181f289-65f5-4a44-8a5a-639a669e60b9
📒 Files selected for processing (5)
server/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates.goserver/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates_test.goserver/datastore/mysql/migrations/tables/20260727084359_AddHostTargetOSVersionAndDeadlineFleetVars.goserver/datastore/mysql/migrations/tables/20260727084359_AddHostTargetOSVersionAndDeadlineFleetVars_test.goserver/datastore/mysql/schema.sql
There was a problem hiding this comment.
Pull request overview
Adds the initial MySQL schema and migrations for AULD software-update assets caching and per-host “resolved target OS version/deadline” tracking, and registers the corresponding Fleet template variables in fleet_variables.
Changes:
- Add
apple_software_update_assetsandhost_mdm_apple_os_updatestables (schema + migration + tests). - Register
FLEET_VAR_HOST_TARGET_OS_VERSIONandFLEET_VAR_HOST_TARGET_OS_DEADLINEinfleet_variables(migration + tests). - Regenerate
server/datastore/mysql/schema.sqlto include the new tables, variables, and migration status entries.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/datastore/mysql/schema.sql | Regenerated schema dump including the two new tables and the two new fleet_variables rows. |
| server/datastore/mysql/migrations/tables/20260727084359_AddHostTargetOSVersionAndDeadlineFleetVars.go | Migration inserting the two new Fleet variables with deterministic created_at. |
| server/datastore/mysql/migrations/tables/20260727084359_AddHostTargetOSVersionAndDeadlineFleetVars_test.go | Tests verifying the variables are absent before and present after the migration (and is_prefix=0). |
| server/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates.go | Migration creating the apple_software_update_assets and host_mdm_apple_os_updates tables. |
| server/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates_test.go | Tests validating key constraints/defaults on both new tables (unique key, enum, NOT NULL, PK uniqueness, defaults). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #50036 +/- ##
==========================================
- Coverage 67.99% 67.97% -0.03%
==========================================
Files 3924 3924
Lines 250120 250101 -19
Branches 13330 13123 -207
==========================================
- Hits 170072 170004 -68
- Misses 64747 64792 +45
- Partials 15301 15305 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
server/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates_test.go:59
- This updated_at assertion can be flaky: the insert and subsequent upsert may happen within the same CURRENT_TIMESTAMP(6) tick, causing
gotUpdatedAtAfterUpsert.After(gotUpdatedAt)to fail even though ON UPDATE is working. Add a small sleep before the upsert to ensure time advances between statements.
require.True(t, gotFirstSeenAt.Equal(gotFirstSeenAtAfterUpsert),
"first_seen_at must not change on upsert")
require.True(t, gotUpdatedAtAfterUpsert.After(gotUpdatedAt),
"updated_at must advance on upsert")
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
server/datastore/mysql/migrations/tables/20260727083533_CreateAppleSoftwareUpdateAssetsAndHostOSUpdates_test.go:52
- The test asserts
updated_atstrictly increases on the upsert, but the upsert happens immediately after the initial insert and can share the sameCURRENT_TIMESTAMP(6)value, making this assertion potentially flaky. Add a small sleep (or otherwise ensure the server timestamp advances) before running the upsert so theAftercheck is reliable.
// An upsert on the same (class, product_version, build) — the shape the GDMF
// refresh uses — keeps first_seen_at from the original insert while
// updated_at advances. posting_date is changed so the row is a real update:
// MySQL leaves updated_at alone when no column value actually changes.
execNoErr(t, db, `
Related issue: Resolves #47713
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Added/updated automated tests
QA'd all new/changed functionality manually
Database migrations
COLLATE utf8mb4_unicode_ci).Summary by CodeRabbit
New Features
Database
Tests