diff --git a/changes/24024-no-setup-exp b/changes/24024-no-setup-exp new file mode 100644 index 000000000000..44ab42bcf059 --- /dev/null +++ b/changes/24024-no-setup-exp @@ -0,0 +1,2 @@ +- Modifies the Fleet setup experience feature to not run if there is no software or script + configured for the setup experience. \ No newline at end of file diff --git a/server/datastore/mysql/setup_experience.go b/server/datastore/mysql/setup_experience.go index fe69a14aef3e..a4ff177352b9 100644 --- a/server/datastore/mysql/setup_experience.go +++ b/server/datastore/mysql/setup_experience.go @@ -109,8 +109,11 @@ WHERE global_or_team_id = ?` } totalInsertions += uint(inserts) // nolint: gosec - if err := setHostAwaitingConfiguration(ctx, tx, hostUUID, true); err != nil { - return ctxerr.Wrap(ctx, err, "setting host awaiting configuration to true") + // Only run setup experience on hosts that have something configured. + if totalInsertions > 0 { + if err := setHostAwaitingConfiguration(ctx, tx, hostUUID, true); err != nil { + return ctxerr.Wrap(ctx, err, "setting host awaiting configuration to true") + } } return nil @@ -529,7 +532,7 @@ WHERE host_uuid = ? if err := sqlx.GetContext(ctx, ds.reader(ctx), &awaitingConfiguration, stmt, hostUUID); err != nil { if errors.Is(err, sql.ErrNoRows) { - return false, nil + return false, notFound("HostAwaitingConfiguration") } return false, ctxerr.Wrap(ctx, err, "getting host awaiting configuration") diff --git a/server/datastore/mysql/setup_experience_test.go b/server/datastore/mysql/setup_experience_test.go index f1f387fe1abf..8ab616a2f728 100644 --- a/server/datastore/mysql/setup_experience_test.go +++ b/server/datastore/mysql/setup_experience_test.go @@ -40,20 +40,22 @@ func TestSetupExperience(t *testing.T) { } } +// TODO(JVE): this test could probably be simplified and most of the ad-hoc SQL removed. func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { ctx := context.Background() test.CreateInsertGlobalVPPToken(t, ds) + // Create some teams team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "team1"}) require.NoError(t, err) team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "team2"}) require.NoError(t, err) - team3, err := ds.NewTeam(ctx, &fleet.Team{Name: "team3"}) require.NoError(t, err) user1 := test.NewUser(t, ds, "Alice", "alice@example.com", true) + // Create some software installers and add them to setup experience tfr1, err := fleet.NewTempFileReader(strings.NewReader("hello"), t.TempDir) require.NoError(t, err) installerID1, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ @@ -97,6 +99,7 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { return err }) + // Create some VPP apps and add them to setup experience app1 := &fleet.VPPApp{Name: "vpp_app_1", VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "1", Platform: fleet.MacOSPlatform}}, BundleIdentifier: "b1"} vpp1, err := ds.InsertVPPAppWithTeam(ctx, app1, &team1.ID) require.NoError(t, err) @@ -110,33 +113,17 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { return err }) - var script1ID, script2ID int64 - ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { - res, err := insertScriptContents(ctx, q, "SCRIPT 1") - if err != nil { - return err - } - id1, _ := res.LastInsertId() - res, err = insertScriptContents(ctx, q, "SCRIPT 2") - if err != nil { - return err - } - id2, _ := res.LastInsertId() - - res, err = q.ExecContext(ctx, "INSERT INTO setup_experience_scripts (team_id, global_or_team_id, name, script_content_id) VALUES (?, ?, ?, ?)", team1.ID, team1.ID, "script1", id1) - if err != nil { - return err - } - script1ID, _ = res.LastInsertId() + // Create some scripts and add them to setup experience + err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script1", ScriptContents: "SCRIPT 1", TeamID: &team1.ID}) + require.NoError(t, err) + err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script2", ScriptContents: "SCRIPT 2", TeamID: &team2.ID}) + require.NoError(t, err) - res, err = q.ExecContext(ctx, "INSERT INTO setup_experience_scripts (team_id, global_or_team_id, name, script_content_id) VALUES (?, ?, ?, ?)", team2.ID, team2.ID, "script2", id2) - if err != nil { - return err - } - script2ID, _ = res.LastInsertId() + script1, err := ds.GetSetupExperienceScript(ctx, &team1.ID) + require.NoError(t, err) - return nil - }) + script2, err := ds.GetSetupExperienceScript(ctx, &team2.ID) + require.NoError(t, err) hostTeam1 := "123" hostTeam2 := "456" @@ -145,14 +132,26 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { anythingEnqueued, err := ds.EnqueueSetupExperienceItems(ctx, hostTeam1, team1.ID) require.NoError(t, err) require.True(t, anythingEnqueued) + awaitingConfig, err := ds.GetHostAwaitingConfiguration(ctx, hostTeam1) + require.NoError(t, err) + require.True(t, awaitingConfig) anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, hostTeam2, team2.ID) require.NoError(t, err) require.True(t, anythingEnqueued) + awaitingConfig, err = ds.GetHostAwaitingConfiguration(ctx, hostTeam2) + require.NoError(t, err) + require.True(t, awaitingConfig) anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, hostTeam3, team3.ID) require.NoError(t, err) require.False(t, anythingEnqueued) + // Nothing is configured for setup experience in team 3, so we do not set + // host_mdm_apple_awaiting_configuration. + awaitingConfig, err = ds.GetHostAwaitingConfiguration(ctx, hostTeam3) + require.Error(t, err) + require.True(t, fleet.IsNotFound(err)) + require.False(t, awaitingConfig) seRows := []setupExperienceInsertTestRows{} @@ -191,13 +190,13 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { HostUUID: hostTeam1, Name: "script1", Status: "pending", - ScriptID: nullableUint(uint(script1ID)), // nolint: gosec + ScriptID: nullableUint(script1.ID), }, { HostUUID: hostTeam2, Name: "script2", Status: "pending", - ScriptID: nullableUint(uint(script2ID)), // nolint: gosec + ScriptID: nullableUint(script2.ID), }, } { var found bool @@ -212,35 +211,28 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { } } - for _, row := range seRows { - if row.HostUUID == hostTeam3 { - t.Error("team 3 shouldn't have any any entries") - } - } - - ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { - _, err := q.ExecContext(ctx, "DELETE FROM setup_experience_scripts WHERE global_or_team_id = ?", team2.ID) - if err != nil { - return err + require.Condition(t, func() (success bool) { + for _, row := range seRows { + if row.HostUUID == hostTeam3 { + return false + } } - _, err = q.ExecContext(ctx, "UPDATE software_installers SET install_during_setup = false WHERE global_or_team_id = ?", team2.ID) - if err != nil { - return err - } + return true + }) - _, err = q.ExecContext(ctx, "UPDATE vpp_apps_teams SET install_during_setup = false WHERE global_or_team_id = ?", team2.ID) - if err != nil { - return err - } + // Remove team2's setup experience items + err = ds.DeleteSetupExperienceScript(ctx, &team2.ID) + require.NoError(t, err) - return nil - }) + err = ds.SetSetupExperienceSoftwareTitles(ctx, team2.ID, []uint{}) + require.NoError(t, err) anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, hostTeam1, team1.ID) require.NoError(t, err) require.True(t, anythingEnqueued) + // team2 now has nothing enqueued anythingEnqueued, err = ds.EnqueueSetupExperienceItems(ctx, hostTeam2, team2.ID) require.NoError(t, err) require.False(t, anythingEnqueued) @@ -272,7 +264,7 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { HostUUID: hostTeam1, Name: "script1", Status: "pending", - ScriptID: nullableUint(uint(script1ID)), // nolint: gosec + ScriptID: nullableUint(script1.ID), }, } { var found bool @@ -908,6 +900,12 @@ func testHostInSetupExperience(t *testing.T, ds *Datastore) { inSetupExperience, err = ds.GetHostAwaitingConfiguration(ctx, "abc") require.NoError(t, err) require.False(t, inSetupExperience) + + // host without a record in the table returns not found + inSetupExperience, err = ds.GetHostAwaitingConfiguration(ctx, "404") + require.Error(t, err) + require.True(t, fleet.IsNotFound(err)) + require.False(t, inSetupExperience) } func testGetSetupExperienceScriptByID(t *testing.T, ds *Datastore) { diff --git a/server/service/integration_mdm_dep_test.go b/server/service/integration_mdm_dep_test.go index 72e91d2ba884..249dddb185a9 100644 --- a/server/service/integration_mdm_dep_test.go +++ b/server/service/integration_mdm_dep_test.go @@ -121,12 +121,6 @@ func (s *integrationMDMTestSuite) TestDEPEnrollReleaseDeviceGlobal() { s.enableABM("fleet_ade_test") - // test manual and automatic release with the new setup experience flow - for _, enableReleaseManually := range []bool{false, true} { - t.Run(fmt.Sprintf("enableReleaseManually=%t", enableReleaseManually), func(t *testing.T) { - s.runDEPEnrollReleaseDeviceTest(t, globalDevice, enableReleaseManually, nil, "I1", false) - }) - } // test manual and automatic release with the old worker flow for _, enableReleaseManually := range []bool{false, true} { t.Run(fmt.Sprintf("enableReleaseManually=%t", enableReleaseManually), func(t *testing.T) { @@ -217,12 +211,6 @@ func (s *integrationMDMTestSuite) TestDEPEnrollReleaseDeviceTeam() { // enable FileVault s.Do("PATCH", "/api/latest/fleet/mdm/apple/settings", json.RawMessage([]byte(fmt.Sprintf(`{"enable_disk_encryption":true,"team_id":%d}`, tm.ID))), http.StatusNoContent) - // test manual and automatic release with the new setup experience flow - for _, enableReleaseManually := range []bool{false, true} { - t.Run(fmt.Sprintf("enableReleaseManually=%t", enableReleaseManually), func(t *testing.T) { - s.runDEPEnrollReleaseDeviceTest(t, teamDevice, enableReleaseManually, &tm.ID, "I2", false) - }) - } // test manual and automatic release with the old worker flow for _, enableReleaseManually := range []bool{false, true} { t.Run(fmt.Sprintf("enableReleaseManually=%t", enableReleaseManually), func(t *testing.T) { @@ -538,7 +526,7 @@ func (s *integrationMDMTestSuite) runDEPEnrollReleaseDeviceTest(t *testing.T, de require.NoError(t, err) require.NoError(t, json.Unmarshal(b, &orbitConfigResp)) // should be notified of the setup experience flow - require.True(t, orbitConfigResp.Notifications.RunSetupExperience) + require.False(t, orbitConfigResp.Notifications.RunSetupExperience) if enableReleaseManually { // get the worker's pending job from the future, there should not be any diff --git a/server/service/orbit.go b/server/service/orbit.go index 0a0e852bb39c..df75ce21c5f0 100644 --- a/server/service/orbit.go +++ b/server/service/orbit.go @@ -235,19 +235,29 @@ func (svc *Service) GetOrbitConfig(ctx context.Context) (fleet.OrbitConfig, erro } if isConnectedToFleetMDM { + // If there is no software or script configured for setup experience and this is the + // first time orbit is calling the /config endpoint, then this host + // will not have a row in host_mdm_apple_awaiting_configuration. + // On subsequent calls to /config, the host WILL have a row in + // host_mdm_apple_awaiting_configuration. inSetupAssistant, err := svc.ds.GetHostAwaitingConfiguration(ctx, host.UUID) - if err != nil { + if err != nil && !fleet.IsNotFound(err) { return fleet.OrbitConfig{}, ctxerr.Wrap(ctx, err, "checking if host is in setup experience") } if inSetupAssistant { notifs.RunSetupExperience = true + } - // check if the client is running an old fleetd version that doesn't support the new - // setup experience flow. + if inSetupAssistant || fleet.IsNotFound(err) { + // If the client is running a fleetd that doesn't support setup experience, or if no + // software/script has been configured for setup experience, then we should fall back to + // the "old way" of releasing the device. We do an additional check for + // !inSetupAssistant to prevent enqueuing a new job every time the /config + // endpoint is hit. mp, ok := capabilities.FromContext(ctx) - if !ok || !mp.Has(fleet.CapabilitySetupExperience) { - level.Debug(svc.logger).Log("msg", "host doesn't support Setup experience, falling back to worker-based device release", "host_uuid", host.UUID) + if !ok || !mp.Has(fleet.CapabilitySetupExperience) || !inSetupAssistant { + level.Debug(svc.logger).Log("msg", "host doesn't support setup experience or no setup experience configured, falling back to worker-based device release", "host_uuid", host.UUID) if err := svc.processReleaseDeviceForOldFleetd(ctx, host); err != nil { return fleet.OrbitConfig{}, err }