Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Create a temp staging dir per-download #1306

Merged
merged 3 commits into from
Aug 29, 2023
Merged

Create a temp staging dir per-download #1306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85c414f
Create a temp staging dir per-download
RebeccaMahany Aug 29, 2023
3ec9ef4
Merge branch 'main' into becca/tuf-temp-staging-dir-per-download
RebeccaMahany Aug 29, 2023
f2e8632
Merge branch 'main' into becca/tuf-temp-staging-dir-per-download
RebeccaMahany Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions pkg/autoupdate/tuf/autoupdate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ type librarian interface {
Available(binary autoupdatableBinary, targetFilename string) bool
AddToLibrary(binary autoupdatableBinary, currentVersion string, targetFilename string, targetMetadata data.TargetFileMeta) error
TidyLibrary(binary autoupdatableBinary, currentVersion string)
Close() error
}

type querier interface {
Expand Down Expand Up @@ -218,9 +217,6 @@ func (ta *TufAutoupdater) Execute() (err error) {
}

func (ta *TufAutoupdater) Interrupt(_ error) {
if err := ta.libraryManager.Close(); err != nil {
level.Debug(ta.logger).Log("msg", "could not close library on interrupt", "err", err)
}
ta.interrupt <- struct{}{}
}

Expand Down
4 changes: 0 additions & 4 deletions pkg/autoupdate/tuf/autoupdate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,8 @@ func TestExecute_osquerydUpdate(t *testing.T) {
require.Contains(t, logLines[len(logLines)-1], "restarted binary after update")

// The autoupdater won't stop after an osqueryd download, so interrupt it and let it shut down
mockLibraryManager.On("Close").Return(nil)
autoupdater.Interrupt(errors.New("test error"))
time.Sleep(1 * time.Second)
mockLibraryManager.AssertExpectations(t)
}

func TestExecute_withInitialDelay(t *testing.T) {
Expand Down Expand Up @@ -246,7 +244,6 @@ func TestExecute_withInitialDelay(t *testing.T) {

// Expect that we interrupt
mockLibraryManager := NewMocklibrarian(t)
mockLibraryManager.On("Close").Return(nil)
autoupdater.libraryManager = mockLibraryManager

// Let the autoupdater run for less than the initial delay
Expand Down Expand Up @@ -357,7 +354,6 @@ func Test_storeError(t *testing.T) {
// We only expect TidyLibrary to run for osqueryd, since we can't get the current running version
// for launcher in tests.
mockLibraryManager.On("TidyLibrary", binaryOsqueryd, mock.Anything).Return().Once()
mockLibraryManager.On("Close").Return(nil)

// Set the check interval to something short so we can accumulate some errors
autoupdater.checkInterval = 1 * time.Second
Expand Down
78 changes: 21 additions & 57 deletions pkg/autoupdate/tuf/library_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ type updateLibraryManager struct {
mirrorUrl string // dl.kolide.co
mirrorClient *http.Client
baseDir string
stagingDir string
lock *libraryLock
logger log.Logger
}
Expand All @@ -50,13 +49,6 @@ func newUpdateLibraryManager(mirrorUrl string, mirrorClient *http.Client, baseDi
return nil, fmt.Errorf("could not make base directory for updates library: %w", err)
}

// Create the directory for staging updates
stagingDir, err := agent.MkdirTemp("staged-updates")
if err != nil {
return nil, fmt.Errorf("could not make staged updates directory: %w", err)
}
ulm.stagingDir = stagingDir

// Create the update library
for _, binary := range binaries {
if err := os.MkdirAll(updatesDirectory(binary, baseDir), 0755); err != nil {
Expand All @@ -67,20 +59,6 @@ func newUpdateLibraryManager(mirrorUrl string, mirrorClient *http.Client, baseDi
return &ulm, nil
}

// Close cleans up the temporary staging directory
func (ulm *updateLibraryManager) Close() error {
// Acquire lock to ensure we aren't interrupting an ongoing operation
for _, binary := range binaries {
ulm.lock.Lock(binary)
defer ulm.lock.Unlock(binary)
}

if err := os.RemoveAll(ulm.stagingDir); err != nil {
return fmt.Errorf("could not remove staging dir %s: %w", ulm.stagingDir, err)
}
return nil
}

// updatesDirectory returns the update library location for the given binary.
func updatesDirectory(binary autoupdatableBinary, baseUpdateDirectory string) string {
return filepath.Join(baseUpdateDirectory, string(binary))
Expand Down Expand Up @@ -115,14 +93,19 @@ func (ulm *updateLibraryManager) AddToLibrary(binary autoupdatableBinary, curren
return nil
}

// Remove downloaded archives after update, regardless of success -- this will run before the unlock
defer ulm.tidyStagedUpdates(binary)

stagedUpdatePath, err := ulm.stageAndVerifyUpdate(binary, targetFilename, targetMetadata)
if err != nil {
return fmt.Errorf("could not stage update: %w", err)
}

// Remove downloaded archives after update, regardless of success -- this will run before the unlock
defer func() {
dirToRemove := filepath.Dir(stagedUpdatePath)
if err := os.RemoveAll(dirToRemove); err != nil {
level.Debug(ulm.logger).Log("msg", "could not remove temp staging directory", "err", err, "directory", dirToRemove)
}
}()

if err := ulm.moveVerifiedUpdate(binary, targetFilename, stagedUpdatePath); err != nil {
return fmt.Errorf("could not move verified update: %w", err)
}
Expand All @@ -133,7 +116,11 @@ func (ulm *updateLibraryManager) AddToLibrary(binary autoupdatableBinary, curren
// stageAndVerifyUpdate downloads the update indicated by `targetFilename` and verifies it against
// the given, validated local metadata.
func (ulm *updateLibraryManager) stageAndVerifyUpdate(binary autoupdatableBinary, targetFilename string, localTargetMetadata data.TargetFileMeta) (string, error) {
stagedUpdatePath := filepath.Join(ulm.stagingDir, targetFilename)
stagingDir, err := agent.MkdirTemp(fmt.Sprintf("staged-updates-%s", versionFromTarget(binary, targetFilename)))
if err != nil {
return "", fmt.Errorf("could not create temporary directory for downloading target: %w", err)
}
stagedUpdatePath := filepath.Join(stagingDir, targetFilename)

// Request download from mirror
downloadPath := path.Join("/", "kolide", string(binary), runtime.GOOS, PlatformArch(), targetFilename)
Expand Down Expand Up @@ -178,9 +165,9 @@ func (ulm *updateLibraryManager) stageAndVerifyUpdate(binary autoupdatableBinary
// Finally, it moves the update to its correct versioned location in the update library for the given binary.
func (ulm *updateLibraryManager) moveVerifiedUpdate(binary autoupdatableBinary, targetFilename string, stagedUpdate string) error {
targetVersion := versionFromTarget(binary, targetFilename)
stagedVersionedDirectory := filepath.Join(ulm.stagingDir, targetVersion)
if err := os.MkdirAll(stagedVersionedDirectory, 0755); err != nil {
return fmt.Errorf("could not create directory %s for untarring and validating new update: %w", stagedVersionedDirectory, err)
stagedVersionedDirectory, err := agent.MkdirTemp(targetVersion)
if err != nil {
return fmt.Errorf("could not create temporary directory for untarring and validating new update: %w", err)
}
defer func() {
// In case of error, clean up the staged version
Expand Down Expand Up @@ -224,39 +211,16 @@ func (ulm *updateLibraryManager) removeUpdate(binary autoupdatableBinary, binary
}
}

// TidyLibrary removes unneeded files from the staged updates and updates directories.
// TidyLibrary reviews all updates in the library for the binary and removes any old versions
// that are no longer needed. It will always preserve the current running binary, and then the
// two most recent valid versions. It will remove versions it cannot validate.
func (ulm *updateLibraryManager) TidyLibrary(binary autoupdatableBinary, currentVersion string) {
// Acquire lock for modifying the library
ulm.lock.Lock(binary)
defer ulm.lock.Unlock(binary)

// First, remove old staged archives
ulm.tidyStagedUpdates(binary)

// Remove any updates we no longer need
ulm.tidyUpdateLibrary(binary, currentVersion)
}

// tidyStagedUpdates removes all old archives from the staged updates directory.
func (ulm *updateLibraryManager) tidyStagedUpdates(binary autoupdatableBinary) {
matches, err := filepath.Glob(filepath.Join(ulm.stagingDir, "*"))
if err != nil {
level.Debug(ulm.logger).Log("msg", "could not glob for staged updates to tidy updates library", "err", err)
return
}

for _, match := range matches {
if err := os.RemoveAll(match); err != nil {
level.Debug(ulm.logger).Log("msg", "could not remove staged update when tidying update library", "file", match, "err", err)
}
}
}

// tidyUpdateLibrary reviews all updates in the library for the binary and removes any old versions
// that are no longer needed. It will always preserve the current running binary, and then the
// two most recent valid versions. It will remove versions it cannot validate.
func (ulm *updateLibraryManager) tidyUpdateLibrary(binary autoupdatableBinary, currentRunningVersion string) {
if currentRunningVersion == "" {
if currentVersion == "" {
level.Debug(ulm.logger).Log("msg", "cannot tidy update library without knowing current running version")
return
}
Expand All @@ -282,7 +246,7 @@ func (ulm *updateLibraryManager) tidyUpdateLibrary(binary autoupdatableBinary, c
nonCurrentlyRunningVersionsKept := 0
for i := len(versionsInLibrary) - 1; i >= 0; i -= 1 {
// Always keep the current running executable
if versionsInLibrary[i] == currentRunningVersion {
if versionsInLibrary[i] == currentVersion {
continue
}

Expand Down
63 changes: 5 additions & 58 deletions pkg/autoupdate/tuf/library_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,38 +30,15 @@ func Test_newUpdateLibraryManager(t *testing.T) {
require.NoError(t, err, "could not stat base dir")
require.True(t, baseDir.IsDir(), "base dir is not a directory")

stagedDownloadDir, err := os.Stat(testLibraryManager.stagingDir)
require.NoError(t, err, "could not stat staged osqueryd download dir")
require.True(t, stagedDownloadDir.IsDir(), "staged osqueryd download dir is not a directory")

osquerydDownloadDir, err := os.Stat(filepath.Join(testBaseDir, "osqueryd"))
osquerydDownloadDir, err := os.Stat(updatesDirectory("osqueryd", testLibraryManager.baseDir))
require.NoError(t, err, "could not stat osqueryd download dir")
require.True(t, osquerydDownloadDir.IsDir(), "osqueryd download dir is not a directory")

launcherDownloadDir, err := os.Stat(filepath.Join(testBaseDir, "launcher"))
launcherDownloadDir, err := os.Stat(updatesDirectory("launcher", testLibraryManager.baseDir))
require.NoError(t, err, "could not stat launcher download dir")
require.True(t, launcherDownloadDir.IsDir(), "launcher download dir is not a directory")
}

func TestClose(t *testing.T) {
t.Parallel()

testBaseDir := filepath.Join(t.TempDir(), "updates")
testLibraryManager, err := newUpdateLibraryManager("", nil, testBaseDir, log.NewNopLogger())
require.NoError(t, err, "unexpected error creating new update library manager")

stagedDownloadDir, err := os.Stat(testLibraryManager.stagingDir)
require.NoError(t, err, "could not stat staged osqueryd download dir")
require.True(t, stagedDownloadDir.IsDir(), "staged osqueryd download dir is not a directory")

// Close the library
require.NoError(t, testLibraryManager.Close(), "expected no error on closing library")

// Confirm staged download directory is gone
_, err = os.Stat(testLibraryManager.stagingDir)
require.True(t, os.IsNotExist(err), "expected staged download dir to be removed on Close")
}

func Test_pathToTargetVersionExecutable(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -171,11 +148,6 @@ func TestAddToLibrary(t *testing.T) {
executableInfo, err := os.Stat(executableLocation(filepath.Join(updatesDirectory(tt.binary, testBaseDir), testReleaseVersion), tt.binary))
require.NoError(t, err, "checking that downloaded update includes executable")
require.False(t, executableInfo.IsDir())

// Confirm the staging directory is empty
matches, err := filepath.Glob(filepath.Join(testLibraryManager.stagingDir, "*"))
require.NoError(t, err, "checking that staging dir was cleaned")
require.Equal(t, 0, len(matches), "unexpected files found in staged updates directory: %+v", matches)
})
}
}
Expand All @@ -198,7 +170,6 @@ func TestAddToLibrary_alreadyRunning(t *testing.T) {
mirrorClient: http.DefaultClient,
logger: log.NewNopLogger(),
baseDir: testBaseDir,
stagingDir: t.TempDir(),
lock: newLibraryLock(),
}

Expand Down Expand Up @@ -238,7 +209,6 @@ func TestAddToLibrary_alreadyAdded(t *testing.T) {
mirrorClient: http.DefaultClient,
logger: log.NewNopLogger(),
baseDir: testBaseDir,
stagingDir: t.TempDir(),
lock: newLibraryLock(),
}

Expand Down Expand Up @@ -328,11 +298,6 @@ func TestAddToLibrary_verifyStagedUpdate_handlesInvalidFiles(t *testing.T) {
// Request download
require.Error(t, testLibraryManager.AddToLibrary(tt.binary, "", tt.targetFile, tt.targetMeta), "expected error when library manager downloads invalid file")

// Confirm the update was removed after download
downloadMatches, err := filepath.Glob(filepath.Join(testLibraryManager.stagingDir, "*"))
require.NoError(t, err, "checking that staging dir did not have any downloads")
require.Equal(t, 0, len(downloadMatches), "unexpected files found in staged updates directory: %+v", downloadMatches)

// Confirm the update was not added to the library
updateMatches, err := filepath.Glob(filepath.Join(updatesDirectory(tt.binary, testBaseDir), "*"))
require.NoError(t, err, "checking that updates directory does not contain any updates")
Expand Down Expand Up @@ -498,22 +463,11 @@ func TestTidyLibrary(t *testing.T) {
// Set up test library manager
testBaseDir := t.TempDir()
testLibraryManager := &updateLibraryManager{
logger: log.NewNopLogger(),
baseDir: testBaseDir,
stagingDir: t.TempDir(),
lock: newLibraryLock(),
logger: log.NewNopLogger(),
baseDir: testBaseDir,
lock: newLibraryLock(),
}

// Make a file in the staged updates directory
f1, err := os.Create(filepath.Join(testLibraryManager.stagingDir, fmt.Sprintf("%s-1.2.3.tar.gz", binary)))
require.NoError(t, err, "creating fake download file")
f1.Close()

// Confirm we made the staging files
stagingMatches, err := filepath.Glob(filepath.Join(testLibraryManager.stagingDir, "*"))
require.NoError(t, err, "could not glob for files in staged download dir")
require.Equal(t, 1, len(stagingMatches))

// Set up existing versions for test
for existingVersion, isExecutable := range tt.existingVersions {
executablePath := executableLocation(filepath.Join(updatesDirectory(binary, testBaseDir), existingVersion), binary)
Expand All @@ -538,13 +492,6 @@ func TestTidyLibrary(t *testing.T) {
// Tidy the library
testLibraryManager.TidyLibrary(binary, tt.currentlyRunningVersion)

// Confirm the staging directory was tidied up
_, err = os.Stat(testLibraryManager.stagingDir)
require.NoError(t, err, "could not stat staged download dir")
matchesAfter, err := filepath.Glob(filepath.Join(testLibraryManager.stagingDir, "*"))
require.NoError(t, err, "could not glob for files in staged download dir")
require.Equal(t, 0, len(matchesAfter))

// Confirm that the versions we expect are still there
for _, expectedPreservedVersion := range tt.expectedPreservedVersions {
info, err := os.Stat(filepath.Join(updatesDirectory(binary, testBaseDir), expectedPreservedVersion))
Expand Down
Loading