From 88492e98ffd04aa16bf1333f66921eb21b41156c Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Fri, 3 Jul 2026 12:40:09 -0300 Subject: [PATCH] Fix TestGitOpsFullGlobal failing on main after Windows BatchSetMDMProfiles change (#48695) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes `TestGitOpsFullGlobal`, which has been failing the `fleetctl` test bundle on every `main` run since #48467 merged (bisected to a90eab6f62, e.g. [this run](https://github.com/fleetdm/fleet/actions/runs/28665588825)). #48467 changed `BatchSetMDMProfiles` so that `assume_enabled` is only honored on dry runs; real runs now validate Windows profiles against the app config persisted in the datastore. That's correct in production, because the GitOps run persists `windows_enabled_and_configured: true` via `ModifyAppConfig` before the profiles batch call. But this test's `AppConfigFunc` mock always returned a fixed app config with Windows MDM disabled, never reflecting what `SaveAppConfigFunc` stored — so the real-run profiles batch now fails with 422 "Windows MDM isn't turned on". The fix makes the mock behave like the real datastore: once `SaveAppConfig` is called, `AppConfigFunc` returns the saved config. Test-only change, no product code touched. # Checklist for submitter - [x] Input data is properly validated, `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. - [x] Timeouts are implemented and retries are limited to avoid infinite loops ## Testing - [x] Added/updated automated tests `go test -run TestGitOps ./cmd/fleetctl/fleetctl/` passes locally (it fails on `main` without this change). ## Summary by CodeRabbit * **Tests** * Improved GitOps test coverage to better match real persistence behavior during an apply run. * Updated test setup so saved app configuration is read back correctly after changes are applied. * Reset test state between subtests to keep each scenario isolated and reliable. --- cmd/fleetctl/fleetctl/gitops_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cmd/fleetctl/fleetctl/gitops_test.go b/cmd/fleetctl/fleetctl/gitops_test.go index e6c401e323..167d0b652f 100644 --- a/cmd/fleetctl/fleetctl/gitops_test.go +++ b/cmd/fleetctl/fleetctl/gitops_test.go @@ -2158,11 +2158,20 @@ func TestGitOpsFullGlobal(t *testing.T) { } // App config + appConfigSaved := false ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + if appConfigSaved { + // Return the config persisted earlier in the same GitOps run, as the real + // datastore would: profile validation re-reads the app config after + // org_settings has enabled Windows MDM. + config := *savedAppConfig + return &config, nil + } return &fleet.AppConfig{MDM: fleet.MDM{EnabledAndConfigured: true}}, nil } ds.SaveAppConfigFunc = func(ctx context.Context, config *fleet.AppConfig) error { savedAppConfig = config + appConfigSaved = true return nil } ds.IsEnrollSecretAvailableFunc = func(ctx context.Context, secret string, isNew bool, teamID *uint) (bool, error) { @@ -2199,6 +2208,7 @@ func TestGitOpsFullGlobal(t *testing.T) { deletedLabels = nil enrolledSecrets = nil savedAppConfig = &fleet.AppConfig{} + appConfigSaved = false policyDeleted = false queryDeleted = false deletedPolicyIDs = nil