Merge release-3.5.1 back into main

internal/config/config.go

@@ -47,7 +47,7 @@ const (
 	// Regular expression to match invalid characters in paths.
 	// It matches whitespace, control characters, non-printable characters, and specific Unicode characters.
 	regexInvalidPath  = "\\s|[[:cntrl:]]|[[:space:]]|[[^:print:]]|ㅤ|\\.\\.|\\*"
-	regexLabelPattern = "^[a-zA-Z0-9]([a-zA-Z0-9-_]{0,254}[a-zA-Z0-9])?$"
+	regexLabelPattern = "^[a-zA-Z0-9]([a-zA-Z0-9-_.]{0,254}[a-zA-Z0-9])?$"
 )
 
 var viperInstance = viper.NewWithOptions(viper.KeyDelimiter(KeyDelimiter))

internal/config/config_test.go

@@ -1559,6 +1559,56 @@ func TestValidateLabel(t *testing.T) {
 			input:    "label 123",
 			expected: false,
 		},
+		{
+			name:     "Test 9: Valid label - cluster id",
+			input:    "73623aef-1d5b-4f6b-b73d-5561c36851cc",
+			expected: true,
+		},
+		{
+			name:     "Test 10: Valid label - installation name",
+			input:    "my-release-nginx-ingress-controller",
+			expected: true,
+		},
+		{
+			name:     "Test 11: Valid label - product",
+			input:    "nic",
+			expected: true,
+		},
+		{
+			name:     "Test 12: Valid label - version",
+			input:    "5.3.0",
+			expected: true,
+		},
+		{
+			name:     "Test 13: Valid label - version snapshot",
+			input:    "5.3.0-SNAPSHOT",
+			expected: true,
+		},
+		{
+			name:     "Test 14: Invalid label - newlines",
+			input:    "label-2\n\n",
+			expected: false,
+		},
+		{
+			name:     "Test 15: Valid label - only numbers",
+			input:    "1234567",
+			expected: true,
+		},
+		{
+			name:     "Test 16: Invalid label - start and end with .",
+			input:    ".label.",
+			expected: false,
+		},
+		{
+			name:     "Test 17: Invalid label - lots of blank spaces",
+			input:    "                 label",
+			expected: false,
+		},
+		{
+			name:     "Test 18: Invalid label - start and end with blank space",
+			input:    " label ",
+			expected: false,
+		},
 	}
 
 	for _, tt := range tests {

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