diff --git a/internal/config/config.go b/internal/config/config.go index e7489310b..540311b3d 100644 --- a/internal/config/config.go +++ b/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)) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 38c8f98e3..334d60502 100644 --- a/internal/config/config_test.go +++ b/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 { 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) }) }