From 3ded77448c3a4211ca7b931dcff8856d5b4e5a4a Mon Sep 17 00:00:00 2001 From: Frederik Ring Date: Tue, 22 Feb 2022 07:49:24 +0100 Subject: [PATCH] Do not skip directories when creating tar archive (#72) * Update targz library to include potential ownership fix * Move archive logic to main repo * Remove assertions for debugging * Use relative path in assertion * Strip local part from archive location * Log when extracting in tests * Fix trimming of prfix * Add license info to archive.go file * Undo change in test assertion * Add test checking for preserved file ownership * use same postgres version in tests * Wrap errors when archiving, handle deletion at script layer --- cmd/backup/archive.go | 142 ++++++++++++++++++++++++++++++ cmd/backup/script.go | 3 +- go.mod | 1 - go.sum | 2 - test/compose/run.sh | 4 +- test/ownership/.gitignore | 1 + test/ownership/docker-compose.yml | 27 ++++++ test/ownership/run.sh | 28 ++++++ test/swarm/docker-compose.yml | 2 +- 9 files changed, 202 insertions(+), 8 deletions(-) create mode 100644 cmd/backup/archive.go create mode 100644 test/ownership/.gitignore create mode 100644 test/ownership/docker-compose.yml create mode 100644 test/ownership/run.sh diff --git a/cmd/backup/archive.go b/cmd/backup/archive.go new file mode 100644 index 0000000..d419eac --- /dev/null +++ b/cmd/backup/archive.go @@ -0,0 +1,142 @@ +// Copyright 2022 - Offen Authors +// SPDX-License-Identifier: MPL-2.0 + +// Portions of this file are taken from package `targz`, Copyright (c) 2014 Fredrik Wallgren +// Licensed under the MIT License: https://github.com/walle/targz/blob/57fe4206da5abf7dd3901b4af3891ec2f08c7b08/LICENSE + +package main + +import ( + "archive/tar" + "compress/gzip" + "fmt" + "io" + "io/fs" + "os" + "path" + "path/filepath" + "strings" +) + +func createArchive(inputFilePath, outputFilePath string) error { + inputFilePath = stripTrailingSlashes(inputFilePath) + inputFilePath, outputFilePath, err := makeAbsolute(inputFilePath, outputFilePath) + if err != nil { + return fmt.Errorf("createArchive: error transposing given file paths: %w", err) + } + if err := os.MkdirAll(filepath.Dir(outputFilePath), 0755); err != nil { + return fmt.Errorf("createArchive: error creating output file path: %w", err) + } + + if err := compress(inputFilePath, outputFilePath, filepath.Dir(inputFilePath)); err != nil { + return fmt.Errorf("createArchive: error creating archive: %w", err) + } + + return nil +} + +func stripTrailingSlashes(path string) string { + if len(path) > 0 && path[len(path)-1] == '/' { + path = path[0 : len(path)-1] + } + + return path +} + +func makeAbsolute(inputFilePath, outputFilePath string) (string, string, error) { + inputFilePath, err := filepath.Abs(inputFilePath) + if err == nil { + outputFilePath, err = filepath.Abs(outputFilePath) + } + + return inputFilePath, outputFilePath, err +} + +func compress(inPath, outFilePath, subPath string) error { + file, err := os.Create(outFilePath) + if err != nil { + return fmt.Errorf("compress: error creating out file: %w", err) + } + + prefix := path.Dir(outFilePath) + gzipWriter := gzip.NewWriter(file) + tarWriter := tar.NewWriter(gzipWriter) + + var paths []string + if err := filepath.WalkDir(inPath, func(path string, di fs.DirEntry, err error) error { + paths = append(paths, path) + return err + }); err != nil { + return fmt.Errorf("compress: error walking filesystem tree: %w", err) + } + + for _, p := range paths { + if err := writeTarGz(p, tarWriter, prefix); err != nil { + return fmt.Errorf("compress error writing %s to archive: %w", p, err) + } + } + + err = tarWriter.Close() + if err != nil { + return fmt.Errorf("compress: error closing tar writer: %w", err) + } + + err = gzipWriter.Close() + if err != nil { + return fmt.Errorf("compress: error closing gzip writer: %w", err) + } + + err = file.Close() + if err != nil { + return fmt.Errorf("compress: error closing file: %w", err) + } + + return nil +} + +func writeTarGz(path string, tarWriter *tar.Writer, prefix string) error { + fileInfo, err := os.Lstat(path) + if err != nil { + return fmt.Errorf("writeTarGz: error getting file infor for %s: %w", path, err) + } + + if fileInfo.Mode()&os.ModeSocket == os.ModeSocket { + return nil + } + + var link string + if fileInfo.Mode()&os.ModeSymlink == os.ModeSymlink { + var err error + if link, err = os.Readlink(path); err != nil { + return fmt.Errorf("writeTarGz: error resolving symlink %s: %w", path, err) + } + } + + header, err := tar.FileInfoHeader(fileInfo, link) + if err != nil { + return fmt.Errorf("writeTarGz: error getting file info header: %w", err) + } + header.Name = strings.TrimPrefix(path, prefix) + + err = tarWriter.WriteHeader(header) + if err != nil { + return fmt.Errorf("writeTarGz: error writing file info header: %w", err) + } + + if !fileInfo.Mode().IsRegular() { + return nil + } + + file, err := os.Open(path) + if err != nil { + return fmt.Errorf("writeTarGz: error opening %s: %w", path, err) + } + defer file.Close() + + _, err = io.Copy(tarWriter, file) + if err != nil { + return fmt.Errorf("writeTarGz: error copying %s to tar writer: %w", path, err) + } + + return nil +} diff --git a/cmd/backup/script.go b/cmd/backup/script.go index 58e6d64..bb6c85f 100644 --- a/cmd/backup/script.go +++ b/cmd/backup/script.go @@ -23,7 +23,6 @@ import ( "github.com/docker/docker/client" "github.com/kelseyhightower/envconfig" "github.com/leekchan/timeutil" - "github.com/m90/targz" "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" "github.com/otiai10/copy" @@ -364,7 +363,7 @@ func (s *script) takeBackup() error { s.logger.Infof("Removed tar file `%s`.", tarFile) return nil }) - if err := targz.Compress(backupSources, tarFile); err != nil { + if err := createArchive(backupSources, tarFile); err != nil { return fmt.Errorf("takeBackup: error compressing backup folder: %w", err) } diff --git a/go.mod b/go.mod index 6550468..ea5d2a7 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/gofrs/flock v0.8.1 github.com/kelseyhightower/envconfig v1.4.0 github.com/leekchan/timeutil v0.0.0-20150802142658-28917288c48d - github.com/m90/targz v0.0.0-20220208141135-d3baeef59a97 github.com/minio/minio-go/v7 v7.0.16 github.com/otiai10/copy v1.7.0 github.com/sirupsen/logrus v1.8.1 diff --git a/go.sum b/go.sum index 9359dca..4496260 100644 --- a/go.sum +++ b/go.sum @@ -450,8 +450,6 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/leekchan/timeutil v0.0.0-20150802142658-28917288c48d h1:2puqoOQwi3Ai1oznMOsFIbifm6kIfJaLLyYzWD4IzTs= github.com/leekchan/timeutil v0.0.0-20150802142658-28917288c48d/go.mod h1:hO90vCP2x3exaSH58BIAowSKvV+0OsY21TtzuFGHON4= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= -github.com/m90/targz v0.0.0-20220208141135-d3baeef59a97 h1:Uc/WzUKI/zvhkqIzk5TyaPE6AY1SD1DWGc7RV7cky4s= -github.com/m90/targz v0.0.0-20220208141135-d3baeef59a97/go.mod h1:YZK3bSO/oVlk9G+v00BxgzxW2Us4p/R4ysHOBjk0fJI= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mailru/easyjson v0.0.0-20190403194419-1ea4449da983/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= diff --git a/test/compose/run.sh b/test/compose/run.sh index 6870086..3628b90 100755 --- a/test/compose/run.sh +++ b/test/compose/run.sh @@ -27,8 +27,8 @@ docker run --rm -it \ -v compose_minio_backup_data:/minio_data \ -v compose_webdav_backup_data:/webdav_data alpine \ ash -c 'apk add gnupg && \ - echo 1234secret | gpg -d --pinentry-mode loopback --passphrase-fd 0 --yes /minio_data/backup/test-hostnametoken.tar.gz.gpg > /tmp/test-hostnametoken.tar.gz && tar -xf /tmp/test-hostnametoken.tar.gz -C /tmp && test -f /tmp/backup/app_data/offen.db && \ - echo 1234secret | gpg -d --pinentry-mode loopback --passphrase-fd 0 --yes /webdav_data/data/my/new/path/test-hostnametoken.tar.gz.gpg > /tmp/test-hostnametoken.tar.gz && tar -xf /tmp/test-hostnametoken.tar.gz -C /tmp && test -f /tmp/backup/app_data/offen.db' + echo 1234secret | gpg -d --pinentry-mode loopback --passphrase-fd 0 --yes /minio_data/backup/test-hostnametoken.tar.gz.gpg > /tmp/test-hostnametoken.tar.gz && tar -xvf /tmp/test-hostnametoken.tar.gz -C /tmp && test -f /tmp/backup/app_data/offen.db && \ + echo 1234secret | gpg -d --pinentry-mode loopback --passphrase-fd 0 --yes /webdav_data/data/my/new/path/test-hostnametoken.tar.gz.gpg > /tmp/test-hostnametoken.tar.gz && tar -xvf /tmp/test-hostnametoken.tar.gz -C /tmp && test -f /tmp/backup/app_data/offen.db' echo "[TEST:PASS] Found relevant files in decrypted and untared remote backups." diff --git a/test/ownership/.gitignore b/test/ownership/.gitignore new file mode 100644 index 0000000..4083037 --- /dev/null +++ b/test/ownership/.gitignore @@ -0,0 +1 @@ +local diff --git a/test/ownership/docker-compose.yml b/test/ownership/docker-compose.yml new file mode 100644 index 0000000..3bb8a65 --- /dev/null +++ b/test/ownership/docker-compose.yml @@ -0,0 +1,27 @@ +version: '3' + +services: + db: + image: postgres:14-alpine + restart: unless-stopped + labels: + - docker-volume-backup.stop-during-backup=true + volumes: + - postgres_data:/var/lib/postgresql/data + environment: + - POSTGRES_PASSWORD=1FHJMSwt0yhIN1zS7I4DilGUhThBKq0x + - POSTGRES_USER=test + - POSTGRES_DB=test + + backup: + image: offen/docker-volume-backup:${TEST_VERSION} + restart: always + environment: + BACKUP_FILENAME: backup.tar.gz + volumes: + - postgres_data:/backup/postgres:ro + - /var/run/docker.sock:/var/run/docker.sock:ro + - ./local:/archive + +volumes: + postgres_data: diff --git a/test/ownership/run.sh b/test/ownership/run.sh new file mode 100644 index 0000000..cde5503 --- /dev/null +++ b/test/ownership/run.sh @@ -0,0 +1,28 @@ +#!/bin/sh +# This test refers to https://github.com/offen/docker-volume-backup/issues/71 + +set -e + +cd $(dirname $0) + +mkdir -p local + +docker-compose up -d +sleep 5 + +docker-compose exec backup backup + +sudo tar --same-owner -xvf ./local/backup.tar.gz -C /tmp + +sudo find /tmp/backup/postgres > /dev/null +echo "[TEST:PASS] Backup contains files at expected location" + +for file in $(sudo find /tmp/backup/postgres); do + if [ "$(sudo stat -c '%u:%g' $file)" != "70:70" ]; then + echo "[TEST:FAIL] Unexpected file ownership for $file: $(sudo stat -c '%u:%g' $file)" + exit 1 + fi +done +echo "[TEST:PASS] All files and directories in backup preserved their ownership." + +docker-compose down --volumes diff --git a/test/swarm/docker-compose.yml b/test/swarm/docker-compose.yml index 5c26e72..ba780ea 100644 --- a/test/swarm/docker-compose.yml +++ b/test/swarm/docker-compose.yml @@ -49,7 +49,7 @@ services: condition: on-failure pg: - image: postgres:12.2-alpine + image: postgres:14-alpine environment: POSTGRES_PASSWORD: example labels: