diff --git a/changes/35309-setup-experience-script b/changes/35309-setup-experience-script index 4cc22d1e42..e628261032 100644 --- a/changes/35309-setup-experience-script +++ b/changes/35309-setup-experience-script @@ -1 +1 @@ -* Added an endpoint to allow updating the macOS setup experience script in-place, and modified gitops to utilize it +* Updated existing /setup_experience/script POST endpoint to allow updating the macOS setup experience script in-place, and modified gitops to remove the DELETE call diff --git a/cmd/fleetctl/fleetctl/testing_utils/testing_utils.go b/cmd/fleetctl/fleetctl/testing_utils/testing_utils.go index db2584a874..d73a94465a 100644 --- a/cmd/fleetctl/fleetctl/testing_utils/testing_utils.go +++ b/cmd/fleetctl/fleetctl/testing_utils/testing_utils.go @@ -452,7 +452,7 @@ func SetupFullGitOpsPremiumServer(t *testing.T) (*mock.Store, **fleet.AppConfig, ds.DeleteSetupExperienceScriptFunc = func(ctx context.Context, teamID *uint) error { return nil } - ds.SetSetupExperienceScriptFunc = func(ctx context.Context, script *fleet.Script, allowUpdate bool) error { + ds.SetSetupExperienceScriptFunc = func(ctx context.Context, script *fleet.Script) error { return nil } ds.ExpandEmbeddedSecretsAndUpdatedAtFunc = func(ctx context.Context, document string) (string, *time.Time, error) { diff --git a/ee/server/service/setup_experience.go b/ee/server/service/setup_experience.go index a1e8ad11ac..cee96941e1 100644 --- a/ee/server/service/setup_experience.go +++ b/ee/server/service/setup_experience.go @@ -95,15 +95,7 @@ func (svc *Service) GetSetupExperienceScript(ctx context.Context, teamID *uint, return script, content, nil } -func (svc *Service) CreateSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { - return svc.setSetupExperienceScript(ctx, teamID, name, r, false) -} - -func (svc *Service) PutSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { - return svc.setSetupExperienceScript(ctx, teamID, name, r, true) -} - -func (svc *Service) setSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader, allowUpdate bool) error { +func (svc *Service) SetSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { if err := svc.authz.Authorize(ctx, &fleet.Script{TeamID: teamID}, fleet.ActionWrite); err != nil { return err } @@ -151,7 +143,7 @@ func (svc *Service) setSetupExperienceScript(ctx context.Context, teamID *uint, return fleet.NewInvalidArgumentError("script", err.Error()) } - if err := svc.ds.SetSetupExperienceScript(ctx, script, allowUpdate); err != nil { + if err := svc.ds.SetSetupExperienceScript(ctx, script); err != nil { var ( existsErr fleet.AlreadyExistsError fkErr fleet.ForeignKeyError diff --git a/ee/server/service/setup_experience_test.go b/ee/server/service/setup_experience_test.go index a985685fcb..a5ed4323bd 100644 --- a/ee/server/service/setup_experience_test.go +++ b/ee/server/service/setup_experience_test.go @@ -244,7 +244,7 @@ func TestSetupExperienceSetWithManualAgentInstall(t *testing.T) { return nil } - ds.SetSetupExperienceScriptFunc = func(ctx context.Context, script *fleet.Script, allowUpdate bool) error { + ds.SetSetupExperienceScriptFunc = func(ctx context.Context, script *fleet.Script) error { return nil } @@ -258,7 +258,7 @@ func TestSetupExperienceSetWithManualAgentInstall(t *testing.T) { require.NoError(t, err) scriptReader := bytes.NewReader([]byte("hello")) - err = svc.CreateSetupExperienceScript(ctx, nil, "potato.sh", scriptReader) + err = svc.SetSetupExperienceScript(ctx, nil, "potato.sh", scriptReader) require.NoError(t, err) _, _ = scriptReader.Seek(0, io.SeekStart) @@ -266,7 +266,7 @@ func TestSetupExperienceSetWithManualAgentInstall(t *testing.T) { err = svc.SetSetupExperienceSoftware(ctx, "darwin", 1, []uint{1, 2}) require.NoError(t, err) - err = svc.CreateSetupExperienceScript(ctx, ptr.Uint(1), "potato.sh", scriptReader) + err = svc.SetSetupExperienceScript(ctx, ptr.Uint(1), "potato.sh", scriptReader) require.NoError(t, err) _, _ = scriptReader.Seek(0, io.SeekStart) @@ -278,7 +278,7 @@ func TestSetupExperienceSetWithManualAgentInstall(t *testing.T) { err = svc.SetSetupExperienceSoftware(ctx, "darwin", 0, []uint{1, 2}) require.ErrorContains(t, err, "Couldn’t add setup experience software. To add software, first disable manual_agent_install.") - err = svc.CreateSetupExperienceScript(ctx, nil, "potato.sh", scriptReader) + err = svc.SetSetupExperienceScript(ctx, nil, "potato.sh", scriptReader) require.ErrorContains(t, err, "Couldn’t add setup experience script. To add script, first disable manual_agent_install.") _, _ = scriptReader.Seek(0, io.SeekStart) @@ -286,7 +286,7 @@ func TestSetupExperienceSetWithManualAgentInstall(t *testing.T) { err = svc.SetSetupExperienceSoftware(ctx, "darwin", 1, []uint{1, 2}) require.ErrorContains(t, err, "Couldn’t add setup experience software. To add software, first disable manual_agent_install.") - err = svc.CreateSetupExperienceScript(ctx, ptr.Uint(1), "potato.sh", scriptReader) + err = svc.SetSetupExperienceScript(ctx, ptr.Uint(1), "potato.sh", scriptReader) require.ErrorContains(t, err, "Couldn’t add setup experience script. To add script, first disable manual_agent_install.") _, _ = scriptReader.Seek(0, io.SeekStart) diff --git a/server/datastore/mysql/activities_test.go b/server/datastore/mysql/activities_test.go index b3cdf5a9bb..59fa952c80 100644 --- a/server/datastore/mysql/activities_test.go +++ b/server/datastore/mysql/activities_test.go @@ -565,7 +565,7 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) { t.Log("h2SelfService", h2SelfService) setupExpScript := &fleet.Script{Name: "setup_experience_script", ScriptContents: "setup_experience"} - err = ds.SetSetupExperienceScript(ctx, setupExpScript, false) + err = ds.SetSetupExperienceScript(ctx, setupExpScript) require.NoError(t, err) ses, err := ds.GetSetupExperienceScript(ctx, h2.TeamID) require.NoError(t, err) diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index b58baaadfb..0637c05eb7 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -8454,7 +8454,7 @@ func testHostsDeleteHosts(t *testing.T, ds *Datastore) { require.NoError(t, err) // Add a setup experience status result - err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "test.sh", ScriptContents: "echo foo"}, false) + err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "test.sh", ScriptContents: "echo foo"}) require.NoError(t, err) added, err := ds.EnqueueSetupExperienceItems(ctx, host.Platform, host.UUID, 0) diff --git a/server/datastore/mysql/setup_experience.go b/server/datastore/mysql/setup_experience.go index 64d79725ce..65a44ceeec 100644 --- a/server/datastore/mysql/setup_experience.go +++ b/server/datastore/mysql/setup_experience.go @@ -589,7 +589,7 @@ WHERE return &script, nil } -func (ds *Datastore) SetSetupExperienceScript(ctx context.Context, script *fleet.Script, allowUpdate bool) error { +func (ds *Datastore) SetSetupExperienceScript(ctx context.Context, script *fleet.Script) error { err := ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { var err error @@ -600,26 +600,24 @@ func (ds *Datastore) SetSetupExperienceScript(ctx context.Context, script *fleet } id, _ := scRes.LastInsertId() - // This clause allows for PUT semantics in some cases. The basic idea is: + // This clause allows for PUT semantics. The basic idea is: // - no existing setup script -> go through the usual insert logic // - existing setup script with different content -> delete(with all side effects) and re-insert // - existing setup script with same content -> no-op - if allowUpdate { - gotSetupExperienceScript, err := ds.getSetupExperienceScript(ctx, tx, script.TeamID) - if err != nil && !fleet.IsNotFound(err) { - return err - } - // We will fall through on a notFound err - nothing to do here - if err == nil { - if gotSetupExperienceScript.ScriptContentID != uint(id) { // nolint:gosec // dismiss G115 - low risk here - err = ds.deleteSetupExperienceScript(ctx, tx, script.TeamID) - if err != nil { - return err - } - } else { - // no change - return nil + gotSetupExperienceScript, err := ds.getSetupExperienceScript(ctx, tx, script.TeamID) + if err != nil && !fleet.IsNotFound(err) { + return err + } + // We will fall through on a notFound err - nothing to do here + if err == nil { + if gotSetupExperienceScript.ScriptContentID != uint(id) { // nolint:gosec // dismiss G115 - low risk here + err = ds.deleteSetupExperienceScript(ctx, tx, script.TeamID) + if err != nil { + return err } + } else { + // no change + return nil } } diff --git a/server/datastore/mysql/setup_experience_test.go b/server/datastore/mysql/setup_experience_test.go index b3a857a7e2..efd4f92d56 100644 --- a/server/datastore/mysql/setup_experience_test.go +++ b/server/datastore/mysql/setup_experience_test.go @@ -305,9 +305,9 @@ func testEnqueueSetupExperienceItems(t *testing.T, ds *Datastore) { }) // Create some scripts and add them to setup experience - err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script1", ScriptContents: "SCRIPT 1", TeamID: &team1.ID}, false) + 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}, false) + err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script2", ScriptContents: "SCRIPT 2", TeamID: &team2.ID}) require.NoError(t, err) script1, err := ds.GetSetupExperienceScript(ctx, &team1.ID) @@ -654,7 +654,7 @@ func testGetSetupExperienceTitles(t *testing.T, ds *Datastore) { TeamID: &team1.ID, Name: "the script.sh", ScriptContents: "hello", - }, false) + }) require.NoError(t, err) sec, err := ds.GetSetupExperienceCount(ctx, "darwin", &team1.ID) @@ -1080,7 +1080,7 @@ func testSetupExperienceScriptCRUD(t *testing.T, ds *Datastore) { ScriptContents: "echo foo", } - err = ds.SetSetupExperienceScript(ctx, wantScript1, false) + err = ds.SetSetupExperienceScript(ctx, wantScript1) require.NoError(t, err) // get the script for team1 @@ -1102,7 +1102,7 @@ func testSetupExperienceScriptCRUD(t *testing.T, ds *Datastore) { ScriptContents: "echo bar", } - err = ds.SetSetupExperienceScript(ctx, wantScript2, false) + err = ds.SetSetupExperienceScript(ctx, wantScript2) require.NoError(t, err) // get the script for team2 @@ -1124,7 +1124,7 @@ func testSetupExperienceScriptCRUD(t *testing.T, ds *Datastore) { ScriptContents: "echo bar", } - err = ds.SetSetupExperienceScript(ctx, wantScriptNoTeam, false) + err = ds.SetSetupExperienceScript(ctx, wantScriptNoTeam) require.NoError(t, err) // get the script nil team id is equivalent to team id 0 @@ -1140,20 +1140,17 @@ func testSetupExperienceScriptCRUD(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Equal(t, wantScriptNoTeam.ScriptContents, string(b)) - // try to create another with name "script" and no team id - var existsErr fleet.AlreadyExistsError - err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script", ScriptContents: "echo baz"}, false) - require.Error(t, err) - require.ErrorAs(t, err, &existsErr) + // try to create another with name "script" and no team id. Should succeed + err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script", ScriptContents: "echo baz"}) + require.NoError(t, err) - // try to create another script with no team id and a different name - err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script2", ScriptContents: "echo baz"}, false) - require.Error(t, err) - require.ErrorAs(t, err, &existsErr) + // try to create another script with no team id and a different name. Should succeed + err = ds.SetSetupExperienceScript(ctx, &fleet.Script{Name: "script2", ScriptContents: "echo baz"}) + require.NoError(t, err) // try to add a script for a team that doesn't exist var fkErr fleet.ForeignKeyError - err = ds.SetSetupExperienceScript(ctx, &fleet.Script{TeamID: ptr.Uint(42), Name: "script", ScriptContents: "echo baz"}, false) + err = ds.SetSetupExperienceScript(ctx, &fleet.Script{TeamID: ptr.Uint(42), Name: "script", ScriptContents: "echo baz"}) require.Error(t, err) require.ErrorAs(t, err, &fkErr) @@ -1174,8 +1171,8 @@ func testSetupExperienceScriptCRUD(t *testing.T, ds *Datastore) { err = ds.DeleteSetupExperienceScript(ctx, ptr.Uint(42)) require.NoError(t, err) // TODO: confirm if we want to return not found on deletes - // add same script for team1 again but with "allowUpdate" set(even though there will be no update since it doesn't exist) - err = ds.SetSetupExperienceScript(ctx, wantScript1, true) + // add same script for team1 again(even though there will be no update since it doesn't exist) + err = ds.SetSetupExperienceScript(ctx, wantScript1) require.NoError(t, err) // get the script for team1 @@ -1190,8 +1187,8 @@ func testSetupExperienceScriptCRUD(t *testing.T, ds *Datastore) { // so the content id should be the same as the old require.Equal(t, oldScript1.ScriptContentID, newScript1.ScriptContentID) - // add same script for team1 again with "allowUpdate" set. - err = ds.SetSetupExperienceScript(ctx, wantScript1, true) + // add same script for team1 again + err = ds.SetSetupExperienceScript(ctx, wantScript1) require.NoError(t, err) // Verify that the script contents remained the same @@ -1234,13 +1231,13 @@ func testUpdateSetupExperienceScriptWhileEnqueued(t *testing.T, ds *Datastore) { ScriptContents: "echo updated foo", } - err = ds.SetSetupExperienceScript(ctx, initialScript1, true) + err = ds.SetSetupExperienceScript(ctx, initialScript1) require.NoError(t, err) team1OriginalScript, err := ds.GetSetupExperienceScript(ctx, &team1.ID) require.NoError(t, err) require.NotNil(t, team1OriginalScript) - err = ds.SetSetupExperienceScript(ctx, initialScript2, true) + err = ds.SetSetupExperienceScript(ctx, initialScript2) require.NoError(t, err) team2OriginalScript, err := ds.GetSetupExperienceScript(ctx, &team2.ID) require.NoError(t, err) @@ -1272,7 +1269,7 @@ func testUpdateSetupExperienceScriptWhileEnqueued(t *testing.T, ds *Datastore) { require.Equal(t, team2OriginalScript.ID, *host2OriginalItems[0].SetupExperienceScriptID) // "Update" the script for team1 with its original contents which should cause no change to the enqueued execution - err = ds.SetSetupExperienceScript(ctx, initialScript1, true) + err = ds.SetSetupExperienceScript(ctx, initialScript1) require.NoError(t, err) team1UpdatedScript, err := ds.GetSetupExperienceScript(ctx, &team1.ID) @@ -1293,7 +1290,7 @@ func testUpdateSetupExperienceScriptWhileEnqueued(t *testing.T, ds *Datastore) { require.Equal(t, team2OriginalScript.ID, *host2NewItems[0].SetupExperienceScriptID) // update script for team1 which should delete the enqueued execution - err = ds.SetSetupExperienceScript(ctx, updatedScript1, true) + err = ds.SetSetupExperienceScript(ctx, updatedScript1) require.NoError(t, err) team1UpdatedScript, err = ds.GetSetupExperienceScript(ctx, &team1.ID) @@ -1354,7 +1351,7 @@ func testGetSetupExperienceScriptByID(t *testing.T, ds *Datastore) { ScriptContents: "echo hello", } - err := ds.SetSetupExperienceScript(ctx, script, false) + err := ds.SetSetupExperienceScript(ctx, script) require.NoError(t, err) scriptByTeamID, err := ds.GetSetupExperienceScript(ctx, nil) diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index c51baab746..2403cb3b72 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -2207,7 +2207,7 @@ type Datastore interface { GetSetupExperienceScriptByID(ctx context.Context, scriptID uint) (*Script, error) // SetSetupExperienceScript sets the setup experience script to the given script. - SetSetupExperienceScript(ctx context.Context, script *Script, allowUpdate bool) error + SetSetupExperienceScript(ctx context.Context, script *Script) error // DeleteSetupExperienceScript deletes the setup experience script for the given team. DeleteSetupExperienceScript(ctx context.Context, teamID *uint) error diff --git a/server/fleet/service.go b/server/fleet/service.go index 6a55e9925c..1d4aecf562 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -1276,13 +1276,10 @@ type Service interface { GetOrbitSetupExperienceStatus(ctx context.Context, orbitNodeKey string, forceRelease bool, resetFailedSetupSteps bool) (*SetupExperienceStatusPayload, error) // GetSetupExperienceScript gets the current setup experience script for the given team. GetSetupExperienceScript(ctx context.Context, teamID *uint, downloadRequested bool) (*Script, []byte, error) - // CreateSetupExperienceScript creates the setup experience script for the given team. An error is returned if a - // script already exists for the given team - CreateSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error - // PutSetupExperienceScript sets the setup experience script for a given team, deleting the existing one if it exists + // SetSetupExperienceScript sets the setup experience script for a given team, deleting the existing one if it exists // and is different and replacing it with a new one. Effectively an upsert operation which does nothing if the contents // do not change - PutSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error + SetSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error // DeleteSetupExperienceScript deletes the setup experience script for the given team. DeleteSetupExperienceScript(ctx context.Context, teamID *uint) error // SetupExperienceNextStep is a callback that processes the diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 52f43501c5..24be549a88 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -257,12 +257,12 @@ type SetOrUpdateCustomHostDeviceMappingFunc func(ctx context.Context, hostID uin type SetOrUpdateIDPHostDeviceMappingFunc func(ctx context.Context, hostID uint, email string) error +type DeleteHostIDPFunc func(ctx context.Context, id uint) error + type SetOrUpdateHostSCIMUserMappingFunc func(ctx context.Context, hostID uint, scimUserID uint) error type DeleteHostSCIMUserMappingFunc func(ctx context.Context, hostID uint) error -type DeleteHostIDPFunc func(ctx context.Context, id uint) error - type ListHostBatteriesFunc func(ctx context.Context, id uint) ([]*fleet.HostBattery, error) type ListUpcomingHostMaintenanceWindowsFunc func(ctx context.Context, hid uint) ([]*fleet.HostMaintenanceWindow, error) @@ -1407,7 +1407,7 @@ type GetSetupExperienceScriptFunc func(ctx context.Context, teamID *uint) (*flee type GetSetupExperienceScriptByIDFunc func(ctx context.Context, scriptID uint) (*fleet.Script, error) -type SetSetupExperienceScriptFunc func(ctx context.Context, script *fleet.Script, allowUpdate bool) error +type SetSetupExperienceScriptFunc func(ctx context.Context, script *fleet.Script) error type DeleteSetupExperienceScriptFunc func(ctx context.Context, teamID *uint) error @@ -1961,15 +1961,15 @@ type DataStore struct { SetOrUpdateIDPHostDeviceMappingFunc SetOrUpdateIDPHostDeviceMappingFunc SetOrUpdateIDPHostDeviceMappingFuncInvoked bool + DeleteHostIDPFunc DeleteHostIDPFunc + DeleteHostIDPFuncInvoked bool + SetOrUpdateHostSCIMUserMappingFunc SetOrUpdateHostSCIMUserMappingFunc SetOrUpdateHostSCIMUserMappingFuncInvoked bool DeleteHostSCIMUserMappingFunc DeleteHostSCIMUserMappingFunc DeleteHostSCIMUserMappingFuncInvoked bool - DeleteHostIDPFunc DeleteHostIDPFunc - DeleteHostIDPFuncInvoked bool - ListHostBatteriesFunc ListHostBatteriesFunc ListHostBatteriesFuncInvoked bool @@ -4811,6 +4811,13 @@ func (s *DataStore) SetOrUpdateIDPHostDeviceMapping(ctx context.Context, hostID return s.SetOrUpdateIDPHostDeviceMappingFunc(ctx, hostID, email) } +func (s *DataStore) DeleteHostIDP(ctx context.Context, id uint) error { + s.mu.Lock() + s.DeleteHostIDPFuncInvoked = true + s.mu.Unlock() + return s.DeleteHostIDPFunc(ctx, id) +} + func (s *DataStore) SetOrUpdateHostSCIMUserMapping(ctx context.Context, hostID uint, scimUserID uint) error { s.mu.Lock() s.SetOrUpdateHostSCIMUserMappingFuncInvoked = true @@ -4825,13 +4832,6 @@ func (s *DataStore) DeleteHostSCIMUserMapping(ctx context.Context, hostID uint) return s.DeleteHostSCIMUserMappingFunc(ctx, hostID) } -func (s *DataStore) DeleteHostIDP(ctx context.Context, id uint) error { - s.mu.Lock() - s.DeleteHostIDPFuncInvoked = true - s.mu.Unlock() - return s.DeleteHostIDPFunc(ctx, id) -} - func (s *DataStore) ListHostBatteries(ctx context.Context, id uint) ([]*fleet.HostBattery, error) { s.mu.Lock() s.ListHostBatteriesFuncInvoked = true @@ -8836,11 +8836,11 @@ func (s *DataStore) GetSetupExperienceScriptByID(ctx context.Context, scriptID u return s.GetSetupExperienceScriptByIDFunc(ctx, scriptID) } -func (s *DataStore) SetSetupExperienceScript(ctx context.Context, script *fleet.Script, allowUpdate bool) error { +func (s *DataStore) SetSetupExperienceScript(ctx context.Context, script *fleet.Script) error { s.mu.Lock() s.SetSetupExperienceScriptFuncInvoked = true s.mu.Unlock() - return s.SetSetupExperienceScriptFunc(ctx, script, allowUpdate) + return s.SetSetupExperienceScriptFunc(ctx, script) } func (s *DataStore) DeleteSetupExperienceScript(ctx context.Context, teamID *uint) error { diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index 10a3359e65..fb1bb1fb5e 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -790,9 +790,7 @@ type GetOrbitSetupExperienceStatusFunc func(ctx context.Context, orbitNodeKey st type GetSetupExperienceScriptFunc func(ctx context.Context, teamID *uint, downloadRequested bool) (*fleet.Script, []byte, error) -type CreateSetupExperienceScriptFunc func(ctx context.Context, teamID *uint, name string, r io.Reader) error - -type PutSetupExperienceScriptFunc func(ctx context.Context, teamID *uint, name string, r io.Reader) error +type SetSetupExperienceScriptFunc func(ctx context.Context, teamID *uint, name string, r io.Reader) error type DeleteSetupExperienceScriptFunc func(ctx context.Context, teamID *uint) error @@ -2011,11 +2009,8 @@ type Service struct { GetSetupExperienceScriptFunc GetSetupExperienceScriptFunc GetSetupExperienceScriptFuncInvoked bool - CreateSetupExperienceScriptFunc CreateSetupExperienceScriptFunc - CreateSetupExperienceScriptFuncInvoked bool - - PutSetupExperienceScriptFunc PutSetupExperienceScriptFunc - PutSetupExperienceScriptFuncInvoked bool + SetSetupExperienceScriptFunc SetSetupExperienceScriptFunc + SetSetupExperienceScriptFuncInvoked bool DeleteSetupExperienceScriptFunc DeleteSetupExperienceScriptFunc DeleteSetupExperienceScriptFuncInvoked bool @@ -4809,18 +4804,11 @@ func (s *Service) GetSetupExperienceScript(ctx context.Context, teamID *uint, do return s.GetSetupExperienceScriptFunc(ctx, teamID, downloadRequested) } -func (s *Service) CreateSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { +func (s *Service) SetSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { s.mu.Lock() - s.CreateSetupExperienceScriptFuncInvoked = true + s.SetSetupExperienceScriptFuncInvoked = true s.mu.Unlock() - return s.CreateSetupExperienceScriptFunc(ctx, teamID, name, r) -} - -func (s *Service) PutSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { - s.mu.Lock() - s.PutSetupExperienceScriptFuncInvoked = true - s.mu.Unlock() - return s.PutSetupExperienceScriptFunc(ctx, teamID, name, r) + return s.SetSetupExperienceScriptFunc(ctx, teamID, name, r) } func (s *Service) DeleteSetupExperienceScript(ctx context.Context, teamID *uint) error { diff --git a/server/service/client_scripts.go b/server/service/client_scripts.go index 821fb34cf0..0bde1ac468 100644 --- a/server/service/client_scripts.go +++ b/server/service/client_scripts.go @@ -168,7 +168,7 @@ func (c *Client) deleteMacOSSetupScript(teamID *uint) error { } func (c *Client) uploadMacOSSetupScript(filename string, data []byte, teamID *uint) error { - verb, path := "PUT", "/api/latest/fleet/setup_experience/script" + verb, path := "POST", "/api/latest/fleet/setup_experience/script" var b bytes.Buffer w := multipart.NewWriter(&b) diff --git a/server/service/handler.go b/server/service/handler.go index 35e47a2bfd..dc5f9b834f 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -409,8 +409,7 @@ func attachFleetAPIRoutes(r *mux.Router, svc fleet.Service, config config.FleetC // Setup experience script endpoints: ue.GET("/api/_version_/fleet/setup_experience/script", getSetupExperienceScriptEndpoint, getSetupExperienceScriptRequest{}) - ue.POST("/api/_version_/fleet/setup_experience/script", createSetupExperienceScriptEndpoint, setSetupExperienceScriptRequest{}) - ue.PUT("/api/_version_/fleet/setup_experience/script", putSetupExperienceScriptEndpoint, setSetupExperienceScriptRequest{}) + ue.POST("/api/_version_/fleet/setup_experience/script", setSetupExperienceScriptEndpoint, setSetupExperienceScriptRequest{}) ue.DELETE("/api/_version_/fleet/setup_experience/script", deleteSetupExperienceScriptEndpoint, deleteSetupExperienceScriptRequest{}) // Fleet-maintained apps diff --git a/server/service/integration_mdm_setup_experience_test.go b/server/service/integration_mdm_setup_experience_test.go index eae3d2a2f9..34e6ac334b 100644 --- a/server/service/integration_mdm_setup_experience_test.go +++ b/server/service/integration_mdm_setup_experience_test.go @@ -69,32 +69,17 @@ func (s *integrationMDMTestSuite) TestSetupExperienceScript() { require.Equal(t, int64(len(`echo "hello"`)), res.ContentLength) require.Equal(t, fmt.Sprintf("attachment;filename=\"%s %s\"", time.Now().Format(time.DateOnly), "script42.sh"), res.Header.Get("Content-Disposition")) - // try to create script with same name, should fail because already exists with this name for this team + // try to update script with same name, should not fail because this is allowed body, headers = generateNewScriptMultipartRequest(t, "script42.sh", []byte(`echo "hello"`), s.token, map[string][]string{"team_id": {fmt.Sprintf("%d", tm.ID)}}) - res = s.DoRawWithHeaders("POST", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusConflict, headers) - errMsg := extractServerErrorText(res.Body) - require.Contains(t, errMsg, "already exists") // TODO: confirm expected error message with product/frontend - - // try to create with a different name for this team, should fail because another script already exists - // for this team - body, headers = generateNewScriptMultipartRequest(t, - "different.sh", []byte(`echo "hello"`), s.token, map[string][]string{"team_id": {fmt.Sprintf("%d", tm.ID)}}) - res = s.DoRawWithHeaders("POST", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusConflict, headers) - errMsg = extractServerErrorText(res.Body) - require.Contains(t, errMsg, "already exists") // TODO: confirm expected error message with product/frontend - - // try to update script with same name via PUT endpoint, should not fail because this is allowed - body, headers = generateNewScriptMultipartRequest(t, - "script42.sh", []byte(`echo "hello"`), s.token, map[string][]string{"team_id": {fmt.Sprintf("%d", tm.ID)}}) - res = s.DoRawWithHeaders("PUT", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusOK, headers) + res = s.DoRawWithHeaders("POST", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusOK, headers) err = json.NewDecoder(res.Body).Decode(&newScriptResp) require.NoError(t, err) // update with a different name and contents via PUT endpoint, should suceed body, headers = generateNewScriptMultipartRequest(t, "different.sh", []byte(`echo "hello2"`), s.token, map[string][]string{"team_id": {fmt.Sprintf("%d", tm.ID)}}) - res = s.DoRawWithHeaders("PUT", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusOK, headers) + res = s.DoRawWithHeaders("POST", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusOK, headers) err = json.NewDecoder(res.Body).Decode(&newScriptResp) require.NoError(t, err) @@ -1109,10 +1094,10 @@ func (s *integrationMDMTestSuite) TestSetupExperienceFlowUpdateScript() { require.NoError(t, err) require.Nil(t, cmd) - // PUT update the script, no changes, it does not get cancelled + // update the script but with no actual changes, it does not get cancelled body, headers := generateNewScriptMultipartRequest(t, "script.sh", []byte(`echo "hello"`), s.token, map[string][]string{"team_id": {fmt.Sprintf("%d", tm.ID)}}) - s.DoRawWithHeaders("PUT", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusOK, headers) + s.DoRawWithHeaders("POST", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusOK, headers) // call the /status endpoint, the software is still running and script should still be pending statusResp = getOrbitSetupExperienceStatusResponse{} @@ -1131,10 +1116,10 @@ func (s *integrationMDMTestSuite) TestSetupExperienceFlowUpdateScript() { require.NotNil(t, statusResp.Results.Software[0].SoftwareTitleID) require.NotZero(t, *statusResp.Results.Software[0].SoftwareTitleID) - // PUT update the script with changes, see it get cancelled + // update the script with changes, see it get cancelled body, headers = generateNewScriptMultipartRequest(t, "script2.sh", []byte(`echo "foobar"`), s.token, map[string][]string{"team_id": {fmt.Sprintf("%d", tm.ID)}}) - s.DoRawWithHeaders("PUT", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusOK, headers) + s.DoRawWithHeaders("POST", "/api/latest/fleet/setup_experience/script", body.Bytes(), http.StatusOK, headers) // call the /status endpoint, software is running, script is removed as it got cancelled by the update statusResp = getOrbitSetupExperienceStatusResponse{} diff --git a/server/service/setup_experience.go b/server/service/setup_experience.go index 31b23a5dbf..b79ea37a79 100644 --- a/server/service/setup_experience.go +++ b/server/service/setup_experience.go @@ -169,7 +169,7 @@ type setSetupExperienceScriptResponse struct { func (r setSetupExperienceScriptResponse) Error() error { return r.Err } -func createSetupExperienceScriptEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) { +func setSetupExperienceScriptEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) { req := request.(*setSetupExperienceScriptRequest) scriptFile, err := req.Script.Open() @@ -178,38 +178,14 @@ func createSetupExperienceScriptEndpoint(ctx context.Context, request interface{ } defer scriptFile.Close() - if err := svc.CreateSetupExperienceScript(ctx, req.TeamID, filepath.Base(req.Script.Filename), scriptFile); err != nil { + if err := svc.SetSetupExperienceScript(ctx, req.TeamID, filepath.Base(req.Script.Filename), scriptFile); err != nil { return setSetupExperienceScriptResponse{Err: err}, nil } return setSetupExperienceScriptResponse{}, nil } -func (svc *Service) CreateSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { - // skipauth: No authorization check needed due to implementation returning - // only license error. - svc.authz.SkipAuthorization(ctx) - - return fleet.ErrMissingLicense -} - -func putSetupExperienceScriptEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) { - req := request.(*setSetupExperienceScriptRequest) - - scriptFile, err := req.Script.Open() - if err != nil { - return setSetupExperienceScriptResponse{Err: err}, nil - } - defer scriptFile.Close() - - if err := svc.PutSetupExperienceScript(ctx, req.TeamID, filepath.Base(req.Script.Filename), scriptFile); err != nil { - return setSetupExperienceScriptResponse{Err: err}, nil - } - - return setSetupExperienceScriptResponse{}, nil -} - -func (svc *Service) PutSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { +func (svc *Service) SetSetupExperienceScript(ctx context.Context, teamID *uint, name string, r io.Reader) error { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index 507c9d9316..6bf773129e 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -25,7 +25,7 @@ func TestSetupExperienceAuth(t *testing.T) { ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { return &fleet.AppConfig{}, nil } - ds.SetSetupExperienceScriptFunc = func(ctx context.Context, script *fleet.Script, allowUpdate bool) error { + ds.SetSetupExperienceScriptFunc = func(ctx context.Context, script *fleet.Script) error { return nil } @@ -199,7 +199,7 @@ func TestSetupExperienceAuth(t *testing.T) { ctx = viewer.NewContext(ctx, viewer.Viewer{User: tt.user}) t.Run("setup experience script", func(t *testing.T) { - err := svc.CreateSetupExperienceScript(ctx, nil, "test.sh", strings.NewReader("echo")) + err := svc.SetSetupExperienceScript(ctx, nil, "test.sh", strings.NewReader("echo")) checkAuthErr(t, tt.shouldFailGlobalWrite, err) err = svc.DeleteSetupExperienceScript(ctx, nil) checkAuthErr(t, tt.shouldFailGlobalWrite, err) @@ -208,7 +208,7 @@ func TestSetupExperienceAuth(t *testing.T) { _, _, err = svc.GetSetupExperienceScript(ctx, nil, true) checkAuthErr(t, tt.shouldFailGlobalRead, err) - err = svc.CreateSetupExperienceScript(ctx, &teamID, "test.sh", strings.NewReader("echo")) + err = svc.SetSetupExperienceScript(ctx, &teamID, "test.sh", strings.NewReader("echo")) checkAuthErr(t, tt.shouldFailTeamWrite, err) err = svc.DeleteSetupExperienceScript(ctx, &teamID) checkAuthErr(t, tt.shouldFailTeamWrite, err)