fix: always use absolute paths for WAL upload (#45)

While pgbackrest can work with relative
ones, such setup requires additional
config flags and a matching Postgresql
working directory configuration, which
seems to be invalid in the context of the plugin's container.

Signed-off-by: Szymon Soloch <ssoloch@opera.com>

Author: Agalin

internal/pgbackrest/archiver/command.go

@@ -37,6 +37,10 @@ import (
 // `requestedWALFile` is the name of the file whose archiving was requested by
 // PostgreSQL, and that file is always the first of the list and is always included.
 // `parallel` is the maximum number of WALs that we can archive in parallel
+// It's important to ensure this method returns absolute paths. While pgbackrest can
+// work with relative ones, such setup requires additional config flags and a matching
+// Postgresql working directory configuration, which seems to be invalid in the context
+// of the plugin's container.
 func (archiver *WALArchiver) GatherWALFilesToArchive(
 	ctx context.Context,
 	requestedWALFile string,
@@ -57,7 +61,10 @@ func (archiver *WALArchiver) GatherWALFilesToArchive(
 	// slightly more optimized, but equivalent to:
 	// walList = []string{requestedWALFile}
 	walList = make([]string, 1, walListLength)
-	walList[0] = requestedWALFile
+	// Ensure it's an absolute path. While Postgres should be configured to use absolute
+	// paths in the archive command, its documentation mentions that even in this mode
+	// a relative path might be used in some cases.
+	walList[0] = filepath.Join(pgWalDirectory, filepath.Base(requestedWALFile))
 
 	err := filepath.WalkDir(archiveStatusPath, func(path string, d os.DirEntry, err error) error {
 		// If err is set, it means the current path is a directory and the readdir raised an error
@@ -96,7 +103,7 @@ func (archiver *WALArchiver) GatherWALFilesToArchive(
 			return nil
 		}
 
-		walList = append(walList, filepath.Join("pg_wal", walFileName))
+		walList = append(walList, filepath.Join(pgWalDirectory, walFileName))
 		return nil
 	})
 

internal/pgbackrest/archiver/command_test.go

@@ -111,51 +111,61 @@ var _ = Describe("GatherWALFilesToArchive", func() {
 		os.Unsetenv("PGDATA")
 	})
 
-	// Helper function to create .ready files
-	createReadyFile := func(walName string) {
+	// Helper function to create .ready files and return the absolute path to the file.
+	// Pgbackrest preffers absolute paths for uploaded files. For relative ones
+	// additional flags must be passed and Postgres config must match those flags.
+	// For some reason it doesn't so we ensure only absolute paths are returned.
+	createReadyFile := func(walName string) string {
 		path := filepath.Join(archiveStatusDir, walName+".ready")
 		err := os.WriteFile(path, []byte{}, 0644)
 		Expect(err).ToNot(HaveOccurred())
+		return filepath.Join(tempPgData, "pg_wal", walName)
 	}
 
 	Context("when parallel=1", func() {
 		It("should gather only the requested file", func(ctx SpecContext) {
 			// Create .ready files for multiple WAL files
-			createReadyFile("000000010000000000000001")
+			wal1 := createReadyFile("000000010000000000000001")
 			createReadyFile("000000010000000000000002")
 			createReadyFile("000000010000000000000003")
 
 			walList := archiver.GatherWALFilesToArchive(ctx, "pg_wal/000000010000000000000001", 1)
 
-			Expect(walList).To(ConsistOf("pg_wal/000000010000000000000001"))
+			Expect(walList).To(ConsistOf(wal1))
+		})
+
+		It("should handle an absolute path as input", func(ctx SpecContext) {
+			// Create .ready files for multiple WAL files
+			wal1 := createReadyFile("000000010000000000000001")
+			createReadyFile("000000010000000000000002")
+			createReadyFile("000000010000000000000003")
+
+			walList := archiver.GatherWALFilesToArchive(ctx, filepath.Join(tempPgData, "pg_wal", "000000010000000000000001"), 1)
+
+			Expect(walList).To(ConsistOf(wal1))
 		})
 
 		It("should handle when no other .ready files exist", func(ctx SpecContext) {
 			// Only create the requested file
-			createReadyFile("000000010000000000000001")
+			wal1 := createReadyFile("000000010000000000000001")
 
 			walList := archiver.GatherWALFilesToArchive(ctx, "pg_wal/000000010000000000000001", 1)
 
-			Expect(walList).To(ConsistOf("pg_wal/000000010000000000000001"))
+			Expect(walList).To(ConsistOf(wal1))
 		})
 	})
 
 	Context("when parallel>1", func() {
 		It("should gather multiple files when parallel=4", func(ctx SpecContext) {
 			// Create .ready files for multiple WAL files
-			createReadyFile("000000010000000000000001")
-			createReadyFile("000000010000000000000002")
-			createReadyFile("000000010000000000000003")
-			createReadyFile("000000010000000000000004")
+			wal1 := createReadyFile("000000010000000000000001")
+			wal2 := createReadyFile("000000010000000000000002")
+			wal3 := createReadyFile("000000010000000000000003")
+			wal4 := createReadyFile("000000010000000000000004")
 
 			walList := archiver.GatherWALFilesToArchive(ctx, "pg_wal/000000010000000000000001", 4)
 
-			Expect(walList).To(ConsistOf(
-				"pg_wal/000000010000000000000001",
-				"pg_wal/000000010000000000000002",
-				"pg_wal/000000010000000000000003",
-				"pg_wal/000000010000000000000004",
-			))
+			Expect(walList).To(ConsistOf(wal1, wal2, wal3, wal4))
 		})
 
 		It("should not exceed parallel limit even when more files are ready", func(ctx SpecContext) {
@@ -171,36 +181,34 @@ var _ = Describe("GatherWALFilesToArchive", func() {
 
 		It("should handle when fewer files exist than parallel limit", func(ctx SpecContext) {
 			// Create only 2 .ready files but request parallel=5
-			createReadyFile("000000010000000000000001")
-			createReadyFile("000000010000000000000002")
+			wal1 := createReadyFile("000000010000000000000001")
+			wal2 := createReadyFile("000000010000000000000002")
 
 			walList := archiver.GatherWALFilesToArchive(ctx, "pg_wal/000000010000000000000001", 5)
 
 			// Should only get the files that exist
-			Expect(walList).To(ConsistOf(
-				"pg_wal/000000010000000000000001",
-				"pg_wal/000000010000000000000002",
-			))
+			Expect(walList).To(ConsistOf(wal1, wal2))
 		})
 	})
 
 	Context("edge cases", func() {
 		It("should handle empty archive_status directory", func(ctx SpecContext) {
 			// Don't create any .ready files
+			expectedWal := filepath.Join(tempPgData, "pg_wal", "000000010000000000000001")
 
 			walList := archiver.GatherWALFilesToArchive(ctx, "pg_wal/000000010000000000000001", 3)
 
 			// Should still return the requested file
-			Expect(walList).To(ConsistOf("pg_wal/000000010000000000000001"))
+			Expect(walList).To(ConsistOf(expectedWal))
 		})
 
 	})
 
 	Context("other files in directory", func() {
 		It("should ignore non-.ready files in archive_status", func(ctx SpecContext) {
 			// Create .ready files
-			createReadyFile("000000010000000000000001")
-			createReadyFile("000000010000000000000002")
+			wal1 := createReadyFile("000000010000000000000001")
+			wal2 := createReadyFile("000000010000000000000002")
 
 			// Create .done files (should be ignored)
 			donePath := filepath.Join(archiveStatusDir, "000000010000000000000003.done")
@@ -215,22 +223,16 @@ var _ = Describe("GatherWALFilesToArchive", func() {
 			walList := archiver.GatherWALFilesToArchive(ctx, "pg_wal/000000010000000000000001", 5)
 
 			// Should only get .ready files, not .done or other files
-			Expect(walList).To(ConsistOf(
-				"pg_wal/000000010000000000000001",
-				"pg_wal/000000010000000000000002",
-			))
+			Expect(walList).To(ConsistOf(wal1, wal2))
 		})
 		It("should handle timeline history files", func(ctx SpecContext) {
 			// Create timeline history file
-			createReadyFile("00000002.history")
-			createReadyFile("000000010000000000000001")
+			history := createReadyFile("00000002.history")
+			wal1 := createReadyFile("000000010000000000000001")
 
 			walList := archiver.GatherWALFilesToArchive(ctx, "pg_wal/00000002.history", 2)
 
-			Expect(walList).To(ConsistOf(
-				"pg_wal/00000002.history",
-				"pg_wal/000000010000000000000001",
-			))
+			Expect(walList).To(ConsistOf(history, wal1))
 		})
 	})
 })