Fix creation of containing directories for files in tar packages (#4100)

* Fix creation of containing directories for files in tar packages
* remove references to paths.Data() in step_unpack

changelog/fragments/1705605734-fix-creation-of-directories-when-unpacking-tar.gz-packages.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: feature
+
+# Change summary; a 80ish characters long description of the change.
+summary: fix creation of directories when unpacking tar.gz packages
+
+# 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; a word indicating the component this changeset affects.
+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/owner/repo/1234
+
+# 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

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

@@ -10,28 +10,28 @@ import (
 	"compress/gzip"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
 
 	"github.com/hashicorp/go-multierror"
 
-	"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
 	"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
 	"github.com/elastic/elastic-agent/pkg/core/logger"
 )
 
 // unpack unpacks archive correctly, skips root (symlink, config...) unpacks data/*
-func (u *Upgrader) unpack(version, archivePath string) (string, error) {
+func (u *Upgrader) unpack(version, archivePath, dataDir string) (string, error) {
 	// unpack must occur in directory that holds the installation directory
 	// or the extraction will be double nested
 	var hash string
 	var err error
 	if runtime.GOOS == windows {
-		hash, err = unzip(u.log, archivePath)
+		hash, err = unzip(u.log, archivePath, dataDir)
 	} else {
-		hash, err = untar(u.log, version, archivePath)
+		hash, err = untar(u.log, version, archivePath, dataDir)
 	}
 
 	if err != nil {
@@ -43,7 +43,7 @@ func (u *Upgrader) unpack(version, archivePath string) (string, error) {
 	return hash, nil
 }
 
-func unzip(log *logger.Logger, archivePath string) (string, error) {
+func unzip(log *logger.Logger, archivePath, dataDir string) (string, error) {
 	var hash, rootDir string
 	r, err := zip.OpenReader(archivePath)
 	if err != nil {
@@ -81,7 +81,7 @@ func unzip(log *logger.Logger, archivePath string) (string, error) {
 			return nil
 		}
 
-		path := filepath.Join(paths.Data(), strings.TrimPrefix(fileName, "data/"))
+		path := filepath.Join(dataDir, strings.TrimPrefix(fileName, "data/"))
 
 		if f.FileInfo().IsDir() {
 			log.Debugw("Unpacking directory", "archive", "zip", "file.path", path)
@@ -126,7 +126,7 @@ func unzip(log *logger.Logger, archivePath string) (string, error) {
 	return hash, nil
 }
 
-func untar(log *logger.Logger, version string, archivePath string) (string, error) {
+func untar(log *logger.Logger, version string, archivePath, dataDir string) (string, error) {
 	r, err := os.Open(archivePath)
 	if err != nil {
 		return "", errors.New(fmt.Sprintf("artifact for 'elastic-agent' version '%s' could not be found at '%s'", version, archivePath), errors.TypeFilesystem, errors.M(errors.MetaKeyPath, archivePath))
@@ -179,7 +179,7 @@ func untar(log *logger.Logger, version string, archivePath string) (string, erro
 		}
 
 		rel := filepath.FromSlash(strings.TrimPrefix(fileName, "data/"))
-		abs := filepath.Join(paths.Data(), rel)
+		abs := filepath.Join(dataDir, rel)
 
 		// find the root dir
 		if currentDir := filepath.Dir(abs); rootDir == "" || len(filepath.Dir(rootDir)) > len(currentDir) {
@@ -193,7 +193,7 @@ func untar(log *logger.Logger, version string, archivePath string) (string, erro
 			log.Debugw("Unpacking file", "archive", "tar", "file.path", abs)
 			// just to be sure, it should already be created by Dir type
 			// remove any world permissions from the directory
-			if err := os.MkdirAll(filepath.Dir(abs), mode.Perm()&0770); err != nil {
+			if err = os.MkdirAll(filepath.Dir(abs), 0o750); err != nil {
 				return "", errors.New(err, "TarInstaller: creating directory for file "+abs, errors.TypeFilesystem, errors.M(errors.MetaKeyPath, abs))
 			}
 
@@ -214,8 +214,19 @@ func untar(log *logger.Logger, version string, archivePath string) (string, erro
 		case mode.IsDir():
 			log.Debugw("Unpacking directory", "archive", "tar", "file.path", abs)
 			// remove any world permissions from the directory
-			if err := os.MkdirAll(abs, mode.Perm()&0770); err != nil {
-				return "", errors.New(err, "TarInstaller: creating directory for file "+abs, errors.TypeFilesystem, errors.M(errors.MetaKeyPath, abs))
+			_, err = os.Stat(abs)
+			if errors.Is(err, fs.ErrNotExist) {
+				if err := os.MkdirAll(abs, mode.Perm()&0770); err != nil {
+					return "", errors.New(err, "TarInstaller: creating directory for file "+abs, errors.TypeFilesystem, errors.M(errors.MetaKeyPath, abs))
+				}
+			} else if err != nil {
+				return "", errors.New(err, "TarInstaller: stat() directory for file "+abs, errors.TypeFilesystem, errors.M(errors.MetaKeyPath, abs))
+			} else {
+				// set the appropriate permissions
+				err = os.Chmod(abs, mode.Perm()&0o770)
+				if err != nil {
+					return "", errors.New(err, fmt.Sprintf("TarInstaller: setting permissions %O for directory %q", mode.Perm()&0o770, abs), errors.TypeFilesystem, errors.M(errors.MetaKeyPath, abs))
+				}
 			}
 		default:
 			return "", errors.New(fmt.Sprintf("tar file entry %s contained unsupported file type %v", fileName, mode), errors.TypeFilesystem, errors.M(errors.MetaKeyPath, fileName))

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

@@ -0,0 +1,212 @@
+// 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 upgrade
+
+import (
+	"archive/tar"
+	"compress/gzip"
+	"fmt"
+	"io"
+	"io/fs"
+	"os"
+	"path"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/elastic/elastic-agent/pkg/core/logger"
+)
+
+const foo_component_spec = `
+version: 2
+inputs:
+  - name: foobar
+    description: "Foo input"
+    platforms:
+      - linux/amd64
+      - linux/arm64
+      - darwin/amd64
+      - darwin/arm64
+    outputs:
+      - elasticsearch
+      - kafka
+      - logstash
+    command:
+      args:
+        - foo
+        - bar
+        - baz
+`
+
+type fileType uint
+
+const (
+	REGULAR fileType = iota
+	DIRECTORY
+	SYMLINK
+)
+
+type files struct {
+	fType   fileType
+	path    string
+	content string
+	mode    fs.FileMode
+}
+
+func (f files) Name() string {
+	return path.Base(f.path)
+}
+
+func (f files) Size() int64 {
+	return int64(len(f.content))
+}
+
+func (f files) Mode() fs.FileMode {
+	return f.mode
+}
+
+func (f files) ModTime() time.Time {
+	return time.Unix(0, 0)
+}
+
+func (f files) IsDir() bool {
+	return f.fType == DIRECTORY
+}
+
+func (f files) Sys() any {
+	return nil
+}
+
+type createArchiveFunc func(t *testing.T, archiveFiles []files) (string, error)
+type checkExtractedPath func(t *testing.T, testDataDir string)
+
+func TestUpgrader_unpack(t *testing.T) {
+	type args struct {
+		version          string
+		archiveGenerator createArchiveFunc
+		archiveFiles     []files
+	}
+	tests := []struct {
+		name       string
+		args       args
+		want       string
+		wantErr    assert.ErrorAssertionFunc
+		checkFiles checkExtractedPath
+	}{
+		{
+			name: "targz with file before containing folder",
+			args: args{
+				version: "1.2.3",
+				archiveFiles: []files{
+					{fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64", mode: fs.ModeDir | (fs.ModePerm & 0o750)},
+					{fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/" + agentCommitFile, content: "abcdefghijklmnopqrstuvwxyz", mode: fs.ModePerm & 0o640},
+					{fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/package.version", content: "1.2.3", mode: fs.ModePerm & 0o640},
+					{fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data", mode: fs.ModeDir | (fs.ModePerm & 0o750)},
+					{fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef", mode: fs.ModeDir | (fs.ModePerm & 0o700)},
+					{fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/" + agentName, content: "Placeholder for the elastic-agent binary", mode: fs.ModePerm & 0o750},
+					{fType: DIRECTORY, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components", mode: fs.ModeDir | (fs.ModePerm & 0o750)},
+					{fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components/comp1", content: "Placeholder for component", mode: fs.ModePerm & 0o750},
+					{fType: REGULAR, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/data/elastic-agent-abcdef/components/comp1.spec.yml", content: foo_component_spec, mode: fs.ModePerm & 0o640},
+					{fType: SYMLINK, path: "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64/" + agentName, content: "data/elastic-agent-abcdef/" + agentName, mode: fs.ModeSymlink | (fs.ModePerm & 0o750)},
+				},
+				archiveGenerator: func(t *testing.T, i []files) (string, error) {
+					return createTarArchive(t, "elastic-agent-1.2.3-SNAPSHOT-linux-x86_64.tar.gz", i)
+				},
+			},
+			want:    "abcdef",
+			wantErr: assert.NoError,
+			checkFiles: func(t *testing.T, testDataDir string) {
+
+				versionedHome := filepath.Join(testDataDir, "elastic-agent-abcdef")
+				require.DirExists(t, versionedHome, "directory for package.version does not exists")
+				stat, err := os.Stat(versionedHome)
+				require.NoErrorf(t, err, "error calling Stat() for versionedHome %q", versionedHome)
+				expectedPermissions := fs.ModePerm & 0o700
+				actualPermissions := fs.ModePerm & stat.Mode()
+				assert.Equalf(t, expectedPermissions, actualPermissions, "Wrong permissions set on versioned home %q: expected %O, got %O", versionedHome, expectedPermissions, actualPermissions)
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if runtime.GOOS == "windows" {
+				t.Skip("tar.gz tests only run on Linux/MacOS")
+			}
+
+			testTop := t.TempDir()
+			testDataDir := filepath.Join(testTop, "data")
+			err := os.MkdirAll(testDataDir, 0o777)
+			assert.NoErrorf(t, err, "error creating initial structure %q", testDataDir)
+			log, _ := logger.NewTesting(tt.name)
+			u := &Upgrader{
+				log: log,
+			}
+
+			archiveFile, err := tt.args.archiveGenerator(t, tt.args.archiveFiles)
+			require.NoError(t, err, "creation of test archive file failed")
+
+			got, err := u.unpack(tt.args.version, archiveFile, testDataDir)
+			if !tt.wantErr(t, err, fmt.Sprintf("unpack(%v, %v, %v)", tt.args.version, archiveFile, testDataDir)) {
+				return
+			}
+			assert.Equalf(t, tt.want, got, "unpack(%v, %v, %v)", tt.args.version, archiveFile, testDataDir)
+			if tt.checkFiles != nil {
+				tt.checkFiles(t, testDataDir)
+			}
+		})
+	}
+}
+
+func createTarArchive(t *testing.T, archiveName string, archiveFiles []files) (string, error) {
+
+	outDir := t.TempDir()
+
+	outFilePath := filepath.Join(outDir, archiveName)
+	file, err := os.OpenFile(outFilePath, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0o644)
+	require.NoErrorf(t, err, "error creating output archive %q", outFilePath)
+	defer file.Close()
+	zipWriter := gzip.NewWriter(file)
+	writer := tar.NewWriter(zipWriter)
+	defer func(writer *tar.Writer) {
+		err := writer.Close()
+		require.NoError(t, err, "error closing tar writer")
+		err = zipWriter.Close()
+		require.NoError(t, err, "error closing gzip writer")
+	}(writer)
+
+	for _, af := range archiveFiles {
+		err = addEntryToTarArchive(af, writer)
+		require.NoErrorf(t, err, "error adding %q to tar archive", af.path)
+	}
+
+	return outFilePath, err
+}
+
+func addEntryToTarArchive(af files, writer *tar.Writer) error {
+	header, err := tar.FileInfoHeader(&af, af.content)
+	if err != nil {
+		return err
+	}
+
+	header.Name = af.path
+
+	if err := writer.WriteHeader(header); err != nil {
+		return err
+	}
+
+	if af.IsDir() || af.fType == SYMLINK {
+		return nil
+	}
+
+	if _, err = io.Copy(writer, strings.NewReader(af.content)); err != nil {
+		return fmt.Errorf("copying file %q content: %w", af.path, err)
+	}
+	return nil
+}

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

@@ -170,7 +170,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string
 
 	det.SetState(details.StateExtracting)
 
-	newHash, err := u.unpack(version, archivePath)
+	newHash, err := u.unpack(version, archivePath, paths.Data())
 	if err != nil {
 		return nil, err
 	}