Ensure only files are in file overview in config apply request (#1395)

Author: dhurley

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
 		}

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)
 		})
 	}