Fix: Agent failed to upgrade from 8.4.2 to 8.5.0 BC1 for MAC 12 agent…

… using agent binary. (#1392)

* Revert "Revert:  Bundle elastic-agent.app for MacOS… #714  (#1387)"

This reverts commit e61c506.

* Fix upgrade for Mac OS

dev-tools/packaging/files/darwin/PkgInfo

@@ -0,0 +1 @@
+APPL????

dev-tools/packaging/templates/darwin/Info.plist.tmpl

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>CFBundleExecutable</key>
+	<string>elastic-agent</string>
+	<key>CFBundleIdentifier</key>
+	<string>co.elastic.elastic-agent</string>
+	<key>CFBundleInfoDictionaryVersion</key>
+	<string>6.0</string>
+	<key>CFBundleName</key>
+	<string>elastic-agent</string>
+	<key>CFBundlePackageType</key>
+	<string>APPL</string>
+	<key>CFBundleShortVersionString</key>
+	<string>{{ beat_version }}</string>
+	<key>CFBundleVersion</key>
+	<string>{{ beat_version }}</string>
+</dict>
+</plist>

dev-tools/packaging/templates/darwin/elastic-agent.tmpl

@@ -0,0 +1,11 @@
+#!/bin/sh
+# Fix up the symlink and exit
+
+set -e
+
+symlink="/Library/Elastic/Agent/elastic-agent"
+
+if test -L "$symlink"; then
+    ln -sfn "data/elastic-agent-{{ commit_short }}/elastic-agent.app/Contents/MacOS/elastic-agent" "$symlink"
+fi
+

internal/pkg/agent/application/info/state.go

@@ -8,18 +8,27 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 
 	"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
 	"github.com/elastic/elastic-agent/internal/pkg/release"
 )
 
+const (
+	darwin = "darwin"
+)
+
 // RunningInstalled returns true when executing Agent is the installed Agent.
 //
 // This verifies the running executable path based on hard-coded paths
 // for each platform type.
 func RunningInstalled() bool {
-	expected := filepath.Join(paths.InstallPath, paths.BinaryName)
+	expectedPaths := []string{filepath.Join(paths.InstallPath, paths.BinaryName)}
+	if runtime.GOOS == darwin {
+		// For the symlink on darwin the execPath is /usr/local/bin/elastic-agent
+		expectedPaths = append(expectedPaths, paths.ShellWrapperPath)
+	}
 	execPath, _ := os.Executable()
 	execPath, _ = filepath.Abs(execPath)
 	execName := filepath.Base(execPath)
@@ -28,13 +37,21 @@ func RunningInstalled() bool {
 		// executable path is being reported as being down inside of data path
 		// move up to directories to perform the comparison
 		execDir = filepath.Dir(filepath.Dir(execDir))
+		if runtime.GOOS == darwin {
+			execDir = filepath.Dir(filepath.Dir(filepath.Dir(execDir)))
+		}
 		execPath = filepath.Join(execDir, execName)
 	}
-	return paths.ArePathsEqual(expected, execPath)
+	for _, expected := range expectedPaths {
+		if paths.ArePathsEqual(expected, execPath) {
+			return true
+		}
+	}
+	return false
 }
 
 // IsInsideData returns true when the exePath is inside of the current Agents data path.
 func IsInsideData(exePath string) bool {
-	expectedPath := filepath.Join("data", fmt.Sprintf("elastic-agent-%s", release.ShortCommit()))
+	expectedPath := paths.BinaryDir(filepath.Join("data", fmt.Sprintf("elastic-agent-%s", release.ShortCommit())))
 	return strings.HasSuffix(exePath, expectedPath)
 }

internal/pkg/agent/application/info/state_test.go

@@ -0,0 +1,48 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License;
+// you may not use this file except in compliance with the Elastic License.
+
+package info
+
+import (
+	"fmt"
+	"path/filepath"
+	"testing"
+
+	"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
+	"github.com/elastic/elastic-agent/internal/pkg/release"
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestIsInsideData(t *testing.T) {
+	validExePath := paths.BinaryDir(filepath.Join("data", fmt.Sprintf("elastic-agent-%s", release.ShortCommit())))
+
+	tests := []struct {
+		name    string
+		exePath string
+		res     bool
+	}{
+		{
+			name: "empty",
+		},
+		{
+			name:    "invalid",
+			exePath: "data/elastic-agent",
+		},
+		{
+			name:    "valid",
+			exePath: validExePath,
+			res:     true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			res := IsInsideData(tc.exePath)
+			diff := cmp.Diff(tc.res, res)
+			if diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}

internal/pkg/agent/application/paths/common.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"sync"
 
@@ -21,6 +22,8 @@ const (
 	// AgentLockFileName is the name of the overall Elastic Agent file lock.
 	AgentLockFileName = "agent.lock"
 	tempSubdir        = "tmp"
+
+	darwin = "darwin"
 )
 
 var (
@@ -68,7 +71,9 @@ func SetTop(path string) {
 func TempDir() string {
 	tmpDir := filepath.Join(Data(), tempSubdir)
 	tmpCreator.Do(func() {
-		// create tempdir as it probably don't exists
+		// Create tempdir as it probably don't exists.
+		// The error was not checked here before and the linter is not happy about it.
+		// Changing this now would lead to the wide change scope that intended at the moment, so just making the linter happy for now.
 		_ = os.MkdirAll(tmpDir, 0750)
 	})
 	return tmpDir
@@ -172,10 +177,15 @@ func SetInstall(path string) {
 // initialTop returns the initial top-level path for the binary
 //
 // When nested in top-level/data/elastic-agent-${hash}/ the result is top-level/.
+// The agent fexecutable for MacOS is wrappend in the bundle, so the path to the binary is
+// top-level/data/elastic-agent-${hash}/elastic-agent.app/Contents/MacOS
 func initialTop() string {
 	exePath := retrieveExecutablePath()
 	if insideData(exePath) {
-		return filepath.Dir(filepath.Dir(exePath))
+		exePath = filepath.Dir(filepath.Dir(exePath))
+		if runtime.GOOS == darwin {
+			exePath = filepath.Dir(filepath.Dir(filepath.Dir(exePath)))
+		}
 	}
 	return exePath
 }
@@ -195,6 +205,21 @@ func retrieveExecutablePath() string {
 
 // insideData returns true when the exePath is inside of the current Agents data path.
 func insideData(exePath string) bool {
-	expectedPath := filepath.Join("data", fmt.Sprintf("elastic-agent-%s", release.ShortCommit()))
+	expectedPath := BinaryDir(filepath.Join("data", fmt.Sprintf("elastic-agent-%s", release.ShortCommit())))
 	return strings.HasSuffix(exePath, expectedPath)
 }
+
+// BinaryDir returns the application binary directory
+// For macOS it appends the path inside of the app bundle
+// For other platforms it returns the same dir
+func BinaryDir(baseDir string) string {
+	if runtime.GOOS == darwin {
+		baseDir = filepath.Join(baseDir, "elastic-agent.app", "Contents", "MacOS")
+	}
+	return baseDir
+}
+
+// BinaryPath returns the application binary path that is concatenation of the directory and the agentName
+func BinaryPath(baseDir, agentName string) string {
+	return filepath.Join(BinaryDir(baseDir), agentName)
+}

internal/pkg/agent/application/upgrade/service_darwin.go

@@ -14,7 +14,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -115,8 +114,7 @@ func (p *darwinPidProvider) piderFromCmd(ctx context.Context, name string, args
 }
 
 func invokeCmd(topPath string) *exec.Cmd {
-	homeExePath := filepath.Join(topPath, agentName)
-
+	homeExePath := paths.BinaryPath(topPath, agentName)
 	cmd := exec.Command(homeExePath, watcherSubcommand,
 		"--path.config", paths.Config(),
 		"--path.home", paths.Top(),

internal/pkg/agent/application/upgrade/step_relink.go

@@ -23,7 +23,8 @@ func ChangeSymlink(ctx context.Context, log *logger.Logger, targetHash string) e
 	hashedDir := fmt.Sprintf("%s-%s", agentName, targetHash)
 
 	symlinkPath := filepath.Join(paths.Top(), agentName)
-	newPath := filepath.Join(paths.Top(), "data", hashedDir, agentName)
+
+	newPath := paths.BinaryPath(filepath.Join(paths.Top(), "data", hashedDir), agentName)
 
 	// handle windows suffixes
 	if runtime.GOOS == "windows" {

internal/pkg/agent/install/install.go

@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"runtime"
 
 	"github.com/otiai10/copy"
 
@@ -17,6 +18,10 @@ import (
 	"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
 )
 
+const (
+	darwin = "darwin"
+)
+
 // Install installs Elastic Agent persistently on the system including creating and starting its service.
 func Install(cfgFile string) error {
 	dir, err := findDirectory()
@@ -53,15 +58,36 @@ func Install(cfgFile string) error {
 
 	// place shell wrapper, if present on platform
 	if paths.ShellWrapperPath != "" {
-		err = os.MkdirAll(filepath.Dir(paths.ShellWrapperPath), 0755)
-		if err == nil {
-			err = ioutil.WriteFile(paths.ShellWrapperPath, []byte(paths.ShellWrapper), 0755) // nolint:gosec,G306 // this is fine.
-		}
-		if err != nil {
-			return errors.New(
-				err,
-				fmt.Sprintf("failed to write shell wrapper (%s)", paths.ShellWrapperPath),
-				errors.M("destination", paths.ShellWrapperPath))
+		// Install symlink for darwin instead
+		if runtime.GOOS == darwin {
+			// Check if previous shell wrapper or symlink exists and remove it so it can be overwritten
+			if _, err := os.Lstat(paths.ShellWrapperPath); err == nil {
+				if err := os.Remove(paths.ShellWrapperPath); err != nil {
+					return errors.New(
+						err,
+						fmt.Sprintf("failed to remove (%s)", paths.ShellWrapperPath),
+						errors.M("destination", paths.ShellWrapperPath))
+				}
+			}
+			err = os.Symlink("/Library/Elastic/Agent/elastic-agent", paths.ShellWrapperPath)
+			if err != nil {
+				return errors.New(
+					err,
+					fmt.Sprintf("failed to create elastic-agent symlink (%s)", paths.ShellWrapperPath),
+					errors.M("destination", paths.ShellWrapperPath))
+			}
+		} else {
+			err = os.MkdirAll(filepath.Dir(paths.ShellWrapperPath), 0755)
+			if err == nil {
+				//nolint: gosec // this is intended to be an executable shell script, not chaning the permissions for the linter
+				err = ioutil.WriteFile(paths.ShellWrapperPath, []byte(paths.ShellWrapper), 0755)
+			}
+			if err != nil {
+				return errors.New(
+					err,
+					fmt.Sprintf("failed to write shell wrapper (%s)", paths.ShellWrapperPath),
+					errors.M("destination", paths.ShellWrapperPath))
+			}
 		}
 	}
 
@@ -151,6 +177,9 @@ func findDirectory() (string, error) {
 		// executable path is being reported as being down inside of data path
 		// move up to directories to perform the copy
 		sourceDir = filepath.Dir(filepath.Dir(sourceDir))
+		if runtime.GOOS == darwin {
+			sourceDir = filepath.Dir(filepath.Dir(filepath.Dir(sourceDir)))
+		}
 	}
 	err = verifyDirectory(sourceDir)
 	if err != nil {