From b3f8b4c485db891710c24eb658abecf66d91776b Mon Sep 17 00:00:00 2001
From: Brandon Johnson <brandon.johnson@observiq.com>
Date: Fri, 7 Jun 2024 12:29:23 -0400
Subject: [PATCH] [cmd/opampsupervisor] remove executable check for collector
 binary (#33431)

**Description:** <Describe what has changed.>
* Remove the check for if an executable bit is set on the collector
binary. Instead, we'll allow the supervisor to attempt to run it and
fail then.

**Link to tracking Issue:** Closes #33430

**Testing:**
* Unit tests
---
 .../supervisor/config/config.go               |  6 +--
 .../supervisor/config/config_test.go          | 53 ++++---------------
 2 files changed, 11 insertions(+), 48 deletions(-)

diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go
index e71e29e6a79d..60244e9d9c9e 100644
--- a/cmd/opampsupervisor/supervisor/config/config.go
+++ b/cmd/opampsupervisor/supervisor/config/config.go
@@ -132,15 +132,11 @@ func (a Agent) Validate() error {
 		return errors.New("agent::executable must be specified")
 	}
 
-	f, err := os.Stat(a.Executable)
+	_, err := os.Stat(a.Executable)
 	if err != nil {
 		return fmt.Errorf("could not stat agent::executable path: %w", err)
 	}
 
-	if f.Mode().Perm()&0111 == 0 {
-		return fmt.Errorf("agent::executable does not have executable bit set")
-	}
-
 	return nil
 }
 
diff --git a/cmd/opampsupervisor/supervisor/config/config_test.go b/cmd/opampsupervisor/supervisor/config/config_test.go
index 25219bce2518..afc3e9c0f462 100644
--- a/cmd/opampsupervisor/supervisor/config/config_test.go
+++ b/cmd/opampsupervisor/supervisor/config/config_test.go
@@ -34,7 +34,7 @@ func TestValidate(t *testing.T) {
 					},
 				},
 				Agent: Agent{
-					Executable:              "${executable_path}",
+					Executable:              "${file_path}",
 					OrphanDetectionInterval: 5 * time.Second,
 				},
 				Capabilities: Capabilities{
@@ -57,7 +57,7 @@ func TestValidate(t *testing.T) {
 					},
 				},
 				Agent: Agent{
-					Executable:              "${executable_path}",
+					Executable:              "${file_path}",
 					OrphanDetectionInterval: 5 * time.Second,
 				},
 				Capabilities: Capabilities{
@@ -82,7 +82,7 @@ func TestValidate(t *testing.T) {
 					},
 				},
 				Agent: Agent{
-					Executable:              "${executable_path}",
+					Executable:              "${file_path}",
 					OrphanDetectionInterval: 5 * time.Second,
 				},
 				Capabilities: Capabilities{
@@ -107,7 +107,7 @@ func TestValidate(t *testing.T) {
 					},
 				},
 				Agent: Agent{
-					Executable:              "${executable_path}",
+					Executable:              "${file_path}",
 					OrphanDetectionInterval: 5 * time.Second,
 				},
 				Capabilities: Capabilities{
@@ -136,7 +136,7 @@ func TestValidate(t *testing.T) {
 					},
 				},
 				Agent: Agent{
-					Executable:              "${executable_path}",
+					Executable:              "${file_path}",
 					OrphanDetectionInterval: 5 * time.Second,
 				},
 				Capabilities: Capabilities{
@@ -198,31 +198,6 @@ func TestValidate(t *testing.T) {
 			},
 			expectedError: "could not stat agent::executable path:",
 		},
-		{
-			name: "agent executable has no exec bits set",
-			config: Supervisor{
-				Server: OpAMPServer{
-					Endpoint: "wss://localhost:9090/opamp",
-					Headers: http.Header{
-						"Header1": []string{"HeaderValue"},
-					},
-					TLSSetting: configtls.ClientConfig{
-						Insecure: true,
-					},
-				},
-				Agent: Agent{
-					Executable:              "${non_executable_path}",
-					OrphanDetectionInterval: 5 * time.Second,
-				},
-				Capabilities: Capabilities{
-					AcceptsRemoteConfig: true,
-				},
-				Storage: Storage{
-					Directory: "/etc/opamp-supervisor/storage",
-				},
-			},
-			expectedError: "agent::executable does not have executable bit set",
-		},
 		{
 			name: "Invalid orphan detection interval",
 			config: Supervisor{
@@ -236,7 +211,7 @@ func TestValidate(t *testing.T) {
 					},
 				},
 				Agent: Agent{
-					Executable:              "${executable_path}",
+					Executable:              "${file_path}",
 					OrphanDetectionInterval: -1,
 				},
 				Capabilities: Capabilities{
@@ -253,25 +228,17 @@ func TestValidate(t *testing.T) {
 	// create some fake files for validating agent config
 	tmpDir := t.TempDir()
 
-	executablePath := filepath.Join(tmpDir, "agent.exe")
-	//#nosec G306 -- need to write executable file for test
-	require.NoError(t, os.WriteFile(executablePath, []byte{}, 0100))
-
-	nonExecutablePath := filepath.Join(tmpDir, "file")
-	require.NoError(t, os.WriteFile(nonExecutablePath, []byte{}, 0600))
+	filePath := filepath.Join(tmpDir, "file")
+	require.NoError(t, os.WriteFile(filePath, []byte{}, 0600))
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			// Fill in path to agent executable
 			tc.config.Agent.Executable = os.Expand(tc.config.Agent.Executable,
 				func(s string) string {
-					switch s {
-					case "executable_path":
-						return executablePath
-					case "non_executable_path":
-						return nonExecutablePath
+					if s == "file_path" {
+						return filePath
 					}
-
 					return ""
 				})