diff --git a/internal/constants/constants.go b/internal/constants/constants.go index 670063f..5fad68b 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -3,6 +3,7 @@ package constants import ( + "encoding/json" "log/slog" "os" "path/filepath" @@ -38,7 +39,7 @@ const ( ) // OptOutJSON is the data sent in case of Opt-Out choice. -var OptOutJSON = struct{ OptOut bool }{OptOut: true} +var OptOutJSON = json.RawMessage(`{"OptOut": true}`) type options struct { baseDir func() (string, error) diff --git a/internal/testutils/files.go b/internal/testutils/files.go index 84a0cf1..681cb19 100644 --- a/internal/testutils/files.go +++ b/internal/testutils/files.go @@ -3,7 +3,10 @@ package testutils import ( + "bytes" + "fmt" "io" + "io/fs" "os" "path/filepath" "testing" @@ -52,3 +55,46 @@ func CopyDir(srcDir, dstDir string) error { return CopyFile(path, dstPath) }) } + +// GetDirContents returns the contents of a directory as a map of file paths to file contents. +// The contents are read as strings. +// The maxDepth parameter limits the depth of the directory tree to read. +func GetDirContents(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 + } + // Normalize content between Windows and Linux + content = bytes.ReplaceAll(content, []byte("\r\n"), []byte("\n")) + files[filepath.ToSlash(relPath)] = string(content) + } + + return nil + }) + + return files, err +} diff --git a/internal/uploader/internal_test.go b/internal/uploader/internal_test.go index 01aafa6..84b716b 100644 --- a/internal/uploader/internal_test.go +++ b/internal/uploader/internal_test.go @@ -1 +1,192 @@ package uploader + +import ( + "math" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "github.com/ubuntu/ubuntu-insights/internal/constants" + "github.com/ubuntu/ubuntu-insights/internal/fileutils" +) + +type mockTimeProvider struct { + currentTime int64 +} + +func (m mockTimeProvider) NowUnix() int64 { + return m.currentTime +} + +func TestUploadBadFile(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + fName string + fileContents string + missingFile bool + fileIsDir bool + url string + consent bool + minAge uint + + wantErr bool + }{ + "Ok": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, wantErr: false}, + "Missing File": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, missingFile: true, wantErr: true}, + "File Is Dir": {fName: "0.json", fileIsDir: true}, + "Non-numeric": {fName: "not-numeric.json", fileContents: `{"Content":true, "string": "string"}`, wantErr: true}, + "Bad File Ext": {fName: "0.txt", fileContents: `{"Content":true, "string": "string"}`}, + "Bad Contents": {fName: "0.json", fileContents: `bad content`}, + "minAge Overflow": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, minAge: math.MaxUint64, wantErr: true}, + "Bad URL": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, url: "http://bad host:1234", wantErr: true}, + + "Ok Consent": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, consent: true, wantErr: false}, + "Missing File Consent": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, missingFile: true, consent: true, wantErr: true}, + "File Is Dir Consent": {fName: "0.json", fileIsDir: true, consent: true, wantErr: true}, + "Non-numeric Consent": {fName: "not-numeric.json", fileContents: `{"Content":true, "string": "string"}`, consent: true, wantErr: true}, + "Bad File Ext Consent": {fName: "0.txt", fileContents: `{"Content":true, "string": "string"}`, consent: true}, + "Bad Contents Consent": {fName: "0.json", fileContents: `bad content`, consent: true, wantErr: true}, + "minAge Overflow Consent": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, minAge: math.MaxUint64, consent: true, wantErr: true}, + "Bad URL Consent": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, url: "http://bad host:1234", consent: true, wantErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + require.False(t, tc.missingFile && tc.fileIsDir, "Test case cannot have both missing file and file is dir") + + dir := t.TempDir() + + um := &Manager{ + collectedDir: filepath.Join(dir, constants.LocalFolder), + uploadedDir: filepath.Join(dir, constants.UploadedFolder), + minAge: tc.minAge, + timeProvider: mockTimeProvider{currentTime: 0}, + } + + require.NoError(t, os.Mkdir(um.collectedDir, 0750), "Setup: failed to create uploaded folder") + require.NoError(t, os.Mkdir(um.uploadedDir, 0750), "Setup: failed to create collected folder") + fPath := filepath.Join(um.collectedDir, tc.fName) + + if !tc.missingFile && !tc.fileIsDir { + require.NoError(t, fileutils.AtomicWrite(fPath, []byte(tc.fileContents)), "Setup: failed to create report file") + } + if tc.fileIsDir { + require.NoError(t, os.Mkdir(fPath, 0750), "Setup: failed to create directory") + } + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(func() { ts.Close() }) + + if tc.url == "" { + tc.url = ts.URL + } + + err := um.upload(tc.fName, tc.url, tc.consent) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} + +func TestMoveReport(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + fileExists bool + + wantErr bool + }{ + "File Exists": {fileExists: true}, + + "File Not Found": {fileExists: false, wantErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + + um := &Manager{ + collectedDir: filepath.Join(dir, constants.LocalFolder), + uploadedDir: filepath.Join(dir, constants.UploadedFolder), + } + + require.NoError(t, os.MkdirAll(um.collectedDir, 0750), "Setup: failed to create uploaded folder") + require.NoError(t, os.MkdirAll(um.uploadedDir, 0750), "Setup: failed to create collected folder") + + if tc.fileExists { + f, err := os.Create(filepath.Join(um.collectedDir, "report.json")) + require.NoError(t, err) + f.Close() + } + + err := um.moveReport("report.json", []byte("payload")) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(um.uploadedDir, "report.json")) + if !tc.fileExists { + require.Error(t, err, "File should not exist in the uploaded directory") + return + } + + require.NoError(t, err, "File should exist in the uploaded directory") + }) + } +} + +func TestSend(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + url string + noServer bool + serverResponse int + + wantErr bool + }{ + "No Server": {noServer: true, wantErr: true}, + "Bad URL": {url: "http://local host:1234", serverResponse: http.StatusOK, wantErr: true}, + "Bad Response": {serverResponse: http.StatusForbidden, wantErr: true}, + + "Success": {serverResponse: http.StatusOK}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tc.serverResponse) + })) + t.Cleanup(func() { ts.Close() }) + + if tc.url == "" { + tc.url = ts.URL + } + if tc.noServer { + ts.Close() + } + + err := send(tc.url, []byte("payload")) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} diff --git a/internal/uploader/testdata/TestUpload/golden/consent_manager_false b/internal/uploader/testdata/TestUpload/golden/consent_manager_false index 4d2381a..4b69e98 100644 --- a/internal/uploader/testdata/TestUpload/golden/consent_manager_false +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_false @@ -1 +1 @@ -uploaded/1.json: '{"OptOut":true}' +uploaded/1.json: '{"OptOut": true}' 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 index 4d2381a..4b69e98 100644 --- a/internal/uploader/testdata/TestUpload/golden/consent_manager_global_false,_source_true +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_false,_source_true @@ -1 +1 @@ -uploaded/1.json: '{"OptOut":true}' +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 index 4d2381a..4b69e98 100644 --- a/internal/uploader/testdata/TestUpload/golden/consent_manager_global_true,_source_false +++ b/internal/uploader/testdata/TestUpload/golden/consent_manager_global_true,_source_false @@ -1 +1 @@ -uploaded/1.json: '{"OptOut":true}' +uploaded/1.json: '{"OptOut": true}' diff --git a/internal/uploader/testdata/TestUpload/golden/invalid_directory b/internal/uploader/testdata/TestUpload/golden/invalid_directory new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/internal/uploader/testdata/TestUpload/golden/invalid_directory @@ -0,0 +1 @@ +{} diff --git a/internal/uploader/upload.go b/internal/uploader/upload.go index 4775582..c2aba24 100644 --- a/internal/uploader/upload.go +++ b/internal/uploader/upload.go @@ -58,37 +58,40 @@ func (um Manager) Upload() error { return nil } -func (um Manager) upload(file, url string, consent bool) error { - slog.Debug("Uploading report", "file", file, "consent", consent) +func (um Manager) upload(fName, url string, consent bool) error { + slog.Debug("Uploading report", "file", fName, "consent", consent) - ts, err := reportutils.GetReportTime(file) + ts, err := reportutils.GetReportTime(fName) 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) + return ErrMinAgeOverflow } - if ts+int64(um.minAge) > um.timeProvider.NowUnix() { - slog.Debug("Skipping report due to min age", "timestamp", file, "minAge", um.minAge) + if ts > um.timeProvider.NowUnix()-int64(um.minAge) { + slog.Debug("Skipping report due to min age", "timestamp", fName, "minAge", um.minAge) return ErrReportNotMature } - payload, err := um.getPayload(file, consent) + payload, err := um.getPayload(fName, 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 um.dryRun { + slog.Debug("Dry run, skipping upload") + return nil + } - if err := um.moveReport(file, payload); err != nil { - return fmt.Errorf("failed to move report after uploading: %v", err) - } + if err := send(url, payload); err != nil { + return fmt.Errorf("failed to send data: %v", err) + } + + if err := um.moveReport(fName, payload); err != nil { + return fmt.Errorf("failed to move report after uploading: %v", err) } return nil @@ -104,32 +107,31 @@ func (um Manager) getURL() (string, error) { } func (um Manager) getPayload(file string, consent bool) ([]byte, error) { - path := path.Join(um.collectedDir, file) + if !consent { + // Return the opt-out JSON + data, err := constants.OptOutJSON.MarshalJSON() + if err != nil { + return nil, fmt.Errorf("failed to marshal opt-out JSON data: %v", err) + } + return data, nil + } + var jsonData map[string]interface{} - data, err := json.Marshal(constants.OptOutJSON) + // Read the report file + data, err := os.ReadFile(path.Join(um.collectedDir, file)) if err != nil { - return nil, fmt.Errorf("failed to marshal JSON data") + return nil, fmt.Errorf("failed to read report file: %v", err) } - 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) - } + // Remarshal 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) + } - return data, nil + data, err = json.Marshal(jsonData) + if err != nil { + return nil, fmt.Errorf("failed to marshal JSON data: %v", err) } return data, nil diff --git a/internal/uploader/uploader.go b/internal/uploader/uploader.go index a1eebe3..db8d995 100644 --- a/internal/uploader/uploader.go +++ b/internal/uploader/uploader.go @@ -5,6 +5,7 @@ package uploader import ( "errors" "log/slog" + "math" "path/filepath" "time" @@ -18,6 +19,8 @@ var ( 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") + // ErrMinAgeOverflow is returned when the min age is too large and would overflow. + ErrMinAgeOverflow = errors.New("min age is too large, would overflow") ) type timeProvider interface { @@ -65,6 +68,10 @@ func New(cm consentManager, source string, minAge uint, dryRun bool, args ...Opt return Manager{}, ErrEmptySource } + if minAge > math.MaxInt64 { + return Manager{}, ErrMinAgeOverflow + } + opts := options{ baseServerURL: constants.DefaultServerURL, cachePath: constants.GetDefaultCachePath(), diff --git a/internal/uploader/uploader_test.go b/internal/uploader/uploader_test.go index 720c2e4..62fe2a6 100644 --- a/internal/uploader/uploader_test.go +++ b/internal/uploader/uploader_test.go @@ -1,10 +1,9 @@ package uploader_test import ( - "bytes" "encoding/json" "fmt" - "io/fs" + "math" "net/http" "net/http/httptest" "os" @@ -32,19 +31,51 @@ func (m mockTimeProvider) NowUnix() int64 { return m.currentTime } -func TestUpload(t *testing.T) { +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} +) + +func TestNew(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} - ) + tests := map[string]struct { + cm testConsentManager + source string + minAge uint + dryRun bool + + wantErr bool + }{ + "Valid": {cm: cmTrue, source: "source", minAge: 5, dryRun: true}, + "Zero Min Age": {cm: cmTrue, source: "source", minAge: 0}, + + "Empty Source": {cm: cmTrue, source: "", wantErr: true}, + "Minage Overflow": {cm: cmTrue, source: "source", minAge: math.MaxUint64, wantErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + _, err := uploader.New(tc.cm, tc.source, tc.minAge, tc.dryRun) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} + +func TestUpload(t *testing.T) { + t.Parallel() const mockTime = 10 @@ -54,6 +85,7 @@ func TestUpload(t *testing.T) { serverResponse int serverOffline bool url string + invalidDir bool cm testConsentManager minAge uint @@ -83,6 +115,8 @@ func TestUpload(t *testing.T) { "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}, + + "Invalid Directory": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, invalidDir: true, wantErr: true}, } for name, tc := range tests { @@ -92,14 +126,19 @@ func TestUpload(t *testing.T) { dir := setupTmpDir(t, tc.localFiles, tc.uploadedFiles, tc.dummy) if !tc.serverOffline { - status := statusHandler(tc.serverResponse) - ts := httptest.NewServer(&status) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tc.serverResponse) + })) t.Cleanup(func() { ts.Close() }) if tc.url == "" { tc.url = ts.URL } } + if tc.invalidDir { + require.NoError(t, os.RemoveAll(filepath.Join(dir, "local")), "Setup: failed to remove local directory") + } + 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") @@ -111,7 +150,7 @@ func TestUpload(t *testing.T) { require.NoError(t, err) } - got, err := getDirResult(t, dir, 3) + got, err := testutils.GetDirContents(t, dir, 3) require.NoError(t, err) want := testutils.LoadWithUpdateFromGoldenYAML(t, got) require.EqualValues(t, want, got) @@ -121,10 +160,7 @@ func TestUpload(t *testing.T) { 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) }) + dir := t.TempDir() localDir := filepath.Join(dir, "local") uploadedDir := filepath.Join(dir, "uploaded") @@ -165,46 +201,6 @@ func writeFiles(t *testing.T, targetDir string, files map[string]reportType) { } } -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 - } - // Normalize content between windows and linux - content = bytes.ReplaceAll(content, []byte("\r\n"), []byte("\n")) - files[filepath.ToSlash(relPath)] = string(content) - } - - return nil - }) - - return files, err -} - type testConsentManager struct { sState bool gState bool @@ -218,9 +214,3 @@ func (m testConsentManager) GetConsentState(source string) (bool, error) { } return m.gState, m.gErr } - -type statusHandler int - -func (h *statusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(int(*h)) -}