From 0ed78b333de32aab4e9f157e5be32fce0a3292e0 Mon Sep 17 00:00:00 2001 From: Frederik Ring Date: Tue, 13 Feb 2024 12:01:08 +0100 Subject: [PATCH 1/3] Extend confd test case to test for env var propagation --- test/confd/01backup.env | 2 +- test/confd/02backup.env | 2 +- test/confd/03never.env | 2 +- test/confd/docker-compose.yml | 3 +++ 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/confd/01backup.env b/test/confd/01backup.env index aaeee072..8d6d7e3f 100644 --- a/test/confd/01backup.env +++ b/test/confd/01backup.env @@ -1,2 +1,2 @@ -BACKUP_FILENAME="conf.tar.gz" BACKUP_CRON_EXPRESSION="*/1 * * * *" +NAME="conf" diff --git a/test/confd/02backup.env b/test/confd/02backup.env index 7f33b66b..4278acb4 100644 --- a/test/confd/02backup.env +++ b/test/confd/02backup.env @@ -1,2 +1,2 @@ -BACKUP_FILENAME="other.tar.gz" +NAME="other" BACKUP_CRON_EXPRESSION="*/1 * * * *" diff --git a/test/confd/03never.env b/test/confd/03never.env index df320c58..17c5c70a 100644 --- a/test/confd/03never.env +++ b/test/confd/03never.env @@ -1,2 +1,2 @@ -BACKUP_FILENAME="never.tar.gz" +NAME="never" BACKUP_CRON_EXPRESSION="0 0 5 31 2 ?" diff --git a/test/confd/docker-compose.yml b/test/confd/docker-compose.yml index 832ec6ff..86c21f57 100644 --- a/test/confd/docker-compose.yml +++ b/test/confd/docker-compose.yml @@ -4,6 +4,9 @@ services: backup: image: offen/docker-volume-backup:${TEST_VERSION:-canary} restart: always + environment: + BACKUP_FILENAME: $$NAME.tar.gz + BACKUP_FILENAME_EXPAND: 'true' volumes: - ${LOCAL_DIR:-./local}:/archive - app_data:/backup/app_data:ro From 36f4b98ec8970db0db225b94fa6bdedf9becfc78 Mon Sep 17 00:00:00 2001 From: Frederik Ring Date: Tue, 13 Feb 2024 13:09:01 +0100 Subject: [PATCH 2/3] Env vars set in conf.d files are expected to propagate --- cmd/backup/config_provider.go | 12 +++++++---- cmd/backup/main.go | 40 +++++++++++++++++++++++++++++------ cmd/backup/script.go | 5 ----- test/confd/01backup.env | 2 +- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/cmd/backup/config_provider.go b/cmd/backup/config_provider.go index 50a588d4..306b7592 100644 --- a/cmd/backup/config_provider.go +++ b/cmd/backup/config_provider.go @@ -49,8 +49,9 @@ func loadEnvVars() (*Config, error) { } type configFile struct { - name string - config *Config + name string + config *Config + additionalEnvVars map[string]string } func loadEnvFiles(directory string) ([]configFile, error) { @@ -74,13 +75,16 @@ func loadEnvFiles(directory string) ([]configFile, error) { } lookup := func(key string) (string, bool) { val, ok := envFile[key] - return val, ok + if ok { + return val, ok + } + return os.LookupEnv(key) } c, err := loadConfig(lookup) if err != nil { return nil, fmt.Errorf("loadEnvFiles: error loading config from file %s: %w", p, err) } - cs = append(cs, configFile{config: c, name: item.Name()}) + cs = append(cs, configFile{config: c, name: item.Name(), additionalEnvVars: envFile}) } return cs, nil diff --git a/cmd/backup/main.go b/cmd/backup/main.go index f52183c3..1aff16b6 100644 --- a/cmd/backup/main.go +++ b/cmd/backup/main.go @@ -36,7 +36,7 @@ func (c *command) must(err error) { } } -func runScript(c *Config) (err error) { +func runScript(c *Config, envVars map[string]string) (err error) { defer func() { if derr := recover(); derr != nil { err = fmt.Errorf("runScript: unexpected panic running script: %v", err) @@ -63,6 +63,33 @@ func runScript(c *Config) (err error) { } }() + for key, value := range envVars { + currentVal, currentOk := os.LookupEnv(key) + defer func(currentKey, currentVal string, currentOk bool) { + if !currentOk { + _ = os.Unsetenv(currentKey) + } else { + _ = os.Setenv(currentKey, currentVal) + } + s.logger.Info(fmt.Sprintf("unset %v: %v", currentKey, currentVal)) + }(key, currentVal, currentOk) + + if err := os.Setenv(key, value); err != nil { + return fmt.Errorf( + "Unexpected error overloading environment %s: %w", + s.c.BackupCronExpression, + err, + ) + } + s.logger.Info(fmt.Sprintf("set %v: %v", key, value)) + } + + if s.c.BackupFilenameExpand { + s.file = os.ExpandEnv(s.file) + s.c.BackupLatestSymlink = os.ExpandEnv(s.c.BackupLatestSymlink) + s.c.BackupPruningPrefix = os.ExpandEnv(s.c.BackupPruningPrefix) + } + scriptErr := func() error { if err := s.withLabeledCommands(lifecyclePhaseArchive, func() (err error) { restartContainersAndServices, err := s.stopContainersAndServices() @@ -132,7 +159,7 @@ func (c *command) runInForeground(profileCronExpression string) error { ), ) - addJob := func(config *Config, name string) error { + addJob := func(config *Config, name string, envVars map[string]string) error { if _, err := cr.AddFunc(config.BackupCronExpression, func() { c.logger.Info( fmt.Sprintf( @@ -140,7 +167,8 @@ func (c *command) runInForeground(profileCronExpression string) error { config.BackupCronExpression, ), ) - if err := runScript(config); err != nil { + + if err := runScript(config, envVars); err != nil { c.logger.Error( fmt.Sprintf( "Unexpected error running schedule %s: %v", @@ -175,7 +203,7 @@ func (c *command) runInForeground(profileCronExpression string) error { if err != nil { return fmt.Errorf("runInForeground: could not load config from environment variables: %w", err) } else { - err = addJob(c, "from environment") + err = addJob(c, "from environment", nil) if err != nil { return fmt.Errorf("runInForeground: error adding job from env: %w", err) } @@ -183,7 +211,7 @@ func (c *command) runInForeground(profileCronExpression string) error { } else { c.logger.Info("/etc/dockervolumebackup/conf.d was found, using configuration files from this directory.") for _, config := range cs { - err = addJob(config.config, config.name) + err = addJob(config.config, config.name, config.additionalEnvVars) if err != nil { return fmt.Errorf("runInForeground: error adding jobs from conf files: %w", err) } @@ -227,7 +255,7 @@ func (c *command) runAsCommand() error { if err != nil { return fmt.Errorf("runAsCommand: error loading env vars: %w", err) } - err = runScript(config) + err = runScript(config, nil) if err != nil { return fmt.Errorf("runAsCommand: error running script: %w", err) } diff --git a/cmd/backup/script.go b/cmd/backup/script.go index b16d23b1..c3606259 100644 --- a/cmd/backup/script.go +++ b/cmd/backup/script.go @@ -97,11 +97,6 @@ func newScript(c *Config) (*script, error) { } s.file = bf.String() - if s.c.BackupFilenameExpand { - s.file = os.ExpandEnv(s.file) - s.c.BackupLatestSymlink = os.ExpandEnv(s.c.BackupLatestSymlink) - s.c.BackupPruningPrefix = os.ExpandEnv(s.c.BackupPruningPrefix) - } s.file = timeutil.Strftime(&s.stats.StartTime, s.file) _, err := os.Stat("/var/run/docker.sock") diff --git a/test/confd/01backup.env b/test/confd/01backup.env index 8d6d7e3f..1d9d2b5f 100644 --- a/test/confd/01backup.env +++ b/test/confd/01backup.env @@ -1,2 +1,2 @@ -BACKUP_CRON_EXPRESSION="*/1 * * * *" NAME="conf" +BACKUP_CRON_EXPRESSION="*/1 * * * *" From 44f7ab9e5f1082cc21e0fb3a4b09c88674c7544a Mon Sep 17 00:00:00 2001 From: Frederik Ring Date: Tue, 13 Feb 2024 14:58:47 +0100 Subject: [PATCH 3/3] Lock needs to be acquired when instantiating script --- cmd/backup/main.go | 46 +++++---------------------------- cmd/backup/script.go | 60 +++++++++++++++++++++++++++++++++----------- 2 files changed, 51 insertions(+), 55 deletions(-) diff --git a/cmd/backup/main.go b/cmd/backup/main.go index 1aff16b6..fb23c79b 100644 --- a/cmd/backup/main.go +++ b/cmd/backup/main.go @@ -43,53 +43,19 @@ func runScript(c *Config, envVars map[string]string) (err error) { } }() - s, err := newScript(c) + s, unlock, err := newScript(c, envVars) if err != nil { err = fmt.Errorf("runScript: error instantiating script: %w", err) return } - - runErr := func() (err error) { - unlock, err := s.lock("/var/lock/dockervolumebackup.lock") + defer func() { + derr := unlock() if err != nil { - err = fmt.Errorf("runScript: error acquiring file lock: %w", err) - return - } - - defer func() { - derr := unlock() - if err == nil && derr != nil { - err = fmt.Errorf("runScript: error releasing file lock: %w", derr) - } - }() - - for key, value := range envVars { - currentVal, currentOk := os.LookupEnv(key) - defer func(currentKey, currentVal string, currentOk bool) { - if !currentOk { - _ = os.Unsetenv(currentKey) - } else { - _ = os.Setenv(currentKey, currentVal) - } - s.logger.Info(fmt.Sprintf("unset %v: %v", currentKey, currentVal)) - }(key, currentVal, currentOk) - - if err := os.Setenv(key, value); err != nil { - return fmt.Errorf( - "Unexpected error overloading environment %s: %w", - s.c.BackupCronExpression, - err, - ) - } - s.logger.Info(fmt.Sprintf("set %v: %v", key, value)) - } - - if s.c.BackupFilenameExpand { - s.file = os.ExpandEnv(s.file) - s.c.BackupLatestSymlink = os.ExpandEnv(s.c.BackupLatestSymlink) - s.c.BackupPruningPrefix = os.ExpandEnv(s.c.BackupPruningPrefix) + err = fmt.Errorf("runScript: error releasing file lock: %w", derr) } + }() + runErr := func() (err error) { scriptErr := func() error { if err := s.withLabeledCommands(lifecyclePhaseArchive, func() (err error) { restartContainersAndServices, err := s.stopContainersAndServices() diff --git a/cmd/backup/script.go b/cmd/backup/script.go index c3606259..890c99f8 100644 --- a/cmd/backup/script.go +++ b/cmd/backup/script.go @@ -57,7 +57,7 @@ type script struct { // remote resources like the Docker engine or remote storage locations. All // reading from env vars or other configuration sources is expected to happen // in this method. -func newScript(c *Config) (*script, error) { +func newScript(c *Config, envVars map[string]string) (*script, func() error, error) { stdOut, logBuffer := buffer(os.Stdout) s := &script{ c: c, @@ -76,6 +76,31 @@ func newScript(c *Config) (*script, error) { }, } + unlock, err := s.lock("/var/lock/dockervolumebackup.lock") + if err != nil { + return nil, noop, fmt.Errorf("runScript: error acquiring file lock: %w", err) + } + + for key, value := range envVars { + currentVal, currentOk := os.LookupEnv(key) + defer func(currentKey, currentVal string, currentOk bool) { + if !currentOk { + _ = os.Unsetenv(currentKey) + } else { + _ = os.Setenv(currentKey, currentVal) + } + s.logger.Info(fmt.Sprintf("unset %v: %v", currentKey, currentVal)) + }(key, currentVal, currentOk) + + if err := os.Setenv(key, value); err != nil { + return nil, unlock, fmt.Errorf( + "Unexpected error overloading environment %s: %w", + s.c.BackupCronExpression, + err, + ) + } + s.logger.Info(fmt.Sprintf("set %v: %v", key, value)) + } s.registerHook(hookLevelPlumbing, func(error) error { s.stats.EndTime = time.Now() s.stats.TookTime = s.stats.EndTime.Sub(s.stats.StartTime) @@ -86,25 +111,30 @@ func newScript(c *Config) (*script, error) { tmplFileName, tErr := template.New("extension").Parse(s.file) if tErr != nil { - return nil, fmt.Errorf("newScript: unable to parse backup file extension template: %w", tErr) + return nil, unlock, fmt.Errorf("newScript: unable to parse backup file extension template: %w", tErr) } var bf bytes.Buffer if tErr := tmplFileName.Execute(&bf, map[string]string{ "Extension": fmt.Sprintf("tar.%s", s.c.BackupCompression), }); tErr != nil { - return nil, fmt.Errorf("newScript: error executing backup file extension template: %w", tErr) + return nil, unlock, fmt.Errorf("newScript: error executing backup file extension template: %w", tErr) } s.file = bf.String() + if s.c.BackupFilenameExpand { + s.file = os.ExpandEnv(s.file) + s.c.BackupLatestSymlink = os.ExpandEnv(s.c.BackupLatestSymlink) + s.c.BackupPruningPrefix = os.ExpandEnv(s.c.BackupPruningPrefix) + } s.file = timeutil.Strftime(&s.stats.StartTime, s.file) - _, err := os.Stat("/var/run/docker.sock") + _, err = os.Stat("/var/run/docker.sock") _, dockerHostSet := os.LookupEnv("DOCKER_HOST") if !os.IsNotExist(err) || dockerHostSet { cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) if err != nil { - return nil, fmt.Errorf("newScript: failed to create docker client") + return nil, unlock, fmt.Errorf("newScript: failed to create docker client") } s.cli = cli s.registerHook(hookLevelPlumbing, func(err error) error { @@ -142,7 +172,7 @@ func newScript(c *Config) (*script, error) { } s3Backend, err := s3.NewStorageBackend(s3Config, logFunc) if err != nil { - return nil, fmt.Errorf("newScript: error creating s3 storage backend: %w", err) + return nil, unlock, fmt.Errorf("newScript: error creating s3 storage backend: %w", err) } s.storages = append(s.storages, s3Backend) } @@ -157,7 +187,7 @@ func newScript(c *Config) (*script, error) { } webdavBackend, err := webdav.NewStorageBackend(webDavConfig, logFunc) if err != nil { - return nil, fmt.Errorf("newScript: error creating webdav storage backend: %w", err) + return nil, unlock, fmt.Errorf("newScript: error creating webdav storage backend: %w", err) } s.storages = append(s.storages, webdavBackend) } @@ -174,7 +204,7 @@ func newScript(c *Config) (*script, error) { } sshBackend, err := ssh.NewStorageBackend(sshConfig, logFunc) if err != nil { - return nil, fmt.Errorf("newScript: error creating ssh storage backend: %w", err) + return nil, unlock, fmt.Errorf("newScript: error creating ssh storage backend: %w", err) } s.storages = append(s.storages, sshBackend) } @@ -198,7 +228,7 @@ func newScript(c *Config) (*script, error) { } azureBackend, err := azure.NewStorageBackend(azureConfig, logFunc) if err != nil { - return nil, fmt.Errorf("newScript: error creating azure storage backend: %w", err) + return nil, unlock, fmt.Errorf("newScript: error creating azure storage backend: %w", err) } s.storages = append(s.storages, azureBackend) } @@ -215,7 +245,7 @@ func newScript(c *Config) (*script, error) { } dropboxBackend, err := dropbox.NewStorageBackend(dropboxConfig, logFunc) if err != nil { - return nil, fmt.Errorf("newScript: error creating dropbox storage backend: %w", err) + return nil, unlock, fmt.Errorf("newScript: error creating dropbox storage backend: %w", err) } s.storages = append(s.storages, dropboxBackend) } @@ -241,14 +271,14 @@ func newScript(c *Config) (*script, error) { hookLevel, ok := hookLevels[s.c.NotificationLevel] if !ok { - return nil, fmt.Errorf("newScript: unknown NOTIFICATION_LEVEL %s", s.c.NotificationLevel) + return nil, unlock, fmt.Errorf("newScript: unknown NOTIFICATION_LEVEL %s", s.c.NotificationLevel) } s.hookLevel = hookLevel if len(s.c.NotificationURLs) > 0 { sender, senderErr := shoutrrr.CreateSender(s.c.NotificationURLs...) if senderErr != nil { - return nil, fmt.Errorf("newScript: error creating sender: %w", senderErr) + return nil, unlock, fmt.Errorf("newScript: error creating sender: %w", senderErr) } s.sender = sender @@ -256,13 +286,13 @@ func newScript(c *Config) (*script, error) { tmpl.Funcs(templateHelpers) tmpl, err = tmpl.Parse(defaultNotifications) if err != nil { - return nil, fmt.Errorf("newScript: unable to parse default notifications templates: %w", err) + return nil, unlock, fmt.Errorf("newScript: unable to parse default notifications templates: %w", err) } if fi, err := os.Stat("/etc/dockervolumebackup/notifications.d"); err == nil && fi.IsDir() { tmpl, err = tmpl.ParseGlob("/etc/dockervolumebackup/notifications.d/*.*") if err != nil { - return nil, fmt.Errorf("newScript: unable to parse user defined notifications templates: %w", err) + return nil, unlock, fmt.Errorf("newScript: unable to parse user defined notifications templates: %w", err) } } s.template = tmpl @@ -283,7 +313,7 @@ func newScript(c *Config) (*script, error) { }) } - return s, nil + return s, unlock, nil } // createArchive creates a tar archive of the configured backup location and