Moved common_mysql package to server/platform/mysql #38017
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #38017 +/- ##
==========================================
- Coverage 65.82% 65.81% -0.01%
==========================================
Files 2387 2387
Lines 190158 190158
Branches 8428 8428
==========================================
- Hits 125168 125160 -8
- Misses 53609 53613 +4
- Partials 11381 11385 +4
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:
|
WalkthroughRefactors the MySQL common utilities package by moving Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (14)
server/datastore/mysql/apple_mdm_test.go (1)
18-37: Fix import alias for movedcommon_mysqlpackage and avoidmysqlname collisionThe new import
"github.com/fleetdm/fleet/v4/server/platform/mysql"introduces two problems:
- There is no imported symbol named
common_mysql, but the file still usescommon_mysql.NotFoundError, which will not compile.- Without an alias this package would get the local name
mysql, colliding withgithub.com/go-sql-driver/mysqlalso imported asmysql.Alias the new path to
common_mysql(preserving existing call sites) and avoid themysqlname clash:🔧 Suggested import fix
@@ - "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/push" - "github.com/fleetdm/fleet/v4/server/platform/mysql" + "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/push" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"server/datastore/mysql/android_enterprises.go (1)
31-31: Critical: Update references to use new import path.The import was changed to
platform/mysql(line 10), but these lines still referencecommon_mysql.NotFound. Without an import alias, the package name ismysql, so these should be updated tomysql.NotFound.🔧 Proposed fix
- return nil, common_mysql.NotFound("Android enterprise").WithID(id) + return nil, mysql.NotFound("Android enterprise").WithID(id)- return nil, common_mysql.NotFound("Android enterprise") + return nil, mysql.NotFound("Android enterprise")- return nil, common_mysql.NotFound("Android enterprise") + return nil, mysql.NotFound("Android enterprise")- return common_mysql.NotFound("Android enterprise").WithID(enterprise.ID) + return mysql.NotFound("Android enterprise").WithID(enterprise.ID)Also applies to: 44-44, 57-57, 80-80
server/datastore/mysql/vulnerabilities_test.go (1)
176-177: Critical: Update references to use new import path.The import was changed to
platform/mysql(line 10), but these lines still referencecommon_mysql.NotFoundError. The package should now be referenced asmysql.NotFoundError.🔧 Proposed fix
- var nfe *common_mysql.NotFoundError + var nfe *mysql.NotFoundError require.ErrorAs(t, err, &nfe)Also applies to: 285-286
server/datastore/mysql/operating_system_vulnerabilities.go (1)
1038-1038: Critical: Update reference to use new import path.The import was changed to
platform/mysql(line 12), but this line still referencescommon_mysql.BatchProcessSimple. It should be updated tomysql.BatchProcessSimple.🔧 Proposed fix
- err := common_mysql.BatchProcessSimple(cveList, batchSize, func(batch []string) error { + err := mysql.BatchProcessSimple(cveList, batchSize, func(batch []string) error {server/datastore/mysql/apple_mdm.go (1)
21-36: Brokencommon_mysqlreference after import path moveThe new import at Line 29 points to
server/platform/mysqlwithout an alias, but the code still refers tocommon_mysql.DBReadTxandcommon_mysql.BatchProcessSimple(e.g., Lines 3256, 3286, 5547). As written:
common_mysqlis now an undefined identifier → compile error.- The imported
github.com/fleetdm/fleet/v4/server/platform/mysqlpackage is unused → additional compile/lint error.If the intent is to keep the existing
common_mysqlAPI surface while changing its location, you should alias the new package tocommon_mysqlso the rest of the file compiles unchanged.🔧 Suggested fix: alias the new platform mysql package
@@ - "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/mdm" - "github.com/fleetdm/fleet/v4/server/platform/mysql" + "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/mdm" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"This preserves all existing
common_mysql.DBReadTxandcommon_mysql.BatchProcessSimpleusages without further changes in this file.Also applies to: 3253-3266, 5547-5563
tools/mysql-tests/rds/iam_auth.go (1)
11-17: Resolve MySQL import conflict and restorecommon_mysql.DBOptionsThis file now imports both:
github.com/fleetdm/fleet/v4/server/datastore/mysqlgithub.com/fleetdm/fleet/v4/server/platform/mysqlwithout aliasing, while still using
common_mysql.DBOptions. This leads to:
- A name clash on
mysqlbetween the two imports.common_mysqlbeing undefined.Alias the new platform package as
common_mysqlsoDBOptionsresolves and the datastoremysqlimport remains unchanged.🔧 Suggested import change
- "github.com/fleetdm/fleet/v4/server/platform/mysql" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"With this change:
mysql.NewDB(...)continues to refer toserver/datastore/mysql.dbOpts := &common_mysql.DBOptions{...}compiles againstserver/platform/mysql.server/service/software_title_icons_test.go (1)
11-21: Alias moved MySQL helpers tocommon_mysqlin testsThis test still constructs errors using
&common_mysql.NotFoundError{...}, but now importsserver/platform/mysqlwithout an alias. As written:
common_mysqlis undefined.mysqlis imported but never used.Mirror the old alias by importing the new package as
common_mysql:Suggested import fix
import ( @@ - "github.com/fleetdm/fleet/v4/server/mock" - "github.com/fleetdm/fleet/v4/server/platform/mysql" + "github.com/fleetdm/fleet/v4/server/mock" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"This keeps all
common_mysql.NotFoundErroruses compiling against the relocated package.Also applies to: 50-60, 147-155, 185-191, 229-239, 289-296, 381-383, 461-463
server/datastore/mysql/policies.go (1)
15-22: WireWithRetryTxx/BatchProcessSimpleto new platform/mysql packageAfter switching the import to
server/platform/mysql, this file still referencescommon_mysql.WithRetryTxxandcommon_mysql.BatchProcessSimple, but no longer importscommon_mysql. The newmysqlimport is otherwise unused.Alias the new package so existing helper calls compile:
Suggested import fix
import ( @@ - "github.com/fleetdm/fleet/v4/server/fleet" - "github.com/fleetdm/fleet/v4/server/platform/mysql" + "github.com/fleetdm/fleet/v4/server/fleet" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"With that alias:
common_mysql.WithRetryTxxincleanupPolicywill resolve correctly.common_mysql.BatchProcessSimpleingetPoliciesBySoftwareTitleIDswill also bind to the relocated utilities.Also applies to: 441-446, 2163-2179
server/datastore/mysql/hosts_test.go (1)
30-40: Brokencommon_mysqlreference after import refactor—will not compileThe import was changed to
server/platform/mysqlbut line 40 still callscommon_mysql.GetDefaultNonZeroTime(). Sincecommon_mysqlis not imported, this will fail to compile. Also, importingplatform/mysqlwithout an alias inpackage mysqlcreates ambiguity.Alias the import and update the usage:
Proposed fix
- "github.com/fleetdm/fleet/v4/server/platform/mysql" + platformmysql "github.com/fleetdm/fleet/v4/server/platform/mysql"-var expLastExec = common_mysql.GetDefaultNonZeroTime() +var expLastExec = platformmysql.GetDefaultNonZeroTime()server/datastore/mysql/scim.go (3)
693-693: Critical: Update package reference to match new import.The import was changed to
github.com/fleetdm/fleet/v4/server/platform/mysql(line 12), which provides the package namemysql, but this line still referencescommon_mysql.BatchProcessSimple. This will cause a compilation error.🔧 Required fix
- return common_mysql.BatchProcessSimple(userIDs, batchSize, func(userIDsInBatch []uint) error { + return mysql.BatchProcessSimple(userIDs, batchSize, func(userIDsInBatch []uint) error {
879-879: Critical: Update package reference to match new import.The package reference must be updated from
common_mysqltomysqlto match the new import path.🔧 Required fix
- err = common_mysql.BatchProcessSimple(usersToRemove, batchSize, func(usersToRemoveInBatch []uint) error { + err = mysql.BatchProcessSimple(usersToRemove, batchSize, func(usersToRemoveInBatch []uint) error {
1381-1381: Critical: Update package reference to match new import.The package reference must be updated from
common_mysqltomysqlto match the new import path.🔧 Required fix
- err := common_mysql.BatchProcessSimple(ids, batchSize, func(batchIDs []uint) error { + err := mysql.BatchProcessSimple(ids, batchSize, func(batchIDs []uint) error {server/datastore/mysql/errors.go (1)
20-24: Critical: Update package references to match new import.The import was changed to
github.com/fleetdm/fleet/v4/server/platform/mysql(line 16), which provides the package namemysql, but lines 20-21 still referencecommon_mysql.NotFoundError. This will cause compilation errors.🔧 Required fix
-func notFound(kind string) *common_mysql.NotFoundError { - return &common_mysql.NotFoundError{ +func notFound(kind string) *mysql.NotFoundError { + return &mysql.NotFoundError{ ResourceType: kind, } }server/mdm/nanomdm/storage/mysql/mysql.go (1)
341-341: Critical: Update package reference to match new import.The import was changed to
github.com/fleetdm/fleet/v4/server/platform/mysql(line 16), which provides the package namemysql, but this line still referencescommon_mysql.WithRetryTxx. This will cause a compilation error.🔧 Required fix
- err = common_mysql.WithRetryTxx(ctx, sqlx.NewDb(s.db, ""), func(tx sqlx.ExtContext) error { + err = mysql.WithRetryTxx(ctx, sqlx.NewDb(s.db, ""), func(tx sqlx.ExtContext) error {
🤖 Fix all issues with AI agents
In @ee/server/service/certificate_authorities_test.go:
- Line 21: The test import was changed to use the package name mysql but the
code still calls common_mysql.NotFound(...) causing undefined/unused import
errors; restore the import alias by importing the package with the alias
common_mysql (i.e., import common_mysql
"github.com/fleetdm/fleet/v4/server/platform/mysql") or alternatively update all
calls to common_mysql.NotFound to mysql.NotFound so the symbol and import match;
update references at the noted call sites (functions/lines that call
common_mysql.NotFound) to keep the import and usage consistent.
In @ee/server/service/condaccess/idp_test.go:
- Line 14: The test imports the moved package under the plain name mysql but the
code still calls common_mysql.NotFound("certificate"), leaving common_mysql
undefined and mysql unused; update the import to use the original alias (e.g.,
common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql") or rename all
call sites to use mysql.NotFound so the symbol resolves; apply the same alias
fix to the other occurrences noted around lines 307-309 that reference
common_mysql.NotFound.
In @ee/server/service/hostidentity/depot/depot.go:
- Around line 21-22: The compile error is caused by calling
common_mysql.WithRetryTxx while the import is named mysql; in the Put method
replace common_mysql.WithRetryTxx(...) with mysql.WithRetryTxx(...) (keeping the
same arguments, including context.Background()), so the call uses the imported
github.com/fleetdm/fleet/v4/server/platform/mysql package; ensure the Put
function references mysql.WithRetryTxx and no other behavioral changes are made.
In @ee/server/service/request_certificate_test.go:
- Line 20: The test imports were changed to platform/mysql but code still uses
the old symbol common_mysql.NotFound; update all references to the undefined
symbol (e.g., common_mysql.NotFound) to use the imported package name
(mysql.NotFound) or adjust the import alias to match the symbol used in the file
(for example, alias platform/mysql as common_mysql) so the symbol resolves in
request_certificate_test.go (ensure you update the reference around the
assertion at the NotFound check and any other occurrences).
In @ee/server/service/software_installers.go:
- Around line 30-31: The import uses the package name mysql but the code still
calls common_mysql.NotFound; update all references of common_mysql.* in this
file to use mysql.* (e.g., change common_mysql.NotFound(...) to
mysql.NotFound(...)) so the unaliased import is actually used, and scan the file
for any other lingering common_mysql identifiers and replace them with mysql.
In @server/datastore/mysql/android_device_test.go:
- Around line 13-15: The tests reference common_mysql.NotFound and
common_mysql.NotFoundError but the import was changed to an unaliased path;
update the import list in android_device_test.go (and also
software_installers_test.go and policies_test.go) to alias
github.com/fleetdm/fleet/v4/server/platform/mysql as common_mysql so all
existing references to common_mysql.NotFound and common_mysql.NotFoundError
resolve to the new package name.
In @server/datastore/mysql/config.go:
- Around line 6-10: The file imports server/platform/mysql but the code still
references common_mysql.DBOptions (e.g., in DBOption and helpers), causing
undefined symbol and unused import errors; change the import statement to alias
the package as common_mysql (import common_mysql
"github.com/fleetdm/fleet/v4/server/platform/mysql") so all existing references
to common_mysql.DBOptions compile, and remove any unused plain mysql import
references if present.
In @server/datastore/mysql/hosts.go:
- Line 23: The import for platform/mysql must be aliased to common_mysql because
the code uses common_mysql.DefaultNonZeroTime in this file; update the import
line that currently imports "github.com/fleetdm/fleet/v4/server/platform/mysql"
to use the alias common_mysql (e.g., common_mysql
"github.com/fleetdm/fleet/v4/server/platform/mysql") so references to
common_mysql.DefaultNonZeroTime compile.
In
@server/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.go:
- Around line 9-13: The code still calls common_mysql.BatchProcessSimple but the
import for server/platform/mysql was added without the common_mysql alias (and
the plain mysql import is now unused); change the import line that currently
imports server/platform/mysql to use the alias common_mysql (e.g., import
common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql") so
common_mysql.BatchProcessSimple resolves, and remove any unused plain mysql
import to keep the build clean.
In @server/datastore/mysql/mysql.go:
- Line 33: The file imports the package as
"github.com/fleetdm/fleet/v4/server/platform/mysql" but the code still
references the old package name `common_mysql`; replace all `common_mysql`
identifiers with `mysql` throughout the file (e.g., change types and usages like
`common_mysql.MysqlConfig`, `common_mysql.TxFn`, `common_mysql.WithRetryTxx`,
`common_mysql.WithTxx`, `common_mysql.ReadTxFn`, `common_mysql.WithReadOnlyTxx`,
`common_mysql.DBConnections`, `common_mysql.DBOptions`, `common_mysql.NewDB`,
`common_mysql.LoggingConfig`, `common_mysql.SanitizeColumn`, and
`common_mysql.AppendListOptionsWithParams` to use `mysql`), ensuring every
occurrence is updated so the import and references match.
In @server/datastore/mysql/nanomdm_storage.go:
- Line 20: The import path was changed to mysql but the code still calls
common_mysql.WithRetryTxx; update every call site to use mysql.WithRetryTxx
instead of common_mysql.WithRetryTxx (i.e., replace the symbol
common_mysql.WithRetryTxx with mysql.WithRetryTxx in nanomdm_storage.go for all
occurrences).
In @server/datastore/mysql/policies_test.go:
- Line 18: The test no longer compiles because the import was changed to
"github.com/fleetdm/fleet/v4/server/platform/mysql" but call sites still
reference the old alias common_mysql and the new mysql import is unused; fix by
either restoring the explicit alias (change the import to common_mysql
"github.com/fleetdm/fleet/v4/server/platform/mysql" and remove the unused plain
mysql import) or update all usages of common_mysql in policies_test.go (and the
other affected files) to use the current package identifier (mysql), ensuring
there are no unused imports left.
In @server/datastore/mysql/software_installers_test.go:
- Around line 19-21: The test references common_mysql.NotFoundError but the
package was renamed; update the imports so the existing references work by
aliasing the new import "github.com/fleetdm/fleet/v4/server/platform/mysql" as
common_mysql (so common_mysql.NotFoundError resolves) in
software_installers_test.go (affecting testDeleteSoftwareInstallers and the
other occurrences noted); remove any unused import warnings by replacing the
unused direct mysql import with the aliased common_mysql import.
In @server/datastore/mysql/software_titles.go:
- Line 15: The code still references common_mysql.BatchProcessSimple even though
the import was changed to "github.com/fleetdm/fleet/v4/server/platform/mysql";
replace all uses of common_mysql.BatchProcessSimple (e.g., the call in the
function around the software titles processing at Line ~352) with
mysql.BatchProcessSimple and ensure any other references to common_mysql are
updated to mysql to match the current import name.
In @server/datastore/mysql/software.go:
- Line 19: The import currently bringing in
"github.com/fleetdm/fleet/v4/server/platform/mysql" must be aliased to
common_mysql so existing call sites using common_mysql.RetryableError and
common_mysql.BatchProcessSimple continue to compile; update the import to use
the alias common_mysql and remove any unused plain mysql import to avoid name
conflict with the package itself.
In @server/datastore/mysql/testing_utils.go:
- Around line 34-35: The code imports the package as "mysql" but still
references "common_mysql", causing undefined symbol errors; update all
references of common_mysql (e.g., common_mysql.TestSQLMode,
common_mysql.DBOptions) to use the imported alias "mysql" (mysql.TestSQLMode,
mysql.DBOptions), or alternatively change the import to use the alias
`common_mysql` if you prefer that name—make the reference and import names
consistent (affecting TestSQLMode and DBOptions usages).
In @server/mdm/android/service/pubsub_test.go:
- Line 18: Update all references to the old import alias common_mysql to the new
import alias mysql: replace occurrences like common_mysql.NotFound(...) with
mysql.NotFound(...) throughout the file (e.g., the calls to NotFound used in
tests/assertions such as common_mysql.NotFound at the spots where errors are
compared). Ensure every reference (previously common_mysql.NotFound at multiple
locations) is changed to mysql.NotFound so the code compiles against the updated
import.
In @server/mdm/nanomdm/storage/mysql/queue.go:
- Line 12: The code still refers to the old package alias common_mysql but the
import uses mysql; update all references to the new package name (replace
common_mysql.WithRetryTxx and common_mysql.BatchProcessSimple with
mysql.WithRetryTxx and mysql.BatchProcessSimple) so the calls at the spots using
WithRetryTxx (around the function referencing retry wrappers) and
BatchProcessSimple (the batch processing call) use the imported mysql package
symbol; ensure the import line remains `mysql` (or add an explicit alias
`common_mysql` if you prefer the old name) and adjust any usages accordingly so
names match.
In @server/service/osquery_utils/queries_test.go:
- Around line 31-32: Tests currently reference common_mysql.NotFound(...) but
the import was changed to github.com/fleetdm/fleet/v4/server/platform/mysql,
leaving common_mysql undefined and mysql unused; update all occurrences of
common_mysql.NotFound(...) to mysql.NotFound(...) (e.g., in
TestDirectIngestMDMWindows, LUKS tests, SCIM-related tests) so the tests compile
and use the new package name, and remove any remaining unused imports if
necessary.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
changes/37244-refactor-common_mysqlee/server/service/certificate_authorities_test.goee/server/service/condaccess/idp_test.goee/server/service/hostidentity/depot/depot.goee/server/service/request_certificate_test.goee/server/service/software_installers.goserver/datastore/mysql/android.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/android_mysql.goserver/datastore/mysql/android_test.goserver/datastore/mysql/apple_mdm.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/certificate_authorities.goserver/datastore/mysql/common_mysql/arch_test.goserver/datastore/mysql/config.goserver/datastore/mysql/errors.goserver/datastore/mysql/host_identity_scep.goserver/datastore/mysql/hosts.goserver/datastore/mysql/hosts_test.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/migrations_test.goserver/datastore/mysql/mysql.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/nanomdm_storage.goserver/datastore/mysql/operating_system_vulnerabilities.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/policies.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/scripts_test.goserver/datastore/mysql/software.goserver/datastore/mysql/software_installers_test.goserver/datastore/mysql/software_titles.goserver/datastore/mysql/testing_utils.goserver/datastore/mysql/vulnerabilities_test.goserver/mdm/android/service/pubsub_test.goserver/mdm/nanomdm/storage/mysql/mysql.goserver/mdm/nanomdm/storage/mysql/queue.goserver/platform/README.mdserver/platform/arch_test.goserver/platform/mysql/batch.goserver/platform/mysql/batch_test.goserver/platform/mysql/common.goserver/platform/mysql/config.goserver/platform/mysql/errors.goserver/platform/mysql/list_options.goserver/platform/mysql/retry.goserver/platform/mysql/testing_utils/testing_utils.goserver/service/integrationtest/suite.goserver/service/labels_test.goserver/service/osquery_utils/queries_test.goserver/service/software_title_icons_test.gotools/mysql-tests/rds/iam_auth.go
💤 Files with no reviewable changes (1)
- server/datastore/mysql/common_mysql/arch_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
When reviewing SQL queries that are added or modified, ensure that appropriate filtering criteria are applied—especially when a query is intended to return data for a specific entity (e.g., a single host). Check for missing WHERE clauses or incorrect filtering that could lead to incorrect or non-deterministic results (e.g., returning the first row instead of the correct one). Flag any queries that may return unintended results due to lack of precise scoping.
Files:
server/datastore/mysql/scripts_test.goserver/datastore/mysql/testing_utils.goserver/mdm/nanomdm/storage/mysql/queue.goserver/platform/arch_test.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/migrations_test.goserver/datastore/mysql/android_test.goserver/service/labels_test.goee/server/service/request_certificate_test.goserver/service/integrationtest/suite.goserver/datastore/mysql/software_installers_test.goserver/service/osquery_utils/queries_test.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/host_identity_scep.goserver/datastore/mysql/mysql.goserver/datastore/mysql/hosts.goee/server/service/software_installers.goserver/datastore/mysql/operating_system_vulnerabilities.goee/server/service/condaccess/idp_test.goserver/mdm/android/service/pubsub_test.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/apple_mdm.goserver/mdm/nanomdm/storage/mysql/mysql.goee/server/service/certificate_authorities_test.goserver/platform/mysql/testing_utils/testing_utils.goserver/service/software_title_icons_test.goserver/datastore/mysql/android_mysql.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/nanomdm_storage.goserver/datastore/mysql/policies.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/android.goserver/datastore/mysql/certificate_authorities.goserver/datastore/mysql/software_titles.gotools/mysql-tests/rds/iam_auth.goserver/datastore/mysql/config.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/errors.goserver/datastore/mysql/hosts_test.goee/server/service/hostidentity/depot/depot.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/vulnerabilities_test.goserver/datastore/mysql/software.go
🧠 Learnings (14)
📚 Learning: 2025-08-08T07:40:05.301Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 31726
File: server/datastore/mysql/labels_test.go:2031-2031
Timestamp: 2025-08-08T07:40:05.301Z
Learning: Fleet repo targets Go 1.24.5 (root go.mod), which supports testing.T.Context(). Do not flag usage of t.Context() or suggest replacing it with context.Background() in tests (e.g., server/datastore/mysql/labels_test.go Line 2031 and similar).
Applied to files:
server/datastore/mysql/scripts_test.goserver/datastore/mysql/testing_utils.goserver/mdm/nanomdm/storage/mysql/queue.goserver/platform/arch_test.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/migrations_test.gochanges/37244-refactor-common_mysqlserver/datastore/mysql/android_test.goserver/service/labels_test.goee/server/service/request_certificate_test.goserver/service/integrationtest/suite.goserver/datastore/mysql/software_installers_test.goserver/service/osquery_utils/queries_test.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/mysql.goserver/datastore/mysql/hosts.goee/server/service/software_installers.goserver/datastore/mysql/operating_system_vulnerabilities.goee/server/service/condaccess/idp_test.goserver/mdm/android/service/pubsub_test.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/apple_mdm.goserver/mdm/nanomdm/storage/mysql/mysql.goee/server/service/certificate_authorities_test.goserver/platform/mysql/testing_utils/testing_utils.goserver/service/software_title_icons_test.goserver/datastore/mysql/android_mysql.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/nanomdm_storage.goserver/datastore/mysql/policies.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/android.goserver/datastore/mysql/certificate_authorities.goserver/datastore/mysql/software_titles.gotools/mysql-tests/rds/iam_auth.goserver/datastore/mysql/config.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/errors.goserver/datastore/mysql/hosts_test.goee/server/service/hostidentity/depot/depot.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/vulnerabilities_test.goserver/datastore/mysql/software.go
📚 Learning: 2025-08-08T07:40:05.301Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 31726
File: server/datastore/mysql/labels_test.go:2031-2031
Timestamp: 2025-08-08T07:40:05.301Z
Learning: In fleetdm/fleet repository tests (server/datastore/mysql/labels_test.go and similar), using testing.T.Context() is valid because the project targets a recent Go version where testing.T.Context() exists. Do not suggest replacing t.Context() with context.Background() in this codebase.
Applied to files:
server/datastore/mysql/scripts_test.goserver/datastore/mysql/testing_utils.goserver/mdm/nanomdm/storage/mysql/queue.goserver/platform/arch_test.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/migrations_test.gochanges/37244-refactor-common_mysqlserver/datastore/mysql/android_test.goserver/service/labels_test.goee/server/service/request_certificate_test.goserver/service/integrationtest/suite.goserver/datastore/mysql/software_installers_test.goserver/service/osquery_utils/queries_test.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/host_identity_scep.goserver/datastore/mysql/mysql.goserver/datastore/mysql/hosts.goee/server/service/software_installers.goserver/datastore/mysql/operating_system_vulnerabilities.goee/server/service/condaccess/idp_test.goserver/mdm/android/service/pubsub_test.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/apple_mdm.goserver/mdm/nanomdm/storage/mysql/mysql.goee/server/service/certificate_authorities_test.goserver/platform/mysql/testing_utils/testing_utils.goserver/service/software_title_icons_test.goserver/datastore/mysql/android_mysql.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/nanomdm_storage.goserver/datastore/mysql/policies.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/android.goserver/datastore/mysql/certificate_authorities.goserver/datastore/mysql/software_titles.gotools/mysql-tests/rds/iam_auth.goserver/datastore/mysql/config.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/errors.goserver/datastore/mysql/hosts_test.goee/server/service/hostidentity/depot/depot.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/vulnerabilities_test.goserver/datastore/mysql/software.go
📚 Learning: 2025-08-01T15:08:16.858Z
Learnt from: sgress454
Repo: fleetdm/fleet PR: 31508
File: server/datastore/mysql/schema.sql:102-116
Timestamp: 2025-08-01T15:08:16.858Z
Learning: The schema.sql file in server/datastore/mysql/ is auto-generated from migrations for use with tests, so it cannot be manually edited. Any changes must be made through migrations.
Applied to files:
server/datastore/mysql/scripts_test.goserver/datastore/mysql/migrations_test.go
📚 Learning: 2025-08-13T18:20:42.136Z
Learnt from: titanous
Repo: fleetdm/fleet PR: 31075
File: tools/redis-tests/elasticache/iam_auth.go:4-10
Timestamp: 2025-08-13T18:20:42.136Z
Learning: For test harnesses and CLI tools in the Fleet codebase, resource cleanup on error paths (like closing connections before log.Fatalf) may not be necessary since the OS handles cleanup when the process exits. These tools prioritize simplicity over defensive programming patterns used in production code.
Applied to files:
server/datastore/mysql/scripts_test.goserver/mdm/nanomdm/storage/mysql/queue.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/migrations_test.goserver/datastore/mysql/android_test.goserver/service/labels_test.goserver/service/integrationtest/suite.goserver/datastore/mysql/software_installers_test.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/operating_system_vulnerabilities.goee/server/service/condaccess/idp_test.goserver/datastore/mysql/policies_test.goserver/mdm/nanomdm/storage/mysql/mysql.goserver/platform/mysql/testing_utils/testing_utils.goserver/service/software_title_icons_test.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/policies.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/android.goserver/datastore/mysql/certificate_authorities.goserver/platform/README.mdserver/datastore/mysql/software_titles.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/errors.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/vulnerabilities_test.goserver/datastore/mysql/software.go
📚 Learning: 2025-08-08T07:40:05.301Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 31726
File: server/datastore/mysql/labels_test.go:2031-2031
Timestamp: 2025-08-08T07:40:05.301Z
Learning: In fleetdm/fleet, tests may validly use testing.T.Context() when the module/toolchain targets Go 1.24+. Do not flag t.Context() usage in this codebase if go.mod/toolchain indicates Go >= 1.24.
Applied to files:
server/datastore/mysql/testing_utils.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/migrations_test.goserver/service/labels_test.goserver/service/integrationtest/suite.goserver/datastore/mysql/software_installers_test.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/android_mysql.goserver/datastore/mysql/nanomdm_storage.goserver/datastore/mysql/policies.goserver/datastore/mysql/android.goserver/datastore/mysql/certificate_authorities.goserver/datastore/mysql/software_titles.goserver/datastore/mysql/android_device_test.go
📚 Learning: 2025-07-08T16:12:48.797Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 30589
File: ee/server/service/hostidentity/depot/depot.go:108-111
Timestamp: 2025-07-08T16:12:48.797Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the SCEP depot interface methods like Put() do not accept context parameters, and the common_mysql.WithRetryTxx callback function type TxFn only receives a transaction parameter, not a context. Therefore, using context.Background() in tx.ExecContext calls within the transaction callback is the correct approach.
Applied to files:
server/datastore/mysql/host_identity_scep.goserver/datastore/mysql/android_mysql.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/nanomdm_storage.goserver/datastore/mysql/operating_systems.goee/server/service/hostidentity/depot/depot.go
📚 Learning: 2025-07-08T16:06:54.576Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 30589
File: ee/server/service/hostidentity/depot/depot.go:104-119
Timestamp: 2025-07-08T16:06:54.576Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the security concern where shared challenges allow certificate revocation (lines 104-119) is a known issue that will be addressed in a later feature, not an immediate concern to fix.
Applied to files:
server/datastore/mysql/host_identity_scep.goee/server/service/hostidentity/depot/depot.go
📚 Learning: 2025-08-20T21:24:59.261Z
Learnt from: sgress454
Repo: fleetdm/fleet PR: 31075
File: server/datastore/mysql/common_mysql/aws_iam_auth.go:111-130
Timestamp: 2025-08-20T21:24:59.261Z
Learning: In Fleet's MySQL IAM authentication implementation, the TLS and AllowCleartextPasswords configuration is handled in the generateMysqlConnectionString function in server/datastore/mysql/common_mysql/common.go, not in the awsIAMAuthConnector.Connect method. The connector receives a base DSN that already includes the necessary IAM authentication parameters.
Applied to files:
server/datastore/mysql/host_identity_scep.goserver/datastore/mysql/mysql.gotools/mysql-tests/rds/iam_auth.goserver/datastore/mysql/errors.goserver/datastore/mysql/apple_mdm_test.go
📚 Learning: 2025-07-08T16:11:49.555Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 30589
File: ee/server/service/hostidentity/depot/depot.go:115-115
Timestamp: 2025-07-08T16:11:49.555Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the error from result.RowsAffected() is intentionally ignored because the information is only used for logging purposes, not for critical program logic.
Applied to files:
server/datastore/mysql/host_identity_scep.goee/server/service/hostidentity/depot/depot.go
📚 Learning: 2025-12-11T23:02:59.284Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 36978
File: server/mdm/android/service/profiles.go:441-449
Timestamp: 2025-12-11T23:02:59.284Z
Learning: Fleet’s server/mdm/android/service.Service has fields ds (type fleet.AndroidDatastore) and fleetDS (type fleet.Datastore). The fleet.Datastore interface embeds AndroidDatastore (server/fleet/datastore.go), so assigning r.DS (a fleet.Datastore) to Service.ds is valid in Go due to interface-to-interface assignment using the dynamic type.
Applied to files:
server/datastore/mysql/mysql.goserver/datastore/mysql/hosts.goee/server/service/software_installers.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/apple_mdm.goserver/datastore/mysql/hosts_test.goserver/datastore/mysql/apple_mdm_test.go
📚 Learning: 2025-12-11T23:02:52.191Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 36978
File: server/mdm/android/service/profiles.go:441-449
Timestamp: 2025-12-11T23:02:52.191Z
Learning: In server/mdm/android/service, Go allows assigning an interface value of type fleet.Datastore to a field ds of type fleet.Datastore because the underlying dynamic type implements both the embedded AndroidDatastore (via fleet.Datastore embedding AndroidDatastore). When reviewing code in profiles.go and related files, verify assignments between interfaces respect embedding and ensure the concrete type actually implements all required interfaces. This pattern is valid wherever fleet.Datastore embeds AndroidDatastore and the receiver expects fleetDS or similar fields; prefer keeping interface boundaries clear and rely on the dynamic type to satisfy both interfaces.
Applied to files:
server/mdm/android/service/pubsub_test.go
📚 Learning: 2025-10-03T18:16:11.482Z
Learnt from: MagnusHJensen
Repo: fleetdm/fleet PR: 33805
File: server/service/integration_mdm_test.go:1248-1251
Timestamp: 2025-10-03T18:16:11.482Z
Learning: In server/service/integration_mdm_test.go, the helper createAppleMobileHostThenEnrollMDM(platform string) is exclusively for iOS/iPadOS hosts (mobile). Do not flag macOS model/behavior issues based on changes within this helper; macOS provisioning uses different helpers such as createHostThenEnrollMDM.
Applied to files:
server/datastore/mysql/apple_mdm.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/apple_mdm_test.go
📚 Learning: 2025-12-31T23:23:07.852Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 37245
File: server/datastore/mysql/common_mysql/list_options.go:56-73
Timestamp: 2025-12-31T23:23:07.852Z
Learning: In server/datastore/mysql/common_mysql/list_options.go, the strings.HasSuffix(orderKey, "id") check at line 62 never matches because orderKey is already sanitized with backticks, making the integer conversion code at line 63 unreachable dead code. However, this doesn't cause functional issues because MySQL automatically coerces string parameters to integers when comparing with integer columns in WHERE clauses.
Applied to files:
server/datastore/mysql/software_titles.go
📚 Learning: 2025-08-20T21:24:59.261Z
Learnt from: sgress454
Repo: fleetdm/fleet PR: 31075
File: server/datastore/mysql/common_mysql/aws_iam_auth.go:111-130
Timestamp: 2025-08-20T21:24:59.261Z
Learning: In Fleet's MySQL IAM authentication implementation, the TLS and AllowCleartextPasswords configuration is handled in the generateMysqlConnectionString function in server/datastore/mysql/common_mysql/common.go. When no password is configured and the endpoint is an RDS endpoint, the function automatically sets allowCleartextPasswords=true and configures appropriate TLS settings. The awsIAMAuthConnector receives a properly configured base DSN and only needs to inject the IAM token as the password.
Applied to files:
tools/mysql-tests/rds/iam_auth.go
🧬 Code graph analysis (1)
server/platform/arch_test.go (2)
server/ptr/ptr.go (1)
T(86-88)server/archtest/archtest.go (1)
NewPackageTest(43-45)
🔇 Additional comments (16)
server/datastore/mysql/android_mysql.go (1)
10-10: Critical: Import package identifier mismatch with usage below.The import at line 10 uses the default package name
mysql, but lines 47-52 referencecommon_mysql.TxFn,common_mysql.WithRetryTxx, andcommon_mysql.WithTxx. This code will not compile.Resolution options:
- Add an import alias:
common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"- OR update all references below from
common_mysql.*tomysql.*⛔ Skipped due to learnings
Learnt from: getvictor Repo: fleetdm/fleet PR: 31726 File: server/datastore/mysql/labels_test.go:2031-2031 Timestamp: 2025-08-08T07:40:05.301Z Learning: In fleetdm/fleet repository tests (server/datastore/mysql/labels_test.go and similar), using testing.T.Context() is valid because the project targets a recent Go version where testing.T.Context() exists. Do not suggest replacing t.Context() with context.Background() in this codebase.Learnt from: getvictor Repo: fleetdm/fleet PR: 37245 File: server/datastore/mysql/common_mysql/list_options.go:56-73 Timestamp: 2025-12-31T23:23:07.852Z Learning: In server/datastore/mysql/common_mysql/list_options.go, the strings.HasSuffix(orderKey, "id") check at line 62 never matches because orderKey is already sanitized with backticks, making the integer conversion code at line 63 unreachable dead code. However, this doesn't cause functional issues because MySQL automatically coerces string parameters to integers when comparing with integer columns in WHERE clauses.Learnt from: getvictor Repo: fleetdm/fleet PR: 31726 File: server/datastore/mysql/labels_test.go:2031-2031 Timestamp: 2025-08-08T07:40:05.301Z Learning: Fleet repo targets Go 1.24.5 (root go.mod), which supports testing.T.Context(). Do not flag usage of t.Context() or suggest replacing it with context.Background() in tests (e.g., server/datastore/mysql/labels_test.go Line 2031 and similar).Learnt from: sgress454 Repo: fleetdm/fleet PR: 31075 File: server/datastore/mysql/common_mysql/aws_iam_auth.go:111-130 Timestamp: 2025-08-20T21:24:59.261Z Learning: In Fleet's MySQL IAM authentication implementation, the TLS and AllowCleartextPasswords configuration is handled in the generateMysqlConnectionString function in server/datastore/mysql/common_mysql/common.go, not in the awsIAMAuthConnector.Connect method. The connector receives a base DSN that already includes the necessary IAM authentication parameters.server/datastore/mysql/android.go (1)
11-19: Import path refactor to platform/mysql looks correctThe updated import to
github.com/fleetdm/fleet/v4/server/platform/mysqlis consistent with the common_mysql → platform/mysql move and doesn’t alter any call sites, so behavior should remain unchanged as long as the moved package preserves the same API.server/datastore/mysql/android_test.go (1)
11-19: Test import updated to new platform/mysql locationSwitching the MySQL helper import to
github.com/fleetdm/fleet/v4/server/platform/mysqlmatches the refactor and leaves test logic intact.server/datastore/mysql/scripts_test.go (1)
12-20: Scripts tests correctly updated to use platform/mysqlThe import now points at
github.com/fleetdm/fleet/v4/server/platform/mysql, which aligns with the common_mysql relocation without impacting the test behavior.server/datastore/mysql/host_identity_scep.go (1)
13-16: Host-identity SCEP datastore now imports platform/mysqlUpdating the helper import to
github.com/fleetdm/fleet/v4/server/platform/mysqlkeeps the retry/transaction behavior the same while matching the new package location.server/datastore/mysql/certificate_authorities.go (1)
11-16: Certificate authority datastore correctly switched to platform/mysqlThe import now targets
github.com/fleetdm/fleet/v4/server/platform/mysql, which matches the common_mysql move; the rest of the CA logic remains untouched.server/platform/README.md (1)
1-16: Platform README clearly documents architecture boundariesThe description of platform vs domain responsibilities and the “platform must not import domain” rule is clear and matches the new arch tests. No changes needed.
changes/37244-refactor-common_mysql (1)
1-1: Changelog entry matches the refactorThe note correctly summarizes moving
common_mysqltoserver/platform/mysql. Looks good.server/service/labels_test.go (1)
14-18: Import migration toplatform/mysql/testing_utilslooks correctThe test now pulls
DatastoreTestOptionsandCreateMySQLDSWithOptionsfromserver/platform/mysql/testing_utils, and the call sites compile cleanly and preserve behavior. No further changes needed here.Also applies to: 434-436
server/datastore/mysql/migrations_test.go (1)
12-12: LGTM! Import path updated correctly.The import path has been successfully updated to
server/platform/mysql/testing_utils, and all references to thetesting_utilspackage throughout the file remain correct.server/platform/mysql/testing_utils/testing_utils.go (1)
12-12: Critical: Package references not updated after import change.The import path was updated to
server/platform/mysqlat line 12, but the code still usescommon_mysqlreferences:
- Line 49:
common_mysql.WithTxxshould bemysql.WithTxx- Lines 167-174: Function returns
*common_mysql.MysqlConfigshould be*mysql.MysqlConfigThese will cause compilation failures.
🔧 Required fixes
- require.NoError(t, common_mysql.WithTxx(ctx, db, func(tx sqlx.ExtContext) error { + require.NoError(t, mysql.WithTxx(ctx, db, func(tx sqlx.ExtContext) error {-func MysqlTestConfig(testName string) *common_mysql.MysqlConfig { - return &common_mysql.MysqlConfig{ +func MysqlTestConfig(testName string) *mysql.MysqlConfig { + return &mysql.MysqlConfig{Also applies to: 49-49, 167-174
⛔ Skipped due to learnings
Learnt from: getvictor Repo: fleetdm/fleet PR: 31726 File: server/datastore/mysql/labels_test.go:2031-2031 Timestamp: 2025-08-08T07:40:05.301Z Learning: In fleetdm/fleet repository tests (server/datastore/mysql/labels_test.go and similar), using testing.T.Context() is valid because the project targets a recent Go version where testing.T.Context() exists. Do not suggest replacing t.Context() with context.Background() in this codebase.Learnt from: getvictor Repo: fleetdm/fleet PR: 31726 File: server/datastore/mysql/labels_test.go:2031-2031 Timestamp: 2025-08-08T07:40:05.301Z Learning: Fleet repo targets Go 1.24.5 (root go.mod), which supports testing.T.Context(). Do not flag usage of t.Context() or suggest replacing it with context.Background() in tests (e.g., server/datastore/mysql/labels_test.go Line 2031 and similar).Learnt from: sgress454 Repo: fleetdm/fleet PR: 31075 File: server/datastore/mysql/common_mysql/aws_iam_auth.go:111-130 Timestamp: 2025-08-20T21:24:59.261Z Learning: In Fleet's MySQL IAM authentication implementation, the TLS and AllowCleartextPasswords configuration is handled in the generateMysqlConnectionString function in server/datastore/mysql/common_mysql/common.go, not in the awsIAMAuthConnector.Connect method. The connector receives a base DSN that already includes the necessary IAM authentication parameters.server/service/integrationtest/suite.go (1)
12-12: LGTM! Import path refactored correctly.The import path has been successfully updated to reference the new platform location, and all usages in the file remain consistent.
server/datastore/mysql/mysql_test.go (1)
25-25: LGTM! Import path updated correctly.The testing utilities import has been successfully migrated to the platform package location.
server/datastore/mysql/android_enterprise_test.go (1)
3-11: Import path migration for testing_utils looks correctThe switch to
server/platform/mysql/testing_utilswhile continuing to calltesting_utils.TruncateTablesis consistent with the package move and keeps the test behavior unchanged.Also applies to: 25-31
server/platform/arch_test.go (1)
93-110: LGTM! Well-structured architecture test.The new test properly enforces that the
platform/mysqlpackage maintains decoupling from Fleet domain packages, consistent with the other platform package tests in this file.server/datastore/mysql/operating_systems.go (1)
10-10: The code is correct as-is and requires no changes.In Go, when you import a package without an alias, the package is referenced by its package declaration, not by its directory name or import path. Since all files in
server/platform/mysql/declarepackage common_mysql, the import statement"github.com/fleetdm/fleet/v4/server/platform/mysql"at line 10 automatically makes the package available ascommon_mysql. This is whycommon_mysql.DoRetryErrat line 102 compiles correctly and the import is actively used.Likely an incorrect or invalid review comment.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/platform/mysql/list_options.go (1)
1-23: ListOptions interface + helpers look correct for platform boundaryThe new
ListOptionsinterface andAppendListOptionsWithParamsmirror the previous pagination behavior while removing the direct dependency onfleet.ListOptions, which is appropriate forserver/platform/mysql. Cursor predicate, ordering, and LIMIT/OFFSET are applied safely on sanitized column names.Optionally, you might update the comment on Line 13 to say “this package” instead of “common_mysql package” to avoid confusion, but that’s purely cosmetic.
Also applies to: 53-103
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
changes/37244-refactor-common_mysqlee/server/service/certificate_authorities_test.goee/server/service/condaccess/idp_test.goee/server/service/hostidentity/depot/depot.goee/server/service/request_certificate_test.goee/server/service/software_installers.goserver/datastore/mysql/android.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/android_mysql.goserver/datastore/mysql/android_test.goserver/datastore/mysql/apple_mdm.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/certificate_authorities.goserver/datastore/mysql/common_mysql/arch_test.goserver/datastore/mysql/config.goserver/datastore/mysql/errors.goserver/datastore/mysql/host_identity_scep.goserver/datastore/mysql/hosts.goserver/datastore/mysql/hosts_test.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/migrations_test.goserver/datastore/mysql/mysql.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/nanomdm_storage.goserver/datastore/mysql/operating_system_vulnerabilities.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/policies.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/scripts_test.goserver/datastore/mysql/software.goserver/datastore/mysql/software_installers_test.goserver/datastore/mysql/software_titles.goserver/datastore/mysql/testing_utils.goserver/datastore/mysql/vulnerabilities_test.goserver/mdm/android/service/pubsub_test.goserver/mdm/nanomdm/storage/mysql/mysql.goserver/mdm/nanomdm/storage/mysql/queue.goserver/platform/README.mdserver/platform/arch_test.goserver/platform/mysql/batch.goserver/platform/mysql/batch_test.goserver/platform/mysql/common.goserver/platform/mysql/config.goserver/platform/mysql/errors.goserver/platform/mysql/list_options.goserver/platform/mysql/retry.goserver/platform/mysql/testing_utils/testing_utils.goserver/service/integrationtest/suite.goserver/service/labels_test.goserver/service/osquery_utils/queries_test.goserver/service/software_title_icons_test.gotools/mysql-tests/rds/iam_auth.go
💤 Files with no reviewable changes (1)
- server/datastore/mysql/common_mysql/arch_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
When reviewing SQL queries that are added or modified, ensure that appropriate filtering criteria are applied—especially when a query is intended to return data for a specific entity (e.g., a single host). Check for missing WHERE clauses or incorrect filtering that could lead to incorrect or non-deterministic results (e.g., returning the first row instead of the correct one). Flag any queries that may return unintended results due to lack of precise scoping.
Files:
server/platform/mysql/config.goserver/datastore/mysql/android_mysql.goee/server/service/hostidentity/depot/depot.goserver/service/integrationtest/suite.goee/server/service/request_certificate_test.goserver/datastore/mysql/vulnerabilities_test.goserver/datastore/mysql/policies.goee/server/service/condaccess/idp_test.goserver/datastore/mysql/host_identity_scep.goserver/platform/arch_test.goserver/datastore/mysql/nanomdm_storage.goserver/platform/mysql/common.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/software_installers_test.gotools/mysql-tests/rds/iam_auth.goserver/service/labels_test.goserver/datastore/mysql/android_test.goserver/datastore/mysql/migrations_test.goserver/service/software_title_icons_test.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/mysql.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/operating_system_vulnerabilities.goee/server/service/certificate_authorities_test.goserver/platform/mysql/list_options.goserver/platform/mysql/errors.goserver/datastore/mysql/scripts_test.goserver/datastore/mysql/apple_mdm.goserver/mdm/android/service/pubsub_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/config.goserver/mdm/nanomdm/storage/mysql/mysql.goserver/platform/mysql/batch.goserver/datastore/mysql/errors.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/policies_test.goserver/platform/mysql/batch_test.goserver/datastore/mysql/android.goserver/datastore/mysql/software.goserver/platform/mysql/testing_utils/testing_utils.goserver/datastore/mysql/certificate_authorities.goee/server/service/software_installers.goserver/platform/mysql/retry.goserver/datastore/mysql/software_titles.goserver/mdm/nanomdm/storage/mysql/queue.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/testing_utils.goserver/datastore/mysql/hosts.goserver/service/osquery_utils/queries_test.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/hosts_test.go
🧠 Learnings (14)
📚 Learning: 2025-08-08T07:40:05.301Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 31726
File: server/datastore/mysql/labels_test.go:2031-2031
Timestamp: 2025-08-08T07:40:05.301Z
Learning: In fleetdm/fleet repository tests (server/datastore/mysql/labels_test.go and similar), using testing.T.Context() is valid because the project targets a recent Go version where testing.T.Context() exists. Do not suggest replacing t.Context() with context.Background() in this codebase.
Applied to files:
server/datastore/mysql/android_mysql.goee/server/service/hostidentity/depot/depot.goserver/service/integrationtest/suite.goee/server/service/request_certificate_test.goserver/datastore/mysql/vulnerabilities_test.goserver/datastore/mysql/policies.goee/server/service/condaccess/idp_test.goserver/platform/arch_test.goserver/datastore/mysql/nanomdm_storage.goserver/platform/mysql/common.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/software_installers_test.gotools/mysql-tests/rds/iam_auth.goserver/service/labels_test.goserver/datastore/mysql/android_test.goserver/datastore/mysql/migrations_test.goserver/service/software_title_icons_test.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/mysql.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/operating_system_vulnerabilities.goee/server/service/certificate_authorities_test.goserver/datastore/mysql/scripts_test.goserver/datastore/mysql/apple_mdm.goserver/mdm/android/service/pubsub_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/config.goserver/mdm/nanomdm/storage/mysql/mysql.goserver/datastore/mysql/errors.gochanges/37244-refactor-common_mysqlserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/android.goserver/datastore/mysql/software.goserver/platform/mysql/testing_utils/testing_utils.goserver/datastore/mysql/certificate_authorities.goee/server/service/software_installers.goserver/platform/mysql/retry.goserver/datastore/mysql/software_titles.goserver/mdm/nanomdm/storage/mysql/queue.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/testing_utils.goserver/datastore/mysql/hosts.goserver/service/osquery_utils/queries_test.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/hosts_test.go
📚 Learning: 2025-08-08T07:40:05.301Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 31726
File: server/datastore/mysql/labels_test.go:2031-2031
Timestamp: 2025-08-08T07:40:05.301Z
Learning: Fleet repo targets Go 1.24.5 (root go.mod), which supports testing.T.Context(). Do not flag usage of t.Context() or suggest replacing it with context.Background() in tests (e.g., server/datastore/mysql/labels_test.go Line 2031 and similar).
Applied to files:
server/datastore/mysql/android_mysql.goee/server/service/hostidentity/depot/depot.goserver/service/integrationtest/suite.goee/server/service/request_certificate_test.goserver/datastore/mysql/vulnerabilities_test.goserver/datastore/mysql/policies.goee/server/service/condaccess/idp_test.goserver/platform/arch_test.goserver/datastore/mysql/nanomdm_storage.goserver/platform/mysql/common.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/software_installers_test.gotools/mysql-tests/rds/iam_auth.goserver/service/labels_test.goserver/datastore/mysql/android_test.goserver/datastore/mysql/migrations_test.goserver/service/software_title_icons_test.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/mysql.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/operating_system_vulnerabilities.goee/server/service/certificate_authorities_test.goserver/datastore/mysql/scripts_test.goserver/datastore/mysql/apple_mdm.goserver/mdm/android/service/pubsub_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/config.goserver/mdm/nanomdm/storage/mysql/mysql.goserver/datastore/mysql/errors.gochanges/37244-refactor-common_mysqlserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/android.goserver/datastore/mysql/software.goserver/platform/mysql/testing_utils/testing_utils.goserver/datastore/mysql/certificate_authorities.goee/server/service/software_installers.goserver/platform/mysql/retry.goserver/datastore/mysql/software_titles.goserver/mdm/nanomdm/storage/mysql/queue.goserver/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.goserver/datastore/mysql/testing_utils.goserver/datastore/mysql/hosts.goserver/service/osquery_utils/queries_test.goserver/datastore/mysql/android_enterprises.goserver/datastore/mysql/hosts_test.go
📚 Learning: 2025-08-08T07:40:05.301Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 31726
File: server/datastore/mysql/labels_test.go:2031-2031
Timestamp: 2025-08-08T07:40:05.301Z
Learning: In fleetdm/fleet, tests may validly use testing.T.Context() when the module/toolchain targets Go 1.24+. Do not flag t.Context() usage in this codebase if go.mod/toolchain indicates Go >= 1.24.
Applied to files:
server/datastore/mysql/android_mysql.goserver/service/integrationtest/suite.goserver/datastore/mysql/policies.goserver/datastore/mysql/nanomdm_storage.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/software_installers_test.goserver/service/labels_test.goserver/datastore/mysql/migrations_test.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/mysql_test.goserver/datastore/mysql/android.goserver/datastore/mysql/software_titles.goserver/datastore/mysql/testing_utils.goserver/service/osquery_utils/queries_test.go
📚 Learning: 2025-07-08T16:12:48.797Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 30589
File: ee/server/service/hostidentity/depot/depot.go:108-111
Timestamp: 2025-07-08T16:12:48.797Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the SCEP depot interface methods like Put() do not accept context parameters, and the common_mysql.WithRetryTxx callback function type TxFn only receives a transaction parameter, not a context. Therefore, using context.Background() in tx.ExecContext calls within the transaction callback is the correct approach.
Applied to files:
server/datastore/mysql/android_mysql.goee/server/service/hostidentity/depot/depot.goserver/datastore/mysql/nanomdm_storage.goserver/platform/mysql/common.goserver/platform/mysql/retry.goserver/datastore/mysql/android_enterprises.go
📚 Learning: 2025-07-08T16:11:49.555Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 30589
File: ee/server/service/hostidentity/depot/depot.go:115-115
Timestamp: 2025-07-08T16:11:49.555Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the error from result.RowsAffected() is intentionally ignored because the information is only used for logging purposes, not for critical program logic.
Applied to files:
ee/server/service/hostidentity/depot/depot.go
📚 Learning: 2025-07-08T16:06:54.576Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 30589
File: ee/server/service/hostidentity/depot/depot.go:104-119
Timestamp: 2025-07-08T16:06:54.576Z
Learning: In ee/server/service/hostidentity/depot/depot.go, the security concern where shared challenges allow certificate revocation (lines 104-119) is a known issue that will be addressed in a later feature, not an immediate concern to fix.
Applied to files:
ee/server/service/hostidentity/depot/depot.goee/server/service/request_certificate_test.goee/server/service/condaccess/idp_test.goee/server/service/certificate_authorities_test.go
📚 Learning: 2025-08-13T18:20:42.136Z
Learnt from: titanous
Repo: fleetdm/fleet PR: 31075
File: tools/redis-tests/elasticache/iam_auth.go:4-10
Timestamp: 2025-08-13T18:20:42.136Z
Learning: For test harnesses and CLI tools in the Fleet codebase, resource cleanup on error paths (like closing connections before log.Fatalf) may not be necessary since the OS handles cleanup when the process exits. These tools prioritize simplicity over defensive programming patterns used in production code.
Applied to files:
server/service/integrationtest/suite.goserver/datastore/mysql/vulnerabilities_test.goserver/datastore/mysql/policies.goee/server/service/condaccess/idp_test.goserver/datastore/mysql/android_enterprise_test.goserver/datastore/mysql/software_installers_test.goserver/service/labels_test.goserver/datastore/mysql/android_test.goserver/datastore/mysql/migrations_test.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/operating_systems.goserver/datastore/mysql/operating_system_vulnerabilities.goserver/platform/README.mdserver/datastore/mysql/scripts_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/mysql_test.goserver/mdm/nanomdm/storage/mysql/mysql.goserver/datastore/mysql/errors.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/android.goserver/datastore/mysql/software.goserver/platform/mysql/testing_utils/testing_utils.goserver/datastore/mysql/certificate_authorities.goserver/datastore/mysql/software_titles.goserver/mdm/nanomdm/storage/mysql/queue.goserver/service/osquery_utils/queries_test.go
📚 Learning: 2025-12-31T23:23:07.852Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 37245
File: server/datastore/mysql/common_mysql/list_options.go:56-73
Timestamp: 2025-12-31T23:23:07.852Z
Learning: In server/datastore/mysql/common_mysql/list_options.go, the strings.HasSuffix(orderKey, "id") check at line 62 never matches because orderKey is already sanitized with backticks, making the integer conversion code at line 63 unreachable dead code. However, this doesn't cause functional issues because MySQL automatically coerces string parameters to integers when comparing with integer columns in WHERE clauses.
Applied to files:
ee/server/service/condaccess/idp_test.goserver/datastore/mysql/nanomdm_storage.gotools/mysql-tests/rds/iam_auth.goserver/datastore/mysql/android_device_test.goserver/datastore/mysql/mysql.goee/server/service/certificate_authorities_test.goserver/platform/mysql/list_options.goserver/mdm/android/service/pubsub_test.goserver/datastore/mysql/policies_test.goserver/datastore/mysql/software.goee/server/service/software_installers.goserver/datastore/mysql/software_titles.goserver/mdm/nanomdm/storage/mysql/queue.goserver/datastore/mysql/testing_utils.goserver/datastore/mysql/hosts.goserver/service/osquery_utils/queries_test.go
📚 Learning: 2025-08-20T21:24:59.261Z
Learnt from: sgress454
Repo: fleetdm/fleet PR: 31075
File: server/datastore/mysql/common_mysql/aws_iam_auth.go:111-130
Timestamp: 2025-08-20T21:24:59.261Z
Learning: In Fleet's MySQL IAM authentication implementation, the TLS and AllowCleartextPasswords configuration is handled in the generateMysqlConnectionString function in server/datastore/mysql/common_mysql/common.go, not in the awsIAMAuthConnector.Connect method. The connector receives a base DSN that already includes the necessary IAM authentication parameters.
Applied to files:
tools/mysql-tests/rds/iam_auth.goserver/datastore/mysql/mysql.goserver/datastore/mysql/errors.goserver/datastore/mysql/apple_mdm_test.go
📚 Learning: 2025-08-20T21:24:59.261Z
Learnt from: sgress454
Repo: fleetdm/fleet PR: 31075
File: server/datastore/mysql/common_mysql/aws_iam_auth.go:111-130
Timestamp: 2025-08-20T21:24:59.261Z
Learning: In Fleet's MySQL IAM authentication implementation, the TLS and AllowCleartextPasswords configuration is handled in the generateMysqlConnectionString function in server/datastore/mysql/common_mysql/common.go. When no password is configured and the endpoint is an RDS endpoint, the function automatically sets allowCleartextPasswords=true and configures appropriate TLS settings. The awsIAMAuthConnector receives a properly configured base DSN and only needs to inject the IAM token as the password.
Applied to files:
tools/mysql-tests/rds/iam_auth.go
📚 Learning: 2025-08-01T15:08:16.858Z
Learnt from: sgress454
Repo: fleetdm/fleet PR: 31508
File: server/datastore/mysql/schema.sql:102-116
Timestamp: 2025-08-01T15:08:16.858Z
Learning: The schema.sql file in server/datastore/mysql/ is auto-generated from migrations for use with tests, so it cannot be manually edited. Any changes must be made through migrations.
Applied to files:
server/datastore/mysql/migrations_test.go
📚 Learning: 2025-12-11T23:02:59.284Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 36978
File: server/mdm/android/service/profiles.go:441-449
Timestamp: 2025-12-11T23:02:59.284Z
Learning: Fleet’s server/mdm/android/service.Service has fields ds (type fleet.AndroidDatastore) and fleetDS (type fleet.Datastore). The fleet.Datastore interface embeds AndroidDatastore (server/fleet/datastore.go), so assigning r.DS (a fleet.Datastore) to Service.ds is valid in Go due to interface-to-interface assignment using the dynamic type.
Applied to files:
server/datastore/mysql/mysql.goserver/datastore/mysql/apple_mdm.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/policies_test.goee/server/service/software_installers.goserver/datastore/mysql/hosts.goserver/datastore/mysql/hosts_test.go
📚 Learning: 2025-10-03T18:16:11.482Z
Learnt from: MagnusHJensen
Repo: fleetdm/fleet PR: 33805
File: server/service/integration_mdm_test.go:1248-1251
Timestamp: 2025-10-03T18:16:11.482Z
Learning: In server/service/integration_mdm_test.go, the helper createAppleMobileHostThenEnrollMDM(platform string) is exclusively for iOS/iPadOS hosts (mobile). Do not flag macOS model/behavior issues based on changes within this helper; macOS provisioning uses different helpers such as createHostThenEnrollMDM.
Applied to files:
server/datastore/mysql/apple_mdm.goserver/datastore/mysql/apple_mdm_test.go
📚 Learning: 2025-12-11T23:02:52.191Z
Learnt from: getvictor
Repo: fleetdm/fleet PR: 36978
File: server/mdm/android/service/profiles.go:441-449
Timestamp: 2025-12-11T23:02:52.191Z
Learning: In server/mdm/android/service, Go allows assigning an interface value of type fleet.Datastore to a field ds of type fleet.Datastore because the underlying dynamic type implements both the embedded AndroidDatastore (via fleet.Datastore embedding AndroidDatastore). When reviewing code in profiles.go and related files, verify assignments between interfaces respect embedding and ensure the concrete type actually implements all required interfaces. This pattern is valid wherever fleet.Datastore embeds AndroidDatastore and the receiver expects fleetDS or similar fields; prefer keeping interface boundaries clear and rely on the dynamic type to satisfy both interfaces.
Applied to files:
server/mdm/android/service/pubsub_test.go
🧬 Code graph analysis (1)
server/platform/arch_test.go (1)
server/archtest/archtest.go (1)
NewPackageTest(43-45)
🔇 Additional comments (52)
server/datastore/mysql/mysql_test.go (1)
25-25: LGTM! Clean import path update.The import path update correctly reflects the package relocation from
server/datastore/mysql/common_mysql/testing_utilstoserver/platform/mysql/testing_utils, consistent with the PR's refactoring objectives. All usages oftesting_utilsthroughout the file remain syntactically valid.ee/server/service/hostidentity/depot/depot.go (1)
21-21: LGTM! Clean refactoring with minimal changes.The import path has been correctly updated to reference the new package location at
server/platform/mysql, while preserving thecommon_mysqlalias. This approach minimizes changes by keeping all usage sites (like line 125'scommon_mysql.WithRetryTxx) unchanged. The refactoring is consistent with the PR objective to relocate the common MySQL utilities.server/datastore/mysql/host_identity_scep.go (1)
14-14: LGTM! Import path updated correctly.The import path has been successfully updated to reference the relocated
platform/mysqlpackage with thecommon_mysqlalias maintained for backward compatibility.changes/37244-refactor-common_mysql (1)
1-1: LGTM! Changelog entry is clear and accurate.The changelog appropriately documents the package refactoring and relocation to support bounded contexts within the Fleet codebase.
server/datastore/mysql/android.go (1)
14-14: LGTM! Import path updated correctly.The import path has been successfully updated to reference the relocated
platform/mysqlpackage with thecommon_mysqlalias. All usages ofcommon_mysql.GetDefaultNonZeroTime()andcommon_mysql.NotFound()throughout the file remain consistent with the new import path.server/datastore/mysql/operating_system_vulnerabilities.go (1)
12-12: LGTM! Import path updated correctly.The import path has been successfully updated to reference the relocated
platform/mysqlpackage with thecommon_mysqlalias. The usage ofcommon_mysql.BatchProcessSimple()at line 1038 remains consistent with the new import path.server/service/software_title_icons_test.go (1)
17-17: LGTM! Import path updated correctly.The import path has been successfully updated to reference the relocated
platform/mysqlpackage with thecommon_mysqlalias. All test mock functions using&common_mysql.NotFoundError{}throughout the file remain consistent with the new import path.server/datastore/mysql/config.go (1)
7-7: LGTM! Import alias correctly applied.The import now properly aliases
server/platform/mysqlascommon_mysql, which aligns with all thecommon_mysql.DBOptionsreferences throughout the file (lines 18-77). This resolves the concern from the previous review.server/platform/mysql/testing_utils/testing_utils.go (1)
12-12: LGTM! Import path successfully updated.The import correctly references the new
server/platform/mysqllocation with thecommon_mysqlalias, maintaining compatibility with all existing usages throughout the file.server/datastore/mysql/migrations_test.go (1)
12-12: LGTM! Testing utilities import updated.The import path correctly points to the new location at
server/platform/mysql/testing_utils, and all references totesting_utilsconstants remain intact.server/datastore/mysql/android_enterprise_test.go (1)
8-8: LGTM! Import path updated correctly.The import now references
server/platform/mysql/testing_utils, consistent with the package reorganization. Usage at line 27 remains unchanged.server/datastore/mysql/migrations/tables/20250502222222_AddMdmEnrollTables.go (2)
10-10: LGTM! Import alias correctly applied.The import properly aliases
server/platform/mysqlascommon_mysql, which aligns with the usage at line 248 (common_mysql.BatchProcessSimple). This resolves the concern from the previous review.
72-81: SQL queries properly scoped.All SQL queries in this migration include appropriate filtering:
- Lines 72-81: JOIN with WHERE clause to filter by
fleet_enroll_ref- Lines 87-102: JOIN with WHERE clause filtering by source
- Lines 123-135: Proper ordering for deterministic results
- Lines 161-168: JOIN to match accounts by UUID
- Lines 237-245: INSERT with ON DUPLICATE KEY UPDATE for idempotency
Also applies to: 87-102, 123-135, 161-168, 237-245
server/datastore/mysql/scripts_test.go (1)
13-13: LGTM: Import path correctly updated.The import path has been properly updated to reflect the new package location at
server/platform/mysql, while preserving thecommon_mysqlalias to maintain compatibility with existing references throughout the test file.server/platform/mysql/batch_test.go (1)
1-1: LGTM: Package declaration correctly updated.The package name has been appropriately changed from
common_mysqltomysql, aligning with the new directory structure and following Go naming conventions where the package name matches the directory name.server/datastore/mysql/policies_test.go (1)
15-25: Import alias now correctly targets platform/mysqlUsing
common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"keeps existingcommon_mysql.NotFoundErrorusages compiling and aligns this test with the new package location. No further changes needed here.tools/mysql-tests/rds/iam_auth.go (1)
11-17: RDS test tool correctly updated to use platform/mysql DBOptionsThe alias switch to
server/platform/mysqland continued use ofcommon_mysql.DBOptionswithmysql.NewDBis consistent with the new shared MySQL utilities; IAM and TLS behavior remain centralized in the platform package.Also applies to: 55-58, 65-67
server/datastore/mysql/mysql.go (1)
20-41: common_mysql alias correctly re-pointed to platform/mysqlRebinding
common_mysqltoserver/platform/mysqlwhile keeping all existingcommon_mysql.*usages (DBOptions/DBConnections, Tx helpers, NewDB, list-options facades) preserves behavior and fixes the earlier broken import/alias combination. The datastore continues to treat platform/mysql as the shared MySQL utility layer without pulling in domain types into the platform package.Also applies to: 72-75, 207-227, 232-283, 382-446, 829-846
server/datastore/mysql/apple_mdm.go (1)
29-29: Import alias update to platform/mysql is consistentThe alias
common_mysqlnow pointing atserver/platform/mysqlmatches the refactor and keeps existing usages (e.g.,common_mysql.BatchProcessSimple,common_mysql.DBReadTx) working without behavioral change.server/platform/mysql/batch.go (1)
1-1: Package rename tomysqlaligns with new platform packageRenaming the package to
mysqlunderserver/platform/mysqlis consistent with the refactor, and existing callers that import it ascommon_mysqlremain valid.server/mdm/nanomdm/storage/mysql/mysql.go (1)
16-16: WithRetryTxx import moved to platform/mysql without behavior changeSwitching
common_mysqltoserver/platform/mysqlkeepsupdateLastSeenBatch’sWithRetryTxxusage intact and matches the new shared MySQL helpers location.server/platform/mysql/retry.go (1)
1-1: Retry helper package renamed tomysqlas expectedThe retry utilities now live under
server/platform/mysqlwith packagemysql; callers importing this ascommon_mysqlcontinue to work with unchanged behavior.server/service/labels_test.go (1)
17-17: Label service tests now target platform/mysql testing utilitiesUpdating the testing utilities import to
server/platform/mysql/testing_utilsaligns with the common_mysql → platform/mysql move without altering test behavior.server/platform/mysql/config.go (1)
1-1: LGTM! Package declaration correctly updated.The package declaration has been correctly changed from
package common_mysqltopackage mysql, aligning with the new location atserver/platform/mysql. Import sites maintain backward compatibility by aliasing the import ascommon_mysql.server/datastore/mysql/android_enterprises.go (1)
10-10: LGTM! Import correctly updated with alias.The import has been successfully updated to reference the new
server/platform/mysqllocation with thecommon_mysqlalias. All usages ofcommon_mysql.NotFoundthroughout the file (Lines 31, 44, 57, 80) remain correct.server/datastore/mysql/vulnerabilities_test.go (1)
10-10: LGTM! Test import correctly updated.The import has been successfully updated to the new
server/platform/mysqlpath with thecommon_mysqlalias preserved. All usages ofcommon_mysql.NotFoundError(Lines 176, 285) remain correct and compilation-safe.ee/server/service/condaccess/idp_test.go (1)
14-14: LGTM! Import path correctly updated with alias.The import has been properly updated to reference the new
platform/mysqllocation while preserving thecommon_mysqlalias, ensuring all existing call sites (like line 308'scommon_mysql.NotFound("certificate")) continue to work without modification.server/platform/mysql/errors.go (1)
1-1: LGTM! Package renamed as part of refactor.The package declaration correctly changes from
common_mysqltomysqlas part of the relocation toserver/platform/mysql. This aligns with the PR objective to move and reorganize the MySQL utilities under the platform package.server/mdm/nanomdm/storage/mysql/queue.go (1)
12-12: LGTM! Import migration complete and correct.The import path has been successfully updated to
server/platform/mysqlwith thecommon_mysqlalias, ensuring all usages ofcommon_mysql.WithRetryTxx(lines 69, 271) andcommon_mysql.BatchProcessSimple(line 297) remain functional without code changes.server/service/integrationtest/suite.go (1)
12-12: LGTM! Test utilities import path updated correctly.The import path for
testing_utilshas been properly migrated from the olddatastore/mysql/common_mysql/testing_utilslocation to the newplatform/mysql/testing_utilspath, maintaining compatibility with existing usage at line 57.server/datastore/mysql/android_mysql.go (1)
10-10: LGTM! Android datastore import successfully migrated.The import has been correctly updated to reference
server/platform/mysqlwith thecommon_mysqlalias, preserving all existing references tocommon_mysql.TxFnand the retry helper functions at lines 47-48 and 51-52.server/datastore/mysql/operating_systems.go (1)
10-10: Import alias points correctly to relocated common MySQL helpersThe alias
common_mysqlnow targetsserver/platform/mysqlwhile preserving existing usages (e.g.,common_mysql.DoRetryErr), so this refactor is consistent and safe.server/datastore/mysql/hosts.go (1)
23-23: Alias for platform/mysql correctly matches existing usagesUsing
common_mysqlas an alias forserver/platform/mysqlaligns with allcommon_mysql.DefaultNonZeroTimereferences and fixes the earlier missing-alias problem.server/mdm/android/service/pubsub_test.go (1)
18-18: Test import alias correctly updated to new platform packageThe
common_mysqlalias now targetsserver/platform/mysql, so allcommon_mysql.NotFound(...)test expectations remain valid and compile.server/datastore/mysql/android_test.go (1)
13-13: LGTM! Import path correctly updated.The import path has been successfully updated from the old
server/datastore/mysql/common_mysqllocation to the newserver/platform/mysqllocation, maintaining thecommon_mysqlalias to preserve existing code references (e.g., line 1733). This change aligns with the PR's objective to consolidate MySQL utilities under the platform package.server/datastore/mysql/apple_mdm_test.go (1)
29-29: Import path relocation looks correct and non-breakingThe alias
common_mysqlis preserved while updating the import toserver/platform/mysql, so existing uses (e.g.,common_mysql.NotFoundError) remain valid and the change cleanly aligns tests with the new package location.ee/server/service/certificate_authorities_test.go (1)
21-21: Import refactor correctly implemented.The import path has been successfully updated to the new location with the
common_mysqlalias, and all references throughout the file use this alias correctly. The past review concerns about compilation errors have been resolved.ee/server/service/request_certificate_test.go (1)
20-20: Import path correctly updated.The import has been properly updated to reference the new package location with the
common_mysqlalias, and line 146's usage ofcommon_mysql.NotFoundis correct.server/datastore/mysql/certificate_authorities.go (1)
13-13: Import refactor correctly applied.The import path update is correct with the
common_mysqlalias properly maintained. All references tocommon_mysql.NotFoundthroughout the file (lines 57, 389, 406) are consistent with the alias.server/platform/mysql/common.go (1)
1-1: Package rename correctly implemented.The package declaration has been appropriately changed from
common_mysqltomysqlto align with the new location atserver/platform/mysql. Import sites maintain backward compatibility by using thecommon_mysqlalias.server/datastore/mysql/nanomdm_storage.go (1)
20-20: Import path successfully updated.The import has been correctly updated to the new location with the
common_mysqlalias. All references tocommon_mysql.WithRetryTxxthroughout the file (lines 199, 246, 272, 314) properly use the alias and will compile without errors.server/datastore/mysql/scim.go (1)
12-12: Import alias correctly updated to new common MySQL utilitiesAliasing
server/platform/mysqlascommon_mysqlkeeps existingcommon_mysql.BatchProcessSimplecall sites compiling and avoids any package-name conflict with the localmysqlpackage. Looks good.server/datastore/mysql/software.go (1)
19-19: Alias to platform MySQL utilities is consistent with call sitesThe import now correctly aliases
server/platform/mysqlascommon_mysql, matching usages ofcommon_mysql.RetryableErrorandcommon_mysql.BatchProcessSimpleand avoiding any name clash with themysqlpackage.ee/server/service/software_installers.go (1)
30-30: Import alias correctly updated toplatform/mysqlThe
common_mysqlalias now points toserver/platform/mysqland matches thecommon_mysql.NotFound(...)usage later in the file, so the refactor compiles cleanly and behavior remains unchanged.server/datastore/mysql/testing_utils.go (1)
34-35: Platform MySQL imports and alias are now consistent with usages
common_mysqlis aliased toserver/platform/mysql, andtesting_utilstoserver/platform/mysql/testing_utils, matching allcommon_mysql.TestSQLMode/common_mysql.DBOptionsandtesting_utils.MysqlTestConfigusages so tests will build correctly after the move.server/datastore/mysql/software_installers_test.go (1)
19-20: Aliascommon_mysqlnow correctly targetsplatform/mysqlThe tests now import
server/platform/mysqlascommon_mysql, socommon_mysql.NotFoundErrorand the shared MySQL helpers resolve correctly, and the new testing_utils path matches the refactored location.server/datastore/mysql/policies.go (1)
17-17: LGTM! Clean import path update.The import path has been correctly updated to reference the new package location while maintaining the
common_mysqlalias for backward compatibility with existing usages in this file (lines 443 and 2164).server/datastore/mysql/hosts_test.go (1)
30-40: Import relocation toserver/platform/mysqlis correct and non-breakingThe alias remains
common_mysqlandGetDefaultNonZeroTime()is used exactly as before, so this cleanly switches the dependency to the new sharedplatform/mysqlpackage without behavioral changes. This matches the PR’s refactor goal.server/service/osquery_utils/queries_test.go (1)
31-31: LGTM! Import path correctly updated.The import path has been successfully updated to use
server/platform/mysqlwith thecommon_mysqlalias, maintaining compatibility with existing usages throughout the test file.server/platform/README.md (1)
1-16: Excellent documentation of architectural boundaries.This README clearly explains the platform vs domain separation, provides concrete guidelines, and references the enforcement mechanism via
arch_test.go. The documentation will help maintainers understand and respect these architectural boundaries.server/platform/arch_test.go (1)
94-110: Well-structured architectural test.The
TestMysqlPackageDependenciestest correctly enforces that the mysql platform package doesn't depend on Fleet domain packages. It follows the same pattern as other tests in this file and appropriately ignores internal platform dependencies.server/datastore/mysql/errors.go (1)
16-16: Import correctly updated.The import path now references
server/platform/mysqlwith thecommon_mysqlalias, maintaining compatibility with the existingnotFoundfunction and other error helpers.
lucasmrod
left a comment
lucasmrod
left a comment
There was a problem hiding this comment.
Nit: Worth sticking to one either platform_ or common_ for import prefixing (we used platform_http for imports of server/platform/http).
Yes, I prefer |
Related issue: Resolves #37244
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.