Env vars should propagate when using conf.d (#358)

* Extend confd test case to test for env var propagation * Env vars set in conf.d files are expected to propagate * Lock needs to be acquired when instantiating script
offen · Feb 13, 2024 · 9a1e885 · 9a1e885
1 parent 241b5d2
commit 9a1e885
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 41 deletions.
diff --git 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
@@ -36,33 +36,26 @@ 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)
 		}
 	}()
 
-	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
+			err = fmt.Errorf("runScript: error releasing file lock: %w", derr)
 		}
+	}()
 
-		defer func() {
-			derr := unlock()
-			if err == nil && derr != nil {
-				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()
@@ -132,15 +125,16 @@ 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(
 					"Now running script on schedule %s",
 					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,15 +169,15 @@ 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)
 			}
 		}
 	} 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 +221,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
@@ -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,14 +111,14 @@ 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()
 
@@ -104,12 +129,12 @@ func newScript(c *Config) (*script, error) {
 	}
 	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 {
@@ -147,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)
 	}
@@ -162,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)
 	}
@@ -179,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)
 	}
@@ -203,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)
 	}
@@ -220,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)
 	}
@@ -246,28 +271,28 @@ 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
 
 		tmpl := template.New("")
 		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
@@ -288,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

diff --git a/test/confd/01backup.env b/test/confd/01backup.env
@@ -1,2 +1,2 @@
-BACKUP_FILENAME="conf.tar.gz"
+NAME="conf"
 BACKUP_CRON_EXPRESSION="*/1 * * * *"
diff --git 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
@@ -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
@@ -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