Update atomic writes logic to create hidden files in same directory as the files being updated (#1348)

Author: dhurley

internal/datasource/config/nginx_config_parser.go

@@ -44,6 +44,29 @@ const (
 	locationDirective                 = "location"
 )
 
+var globFunction = func(path string) ([]string, error) {
+	matches, err := filepath.Glob(path)
+	if err != nil {
+		return nil, err
+	}
+
+	// Exclude hidden files unless the glob pattern itself starts with a dot
+	if !strings.HasPrefix(filepath.Base(path), ".") {
+		filteredMatches := make([]string, 0)
+
+		for _, match := range matches {
+			base := filepath.Base(match)
+			if !strings.HasPrefix(base, ".") {
+				filteredMatches = append(filteredMatches, match)
+			}
+		}
+
+		return filteredMatches, nil
+	}
+
+	return matches, nil
+}
+
 //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6@v6.8.1 -generate
 //counterfeiter:generate . ConfigParser
 
@@ -95,6 +118,7 @@ func (ncp *NginxConfigParser) Parse(ctx context.Context, instance *mpi.Instance)
 			LexOptions: crossplane.LexOptions{
 				Lexers: []crossplane.RegisterLexer{lua.RegisterLexer()},
 			},
+			Glob: globFunction,
 		},
 	)
 	if err != nil {

internal/file/file_manager_service.go

@@ -13,7 +13,6 @@ import (
 	"fmt"
 	"log/slog"
 	"os"
-	"path"
 	"path/filepath"
 	"strconv"
 	"sync"
@@ -82,7 +81,6 @@ type (
 			err error)
 		Rollback(ctx context.Context, instanceID string) error
 		ClearCache()
-		SetConfigPath(configPath string)
 		ConfigUpload(ctx context.Context, configUploadRequest *mpi.ConfigUploadRequest) error
 		ConfigUpdate(ctx context.Context, nginxConfigContext *model.NginxConfigContext)
 		UpdateCurrentFilesOnDisk(ctx context.Context, updateFiles map[string]*mpi.File, referenced bool) error
@@ -108,9 +106,6 @@ type FileManagerService struct {
 	currentFilesOnDisk    map[string]*mpi.File // key is file path
 	previousManifestFiles map[string]*model.ManifestFile
 	manifestFilePath      string
-	tempConfigDir         string
-	tempRollbackDir       string
-	tempConfigPath        string
 	rollbackManifest      bool
 	filesMutex            sync.RWMutex
 }
@@ -131,11 +126,6 @@ func NewFileManagerService(fileServiceClient mpi.FileServiceClient, agentConfig
 	}
 }
 
-func (fms *FileManagerService) SetConfigPath(configPath string) {
-	fms.tempConfigPath = fmt.Sprintf("%s/.agent-%s", filepath.Dir(configPath), fms.agentConfig.UUID)
-	fms.ClearCache()
-}
-
 func (fms *FileManagerService) ResetClient(ctx context.Context, fileServiceClient mpi.FileServiceClient) {
 	fms.fileServiceOperator.UpdateClient(ctx, fileServiceClient)
 	slog.DebugContext(ctx, "File manager service reset client successfully")
@@ -152,9 +142,6 @@ func (fms *FileManagerService) SetIsConnected(isConnected bool) {
 func (fms *FileManagerService) ConfigApply(ctx context.Context,
 	configApplyRequest *mpi.ConfigApplyRequest,
 ) (status model.WriteStatus, err error) {
-	var configTempErr error
-	var rollbackTempErr error
-
 	fms.rollbackManifest = true
 	fileOverview := configApplyRequest.GetOverview()
 
@@ -189,16 +176,6 @@ func (fms *FileManagerService) ConfigApply(ctx context.Context,
 
 	fms.fileActions = diffFiles
 
-	fms.tempConfigDir, configTempErr = fms.createTempConfigDirectory("config")
-	if configTempErr != nil {
-		return model.Error, configTempErr
-	}
-
-	fms.tempRollbackDir, rollbackTempErr = fms.createTempConfigDirectory("rollback")
-	if rollbackTempErr != nil {
-		return model.Error, rollbackTempErr
-	}
-
 	rollbackTempFilesErr := fms.backupFiles(ctx)
 	if rollbackTempFilesErr != nil {
 		return model.Error, rollbackTempFilesErr
@@ -220,14 +197,22 @@ func (fms *FileManagerService) ConfigApply(ctx context.Context,
 }
 
 func (fms *FileManagerService) ClearCache() {
-	slog.Debug("Clearing cache and temp files")
-	clear(fms.fileActions)
-	clear(fms.previousManifestFiles)
+	slog.Debug("Clearing cache and backup files")
 
-	configErr := os.RemoveAll(fms.tempConfigPath)
-	if configErr != nil && !os.IsNotExist(configErr) {
-		slog.Error("Error removing temp directory", "path", fms.tempConfigDir, "err", configErr)
+	for _, fileAction := range fms.fileActions {
+		if fileAction.Action == model.Update || fileAction.Action == model.Delete {
+			tempFilePath := tempBackupFilePath(fileAction.File.GetFileMeta().GetName())
+			if err := os.Remove(tempFilePath); err != nil && !os.IsNotExist(err) {
+				slog.Warn("Unable to delete backup file",
+					"file", fileAction.File.GetFileMeta().GetName(),
+					"error", err,
+				)
+			}
+		}
 	}
+
+	clear(fms.fileActions)
+	clear(fms.previousManifestFiles)
 }
 
 //nolint:revive,cyclop // cognitive-complexity of 13 max is 12, loop is needed cant be broken up
@@ -502,7 +487,7 @@ func (fms *FileManagerService) backupFiles(ctx context.Context) error {
 			continue
 		}
 
-		tempFilePath := filepath.Join(fms.tempRollbackDir, filePath)
+		tempFilePath := tempBackupFilePath(filePath)
 		slog.DebugContext(ctx, "Attempting to backup file content since file exists", "temp_path", tempFilePath)
 
 		moveErr := fms.fileOperator.MoveFile(ctx, filePath, tempFilePath)
@@ -519,7 +504,7 @@ func (fms *FileManagerService) restoreFiles(fileAction *model.FileCache) ([]byte
 	fileMeta := fileAction.File.GetFileMeta()
 	fileName := fileMeta.GetName()
 
-	tempFilePath := filepath.Join(fms.tempRollbackDir, fileName)
+	tempFilePath := tempBackupFilePath(fileName)
 
 	// Create parent directories for the target file if they don't exist
 	if err := os.MkdirAll(filepath.Dir(fileName), dirPerm); err != nil {
@@ -570,26 +555,24 @@ func (fms *FileManagerService) manifestFile() (map[string]*model.ManifestFile, m
 
 func (fms *FileManagerService) executeFileActions(ctx context.Context) (actionError error) {
 	// Download files to temporary location
-	downloadError := fms.downloadUpdatedFilesToTempLocation(ctx, fms.tempConfigDir)
+	downloadError := fms.downloadUpdatedFilesToTempLocation(ctx)
 	if downloadError != nil {
 		return downloadError
 	}
 
 	// Remove temp files if there is a failure moving or deleting files
-	actionError = fms.moveOrDeleteFiles(ctx, fms.tempConfigDir, actionError)
+	actionError = fms.moveOrDeleteFiles(ctx, actionError)
 	if actionError != nil {
-		fms.deleteTempFiles(ctx, fms.tempConfigDir)
+		fms.deleteTempFiles(ctx)
 	}
 
 	return actionError
 }
 
-func (fms *FileManagerService) downloadUpdatedFilesToTempLocation(
-	ctx context.Context, tempDir string,
-) (updateError error) {
+func (fms *FileManagerService) downloadUpdatedFilesToTempLocation(ctx context.Context) (updateError error) {
 	for _, fileAction := range fms.fileActions {
 		if fileAction.Action == model.Add || fileAction.Action == model.Update {
-			tempFilePath := filepath.Join(tempDir, fileAction.File.GetFileMeta().GetName())
+			tempFilePath := tempFilePath(fileAction.File.GetFileMeta().GetName())
 
 			slog.DebugContext(
 				ctx,
@@ -608,7 +591,7 @@ func (fms *FileManagerService) downloadUpdatedFilesToTempLocation(
 	return updateError
 }
 
-func (fms *FileManagerService) moveOrDeleteFiles(ctx context.Context, tempDir string, actionError error) error {
+func (fms *FileManagerService) moveOrDeleteFiles(ctx context.Context, actionError error) error {
 actionsLoop:
 	for _, fileAction := range fms.fileActions {
 		switch fileAction.Action {
@@ -624,7 +607,8 @@ actionsLoop:
 			continue
 		case model.Add, model.Update:
 			fileMeta := fileAction.File.GetFileMeta()
-			err := fms.fileServiceOperator.RenameFile(ctx, fileMeta.GetHash(), fileMeta.GetName(), tempDir)
+			tempFilePath := tempFilePath(fileAction.File.GetFileMeta().GetName())
+			err := fms.fileServiceOperator.RenameFile(ctx, fileMeta.GetHash(), tempFilePath, fileMeta.GetName())
 			if err != nil {
 				actionError = err
 
@@ -638,11 +622,11 @@ actionsLoop:
 	return actionError
 }
 
-func (fms *FileManagerService) deleteTempFiles(ctx context.Context, tempDir string) {
+func (fms *FileManagerService) deleteTempFiles(ctx context.Context) {
 	for _, fileAction := range fms.fileActions {
 		if fileAction.Action == model.Add || fileAction.Action == model.Update {
-			tempFile := path.Join(tempDir, fileAction.File.GetFileMeta().GetName())
-			if err := os.Remove(tempFile); err != nil && !os.IsNotExist(err) {
+			tempFilePath := tempFilePath(fileAction.File.GetFileMeta().GetName())
+			if err := os.Remove(tempFilePath); err != nil && !os.IsNotExist(err) {
 				slog.ErrorContext(
 					ctx, "Error deleting temp file",
 					"file", fileAction.File.GetFileMeta().GetName(),
@@ -767,21 +751,6 @@ func (fms *FileManagerService) convertToFile(manifestFile *model.ManifestFile) *
 	}
 }
 
-func (fms *FileManagerService) createTempConfigDirectory(pattern string) (string, error) {
-	if _, err := os.Stat(fms.tempConfigPath); os.IsNotExist(err) {
-		mkdirErr := os.MkdirAll(fms.tempConfigPath, dirPerm)
-		if mkdirErr != nil {
-			return "", mkdirErr
-		}
-	}
-	tempDir, tempDirError := os.MkdirTemp(fms.tempConfigPath, pattern)
-	if tempDirError != nil {
-		return "", fmt.Errorf("failed creating temp config directory: %w", tempDirError)
-	}
-
-	return tempDir, nil
-}
-
 // ConvertToMapOfFiles converts a list of files to a map of file caches (file and action) with the file name as the key
 func ConvertToMapOfFileCache(convertFiles []*mpi.File) map[string]*model.FileCache {
 	filesMap := make(map[string]*model.FileCache)
@@ -793,3 +762,13 @@ func ConvertToMapOfFileCache(convertFiles []*mpi.File) map[string]*model.FileCac
 
 	return filesMap
 }
+
+func tempFilePath(fileName string) string {
+	tempFileName := "." + filepath.Base(fileName) + ".agent.tmp"
+	return filepath.Join(filepath.Dir(fileName), tempFileName)
+}
+
+func tempBackupFilePath(fileName string) string {
+	tempFileName := "." + filepath.Base(fileName) + ".agent.backup"
+	return filepath.Join(filepath.Dir(fileName), tempFileName)
+}