Make atomic actually atomic (#7)

Dockerfile

@@ -1,3 +1,4 @@
+# check=skip=RedundantTargetPlatform
 # setup build image
 FROM --platform=$BUILDPLATFORM golang:1.23.4@sha256:c25964d301e6c50174d29deadbbaa5ea6443e94b61087b6d89e8f41ef4ebca35 AS build
 
@@ -16,7 +17,8 @@ RUN --mount=type=cache,target="/root/.cache/go-build" \
     go build -tags -trimpath -ldflags="${GO_LINKER_ARGS}" \
     -o ./build/_output/bin/dynatrace-bootstrapper
 
-FROM public.ecr.aws/dynatrace/dynatrace-codemodules:1.299.73.20250109-153420 AS codemodules
+# platform is required, otherwise the copy command will copy the wrong architecture files, don't trust GitHub Actions linting warnings
+FROM --platform=$TARGETPLATFORM public.ecr.aws/dynatrace/dynatrace-codemodules:1.299.73.20250109-153420 AS codemodules
 
 # copy bootstrapper binary
 COPY --from=build /app/build/_output/bin /opt/dynatrace/oneagent/agent/lib64/
@@ -25,8 +27,8 @@ LABEL name="Dynatrace Bootstrapper" \
       vendor="Dynatrace LLC" \
       maintainer="Dynatrace LLC"
 
-      ENV USER_UID=1001 \
-          USER_NAME=dynatrace-bootstrapper
+ENV USER_UID=1001 \
+    USER_NAME=dynatrace-bootstrapper
 
 USER ${USER_UID}:${USER_UID}
 

pkg/move/atomic.go

@@ -7,39 +7,47 @@ import (
 	"github.com/spf13/afero"
 )
 
-func atomic(copy copyFunc) copyFunc {
-	return func(fs afero.Afero) error {
-		logrus.Infof("Starting to copy (atomic) from %s to %s", sourceFolder, targetFolder)
+func atomic(work string, copy copyFunc) copyFunc {
+	return func(fs afero.Afero, from, to string) (err error) {
+		logrus.Infof("Setting up atomic operation from %s to %s", from, to)
 
-		err := fs.RemoveAll(workFolder)
+		err = fs.RemoveAll(work)
 		if err != nil {
 			logrus.Errorf("Failed initial cleanup of workdir: %v", err)
 
 			return err
 		}
 
-		err = fs.MkdirAll(workFolder, os.ModePerm)
+		err = fs.MkdirAll(work, os.ModePerm)
 		if err != nil {
 			logrus.Errorf("Failed to create the base workdir: %v", err)
 
 			return err
 		}
 
 		defer func() {
-			err := fs.RemoveAll(workFolder)
 			if err != nil {
-				logrus.Errorf("Failed to do cleanup after run: %v", err)
+				if cleanupErr := fs.RemoveAll(work); cleanupErr != nil {
+					logrus.Errorf("Failed cleanup of workdir after failure: %v", err)
+				}
 			}
 		}()
 
-		err = copy(fs)
+		err = copy(fs, from, work)
+		if err != nil {
+			logrus.Errorf("Error copying folder: %v", err)
+
+			return err
+		}
+
+		err = fs.Rename(work, to)
 		if err != nil {
 			logrus.Errorf("Error moving folder: %v", err)
 
 			return err
 		}
 
-		logrus.Infof("Successfully copied from %s to %s", sourceFolder, targetFolder)
+		logrus.Infof("Successfully finalized atomic operation from %s to %s", work, to)
 
 		return nil
 	}

pkg/move/atomic_test.go

@@ -2,57 +2,86 @@ package move
 
 import (
 	"errors"
+	"path/filepath"
 	"testing"
 
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-const tmpWorkFolder = "/work/tmp"
+func mockCopyFuncWithAtomicCheck(t *testing.T, isSuccessful bool) copyFunc {
+	t.Helper()
+
+	return func(fs afero.Afero, _, target string) error {
+		// according to the inner copyFunc, the target should be the workFolder
+		// the actual target will be created outside the copyFunc by the atomic wrapper using fs.Rename
+		require.Equal(t, workFolder, target)
+
+		// the atomic wrapper should already have created the base workFolder
+		exists, err := fs.DirExists(target)
+		require.NoError(t, err)
+		require.True(t, exists)
 
-func mockCopyFunc(isSuccessful bool) copyFunc {
-	return func(fs afero.Afero) error {
 		if isSuccessful {
-			_ = fs.MkdirAll(tmpWorkFolder, 0755)
+			file, err := fs.Create(filepath.Join(target, "test.txt"))
+			require.NoError(t, err)
+			file.Close()
+
 			return nil
 		}
 
 		return errors.New("some mock error")
 	}
 }
 func TestAtomic(t *testing.T) {
-	t.Run("success -> targetFolder is present", func(t *testing.T) {
+	source := "/source"
+	target := "/target"
+	work := "/work"
+
+	t.Run("success -> target is present", func(t *testing.T) {
 		fs := afero.Afero{Fs: afero.NewMemMapFs()}
-		sourceFolder = "/source"
-		targetFolder = "/target"
 		workFolder = "/work"
 
-		err := fs.MkdirAll(sourceFolder, 0755)
+		err := fs.MkdirAll(source, 0755)
 		assert.NoError(t, err)
 
-		atomicCopy := atomic(mockCopyFunc(true))
+		atomicCopy := atomic(work, mockCopyFuncWithAtomicCheck(t, true))
 
-		err = atomicCopy(fs)
+		err = atomicCopy(fs, source, target)
 		assert.NoError(t, err)
 
+		require.NotEqual(t, workFolder, target)
+
 		exists, err := fs.DirExists(workFolder)
 		assert.NoError(t, err)
 		assert.False(t, exists)
+
+		exists, err = fs.DirExists(target)
+		assert.NoError(t, err)
+		assert.True(t, exists)
+
+		isEmpty, err := fs.IsEmpty(target)
+		assert.NoError(t, err)
+		assert.False(t, isEmpty)
 	})
-	t.Run("fail -> targetFolder is not present", func(t *testing.T) {
+	t.Run("fail -> target is not present", func(t *testing.T) {
 		fs := afero.Afero{Fs: afero.NewMemMapFs()}
-		sourceFolder = "/source"
-		targetFolder = "/target"
-		workFolder = "/work"
 
-		atomicCopy := atomic(mockCopyFunc(false))
+		atomicCopy := atomic(work, mockCopyFuncWithAtomicCheck(t, false))
 
-		err := atomicCopy(fs)
+		err := atomicCopy(fs, source, target)
 		assert.Error(t, err)
 		assert.Equal(t, "some mock error", err.Error())
 
+		require.NotEqual(t, workFolder, target)
+
 		exists, err := fs.DirExists(workFolder)
 		assert.NoError(t, err)
 		assert.False(t, exists)
+
+		exists, err = fs.DirExists(target)
+		assert.NoError(t, err)
+		assert.False(t, exists)
 	})
 }

pkg/move/cmd.go

@@ -42,8 +42,8 @@ func Execute(fs afero.Afero) error {
 	}
 
 	if workFolder != "" {
-		copy = atomic(copy)
+		copy = atomic(workFolder, copy)
 	}
 
-	return copy(fs)
+	return copy(fs, sourceFolder, targetFolder)
 }

pkg/move/copy.go

@@ -10,24 +10,26 @@ import (
 	"github.com/spf13/afero"
 )
 
-type copyFunc func(fs afero.Afero) error
+type copyFunc func(fs afero.Afero, from, to string) error
 
-func simpleCopy(fs afero.Afero) error {
-	logrus.Infof("Starting to copy (simple) from %s to %s", sourceFolder, targetFolder)
+var _ copyFunc = simpleCopy
 
-	err := copyFolder(fs, sourceFolder, targetFolder)
+func simpleCopy(fs afero.Afero, from, to string) error {
+	logrus.Infof("Starting to copy (simple) from %s to %s", from, to)
+
+	err := copyFolder(fs, from, to)
 	if err != nil {
 		logrus.Errorf("Error moving folder: %v", err)
 
 		return err
 	}
 
-	logrus.Infof("Successfully copied from %s to %s", sourceFolder, targetFolder)
+	logrus.Infof("Successfully copied from %s to %s", from, to)
 
 	return nil
 }
 
-func copyFolder(fs afero.Fs, from string, to string) error {
+func copyFolder(fs afero.Fs, from, to string) error {
 	fromInfo, err := fs.Stat(from)
 	if err != nil {
 		return errors.WithStack(err)
@@ -71,8 +73,8 @@ func copyFolder(fs afero.Fs, from string, to string) error {
 	return nil
 }
 
-func copyFile(fs afero.Fs, sourcePath string, destinationPath string) error {
-	sourceFile, err := fs.Open(sourcePath)
+func copyFile(fs afero.Fs, from string, to string) error {
+	sourceFile, err := fs.Open(from)
 	if err != nil {
 		return errors.WithStack(err)
 	}
@@ -83,7 +85,7 @@ func copyFile(fs afero.Fs, sourcePath string, destinationPath string) error {
 		return errors.WithStack(err)
 	}
 
-	destinationFile, err := fs.OpenFile(destinationPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, sourceInfo.Mode())
+	destinationFile, err := fs.OpenFile(to, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, sourceInfo.Mode())
 	if err != nil {
 		return errors.WithStack(err)
 	}

pkg/move/technologies.go

@@ -24,18 +24,20 @@ type FileEntry struct {
 	MD5     string `json:"md5"`
 }
 
-func copyByTechnology(fs afero.Afero) error {
-	logrus.Infof("Starting to copy (filtered) from %s to %s", sourceFolder, targetFolder)
+var _ copyFunc = copyByTechnology
 
-	filteredPaths, err := filterFilesByTechnology(fs, sourceFolder, strings.Split(technology, ","))
+func copyByTechnology(fs afero.Afero, from string, to string) error {
+	logrus.Infof("Starting to copy (filtered) from %s to %s", from, to)
+
+	filteredPaths, err := filterFilesByTechnology(fs, from, strings.Split(technology, ","))
 	if err != nil {
 		return err
 	}
 
 	for _, path := range filteredPaths {
-		targetFile := filepath.Join(targetFolder, strings.Split(path, sourceFolder)[1])
+		targetFile := filepath.Join(to, strings.Split(path, from)[1])
 
-		sourceStatMode, err := fs.Stat(sourceFolder)
+		sourceStatMode, err := fs.Stat(from)
 		if err != nil {
 			logrus.Errorf("Error checking stat mode from source folder: %v", err)
 

pkg/move/technologies_test.go

@@ -15,9 +15,6 @@ func TestCopyFolderWithTechnologyFiltering(t *testing.T) {
 	sourceDir := "/source"
 	targetDir := "/target"
 
-	sourceFolder = sourceDir
-	targetFolder = targetDir
-
 	_ = fs.MkdirAll(sourceDir, 0755)
 	_ = fs.MkdirAll(targetDir, 0755)
 
@@ -51,7 +48,7 @@ func TestCopyFolderWithTechnologyFiltering(t *testing.T) {
 		})
 
 		technology = "java"
-		err := copyByTechnology(fs)
+		err := copyByTechnology(fs, sourceDir, targetDir)
 		require.NoError(t, err)
 
 		assertFileExists(t, fs, filepath.Join(targetDir, "fileA1.txt"))
@@ -66,7 +63,7 @@ func TestCopyFolderWithTechnologyFiltering(t *testing.T) {
 		})
 
 		technology = "java,python"
-		err := copyByTechnology(fs)
+		err := copyByTechnology(fs, sourceDir, targetDir)
 		require.NoError(t, err)
 
 		assertFileExists(t, fs, filepath.Join(targetDir, "fileA1.txt"))
@@ -81,7 +78,7 @@ func TestCopyFolderWithTechnologyFiltering(t *testing.T) {
 		})
 
 		technology = "php"
-		err := copyByTechnology(fs)
+		err := copyByTechnology(fs, sourceDir, targetDir)
 		require.NoError(t, err)
 
 		assertFileNotExists(t, fs, filepath.Join(targetDir, "fileA1.txt"))