Allow Endpoint service to keep running if its install exit code is non-fatal (#9320)

* Allow services to keep running if install exit code is non-fatal

* Improve logging

* Adding a mock binary

* Adding test

* Adding missing headers

* Adding CHANGELOG entry

* Add error assertions

* Use npipe.Dial if OS is Windows

* Add .exe extension to mock binary file name

* Log mock binary filename in test

* Add nil guard

Author: ycombinator

changelog/fragments/1755059673-endpoint-28-not-fatal.yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# Change summary; a 80ish characters long description of the change.
+summary: Treat exit code 28 from Endpoint binary as non-fatal
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: elastic-agent
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+pr: https://github.com/elastic/elastic-agent/pull/9320
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+#issue: https://github.com/owner/repo/1234

pkg/component/runtime/service.go

@@ -9,7 +9,9 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"os/exec"
 	"runtime"
+	"slices"
 	"time"
 
 	"github.com/kardianos/service"
@@ -239,6 +241,18 @@ func (s *serviceRuntime) Run(ctx context.Context, comm Communicator) (err error)
 				// Start service
 				err = s.start(ctx)
 				if err != nil {
+					// If the error is due to a non-fatal exit code, continue running the service
+					var exitErr *exec.ExitError
+					if errors.As(err, &exitErr) {
+						exitCode := exitErr.ExitCode()
+						s.log.Debugf("service %s start failed with exit code %d, err = %s", s.name(), exitCode, err)
+						if s.comp.InputSpec.Spec.Service.Operations.Install != nil &&
+							slices.Contains(s.comp.InputSpec.Spec.Service.Operations.Install.NonFatalExitCodes, exitCode) {
+							s.log.Warnf("exit code %d is non-fatal, continuing to run...", exitCode)
+							continue
+						}
+					}
+
 					cisStop()
 					break
 				}

pkg/component/runtime/service_test.go

@@ -5,13 +5,30 @@
 package runtime
 
 import (
+	"context"
+	"fmt"
+	"net"
 	"net/url"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strconv"
+	"strings"
 	"testing"
+	"time"
+
+	"github.com/elastic/elastic-agent-libs/api/npipe"
+
+	"go.uber.org/zap/zapcore"
+
+	"github.com/stretchr/testify/require"
 
 	"github.com/elastic/elastic-agent-client/v7/pkg/client"
 	"github.com/elastic/elastic-agent-client/v7/pkg/proto"
 	"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
 	"github.com/elastic/elastic-agent/pkg/component"
+	"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -255,3 +272,122 @@ func TestGetConnInfoServerAddress(t *testing.T) {
 		})
 	}
 }
+
+// TestCISKeepsRunningOnNonFatalExitCodeFromStart tests that the connection info
+// server keeps running when starting a service component results in a non-fatal
+// exit code.
+func TestCISKeepsRunningOnNonFatalExitCodeFromStart(t *testing.T) {
+	log, logObs := loggertest.New("test")
+	const nonFatalExitCode = 99
+	const cisPort = 9999
+	const cisSocket = ".teaci.sock"
+
+	// Make an Endpoint component for testing
+	endpoint := makeEndpointComponent(t, map[string]interface{}{})
+	endpoint.InputSpec.Spec.Service = &component.ServiceSpec{
+		CPort:   cisPort,
+		CSocket: cisSocket,
+		Log:     nil,
+		Operations: component.ServiceOperationsSpec{
+			Check: &component.ServiceOperationsCommandSpec{},
+			Install: &component.ServiceOperationsCommandSpec{
+				NonFatalExitCodes: []int{nonFatalExitCode},
+			},
+		},
+		Timeouts: component.ServiceTimeoutSpec{},
+	}
+
+	// Create binary mocking Endpoint such that executing it will return
+	// the non-fatal exit code from the spec above.
+	endpoint.InputSpec.BinaryPath = mockEndpointBinary(t, nonFatalExitCode)
+	endpoint.InputSpec.BinaryName = "endpoint"
+
+	t.Logf("mock binary path: %s\n", endpoint.InputSpec.BinaryPath)
+
+	// Create new service runtime with component
+	service, err := newServiceRuntime(endpoint, log, true)
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	comm := newMockCommunicator("")
+
+	// Observe component state
+	go func() {
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case <-service.ch:
+			}
+		}
+	}()
+
+	// Run the service
+	go func() {
+		err := service.Run(ctx, comm)
+		require.EqualError(t, err, context.Canceled.Error())
+	}()
+
+	service.actionCh <- actionModeSigned{
+		actionMode: actionStart,
+	}
+
+	// Check that connection info server is still running and that we see the
+	// warning log message about Endpoint's install operation failing with a non-fatal exit
+	// code but the service runtime continuing to run.
+	cisAddr, err := getConnInfoServerAddress(runtime.GOOS, true, cisPort, cisSocket)
+	require.NoError(t, err)
+
+	parsedCISAddr, err := url.Parse(cisAddr)
+	require.NoError(t, err)
+
+	expectedWarnLogMsg := fmt.Sprintf("exit code %d is non-fatal, continuing to run...", nonFatalExitCode)
+	require.Eventually(t, func() bool {
+		if runtime.GOOS != "windows" {
+			_, err = net.Dial("unix", parsedCISAddr.Host+parsedCISAddr.Path)
+		} else {
+			if strings.HasPrefix(cisAddr, "npipe:///") {
+				path := strings.TrimPrefix(cisAddr, "npipe:///")
+				cisAddr = `\\.\pipe\` + path
+			}
+			_, err = npipe.Dial(cisAddr)("", "")
+		}
+
+		if err != nil {
+			t.Logf("Connection info server is not running: %v", err)
+			return false
+		}
+
+		logs := logObs.TakeAll()
+		for _, l := range logs {
+			t.Logf("[%s] %s", l.Level, l.Message)
+			if l.Level == zapcore.WarnLevel && l.Message == expectedWarnLogMsg {
+				return true
+			}
+		}
+
+		return false
+	}, 2*time.Second, 200*time.Millisecond)
+}
+
+func mockEndpointBinary(t *testing.T, exitCode int) string {
+	// Build a mock Endpoint binary that can return a specific exit code.
+	outPath := filepath.Join(t.TempDir(), "mock_endpoint")
+	if runtime.GOOS == "windows" {
+		outPath += ".exe"
+	}
+
+	cmd := exec.Command(
+		"go", "build",
+		"-o", outPath,
+		"-ldflags", "-X 'main.ExitCode="+strconv.Itoa(exitCode)+"'",
+		"testdata/exitcode/main.go",
+	)
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	err := cmd.Run()
+	require.NoError(t, err)
+
+	return outPath
+}

pkg/component/runtime/testdata/exitcode/doc.go

@@ -0,0 +1,12 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+package main
+
+// This package contains a go program that exits with an exit code.
+// The desired exit code must be set at build time using
+// go build -ldflags='-X main.ExitCode=<code>'.
+// The resulting binary can be used in tests to simulate an
+// Agent-managed component, e.g. Endpoint, that exits with a specific
+// exit code.

pkg/component/runtime/testdata/exitcode/main.go

@@ -0,0 +1,20 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+package main
+
+import (
+	"os"
+	"strconv"
+)
+
+var ExitCode = "0" // string so it can be set at build time
+
+func main() {
+	exitCode, err := strconv.Atoi(ExitCode)
+	if err != nil {
+		exitCode = -1
+	}
+	os.Exit(exitCode)
+}

pkg/component/spec.go

@@ -134,7 +134,8 @@ type ServiceOperationsSpec struct {
 
 // ServiceOperationsCommandSpec is the specification for execution of binaries to perform the check, install, and uninstall.
 type ServiceOperationsCommandSpec struct {
-	Args    []string         `config:"args,omitempty" yaml:"args,omitempty"`
-	Env     []CommandEnvSpec `config:"env,omitempty" yaml:"env,omitempty"`
-	Timeout time.Duration    `config:"timeout,omitempty" yaml:"timeout,omitempty"`
+	Args              []string         `config:"args,omitempty" yaml:"args,omitempty"`
+	Env               []CommandEnvSpec `config:"env,omitempty" yaml:"env,omitempty"`
+	Timeout           time.Duration    `config:"timeout,omitempty" yaml:"timeout,omitempty"`
+	NonFatalExitCodes []int            `config:"non_fatal_exit_codes,omitempty" yaml:"non_fatal_exit_codes,omitempty"`
 }

specs/endpoint-security.spec.yml

@@ -45,6 +45,8 @@ inputs:
             - "--resources"
             - "endpoint-security-resources.zip"
           timeout: 600s
+          non_fatal_exit_codes:
+            - 28 # tamper protection enabled but could not (re)install
         uninstall:
           args:
             - "uninstall"