From e37d4df1e09653d1efb1cfff78fcac5512b5f333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paolo=20Chil=C3=A0?= Date: Fri, 19 Jan 2024 15:10:37 +0100 Subject: [PATCH] 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 --- ...tories-when-unpacking-tar.gz-packages.yaml | 32 +++ .../agent/application/upgrade/step_unpack.go | 33 ++- .../application/upgrade/step_unpack_test.go | 212 ++++++++++++++++++ .../pkg/agent/application/upgrade/upgrade.go | 2 +- 4 files changed, 267 insertions(+), 12 deletions(-) create mode 100644 changelog/fragments/1705605734-fix-creation-of-directories-when-unpacking-tar.gz-packages.yaml create mode 100644 internal/pkg/agent/application/upgrade/step_unpack_test.go diff --git a/changelog/fragments/1705605734-fix-creation-of-directories-when-unpacking-tar.gz-packages.yaml b/changelog/fragments/1705605734-fix-creation-of-directories-when-unpacking-tar.gz-packages.yaml new file mode 100644 index 00000000000..1ea6577599d --- /dev/null +++ b/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 diff --git a/internal/pkg/agent/application/upgrade/step_unpack.go b/internal/pkg/agent/application/upgrade/step_unpack.go index c418e54b182..4f5b0bd7440 100644 --- a/internal/pkg/agent/application/upgrade/step_unpack.go +++ b/internal/pkg/agent/application/upgrade/step_unpack.go @@ -10,6 +10,7 @@ import ( "compress/gzip" "fmt" "io" + "io/fs" "os" "path/filepath" "runtime" @@ -17,21 +18,20 @@ import ( "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)) diff --git a/internal/pkg/agent/application/upgrade/step_unpack_test.go b/internal/pkg/agent/application/upgrade/step_unpack_test.go new file mode 100644 index 00000000000..c2d5bf0ece7 --- /dev/null +++ b/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 +} diff --git a/internal/pkg/agent/application/upgrade/upgrade.go b/internal/pkg/agent/application/upgrade/upgrade.go index 5fd8d7d989d..46cf449f963 100644 --- a/internal/pkg/agent/application/upgrade/upgrade.go +++ b/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 }