diff --git a/internal/file/file_manager_service.go b/internal/file/file_manager_service.go index 6c7ed4afb..ee591f29f 100644 --- a/internal/file/file_manager_service.go +++ b/internal/file/file_manager_service.go @@ -176,6 +176,8 @@ func (fms *FileManagerService) ConfigApply(ctx context.Context, fms.fileActions = diffFiles + slog.DebugContext(ctx, "Executing config apply file actions", "actions", diffFiles) + rollbackTempFilesErr := fms.backupFiles(ctx) if rollbackTempFilesErr != nil { return model.Error, rollbackTempFilesErr @@ -373,24 +375,58 @@ func (fms *FileManagerService) DetermineFileActions( for _, modifiedFile := range modifiedFiles { fileName := modifiedFile.File.GetFileMeta().GetName() currentFile, ok := filesMap[fileName] - // default to unchanged action modifiedFile.Action = model.Unchanged - // if file is unmanaged, action is set to unchanged so file is skipped when performing actions + // If file is unmanaged, action is set to unchanged so file is skipped when performing actions. if modifiedFile.File.GetUnmanaged() { slog.DebugContext(ctx, "Skipping unmanaged file updates", "file_name", fileName) continue } - // if file doesn't exist in the current files, file has been added - // set file action - if _, statErr := os.Stat(fileName); errors.Is(statErr, os.ErrNotExist) { + + // If file currently exists on disk, is being tracked in manifest and file hash is different. + // Treat it as a file update. + if ok && modifiedFile.File.GetFileMeta().GetHash() != currentFile.GetFileMeta().GetHash() { + slog.DebugContext(ctx, "Tracked file requires updating", "file_name", fileName) + modifiedFile.Action = model.Update + fileDiff[fileName] = modifiedFile + + continue + } + + fileStats, statErr := os.Stat(fileName) + + // If file doesn't exist on disk. + // Treat it as adding a new file. + if errors.Is(statErr, os.ErrNotExist) { + slog.DebugContext(ctx, "New untracked file needs to be created", "file_name", fileName) modifiedFile.Action = model.Add fileDiff[fileName] = modifiedFile continue - // if file currently exists and file hash is different, file has been updated - // copy contents, set file action - } else if ok && modifiedFile.File.GetFileMeta().GetHash() != currentFile.GetFileMeta().GetHash() { + } + + // If there is an error other than not existing, return that error. + if statErr != nil { + return nil, fmt.Errorf("unable to stat file %s: %w", fileName, statErr) + } + + // If there is a directory with the same name, return an error. + if fileStats.IsDir() { + return nil, fmt.Errorf( + "unable to create file %s since a directory with the same name already exists", + fileName, + ) + } + + // If file already exists on disk but is not being tracked in manifest and the file hash is different. + // Treat it as a file update. + metadataOfFileOnDisk, err := files.FileMeta(fileName) + if err != nil { + return nil, fmt.Errorf("unable to get file metadata for %s: %w", fileName, err) + } + + if metadataOfFileOnDisk.GetHash() != modifiedFile.File.GetFileMeta().GetHash() { + slog.DebugContext(ctx, "Untracked file requires updating", "file_name", fileName) modifiedFile.Action = model.Update fileDiff[fileName] = modifiedFile } diff --git a/internal/file/file_manager_service_test.go b/internal/file/file_manager_service_test.go index e42128063..538295093 100644 --- a/internal/file/file_manager_service_test.go +++ b/internal/file/file_manager_service_test.go @@ -763,12 +763,12 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) { modifiedFiles: map[string]*model.FileCache{ addTestFile.Name(): { File: &mpi.File{ - FileMeta: protos.FileMeta(addTestFile.Name(), files.GenerateHash(fileContent)), + FileMeta: protos.FileMeta(addTestFile.Name(), files.GenerateHash(addFileContent)), }, }, updateTestFile.Name(): { File: &mpi.File{ - FileMeta: protos.FileMeta(updateTestFile.Name(), files.GenerateHash(fileContent)), + FileMeta: protos.FileMeta(updateTestFile.Name(), files.GenerateHash(updatedFileContent)), }, }, deleteTestFile.Name(): { @@ -782,10 +782,10 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) { FileMeta: protos.FileMeta(deleteTestFile.Name(), files.GenerateHash(fileContent)), }, updateTestFile.Name(): { - FileMeta: protos.FileMeta(updateTestFile.Name(), files.GenerateHash(fileContent)), + FileMeta: protos.FileMeta(updateTestFile.Name(), files.GenerateHash(updatedFileContent)), }, addTestFile.Name(): { - FileMeta: protos.FileMeta(addTestFile.Name(), files.GenerateHash(fileContent)), + FileMeta: protos.FileMeta(addTestFile.Name(), files.GenerateHash(addFileContent)), }, }, expectedCache: make(map[string]*model.FileCache), @@ -805,6 +805,24 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) { expectedContent: make(map[string][]byte), expectedError: nil, }, + { + name: "Test 4: File is actually a directory", + allowedDirs: []string{tempDir}, + modifiedFiles: map[string]*model.FileCache{ + tempDir: { + File: &mpi.File{ + FileMeta: protos.FileMeta(tempDir, files.GenerateHash(fileContent)), + }, + }, + }, + currentFiles: make(map[string]*mpi.File), + expectedCache: map[string]*model.FileCache(nil), + expectedContent: make(map[string][]byte), + expectedError: fmt.Errorf( + "unable to create file %s since a directory with the same name already exists", + tempDir, + ), + }, } for _, test := range tests { @@ -828,7 +846,13 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) { test.currentFiles, test.modifiedFiles, ) - require.NoError(tt, fileActionErr) + + if test.expectedError != nil { + require.EqualError(tt, fileActionErr, test.expectedError.Error()) + } else { + require.NoError(tt, fileActionErr) + } + assert.Equal(tt, test.expectedCache, diff) }) }