From 60bc8f2ce2f297cb3d455af0729667400c87288b Mon Sep 17 00:00:00 2001 From: Brian Witt Date: Thu, 13 Nov 2025 14:45:35 -0800 Subject: [PATCH] error on out of space --- api/files.go | 39 ++++++++- api/files_test.go | 187 +++++++++++++++++++++++++++++++++++++++++++ files/public.go | 19 ++++- files/public_test.go | 184 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 426 insertions(+), 3 deletions(-) create mode 100644 api/files_test.go diff --git a/api/files.go b/api/files.go index 7c6ad54d4..3f2f39fe8 100644 --- a/api/files.go +++ b/api/files.go @@ -114,34 +114,69 @@ func apiFilesUpload(c *gin.Context) { } stored := []string{} + openFiles := []*os.File{} + // Write all files first for _, files := range c.Request.MultipartForm.File { for _, file := range files { src, err := file.Open() if err != nil { + // Close any files we've opened + for _, f := range openFiles { + _ = f.Close() + } AbortWithJSONError(c, 500, err) return } - defer func() { _ = src.Close() }() destPath := filepath.Join(path, filepath.Base(file.Filename)) dst, err := os.Create(destPath) if err != nil { + _ = src.Close() + // Close any files we've opened + for _, f := range openFiles { + _ = f.Close() + } AbortWithJSONError(c, 500, err) return } - defer func() { _ = dst.Close() }() _, err = io.Copy(dst, src) + _ = src.Close() if err != nil { + _ = dst.Close() + // Close any files we've opened + for _, f := range openFiles { + _ = f.Close() + } AbortWithJSONError(c, 500, err) return } + // Keep file open for batch sync + openFiles = append(openFiles, dst) stored = append(stored, filepath.Join(c.Params.ByName("dir"), filepath.Base(file.Filename))) } } + // Sync all files at once to catch ENOSPC errors + for i, dst := range openFiles { + err := dst.Sync() + if err != nil { + // Close all files + for _, f := range openFiles { + _ = f.Close() + } + AbortWithJSONError(c, 500, fmt.Errorf("error syncing file %s: %s", stored[i], err)) + return + } + } + + // Close all files + for _, dst := range openFiles { + _ = dst.Close() + } + apiFilesUploadedCounter.WithLabelValues(c.Params.ByName("dir")).Inc() c.JSON(200, stored) } diff --git a/api/files_test.go b/api/files_test.go new file mode 100644 index 000000000..e85117fdb --- /dev/null +++ b/api/files_test.go @@ -0,0 +1,187 @@ +package api + +import ( + "bytes" + "encoding/json" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + + "github.com/aptly-dev/aptly/aptly" + ctx "github.com/aptly-dev/aptly/context" + "github.com/gin-gonic/gin" + "github.com/smira/flag" + + . "gopkg.in/check.v1" +) + +type FilesUploadDiskFullSuite struct { + aptlyContext *ctx.AptlyContext + flags *flag.FlagSet + configFile *os.File + router http.Handler +} + +var _ = Suite(&FilesUploadDiskFullSuite{}) + +func (s *FilesUploadDiskFullSuite) SetUpTest(c *C) { + aptly.Version = "testVersion" + + // Create temporary config + file, err := os.CreateTemp("", "aptly") + c.Assert(err, IsNil) + s.configFile = file + + jsonString, err := json.Marshal(gin.H{ + "architectures": []string{}, + "rootDir": c.MkDir(), + }) + c.Assert(err, IsNil) + _, err = file.Write(jsonString) + c.Assert(err, IsNil) + _ = file.Close() + + // Setup flags and context + flags := flag.NewFlagSet("fakeFlags", flag.ContinueOnError) + flags.Bool("no-lock", false, "dummy") + flags.Int("db-open-attempts", 3, "dummy") + flags.String("config", s.configFile.Name(), "dummy") + flags.String("architectures", "", "dummy") + s.flags = flags + + aptlyContext, err := ctx.NewContext(s.flags) + c.Assert(err, IsNil) + + s.aptlyContext = aptlyContext + s.router = Router(aptlyContext) + context = aptlyContext // set global context +} + +func (s *FilesUploadDiskFullSuite) TearDownTest(c *C) { + if s.configFile != nil { + _ = os.Remove(s.configFile.Name()) + } + if s.aptlyContext != nil { + s.aptlyContext.Shutdown() + } +} + +// TestUploadSuccessWithSync verifies that file uploads succeed when there's space +// and that the Sync() call is made (by verifying the file is complete) +func (s *FilesUploadDiskFullSuite) TestUploadSuccessWithSync(c *C) { + // Create a test file to upload + testContent := []byte("test file content for upload") + + // Create multipart form + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + + part, err := writer.CreateFormFile("file", "testfile.txt") + c.Assert(err, IsNil) + + _, err = part.Write(testContent) + c.Assert(err, IsNil) + + err = writer.Close() + c.Assert(err, IsNil) + + // Create request + req, err := http.NewRequest("POST", "/api/files/testdir", body) + c.Assert(err, IsNil) + req.Header.Set("Content-Type", writer.FormDataContentType()) + + // Create response recorder + w := httptest.NewRecorder() + + // Call handler + s.router.ServeHTTP(w, req) + + // Check response + c.Assert(w.Code, Equals, 200) + + // Verify file was written and synced + uploadedFile := filepath.Join(s.aptlyContext.Config().GetRootDir(), "upload", "testdir", "testfile.txt") + content, err := os.ReadFile(uploadedFile) + c.Assert(err, IsNil) + c.Check(content, DeepEquals, testContent) +} + +// TestUploadVerifiesFileIntegrity ensures uploaded files are complete +func (s *FilesUploadDiskFullSuite) TestUploadVerifiesFileIntegrity(c *C) { + // Create larger test file + testContent := bytes.Repeat([]byte("A"), 10000) + + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + + part, err := writer.CreateFormFile("file", "largefile.bin") + c.Assert(err, IsNil) + + _, err = io.Copy(part, bytes.NewReader(testContent)) + c.Assert(err, IsNil) + + err = writer.Close() + c.Assert(err, IsNil) + + req, err := http.NewRequest("POST", "/api/files/testdir2", body) + c.Assert(err, IsNil) + req.Header.Set("Content-Type", writer.FormDataContentType()) + + w := httptest.NewRecorder() + s.router.ServeHTTP(w, req) + + c.Assert(w.Code, Equals, 200) + + // Verify complete file was written + uploadedFile := filepath.Join(s.aptlyContext.Config().GetRootDir(), "upload", "testdir2", "largefile.bin") + content, err := os.ReadFile(uploadedFile) + c.Assert(err, IsNil) + c.Check(len(content), Equals, len(testContent)) + c.Check(content, DeepEquals, testContent) +} + +// TestUploadMultipleFilesWithBatchSync tests that multiple files are synced in batch +func (s *FilesUploadDiskFullSuite) TestUploadMultipleFilesWithBatchSync(c *C) { + // Create multiple test files with different content + testFiles := map[string][]byte{ + "file1.txt": []byte("content of file 1"), + "file2.txt": bytes.Repeat([]byte("B"), 5000), + "file3.deb": []byte("debian package content"), + } + + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + + // Add all files to multipart form + for filename, content := range testFiles { + part, err := writer.CreateFormFile("file", filename) + c.Assert(err, IsNil) + _, err = part.Write(content) + c.Assert(err, IsNil) + } + + err := writer.Close() + c.Assert(err, IsNil) + + req, err := http.NewRequest("POST", "/api/files/multitest", body) + c.Assert(err, IsNil) + req.Header.Set("Content-Type", writer.FormDataContentType()) + + w := httptest.NewRecorder() + s.router.ServeHTTP(w, req) + + // Verify response + c.Assert(w.Code, Equals, 200) + + // Verify all files were written and synced correctly + uploadDir := filepath.Join(s.aptlyContext.Config().GetRootDir(), "upload", "multitest") + for filename, expectedContent := range testFiles { + uploadedFile := filepath.Join(uploadDir, filename) + content, err := os.ReadFile(uploadedFile) + c.Assert(err, IsNil, Commentf("Failed to read %s", filename)) + c.Check(content, DeepEquals, expectedContent, Commentf("Content mismatch for %s", filename)) + } +} diff --git a/files/public.go b/files/public.go index 7cf36ec8f..58271882b 100644 --- a/files/public.go +++ b/files/public.go @@ -119,7 +119,17 @@ func (storage *PublishedStorage) PutFile(path string, sourceFilename string) err }() _, err = io.Copy(f, source) - return err + if err != nil { + return err + } + + // Sync to ensure all data is written to disk and catch ENOSPC errors + err = f.Sync() + if err != nil { + return fmt.Errorf("error syncing file %s: %s", path, err) + } + + return nil } // Remove removes single file under public path @@ -268,6 +278,13 @@ func (storage *PublishedStorage) LinkFromPool(publishedPrefix, publishedRelPath, return err } + // Sync to ensure all data is written to disk and catch ENOSPC errors + err = dst.Sync() + if err != nil { + _ = dst.Close() + return fmt.Errorf("error syncing file %s: %s", destinationPath, err) + } + err = dst.Close() } else if storage.linkMethod == LinkMethodSymLink { err = localSourcePool.Symlink(sourcePath, destinationPath) diff --git a/files/public_test.go b/files/public_test.go index 14413167e..53085a5b4 100644 --- a/files/public_test.go +++ b/files/public_test.go @@ -2,7 +2,10 @@ package files import ( "os" + "os/exec" "path/filepath" + "runtime" + "strings" "syscall" "github.com/aptly-dev/aptly/aptly" @@ -337,3 +340,184 @@ func (s *PublishedStorageSuite) TestRootRemove(c *C) { dirStorage := NewPublishedStorage(pwd, "", "") c.Assert(func() { _ = dirStorage.RemoveDirs("", nil) }, PanicMatches, "trying to remove the root directory") } + +// Disk full error handling tests + +type DiskFullSuite struct { + root string +} + +var _ = Suite(&DiskFullSuite{}) + +func (s *DiskFullSuite) SetUpTest(c *C) { + // Only run on Linux where we can create loopback filesystems + if runtime.GOOS != "linux" { + c.Skip("disk full tests only run on Linux") + } + + // Check if running as root or with sudo capabilities + if os.Geteuid() != 0 { + c.Skip("disk full tests require root privileges") + } + + s.root = c.MkDir() +} + +// TestPutFileOutOfSpace tests that PutFile properly reports disk full errors +func (s *DiskFullSuite) TestPutFileOutOfSpace(c *C) { + mountPoint := filepath.Join(s.root, "smallfs") + err := os.MkdirAll(mountPoint, 0777) + c.Assert(err, IsNil) + + // Create a very small filesystem (1MB) + fsImage := filepath.Join(s.root, "small.img") + + // Create 1MB sparse file + cmd := exec.Command("dd", "if=/dev/zero", "of="+fsImage, "bs=1M", "count=1") + err = cmd.Run() + c.Assert(err, IsNil) + + // Format as ext4 + cmd = exec.Command("mkfs.ext4", "-F", fsImage) + err = cmd.Run() + c.Assert(err, IsNil) + + // Mount the filesystem + cmd = exec.Command("mount", "-o", "loop", fsImage, mountPoint) + err = cmd.Run() + c.Assert(err, IsNil) + defer func() { + _ = exec.Command("umount", mountPoint).Run() + }() + + // Create storage on the small filesystem + storage := NewPublishedStorage(mountPoint, "", "") + + // Create a large source file that won't fit (2MB) + largeFile := filepath.Join(s.root, "largefile") + cmd = exec.Command("dd", "if=/dev/zero", "of="+largeFile, "bs=1M", "count=2") + err = cmd.Run() + c.Assert(err, IsNil) + + // Try to put the large file - should fail with out of space error + err = storage.PutFile("testfile", largeFile) + c.Assert(err, NotNil) + c.Check(strings.Contains(err.Error(), "no space left on device") || + strings.Contains(err.Error(), "sync"), Equals, true, + Commentf("Expected disk full error, got: %v", err)) +} + +// TestLinkFromPoolCopyOutOfSpace tests that LinkFromPool with copy mode properly reports disk full errors +func (s *DiskFullSuite) TestLinkFromPoolCopyOutOfSpace(c *C) { + mountPoint := filepath.Join(s.root, "smallfs") + err := os.MkdirAll(mountPoint, 0777) + c.Assert(err, IsNil) + + // Create a very small filesystem (1MB) + fsImage := filepath.Join(s.root, "small.img") + + cmd := exec.Command("dd", "if=/dev/zero", "of="+fsImage, "bs=1M", "count=1") + err = cmd.Run() + c.Assert(err, IsNil) + + cmd = exec.Command("mkfs.ext4", "-F", fsImage) + err = cmd.Run() + c.Assert(err, IsNil) + + cmd = exec.Command("mount", "-o", "loop", fsImage, mountPoint) + err = cmd.Run() + c.Assert(err, IsNil) + defer func() { + _ = exec.Command("umount", mountPoint).Run() + }() + + // Create storage on the small filesystem using copy mode + storage := NewPublishedStorage(mountPoint, "copy", "") + + // Create a normal pool in a different location + poolPath := filepath.Join(s.root, "pool") + pool := NewPackagePool(poolPath, false) + cs := NewMockChecksumStorage() + + // Create a large package file (2MB) in the pool + largeFile := filepath.Join(s.root, "package.deb") + cmd = exec.Command("dd", "if=/dev/zero", "of="+largeFile, "bs=1M", "count=2") + err = cmd.Run() + c.Assert(err, IsNil) + + sourceChecksum, err := utils.ChecksumsForFile(largeFile) + c.Assert(err, IsNil) + + srcPoolPath, err := pool.Import(largeFile, "package.deb", + &utils.ChecksumInfo{MD5: "d41d8cd98f00b204e9800998ecf8427e"}, false, cs) + c.Assert(err, IsNil) + + // Try to link from pool - should fail with out of space error + err = storage.LinkFromPool("", "pool/main/p/package", "package.deb", + pool, srcPoolPath, sourceChecksum, false) + c.Assert(err, NotNil) + c.Check(strings.Contains(err.Error(), "no space left on device") || + strings.Contains(err.Error(), "sync"), Equals, true, + Commentf("Expected disk full error, got: %v", err)) +} + +// Alternative simpler test that doesn't require root +type DiskFullNoRootSuite struct { + root string +} + +var _ = Suite(&DiskFullNoRootSuite{}) + +func (s *DiskFullNoRootSuite) SetUpTest(c *C) { + s.root = c.MkDir() +} + +// This test verifies Sync() is called by checking it doesn't panic +// The actual disk full behavior is harder to test without root +func (s *DiskFullNoRootSuite) TestSyncIsCalled(c *C) { + storage := NewPublishedStorage(s.root, "", "") + + // Create a small test file + sourceFile := filepath.Join(s.root, "source.txt") + err := os.WriteFile(sourceFile, []byte("test content"), 0644) + c.Assert(err, IsNil) + + // PutFile should succeed with normal disk space + err = storage.PutFile("dest.txt", sourceFile) + c.Assert(err, IsNil) + + // Verify file was written + content, err := os.ReadFile(filepath.Join(s.root, "dest.txt")) + c.Assert(err, IsNil) + c.Check(string(content), Equals, "test content") +} + +func (s *DiskFullNoRootSuite) TestLinkFromPoolCopySyncIsCalled(c *C) { + storage := NewPublishedStorage(s.root, "copy", "") + poolPath := filepath.Join(s.root, "pool") + pool := NewPackagePool(poolPath, false) + cs := NewMockChecksumStorage() + + // Create a test package file + pkgFile := filepath.Join(s.root, "package.deb") + err := os.WriteFile(pkgFile, []byte("package content"), 0644) + c.Assert(err, IsNil) + + sourceChecksum, err := utils.ChecksumsForFile(pkgFile) + c.Assert(err, IsNil) + + srcPoolPath, err := pool.Import(pkgFile, "package.deb", + &utils.ChecksumInfo{MD5: "d41d8cd98f00b204e9800998ecf8427e"}, false, cs) + c.Assert(err, IsNil) + + // LinkFromPool with copy should succeed with normal disk space + err = storage.LinkFromPool("", "pool/main/p/package", "package.deb", + pool, srcPoolPath, sourceChecksum, false) + c.Assert(err, IsNil) + + // Verify file was written + destPath := filepath.Join(s.root, "pool/main/p/package/package.deb") + content, err := os.ReadFile(destPath) + c.Assert(err, IsNil) + c.Check(string(content), Equals, "package content") +}