diff --git a/cmd/insights/commands/upload.go b/cmd/insights/commands/upload.go index 6f680c3..aae5175 100644 --- a/cmd/insights/commands/upload.go +++ b/cmd/insights/commands/upload.go @@ -4,6 +4,7 @@ import ( "log/slog" "github.com/spf13/cobra" + "github.com/ubuntu/ubuntu-insights/internal/constants" ) type uploadConfig struct { @@ -16,7 +17,7 @@ type uploadConfig struct { var defaultUploadConfig = uploadConfig{ sources: []string{""}, - server: "https://metrics.ubuntu.com", + server: constants.DefaultServerURL, minAge: 604800, force: false, dryRun: false, diff --git a/internal/constants/constants.go b/internal/constants/constants.go index b5d600c..670063f 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -18,6 +18,15 @@ const ( // DefaultLogLevel is the default log level selected without any verbosity flags. DefaultLogLevel = slog.LevelInfo + // DefaultServerURL is the default base URL for the server that reports are uploaded to. + DefaultServerURL = "https://metrics.ubuntu.com" + + // LocalFolder is the default name of the local collected reports folder. + LocalFolder = "local" + + // UploadedFolder is the default name of the uploaded reports folder. + UploadedFolder = "uploaded" + // GlobalFileName is the default base name of the consent state files. GlobalFileName = "consent.toml" @@ -26,11 +35,11 @@ const ( // ReportExt is the default extension for the report files. ReportExt = ".json" - - // OptOutJSON is the data sent in case of Opt-Out choice. - OptOutJSON = `{"OptOut": true}` ) +// OptOutJSON is the data sent in case of Opt-Out choice. +var OptOutJSON = struct{ OptOut bool }{OptOut: true} + type options struct { baseDir func() (string, error) } diff --git a/internal/fileutils/fileutils.go b/internal/fileutils/fileutils.go index 0a118d3..b113a2f 100644 --- a/internal/fileutils/fileutils.go +++ b/internal/fileutils/fileutils.go @@ -2,7 +2,6 @@ package fileutils import ( - "errors" "fmt" "log/slog" "os" @@ -13,7 +12,7 @@ import ( // If the file already exists, then it will be overwritten. // Not atomic on Windows. func AtomicWrite(path string, data []byte) error { - tmp, err := os.CreateTemp(filepath.Dir(path), "consent-*.tmp") + tmp, err := os.CreateTemp(filepath.Dir(path), "tmp-*.tmp") if err != nil { return fmt.Errorf("could not create temporary file: %v", err) } @@ -37,12 +36,3 @@ func AtomicWrite(path string, data []byte) error { } return nil } - -// FileExists checks if a file exists at the given path. -func FileExists(path string) (bool, error) { - _, err := os.Stat(path) - if err != nil && !errors.Is(err, os.ErrNotExist) { - return false, err - } - return !errors.Is(err, os.ErrNotExist), nil -} diff --git a/internal/fileutils/fileutils_test.go b/internal/fileutils/fileutils_test.go index fc05209..15c2b96 100644 --- a/internal/fileutils/fileutils_test.go +++ b/internal/fileutils/fileutils_test.go @@ -15,16 +15,19 @@ func TestAtomicWrite(t *testing.T) { tests := map[string]struct { data []byte fileExists bool + invalidDir bool wantError bool }{ - "Empty file": {data: []byte{}, fileExists: false}, - "Non-empty file": {data: []byte("data"), fileExists: false}, + "Empty file": {data: []byte{}}, + "Non-empty file": {data: []byte("data")}, "Override file": {data: []byte("data"), fileExists: true}, "Override empty file": {data: []byte{}, fileExists: true}, "Existing empty file": {data: []byte{}, fileExists: true}, "Existing non-empty file": {data: []byte("data"), fileExists: true}, + + "Invalid Dir": {data: []byte("data"), invalidDir: true, wantError: true}, } for name, tc := range tests { @@ -34,6 +37,10 @@ func TestAtomicWrite(t *testing.T) { oldFile := []byte("Old File!") tempDir := t.TempDir() path := filepath.Join(tempDir, "file") + if tc.invalidDir { + path = filepath.Join(path, "fake_dir") + } + if tc.fileExists { err := fileutils.AtomicWrite(path, oldFile) require.NoError(t, err, "Setup: AtomicWrite should not return an error") @@ -44,60 +51,26 @@ func TestAtomicWrite(t *testing.T) { require.Error(t, err, "AtomicWrite should return an error") // Check that the file was not overwritten - data, err := os.ReadFile(path) - require.NoError(t, err, "ReadFile should not return an error") - require.Equal(t, oldFile, data, "AtomicWrite should not overwrite the file") - } else { - require.NoError(t, err, "AtomicWrite should not return an error") + if !tc.fileExists { + return + } + + if tc.invalidDir { + path = filepath.Dir(path) + } - // Check that the file was written data, err := os.ReadFile(path) require.NoError(t, err, "ReadFile should not return an error") - require.Equal(t, tc.data, data, "AtomicWrite should write the data to the file") - } - }) - } -} - -func TestFileExists(t *testing.T) { - t.Parallel() - - tests := map[string]struct { - fileExists bool - parentDirIsFile bool - - wantExists bool - wantError bool - }{ - "Returns_true_when_file_exists": {fileExists: true, wantExists: true}, - "Returns_false_when_file_does_not_exist": {fileExists: false, wantExists: false}, - "Returns_false_when_parent_directory_does_not_exist": {fileExists: false, wantExists: false}, - - "Error_when_parent_directory_is_a_file": {parentDirIsFile: true, wantError: true}, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - t.Parallel() + require.Equal(t, oldFile, data, "AtomicWrite should not overwrite the file") - tempDir := t.TempDir() - path := filepath.Join(tempDir, "file") - if tc.fileExists { - err := fileutils.AtomicWrite(path, []byte{}) - require.NoError(t, err, "AtomicWrite should not return an error") - } - if tc.parentDirIsFile { - path = filepath.Join(tempDir, "file", "file") - err := fileutils.AtomicWrite(filepath.Join(tempDir, "file"), []byte{}) - require.NoError(t, err, "AtomicWrite should not return an error") + return } + require.NoError(t, err, "AtomicWrite should not return an error") - exists, err := fileutils.FileExists(path) - if tc.wantError { - require.Error(t, err, "FileExists should return an error") - } else { - require.NoError(t, err, "FileExists should not return an error") - } - require.Equal(t, tc.wantExists, exists, "FileExists should return the expected result") + // Check that the file was written + data, err := os.ReadFile(path) + require.NoError(t, err, "ReadFile should not return an error") + require.Equal(t, tc.data, data, "AtomicWrite should write the data to the file") }) } } diff --git a/internal/reportutils/reportutils.go b/internal/reportutils/reportutils.go index 307f7e0..f08a695 100644 --- a/internal/reportutils/reportutils.go +++ b/internal/reportutils/reportutils.go @@ -45,32 +45,41 @@ func GetReportPath(dir string, time int64, period int) (string, error) { // Reports names are utc timestamps. Get the most recent report within the period window. var mostRecentReportPath string - files, err := os.ReadDir(dir) - if err != nil { - slog.Error("Failed to read directory", "directory", dir, "error", err) - return "", err - } + err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { + if err != nil { + slog.Error("Failed to access path", "path", path, "error", err) + return err + } - for _, file := range files { - if filepath.Ext(file.Name()) != constants.ReportExt { - slog.Info("Skipping non-report file, invalid extension", "file", file.Name()) - continue + // Skip subdirectories. + if d.IsDir() && path != dir { + return filepath.SkipDir } - reportTime, err := GetReportTime(file.Name()) + if filepath.Ext(d.Name()) != constants.ReportExt { + slog.Info("Skipping non-report file, invalid extension", "file", d.Name()) + return nil + } + + reportTime, err := GetReportTime(d.Name()) if err != nil { - slog.Info("Skipping non-report file, invalid file name", "file", file.Name()) - continue + slog.Info("Skipping non-report file, invalid file name", "file", d.Name()) + return nil } if reportTime < periodStart { - continue + return nil } if reportTime >= periodEnd { - break + return filepath.SkipDir } - mostRecentReportPath = filepath.Join(dir, file.Name()) + mostRecentReportPath = path + return nil + }) + + if err != nil { + return "", err } return mostRecentReportPath, nil @@ -78,36 +87,80 @@ func GetReportPath(dir string, time int64, period int) (string, error) { // GetReports returns the paths for the latest report within each period window for a given directory. // The key of the map is the start of the period window, and the report timestamp. +// +// If period is 1, then all reports in the dir are returned. func GetReports(dir string, period int) (map[int64]int64, error) { if period <= 0 { return nil, ErrInvalidPeriod } - // Get the most recent report within each period window. - files, err := os.ReadDir(dir) - if err != nil { - slog.Error("Failed to read directory", "directory", dir, "error", err) - return nil, err - } - - // Map to store the most recent report within each period window. reports := make(map[int64]int64) - for _, file := range files { - if filepath.Ext(file.Name()) != constants.ReportExt { - slog.Info("Skipping non-report file, invalid extension", "file", file.Name()) - continue + err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { + if err != nil { + slog.Error("Failed to access path", "path", path, "error", err) + return err } - reportTime, err := GetReportTime(file.Name()) + if d.IsDir() && path != dir { + return filepath.SkipDir + } + + if filepath.Ext(d.Name()) != constants.ReportExt { + slog.Info("Skipping non-report file, invalid extension", "file", d.Name()) + return nil + } + + reportTime, err := GetReportTime(d.Name()) if err != nil { - slog.Info("Skipping non-report file, invalid file name", "file", file.Name()) - continue + slog.Info("Skipping non-report file, invalid file name", "file", d.Name()) + return nil } periodStart := reportTime - (reportTime % int64(period)) if existingReport, ok := reports[periodStart]; !ok || existingReport < reportTime { reports[periodStart] = reportTime } + + return nil + }) + + if err != nil { + return nil, err + } + + return reports, nil +} + +// GetAllReports returns the filename for all reports within a given directory, which match the expected pattern. +// Does not traverse subdirectories. +func GetAllReports(dir string) ([]string, error) { + reports := make([]string, 0) + err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { + if err != nil { + slog.Error("Failed to access path", "path", path, "error", err) + return err + } + + if d.IsDir() && path != dir { + return filepath.SkipDir + } + + if filepath.Ext(d.Name()) != constants.ReportExt { + slog.Info("Skipping non-report file, invalid extension", "file", d.Name()) + return nil + } + + if _, err := GetReportTime(d.Name()); err != nil { + slog.Info("Skipping non-report file, invalid file name", "file", d.Name()) + return nil + } + + reports = append(reports, d.Name()) + return nil + }) + + if err != nil { + return nil, err } return reports, nil diff --git a/internal/reportutils/reportutils_test.go b/internal/reportutils/reportutils_test.go index 1f4a2d5..871eb21 100644 --- a/internal/reportutils/reportutils_test.go +++ b/internal/reportutils/reportutils_test.go @@ -176,6 +176,7 @@ func TestGetReports(t *testing.T) { "Get Newest of Period": {files: []string{"1.json", "7.json"}, period: 100}, "Multiple Consecutive Windows": {files: []string{"1.json", "7.json", "101.json", "107.json", "201.json", "207.json"}, period: 100}, "Multiple Non-Consecutive Windows": {files: []string{"1.json", "7.json", "101.json", "107.json", "251.json", "257.json"}, period: 50}, + "Get All Reports": {files: []string{"1.json", "2.json", "3.json", "101.json", "107.json", "251.json", "257.json"}, period: 1}, } for name, tc := range tests { @@ -198,3 +199,40 @@ func TestGetReports(t *testing.T) { }) } } + +func TestGetAllReports(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + files []string + subDir string + subDirFiles []string + + wantErr error + }{ + "Empty Directory": {}, + "Files in subDir": {files: []string{"1.json", "2.json"}, subDir: "subdir", subDirFiles: []string{"1.json", "2.json"}}, + "Invalid File Extension": {files: []string{"1.txt", "2.txt"}}, + "Invalid File Names": {files: []string{"i-1.json", "i-2.json", "i-3.json", "test.json", "one.json"}}, + "Mix of Valid and Invalid": {files: []string{"1.json", "2.json", "500.json", "i-1.json", "i-2.json", "i-3.json", "test.json", "five.json"}}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + dir, err := setupTmpDir(t, tc.files, tc.subDir, tc.subDirFiles) + require.NoError(t, err, "Setup: failed to setup temporary directory") + defer os.RemoveAll(dir) + + got, err := reportutils.GetAllReports(dir) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return + } + require.NoError(t, err, "got an unexpected error") + + want := testutils.LoadWithUpdateFromGoldenYAML(t, got) + require.Equal(t, want, got, "GetAllReports should return all reports in the directory") + }) + } +} diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/empty_directory b/internal/reportutils/testdata/TestGetAllReports/golden/empty_directory new file mode 100644 index 0000000..fe51488 --- /dev/null +++ b/internal/reportutils/testdata/TestGetAllReports/golden/empty_directory @@ -0,0 +1 @@ +[] diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/files_in_subdir b/internal/reportutils/testdata/TestGetAllReports/golden/files_in_subdir new file mode 100644 index 0000000..6548887 --- /dev/null +++ b/internal/reportutils/testdata/TestGetAllReports/golden/files_in_subdir @@ -0,0 +1,2 @@ +- 1.json +- 2.json diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_extension b/internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_extension new file mode 100644 index 0000000..fe51488 --- /dev/null +++ b/internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_extension @@ -0,0 +1 @@ +[] diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_names b/internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_names new file mode 100644 index 0000000..fe51488 --- /dev/null +++ b/internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_names @@ -0,0 +1 @@ +[] diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/mix_of_valid_and_invalid b/internal/reportutils/testdata/TestGetAllReports/golden/mix_of_valid_and_invalid new file mode 100644 index 0000000..a8eb048 --- /dev/null +++ b/internal/reportutils/testdata/TestGetAllReports/golden/mix_of_valid_and_invalid @@ -0,0 +1,3 @@ +- 1.json +- 2.json +- 500.json diff --git a/internal/reportutils/testdata/TestGetReports/golden/get_all_reports b/internal/reportutils/testdata/TestGetReports/golden/get_all_reports new file mode 100644 index 0000000..e295670 --- /dev/null +++ b/internal/reportutils/testdata/TestGetReports/golden/get_all_reports @@ -0,0 +1,7 @@ +1: 1 +2: 2 +3: 3 +101: 101 +107: 107 +251: 251 +257: 257 diff --git a/internal/reportutils/testdata/TestGetReports/golden/multiple_consecutive_windows b/internal/reportutils/testdata/TestGetReports/golden/multiple_consecutive_windows new file mode 100644 index 0000000..8e9a957 --- /dev/null +++ b/internal/reportutils/testdata/TestGetReports/golden/multiple_consecutive_windows @@ -0,0 +1,3 @@ +0: 7 +100: 107 +200: 207 diff --git a/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consecutive_windows b/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consecutive_windows new file mode 100644 index 0000000..f2cb19d --- /dev/null +++ b/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consecutive_windows @@ -0,0 +1,3 @@ +0: 7 +100: 107 +250: 257 diff --git a/internal/uploader/export_test.go b/internal/uploader/export_test.go index 7619a9b..d34d9f5 100644 --- a/internal/uploader/export_test.go +++ b/internal/uploader/export_test.go @@ -1,2 +1,19 @@ package uploader +func WithCachePath(path string) Options { + return func(o *options) { + o.cachePath = path + } +} + +func WithBaseServerURL(url string) Options { + return func(o *options) { + o.baseServerURL = url + } +} + +func WithTimeProvider(tp timeProvider) Options { + return func(o *options) { + o.timeProvider = tp + } +} diff --git a/internal/uploader/internal_test.go b/internal/uploader/internal_test.go new file mode 100644 index 0000000..01aafa6 --- /dev/null +++ b/internal/uploader/internal_test.go @@ -0,0 +1 @@ +package uploader diff --git a/internal/uploader/testdata/TestUpload/golden/bad_content b/internal/uploader/testdata/TestUpload/golden/bad_content new file mode 100644 index 0000000..be447b6 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/bad_content @@ -0,0 +1 @@ +local/1.json: bad content diff --git a/internal/uploader/testdata/TestUpload/golden/bad_response b/internal/uploader/testdata/TestUpload/golden/bad_response new file mode 100644 index 0000000..0d60902 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/bad_response @@ -0,0 +1 @@ +local/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/bad_url b/internal/uploader/testdata/TestUpload/golden/bad_url new file mode 100644 index 0000000..0d60902 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/bad_url @@ -0,0 +1 @@ +local/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/consent_manager_false b/internal/uploader/testdata/TestUpload/golden/consent_manager_false new file mode 100644 index 0000000..4d2381a --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_false @@ -0,0 +1 @@ +uploaded/1.json: '{"OptOut":true}' diff --git a/internal/uploader/testdata/TestUpload/golden/consent_manager_global_error b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_error new file mode 100644 index 0000000..0d60902 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_error @@ -0,0 +1 @@ +local/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/consent_manager_global_error_with_true b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_error_with_true new file mode 100644 index 0000000..0d60902 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_error_with_true @@ -0,0 +1 @@ +local/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/consent_manager_global_false,_source_true b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_false,_source_true new file mode 100644 index 0000000..4d2381a --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_false,_source_true @@ -0,0 +1 @@ +uploaded/1.json: '{"OptOut":true}' diff --git a/internal/uploader/testdata/TestUpload/golden/consent_manager_global_true,_source_false b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_true,_source_false new file mode 100644 index 0000000..4d2381a --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_true,_source_false @@ -0,0 +1 @@ +uploaded/1.json: '{"OptOut":true}' diff --git a/internal/uploader/testdata/TestUpload/golden/consent_manager_source_error b/internal/uploader/testdata/TestUpload/golden/consent_manager_source_error new file mode 100644 index 0000000..0d60902 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_source_error @@ -0,0 +1 @@ +local/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/consent_manager_source_error_with_true b/internal/uploader/testdata/TestUpload/golden/consent_manager_source_error_with_true new file mode 100644 index 0000000..0d60902 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_source_error_with_true @@ -0,0 +1 @@ +local/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/dry_run b/internal/uploader/testdata/TestUpload/golden/dry_run new file mode 100644 index 0000000..0d60902 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/dry_run @@ -0,0 +1 @@ +local/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/duplicate_upload b/internal/uploader/testdata/TestUpload/golden/duplicate_upload new file mode 100644 index 0000000..382d755 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/duplicate_upload @@ -0,0 +1,2 @@ +local/1.json: '{"Content":"normal content"}' +uploaded/1.json: bad content diff --git a/internal/uploader/testdata/TestUpload/golden/future_timestamp b/internal/uploader/testdata/TestUpload/golden/future_timestamp new file mode 100644 index 0000000..f8a9041 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/future_timestamp @@ -0,0 +1,2 @@ +local/11.json: '{"Content":"normal content"}' +uploaded/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/min_age b/internal/uploader/testdata/TestUpload/golden/min_age new file mode 100644 index 0000000..463c163 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/min_age @@ -0,0 +1,2 @@ +local/9.json: '{"Content":"normal content"}' +uploaded/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/multi_upload b/internal/uploader/testdata/TestUpload/golden/multi_upload new file mode 100644 index 0000000..55b3ee3 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/multi_upload @@ -0,0 +1,2 @@ +uploaded/1.json: '{"Content":"normal content"}' +uploaded/5.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/no_reports b/internal/uploader/testdata/TestUpload/golden/no_reports new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/no_reports @@ -0,0 +1 @@ +{} diff --git a/internal/uploader/testdata/TestUpload/golden/no_reports_with_dummy b/internal/uploader/testdata/TestUpload/golden/no_reports_with_dummy new file mode 100644 index 0000000..0a52bdc --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/no_reports_with_dummy @@ -0,0 +1,33 @@ +-1.json: bad contents +dummy.json: |- + { + "OS": "something" + } +dummy/dummy.json: |- + { + "OS": "something" + } +dummy/empty-dummy.json: "" +empty-dummy.json: "" +local/-1.json: bad contents +local/dummy.json: |- + { + "OS": "something" + } +local/dummy/dummy.json: |- + { + "OS": "something" + } +local/dummy/empty-dummy.json: "" +local/empty-dummy.json: "" +uploaded/-1.json: bad contents +uploaded/dummy.json: |- + { + "OS": "something" + } +uploaded/dummy/dummy.json: |- + { + "OS": "something" + } +uploaded/dummy/empty-dummy.json: "" +uploaded/empty-dummy.json: "" diff --git a/internal/uploader/testdata/TestUpload/golden/offline_server b/internal/uploader/testdata/TestUpload/golden/offline_server new file mode 100644 index 0000000..0d60902 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/offline_server @@ -0,0 +1 @@ +local/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/TestUpload/golden/single_upload b/internal/uploader/testdata/TestUpload/golden/single_upload new file mode 100644 index 0000000..9dc4645 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/single_upload @@ -0,0 +1 @@ +uploaded/1.json: '{"Content":"normal content"}' diff --git a/internal/uploader/testdata/test_source/-1.json b/internal/uploader/testdata/test_source/-1.json new file mode 100644 index 0000000..89d4cda --- /dev/null +++ b/internal/uploader/testdata/test_source/-1.json @@ -0,0 +1 @@ +bad contents \ No newline at end of file diff --git a/internal/uploader/testdata/test_source/dummy.json b/internal/uploader/testdata/test_source/dummy.json new file mode 100644 index 0000000..6012d3c --- /dev/null +++ b/internal/uploader/testdata/test_source/dummy.json @@ -0,0 +1,3 @@ +{ + "OS": "something" +} \ No newline at end of file diff --git a/internal/uploader/testdata/test_source/dummy/dummy.json b/internal/uploader/testdata/test_source/dummy/dummy.json new file mode 100644 index 0000000..6012d3c --- /dev/null +++ b/internal/uploader/testdata/test_source/dummy/dummy.json @@ -0,0 +1,3 @@ +{ + "OS": "something" +} \ No newline at end of file diff --git a/internal/uploader/testdata/test_source/dummy/empty-dummy.json b/internal/uploader/testdata/test_source/dummy/empty-dummy.json new file mode 100644 index 0000000..e69de29 diff --git a/internal/uploader/testdata/test_source/empty-dummy.json b/internal/uploader/testdata/test_source/empty-dummy.json new file mode 100644 index 0000000..e69de29 diff --git a/internal/uploader/upload.go b/internal/uploader/upload.go new file mode 100644 index 0000000..4775582 --- /dev/null +++ b/internal/uploader/upload.go @@ -0,0 +1,175 @@ +package uploader + +import ( + "bytes" + "encoding/json" + "fmt" + "log/slog" + "math" + "net/http" + "net/url" + "os" + "path" + "sync" + "time" + + "github.com/ubuntu/ubuntu-insights/internal/constants" + "github.com/ubuntu/ubuntu-insights/internal/fileutils" + "github.com/ubuntu/ubuntu-insights/internal/reportutils" +) + +// Upload uploads the reports corresponding to the source to the configured server. +// Does not do duplicate checks. +func (um Manager) Upload() error { + slog.Debug("Uploading reports") + + gConsent, err := um.consentManager.GetConsentState("") + if err != nil { + return fmt.Errorf("upload failed to get global consent state: %v", err) + } + + sConsent, err := um.consentManager.GetConsentState(um.source) + if err != nil { + return fmt.Errorf("upload failed to get source consent state: %v", err) + } + + reports, err := reportutils.GetAllReports(um.collectedDir) + if err != nil { + return fmt.Errorf("failed to get reports: %v", err) + } + + url, err := um.getURL() + if err != nil { + return fmt.Errorf("failed to get URL: %v", err) + } + + var wg sync.WaitGroup + for _, file := range reports { + wg.Add(1) + go func(file string) { + defer wg.Done() + if err := um.upload(file, url, gConsent && sConsent); err != nil { + slog.Warn("Failed to upload report", "file", file, "source", um.source, "error", err) + } + }(file) + } + wg.Wait() + + return nil +} + +func (um Manager) upload(file, url string, consent bool) error { + slog.Debug("Uploading report", "file", file, "consent", consent) + + ts, err := reportutils.GetReportTime(file) + if err != nil { + return fmt.Errorf("failed to parse report time from filename: %v", err) + } + + // Report maturity check + if um.minAge > math.MaxInt64 { + return fmt.Errorf("min age is too large: %d", um.minAge) + } + if ts+int64(um.minAge) > um.timeProvider.NowUnix() { + slog.Debug("Skipping report due to min age", "timestamp", file, "minAge", um.minAge) + return ErrReportNotMature + } + + payload, err := um.getPayload(file, consent) + if err != nil { + return fmt.Errorf("failed to get payload: %v", err) + } + slog.Debug("Uploading", "payload", payload) + + if !um.dryRun { + if err := send(url, payload); err != nil { + return fmt.Errorf("failed to send data: %v", err) + } + + if err := um.moveReport(file, payload); err != nil { + return fmt.Errorf("failed to move report after uploading: %v", err) + } + } + + return nil +} + +func (um Manager) getURL() (string, error) { + u, err := url.Parse(um.baseServerURL) + if err != nil { + return "", fmt.Errorf("failed to parse base server URL %s: %v", um.baseServerURL, err) + } + u.Path = path.Join(u.Path, um.source) + return u.String(), nil +} + +func (um Manager) getPayload(file string, consent bool) ([]byte, error) { + path := path.Join(um.collectedDir, file) + var jsonData map[string]interface{} + + data, err := json.Marshal(constants.OptOutJSON) + if err != nil { + return nil, fmt.Errorf("failed to marshal JSON data") + } + if consent { + // Read the report file + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("failed to read report file: %v", err) + } + + // Remashal the JSON data to ensure it is valid + if err := json.Unmarshal(data, &jsonData); err != nil { + return nil, fmt.Errorf("failed to unmarshal JSON data: %v", err) + } + + // Marshal the JSON data back to bytes + data, err = json.Marshal(jsonData) + if err != nil { + return nil, fmt.Errorf("failed to marshal JSON data: %v", err) + } + + return data, nil + } + + return data, nil +} + +// moveReport writes the uploaded report to the uploaded directory, and removes it from the collected directory. +func (um Manager) moveReport(file string, data []byte) error { + err := fileutils.AtomicWrite(path.Join(um.uploadedDir, file), data) + if err != nil { + return fmt.Errorf("failed to write report to uploaded directory: %v", err) + } + + err = os.Remove(path.Join(um.collectedDir, file)) + if err != nil { + return fmt.Errorf("failed to remove report from collected directory: %v", err) + } + + return nil +} + +func send(url string, data []byte) error { + slog.Debug("Sending data to server", "url", url, "data", data) + req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(data)) + if err != nil { + return fmt.Errorf("failed to create request: %v", err) + } + + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{Timeout: time.Second * 10} + + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("failed to send data: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("server returned status code %d", resp.StatusCode) + } + + return nil +} diff --git a/internal/uploader/uploader.go b/internal/uploader/uploader.go index b1c189e..a1eebe3 100644 --- a/internal/uploader/uploader.go +++ b/internal/uploader/uploader.go @@ -1,3 +1,88 @@ // Package uploader implements the uploader component. // The uploader component is responsible for uploading reports to the Ubuntu Insights server. package uploader + +import ( + "errors" + "log/slog" + "path/filepath" + "time" + + "github.com/ubuntu/ubuntu-insights/internal/constants" +) + +var ( + // ErrReportNotMature is returned when a report is not mature enough to be uploaded based on min age. + ErrReportNotMature = errors.New("report is not mature enough to be uploaded") + // ErrDuplicateReport is returned when a report has already been uploaded for this period. + ErrDuplicateReport = errors.New("report has already been uploaded for this period") + // ErrEmptySource is returned when the passed source is incorrectly an empty string. + ErrEmptySource = errors.New("source cannot be an empty string") +) + +type timeProvider interface { + NowUnix() int64 +} + +type realTimeProvider struct{} + +func (realTimeProvider) NowUnix() int64 { + return time.Now().Unix() +} + +// Manager is an abstraction of the uploader component. +type Manager struct { + source string + consentManager consentManager + minAge uint + dryRun bool + + baseServerURL string + collectedDir string + uploadedDir string + timeProvider timeProvider +} + +type options struct { + // Private members exported for tests. + baseServerURL string + cachePath string + timeProvider timeProvider +} + +// Options represents an optional function to override Upload Manager default values. +type Options func(*options) + +type consentManager interface { + GetConsentState(source string) (bool, error) +} + +// New returns a new UploaderManager. +func New(cm consentManager, source string, minAge uint, dryRun bool, args ...Options) (Manager, error) { + slog.Debug("Creating new uploader manager", "source", source, "minAge", minAge, "dryRun", dryRun) + + if source == "" { + return Manager{}, ErrEmptySource + } + + opts := options{ + baseServerURL: constants.DefaultServerURL, + cachePath: constants.GetDefaultCachePath(), + timeProvider: realTimeProvider{}, + } + for _, opt := range args { + opt(&opts) + } + + return Manager{ + source: source, + consentManager: cm, + minAge: minAge, + dryRun: dryRun, + timeProvider: opts.timeProvider, + + baseServerURL: opts.baseServerURL, + collectedDir: filepath.Join(opts.cachePath, constants.LocalFolder), + uploadedDir: filepath.Join(opts.cachePath, constants.UploadedFolder), + }, nil +} diff --git a/internal/uploader/uploader_test.go b/internal/uploader/uploader_test.go new file mode 100644 index 0000000..c76570b --- /dev/null +++ b/internal/uploader/uploader_test.go @@ -0,0 +1,223 @@ +package uploader_test + +import ( + "encoding/json" + "fmt" + "io/fs" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "github.com/ubuntu/ubuntu-insights/internal/fileutils" + "github.com/ubuntu/ubuntu-insights/internal/testutils" + "github.com/ubuntu/ubuntu-insights/internal/uploader" +) + +type reportType any + +var ( + normal reportType = struct{ Content string }{Content: "normal content"} + badContent = `bad content` +) + +type mockTimeProvider struct { + currentTime int64 +} + +func (m mockTimeProvider) NowUnix() int64 { + return m.currentTime +} + +func TestUpload(t *testing.T) { + t.Parallel() + + var ( + cmSErr = testConsentManager{sErr: fmt.Errorf("consent error")} + cmTrueSErr = testConsentManager{sState: true, gState: true, sErr: fmt.Errorf("consent error")} + cmGErr = testConsentManager{gErr: fmt.Errorf("consent error")} + cmTrueGErr = testConsentManager{gState: true, gErr: fmt.Errorf("consent error")} + cmTrue = testConsentManager{sState: true, gState: true} + cmFalse = testConsentManager{sState: false, gState: false} + cmSTrue = testConsentManager{sState: true, gState: false} + cmGTrue = testConsentManager{sState: false, gState: true} + ) + + const mockTime = 10 + + tests := map[string]struct { + localFiles, uploadedFiles map[string]reportType + dummy bool + serverResponse int + serverOffline bool + url string + + cm testConsentManager + minAge uint + dryRun bool + + wantErr bool + }{ + "No Reports": {cm: cmTrue, serverResponse: http.StatusOK}, + "No Reports with Dummy": {dummy: true, cm: cmTrue, serverResponse: http.StatusOK}, + "Single Upload": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, serverResponse: http.StatusOK}, + "Multi Upload": {localFiles: map[string]reportType{"1.json": normal, "5.json": normal}, cm: cmTrue, serverResponse: http.StatusOK}, + "Min Age": {localFiles: map[string]reportType{"1.json": normal, "9.json": normal}, cm: cmTrue, minAge: 5, serverResponse: http.StatusOK}, + "Future Timestamp": {localFiles: map[string]reportType{"1.json": normal, "11.json": normal}, cm: cmTrue, serverResponse: http.StatusOK}, + "Duplicate Upload": {localFiles: map[string]reportType{"1.json": normal}, uploadedFiles: map[string]reportType{"1.json": badContent}, cm: cmTrue, serverResponse: http.StatusAccepted}, + "Bad Content": {localFiles: map[string]reportType{"1.json": badContent}, cm: cmTrue, serverResponse: http.StatusOK}, + + "Consent Manager Source Error": {localFiles: map[string]reportType{"1.json": normal}, cm: cmSErr, serverResponse: http.StatusOK, wantErr: true}, + "Consent Manager Source Error with True": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrueSErr, serverResponse: http.StatusOK, wantErr: true}, + "Consent Manager Global Error": {localFiles: map[string]reportType{"1.json": normal}, cm: cmGErr, serverResponse: http.StatusOK, wantErr: true}, + "Consent Manager Global Error with True": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrueGErr, serverResponse: http.StatusOK, wantErr: true}, + "Consent Manager False": {localFiles: map[string]reportType{"1.json": normal}, cm: cmFalse, serverResponse: http.StatusOK}, + "Consent Manager Global True, Source False": {localFiles: map[string]reportType{"1.json": normal}, cm: cmGTrue, serverResponse: http.StatusOK}, + "Consent Manager Global False, Source True": {localFiles: map[string]reportType{"1.json": normal}, cm: cmSTrue, serverResponse: http.StatusOK}, + + "Dry run": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, dryRun: true}, + + "Bad URL": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, url: "http://a b.com/", wantErr: true}, + "Bad Response": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, serverResponse: http.StatusForbidden}, + "Offline Server": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, serverOffline: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + dir := setupTmpDir(t, tc.localFiles, tc.uploadedFiles, tc.dummy) + + if !tc.serverOffline { + status := statusHandler(tc.serverResponse) + ts := httptest.NewServer(&status) + t.Cleanup(func() { ts.Close() }) + if tc.url == "" { + tc.url = ts.URL + } + } + + mgr, err := uploader.New(tc.cm, "source", tc.minAge, tc.dryRun, + uploader.WithBaseServerURL(tc.url), uploader.WithCachePath(dir), uploader.WithTimeProvider(mockTimeProvider{currentTime: mockTime})) + require.NoError(t, err, "Setup: failed to create new uploader manager") + + err = mgr.Upload() + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + got, err := getDirResult(t, dir, 3) + require.NoError(t, err) + want := testutils.LoadWithUpdateFromGoldenYAML(t, got) + require.EqualValues(t, want, got) + }) + } +} + +func setupTmpDir(t *testing.T, localFiles, uploadedFiles map[string]reportType, dummy bool) string { + t.Helper() + + dir, err := os.MkdirTemp("", "uploader-test") + require.NoError(t, err, "Setup: failed to create temporary directory") + t.Cleanup(func() { os.RemoveAll(dir) }) + + localDir := filepath.Join(dir, "local") + uploadedDir := filepath.Join(dir, "uploaded") + require.NoError(t, os.Mkdir(localDir, 0750), "Setup: failed to create local directory") + require.NoError(t, os.Mkdir(uploadedDir, 0750), "Setup: failed to create uploaded directory") + + if dummy { + copyDummyData(t, "testdata/test_source", dir, localDir, uploadedDir) + } + + writeFiles(t, localDir, localFiles) + writeFiles(t, uploadedDir, uploadedFiles) + + return dir +} + +func copyDummyData(t *testing.T, sourceDir, dir, localDir, uploadedDir string) { + t.Helper() + require.NoError(t, testutils.CopyDir(sourceDir, dir), "Setup: failed to copy dummy data to temporary directory") + require.NoError(t, testutils.CopyDir(sourceDir, localDir), "Setup: failed to copy dummy data to local") + require.NoError(t, testutils.CopyDir(sourceDir, uploadedDir), "Setup: failed to copy dummy data to uploaded") +} + +func writeFiles(t *testing.T, targetDir string, files map[string]reportType) { + t.Helper() + for file, content := range files { + var data []byte + var err error + + switch v := content.(type) { + case string: + data = []byte(v) + default: + data, err = json.Marshal(content) + require.NoError(t, err, "Setup: failed to marshal sample data") + } + require.NoError(t, fileutils.AtomicWrite(filepath.Join(targetDir, file), data), "Setup: failed to write file") + } +} + +func getDirResult(t *testing.T, dir string, maxDepth uint) (map[string]string, error) { + t.Helper() + + files := make(map[string]string) + + err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if path == dir { + return nil + } + + relPath, err := filepath.Rel(dir, path) + if err != nil { + return err + } + + depth := uint(len(filepath.SplitList(relPath))) + if depth > maxDepth { + return fmt.Errorf("max depth %d exceeded at %s", maxDepth, relPath) + } + + if !d.IsDir() { + content, err := os.ReadFile(path) + if err != nil { + return err + } + files[filepath.ToSlash(relPath)] = string(content) + } + + return nil + }) + + return files, err +} + +type testConsentManager struct { + sState bool + gState bool + sErr error + gErr error +} + +func (m testConsentManager) GetConsentState(source string) (bool, error) { + if source != "" { + return m.sState, m.sErr + } + return m.gState, m.gErr +} + +type statusHandler int + +func (h *statusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(int(*h)) +}