From 80b11d873dc984164d459bb1cd6827babe1066ec Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Thu, 11 Jul 2024 15:03:36 -0600 Subject: [PATCH] Part 2 of 2: Script Timeout Agent Options (#20356) --- changes/19875-custom-script-timeout | 3 + cmd/fleetctl/scripts.go | 8 ++- cmd/fleetctl/scripts_test.go | 17 ++---- server/fleet/errors.go | 2 +- server/fleet/scripts.go | 15 +---- server/service/client_scripts.go | 53 ++++++++++++++++- server/service/integration_enterprise_test.go | 59 +++++++++++++++++-- server/service/scripts.go | 4 ++ 8 files changed, 127 insertions(+), 34 deletions(-) create mode 100644 changes/19875-custom-script-timeout diff --git a/changes/19875-custom-script-timeout b/changes/19875-custom-script-timeout new file mode 100644 index 0000000000..cb163c8004 --- /dev/null +++ b/changes/19875-custom-script-timeout @@ -0,0 +1,3 @@ +host script timeouts are now configurable via agent options using `script_execution_timeout`. +`fleetctl` now uses a polling mechanism when running `run-script` to accommodate longer script +timeout values. \ No newline at end of file diff --git a/cmd/fleetctl/scripts.go b/cmd/fleetctl/scripts.go index e1b65f3f26..d5bed94e7d 100644 --- a/cmd/fleetctl/scripts.go +++ b/cmd/fleetctl/scripts.go @@ -8,8 +8,10 @@ import ( "os" "path/filepath" "strings" + "time" "unicode/utf8" + "github.com/briandowns/spinner" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/service" "github.com/urfave/cli/v2" @@ -159,11 +161,15 @@ func runScriptCommand() *cli.Command { return nil } + s := spinner.New(spinner.CharSets[24], 200*time.Millisecond) if !quiet { - fmt.Println("\nScript is running. Please wait for it to finish...") + fmt.Println() + s.Suffix = " Script is running or will run when the host comes online..." + s.Start() } res, err := client.RunHostScriptSync(h.ID, b, name, c.Uint("team")) + s.Stop() if err != nil { if strings.Contains(err.Error(), `Only one of 'script_contents' or 'team_id' is allowed`) { return errors.New("Only one of '--script-path' or '--team' is allowed.") diff --git a/cmd/fleetctl/scripts_test.go b/cmd/fleetctl/scripts_test.go index 49fcb1ca67..e65a183a2e 100644 --- a/cmd/fleetctl/scripts_test.go +++ b/cmd/fleetctl/scripts_test.go @@ -22,6 +22,7 @@ func TestRunScriptCommand(t *testing.T) { License: &fleet.LicenseInfo{ Tier: fleet.TierPremium, }, + NoCacheDatastore: true, }, &service.TestServerOpts{ HTTPServerConfig: &http.Server{WriteTimeout: 90 * time.Second}, // nolint:gosec @@ -43,6 +44,9 @@ func TestRunScriptCommand(t *testing.T) { ds.ListHostBatteriesFunc = func(ctx context.Context, hid uint) ([]*fleet.HostBattery, error) { return nil, nil } + ds.HostLiteFunc = func(ctx context.Context, hid uint) (*fleet.Host, error) { + return &fleet.Host{}, nil + } ds.ListUpcomingHostMaintenanceWindowsFunc = func(ctx context.Context, hid uint) ([]*fleet.HostMaintenanceWindow, error) { return nil, nil } @@ -94,12 +98,6 @@ hello world } cases := []testCase{ - { - name: "host offline", - scriptPath: generateValidPath, - expectErrMsg: fleet.RunScriptHostOfflineErrMsg, - expectOffline: true, - }, { name: "host not found", scriptPath: generateValidPath, @@ -221,12 +219,6 @@ hello world scriptPath: func() string { return writeTmpScriptContents(t, "\xff\xfa", ".sh") }, expectErrMsg: `Wrong data format. Only plain text allowed.`, }, - { - name: "script already running", - scriptPath: generateValidPath, - expectErrMsg: fleet.RunScriptAlreadyRunningErrMsg, - expectPending: true, - }, { name: "script successful", scriptPath: generateValidPath, @@ -368,6 +360,7 @@ Fleet records the last 10,000 characters to prevent downtime. Hostname: "host1", HostID: req.HostID, ScriptContents: req.ScriptContents, + ExecutionID: "123", }, nil } if c.name == "disabled scripts globally" { diff --git a/server/fleet/errors.go b/server/fleet/errors.go index 49d8566bd0..aee72fd458 100644 --- a/server/fleet/errors.go +++ b/server/fleet/errors.go @@ -550,7 +550,7 @@ const ( RunScriptScriptsDisabledGloballyErrMsg = "Running scripts is disabled in organization settings." RunScriptDisabledErrMsg = "Scripts are disabled for this host. To run scripts, deploy the fleetd agent with scripts enabled." RunScriptsOrbitDisabledErrMsg = "Couldn't run script. To run a script, deploy the fleetd agent with --enable-scripts." - RunScriptAsyncScriptEnqueuedErrMsg = "Script is running or will run when the host comes online." + RunScriptAsyncScriptEnqueuedMsg = "Script is running or will run when the host comes online." RunScripSavedMaxLenErrMsg = "Script is too large. It's limited to 500,000 characters (approximately 10,000 lines)." RunScripUnsavedMaxLenErrMsg = "Script is too large. It's limited to 10,000 characters (approximately 125 lines)." RunScriptGatewayTimeoutErrMsg = "Gateway timeout. Fleet didn't hear back from the host and doesn't know if the script ran. Please make sure your load balancer timeout isn't shorter than the Fleet server timeout." diff --git a/server/fleet/scripts.go b/server/fleet/scripts.go index 310b0804ed..d9736c7897 100644 --- a/server/fleet/scripts.go +++ b/server/fleet/scripts.go @@ -152,9 +152,6 @@ type HostScriptRequestPayload struct { func (r HostScriptRequestPayload) ValidateParams(waitForResult time.Duration) error { if r.ScriptContents == "" && r.ScriptID == nil && r.ScriptName == "" { - if waitForResult <= 0 { - return NewInvalidArgumentError("script", `Script contents must not be empty.`) - } return NewInvalidArgumentError("script", `One of 'script_id', 'script_contents', or 'script_name' is required.`) } @@ -176,16 +173,6 @@ func (r HostScriptRequestPayload) ValidateParams(waitForResult time.Duration) er return NewInvalidArgumentError("script_contents", `"Only one of 'script_contents' or 'team_id' is allowed.`) } } - // - // TODO: script_name and team_id are only allowed for synchronous requests; they probably should be allowed for asynchronous requests too, but we need to get a product decision on this - if waitForResult <= 0 { - switch { - case r.ScriptName != "": - return NewInvalidArgumentError("script_name", `Only synchronous script execution requests can use the 'script_name' parameter.`) - case r.TeamID > 0: - return NewInvalidArgumentError("team_id", `Only synchronous script execution requests can use the 'team_id' parameter.`) - } - } return nil } @@ -282,7 +269,7 @@ func (hsr HostScriptResult) UserMessage(hostTimeout bool, hostTimeoutValue *int) } if !hsr.SyncRequest { - return RunScriptAsyncScriptEnqueuedErrMsg + return RunScriptAsyncScriptEnqueuedMsg } return RunScriptAlreadyRunningErrMsg diff --git a/server/service/client_scripts.go b/server/service/client_scripts.go index 373b3b1ebd..faa488cea4 100644 --- a/server/service/client_scripts.go +++ b/server/service/client_scripts.go @@ -7,13 +7,25 @@ import ( "io" "net/http" "strings" + "time" "github.com/fleetdm/fleet/v4/server/fleet" ) +const pollWaitTime = 5 * time.Second + func (c *Client) RunHostScriptSync(hostID uint, scriptContents []byte, scriptName string, teamID uint) (*fleet.HostScriptResult, error) { - verb, path := "POST", "/api/latest/fleet/scripts/run/sync" - return c.runHostScript(verb, path, hostID, scriptContents, scriptName, teamID, http.StatusOK) + verb, path := "POST", "/api/latest/fleet/scripts/run" + res, err := c.runHostScript(verb, path, hostID, scriptContents, scriptName, teamID, http.StatusAccepted) + if err != nil { + return nil, err + } + + if res.ExecutionID == "" { + return nil, errors.New("missing execution id in response") + } + + return c.pollForResult(res.ExecutionID) } func (c *Client) RunHostScriptAsync(hostID uint, scriptContents []byte, scriptName string, teamID uint) (*fleet.HostScriptResult, error) { @@ -81,6 +93,43 @@ func (c *Client) runHostScript(verb, path string, hostID uint, scriptContents [] return &result, nil } +func (c *Client) pollForResult(id string) (*fleet.HostScriptResult, error) { + verb, path := "GET", fmt.Sprintf("/api/latest/fleet/scripts/results/%s", id) + var result *fleet.HostScriptResult + for { + res, err := c.AuthenticatedDo(verb, path, "", nil) + if err != nil { + return nil, fmt.Errorf("polling for result: %w", err) + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK && res.StatusCode != http.StatusNotFound { + + msg, err := extractServerErrMsg(verb, path, res) + if err != nil { + return nil, fmt.Errorf("extracting error message: %w", err) + } + if msg == "" { + msg = fmt.Sprintf("decoding %d response is missing expected message.", res.StatusCode) + } + return nil, errors.New(msg) + } + + if err := json.NewDecoder(res.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("decoding response: %w", err) + } + + if result.ExitCode != nil { + break + } + + time.Sleep(pollWaitTime) + + } + + return result, nil +} + // ApplyNoTeamScripts sends the list of scripts to be applied for the hosts in // no team. func (c *Client) ApplyNoTeamScripts(scripts []fleet.ScriptPayload, opts fleet.ApplySpecOptions) error { diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index d7d26b54bc..790191e948 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -5308,7 +5308,7 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() { require.Equal(t, "echo", scriptResultResp.ScriptContents) require.Nil(t, scriptResultResp.ExitCode) require.False(t, scriptResultResp.HostTimeout) - require.Contains(t, scriptResultResp.Message, fleet.RunScriptAsyncScriptEnqueuedErrMsg) + require.Contains(t, scriptResultResp.Message, fleet.RunScriptAsyncScriptEnqueuedMsg) // an async script doesn't care about timeouts now := time.Now() @@ -5325,7 +5325,7 @@ func (s *integrationEnterpriseTestSuite) TestRunHostScript() { require.Equal(t, "echo", scriptResultResp.ScriptContents) require.Nil(t, scriptResultResp.ExitCode) require.False(t, scriptResultResp.HostTimeout) - require.Contains(t, scriptResultResp.Message, fleet.RunScriptAsyncScriptEnqueuedErrMsg) + require.Contains(t, scriptResultResp.Message, fleet.RunScriptAsyncScriptEnqueuedMsg) // Disable scripts and verify that there are no Orbit notifs acr := appConfigResponse{} @@ -5646,7 +5646,7 @@ func (s *integrationEnterpriseTestSuite) TestRunHostSavedScript() { require.Equal(t, "echo 'no team'", scriptResultResp.ScriptContents) require.Nil(t, scriptResultResp.ExitCode) require.False(t, scriptResultResp.HostTimeout) - require.Contains(t, scriptResultResp.Message, fleet.RunScriptAsyncScriptEnqueuedErrMsg) + require.Contains(t, scriptResultResp.Message, fleet.RunScriptAsyncScriptEnqueuedMsg) require.NotNil(t, scriptResultResp.ScriptID) require.Equal(t, savedNoTmScript.ID, *scriptResultResp.ScriptID) @@ -5715,26 +5715,46 @@ func (s *integrationEnterpriseTestSuite) TestRunHostSavedScript() { errMsg = extractServerErrorText(res.Body) require.Contains(t, errMsg, `Only one of 'script_contents' or 'script_name' is allowed.`) + res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo", ScriptName: savedTmScript.Name}, http.StatusUnprocessableEntity) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, `Only one of 'script_contents' or 'script_name' is allowed.`) + // attempt to run sync with both script id and script name res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptID: ptr.Uint(savedTmScript.ID + 999), ScriptName: savedTmScript.Name}, http.StatusUnprocessableEntity) errMsg = extractServerErrorText(res.Body) require.Contains(t, errMsg, `Only one of 'script_id' or 'script_name' is allowed.`) + res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptID: ptr.Uint(savedTmScript.ID + 999), ScriptName: savedTmScript.Name}, http.StatusUnprocessableEntity) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, `Only one of 'script_id' or 'script_name' is allowed.`) + // attempt to run sync with both script contents and team id res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo", TeamID: 1}, http.StatusUnprocessableEntity) errMsg = extractServerErrorText(res.Body) require.Contains(t, errMsg, `Only one of 'script_contents' or 'team_id' is allowed.`) + res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptContents: "echo", TeamID: 1}, http.StatusUnprocessableEntity) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, `Only one of 'script_contents' or 'team_id' is allowed.`) + // attempt to run sync with both script id and team id res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptID: ptr.Uint(savedTmScript.ID + 999), TeamID: 1}, http.StatusUnprocessableEntity) errMsg = extractServerErrorText(res.Body) require.Contains(t, errMsg, `Only one of 'script_id' or 'team_id' is allowed.`) + res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID, ScriptID: ptr.Uint(savedTmScript.ID + 999), TeamID: 1}, http.StatusUnprocessableEntity) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, `Only one of 'script_id' or 'team_id' is allowed.`) + // attempt to run sync without script contents, script id, or script name res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: host.ID}, http.StatusUnprocessableEntity) errMsg = extractServerErrorText(res.Body) require.Contains(t, errMsg, `One of 'script_id', 'script_contents', or 'script_name' is required.`) + res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host.ID}, http.StatusUnprocessableEntity) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, `One of 'script_id', 'script_contents', or 'script_name' is required.`) + // deleting the saved script does not impact the pending script s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/scripts/%d", savedNoTmScript.ID), nil, http.StatusNoContent) @@ -5792,6 +5812,13 @@ func (s *integrationEnterpriseTestSuite) TestRunHostSavedScript() { require.NoError(t, err) require.NotEqual(t, savedNoTmScript2.ID, savedTmScript2.ID) + _, err = s.ds.NewScript(ctx, &fleet.Script{ + TeamID: nil, + Name: "f13372.sh", + ScriptContents: "echo 'ALL YOUR BASE ARE BELONG TO US'", + }) + require.NoError(t, err) + // make sure the new host is seen as "online" err = s.ds.MarkHostsSeen(ctx, []uint{host2.ID}, time.Now()) require.NoError(t, err) @@ -5836,6 +5863,31 @@ func (s *integrationEnterpriseTestSuite) TestRunHostSavedScript() { require.Contains(t, extractServerErrorText(res.Body), fleet.RunScriptDisabledErrMsg) res = s.Do("POST", "/api/latest/fleet/scripts/run/sync", fleet.HostScriptRequestPayload{HostID: plainOsqueryHost.ID, ScriptID: &script.ID}, http.StatusUnprocessableEntity) require.Contains(t, extractServerErrorText(res.Body), fleet.RunScriptDisabledErrMsg) + + // Async Run Script by Name + + // attempt to run async with a script that does not exist on the specified team + res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host2.ID, ScriptName: "f1337.sh", TeamID: tm.ID}, http.StatusUnprocessableEntity) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, `Script 'f1337.sh' doesn’t exist.`) + + // attempt to run async with an existing team script that belongs to a team different from the host's team + res = s.Do("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host2.ID, ScriptName: "f1337.sh", TeamID: tm2.ID}, http.StatusUnprocessableEntity) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, `The script does not belong to the same team`) + + var runSyncResp3 runScriptSyncResponse + s.DoJSON("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: host2.ID, ScriptName: "f13372.sh"}, http.StatusAccepted, &runSyncResp3) + require.Equal(t, host2.ID, runSyncResp3.HostID) + require.NotEmpty(t, runSyncResp3.ExecutionID) + + // verify pending result + s.DoJSON("GET", "/api/latest/fleet/scripts/results/"+runSyncResp3.ExecutionID, nil, http.StatusOK, &scriptResultResp) + require.Equal(t, host2.ID, scriptResultResp.HostID) + require.Equal(t, "echo 'ALL YOUR BASE ARE BELONG TO US'", scriptResultResp.ScriptContents) + require.Nil(t, scriptResultResp.ExitCode) + require.False(t, scriptResultResp.HostTimeout) + require.Contains(t, scriptResultResp.Message, fleet.RunScriptAsyncScriptEnqueuedMsg) } func (s *integrationEnterpriseTestSuite) TestEnqueueSameScriptTwice() { @@ -11361,5 +11413,4 @@ func (s *integrationEnterpriseTestSuite) TestCalendarCallback() { team1CalendarEvents, err = s.ds.ListCalendarEvents(ctx, &team1.ID) require.NoError(t, err) assert.Empty(t, team1CalendarEvents) - } diff --git a/server/service/scripts.go b/server/service/scripts.go index bb3a332b33..9629b78a09 100644 --- a/server/service/scripts.go +++ b/server/service/scripts.go @@ -29,6 +29,8 @@ type runScriptRequest struct { HostID uint `json:"host_id"` ScriptID *uint `json:"script_id"` ScriptContents string `json:"script_contents"` + ScriptName string `json:"script_name"` + TeamID uint `json:"team_id"` } type runScriptResponse struct { @@ -48,6 +50,8 @@ func runScriptEndpoint(ctx context.Context, request interface{}, svc fleet.Servi HostID: req.HostID, ScriptID: req.ScriptID, ScriptContents: req.ScriptContents, + ScriptName: req.ScriptName, + TeamID: req.TeamID, }, noWait) if err != nil { return runScriptResponse{Err: err}, nil