Retention: Align host filter with "backup"

config/info_customizer.go

@@ -72,18 +72,31 @@ func init() {
 			info.mayBool = true
 			info.examples = []string{"true", "false", fmt.Sprintf(`"%s"`, propertyName)}
 
+			suffixDefaultTrueV2 := fmt.Sprintf(` Defaults to true for config version 2 in "%s".`, sectionName)
+
+			if propertyName == constants.ParameterHost {
+				info.format = "hostname"
+				note = `Boolean true is replaced with the hostname of the system.`
+			} else {
+				note = fmt.Sprintf(`Boolean true is replaced with the %ss from section "backup".`, propertyName)
+			}
+
 			if sectionName == constants.CommandBackup {
 				if propertyName != constants.ParameterHost {
 					info.examples = info.examples[1:] // remove "true" from examples of backup section
 					note = fmt.Sprintf(`Boolean true is unsupported in section "backup".`)
+				} else {
+					note += suffixDefaultTrueV2
+				}
+			} else if sectionName == constants.SectionConfigurationRetention {
+				if propertyName == constants.ParameterHost {
+					note = `Boolean true is replaced with the hostname that applies in section "backup".`
+				}
+				if propertyName == constants.ParameterPath {
+					note += ` Defaults to true in "retention".`
+				} else {
+					note += suffixDefaultTrueV2
 				}
-			} else {
-				note = fmt.Sprintf(`Boolean true is replaced with the %ss from section "backup".`, propertyName)
-			}
-
-			if propertyName == constants.ParameterHost {
-				info.format = "hostname"
-				note = `Boolean true is replaced with the hostname of the system.`
 			}
 
 			if note != "" {

config/info_customizer_test.go

@@ -74,6 +74,12 @@ func TestHostTagPathProperty(t *testing.T) {
 	note := `Boolean true is replaced with the {{property}}s from section "backup".`
 	hostNote := `Boolean true is replaced with the hostname of the system.`
 	backupNote := `Boolean true is unsupported in section "backup".`
+	retentionHostNote := `Boolean true is replaced with the hostname that applies in section "backup".`
+	defaultSuffix := ` Defaults to true in "{{section}}".`
+	defaultSuffixV2 := ` Defaults to true for config version 2 in "{{section}}".`
+
+	backup := constants.CommandBackup
+	retention := constants.SectionConfigurationRetention
 
 	tests := []struct {
 		section, property, note, format string
@@ -83,14 +89,20 @@ func TestHostTagPathProperty(t *testing.T) {
 		{section: "any", property: constants.ParameterPath},
 		{section: "any", property: constants.ParameterTag},
 
-		{section: constants.CommandBackup, property: constants.ParameterHost, note: hostNote, format: "hostname"},
-		{section: constants.CommandBackup, property: constants.ParameterPath, note: backupNote, examples: []string{"false", `"{{property}}"`}},
-		{section: constants.CommandBackup, property: constants.ParameterTag, note: backupNote, examples: []string{"false", `"{{property}}"`}},
+		{section: retention, property: constants.ParameterHost, note: retentionHostNote + defaultSuffixV2, format: "hostname"},
+		{section: retention, property: constants.ParameterPath, note: note + defaultSuffix},
+		{section: retention, property: constants.ParameterTag, note: note + defaultSuffixV2},
+
+		{section: backup, property: constants.ParameterHost, note: hostNote + defaultSuffixV2, format: "hostname"},
+		{section: backup, property: constants.ParameterPath, note: backupNote, examples: []string{"false", `"{{property}}"`}},
+		{section: backup, property: constants.ParameterTag, note: backupNote, examples: []string{"false", `"{{property}}"`}},
 	}
 	for i, test := range tests {
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
 			propertyReplacer := func(s string) string {
-				return strings.ReplaceAll(s, "{{property}}", test.property)
+				s = strings.ReplaceAll(s, "{{property}}", test.property)
+				s = strings.ReplaceAll(s, "{{section}}", test.section)
+				return s
 			}
 			if test.examples == nil {
 				test.examples = examples

config/profile.go

@@ -179,7 +179,7 @@ type BackupSection struct {
 
 func (s *BackupSection) IsEmpty() bool { return s == nil }
 
-func (b *BackupSection) resolve(p *Profile) {
+func (b *BackupSection) resolve(profile *Profile) {
 	// Ensure UseStdin is set when Backup.StdinCommand is defined
 	if len(b.StdinCommand) > 0 {
 		b.UseStdin = true
@@ -188,7 +188,15 @@ func (b *BackupSection) resolve(p *Profile) {
 	if b.unresolvedSource == nil {
 		b.unresolvedSource = b.Source
 	}
-	b.Source = p.resolveSourcePath(b.SourceBase, b.unresolvedSource...)
+	b.Source = profile.resolveSourcePath(b.SourceBase, b.unresolvedSource...)
+
+	// Extras, only enabled for Version >= 2 (to remain backward compatible in version 1)
+	if profile.config != nil && profile.config.version >= Version02 {
+		// Ensure that the host is in sync between backup & retention by setting it if missing
+		if _, found := b.OtherFlags[constants.ParameterHost]; !found {
+			b.SetOtherFlag(constants.ParameterHost, true)
+		}
+	}
 }
 
 func (s *BackupSection) setRootPath(p *Profile, rootPath string) {
@@ -205,26 +213,52 @@ func (s *BackupSection) setRootPath(p *Profile, rootPath string) {
 type RetentionSection struct {
 	ScheduleBaseSection `mapstructure:",squash" deprecated:"0.11.0"`
 	OtherFlagsSection   `mapstructure:",squash"`
-	BeforeBackup        bool `mapstructure:"before-backup" description:"Apply retention before starting the backup command"`
-	AfterBackup         bool `mapstructure:"after-backup" description:"Apply retention after the backup command succeeded"`
+	BeforeBackup        *bool `mapstructure:"before-backup" description:"Apply retention before starting the backup command"`
+	AfterBackup         *bool `mapstructure:"after-backup" description:"Apply retention after the backup command succeeded. Defaults to true if any \"keep-*\" flag is set and \"before-backup\" is unset"`
 }
 
 func (r *RetentionSection) IsEmpty() bool { return r == nil }
 
-func (r *RetentionSection) resolve(p *Profile) {
+func (r *RetentionSection) resolve(profile *Profile) {
 	// Special cases of retention
-	if r.OtherFlags == nil {
-		r.OtherFlags = make(map[string]any)
-	}
+	isSet := func(flags OtherFlags, name string) (found bool) { _, found = flags.GetOtherFlags()[name]; return }
+	hasBackup := !profile.Backup.IsEmpty()
+
 	// Copy "source" from "backup" as "path" if it hasn't been redefined
-	if _, found := r.OtherFlags[constants.ParameterPath]; !found {
-		r.OtherFlags[constants.ParameterPath] = true
-	}
+	if hasBackup && !isSet(r, constants.ParameterPath) {
+		r.SetOtherFlag(constants.ParameterPath, true)
+	}
+
+	// Extras, only enabled for Version >= 2 (to remain backward compatible in version 1)
+	if profile.config != nil && profile.config.version >= Version02 {
+		// Auto-enable "after-backup" if nothing was specified explicitly and any "keep-" was configured
+		if bools.IsUndefined(r.AfterBackup) && bools.IsUndefined(r.BeforeBackup) {
+			for name, _ := range r.OtherFlags {
+				if strings.HasPrefix(name, "keep-") {
+					r.AfterBackup = bools.True()
+					break
+				}
+			}
+		}
+
+		// Copy "tag" from "backup" if it was set and hasn't been redefined here
+		// Allow setting it at profile level when not defined in "backup" nor "retention"
+		if hasBackup &&
+			!isSet(r, constants.ParameterTag) &&
+			isSet(profile.Backup, constants.ParameterTag) {
+
+			r.SetOtherFlag(constants.ParameterTag, true)
+		}
 
-	// Copy "tag" from "backup" if it hasn't been redefined (only for Version >= 2 to be backward compatible)
-	if p.config != nil && p.config.version >= Version02 {
-		if _, found := r.OtherFlags[constants.ParameterTag]; !found {
-			r.OtherFlags[constants.ParameterTag] = true
+		// Copy "host" from "backup" if it was set and hasn't been redefined here
+		// Or use os.Hostname() same as restic does for backup when not setting it, see:
+		// https://github.com/restic/restic/blob/master/cmd/restic/cmd_backup.go#L48
+		if !isSet(r, constants.ParameterHost) {
+			if hasBackup && isSet(profile.Backup, constants.ParameterHost) {
+				r.SetOtherFlag(constants.ParameterHost, profile.Backup.OtherFlags[constants.ParameterHost])
+			} else if !isSet(profile, constants.ParameterHost) {
+				r.SetOtherFlag(constants.ParameterHost, true) // resolved with os.Hostname()
+			}
 		}
 	}
 }
@@ -423,7 +457,14 @@ type OtherFlagsSection struct {
 	OtherFlags map[string]any `mapstructure:",remain"`
 }
 
-func (o OtherFlagsSection) GetOtherFlags() map[string]any { return o.OtherFlags }
+func (o *OtherFlagsSection) GetOtherFlags() map[string]any { return o.OtherFlags }
+
+func (o *OtherFlagsSection) SetOtherFlag(name string, value any) {
+	if o.OtherFlags == nil {
+		o.OtherFlags = make(map[string]any)
+	}
+	o.OtherFlags[name] = value
+}
 
 // NewProfile instantiates a new blank profile
 func NewProfile(c *Config, name string) (p *Profile) {
@@ -502,7 +543,7 @@ func (p *Profile) ResolveConfiguration() {
 	if p.Backup != nil {
 		// Copy tags from backup if tag is set to boolean true
 		if tags, ok := stringifyValueOf(p.Backup.OtherFlags[constants.ParameterTag]); ok {
-			p.SetTag(tags...)
+			p.SetTag(strings.Join(tags, ",")) // must use "tag1,tag2,..." to require all tags
 		} else {
 			p.SetTag() // resolve tag parameters when no tag is set in backup
 		}

config/profile_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/creativeprojects/resticprofile/restic"
 	"github.com/creativeprojects/resticprofile/shell"
 	"github.com/creativeprojects/resticprofile/util"
+	"github.com/creativeprojects/resticprofile/util/bools"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/exp/maps"
@@ -580,19 +581,26 @@ func TestPathAndTagInRetention(t *testing.T) {
 	cwd, err := filepath.Abs(".")
 	require.NoError(t, err)
 	examples := filepath.Join(cwd, "../examples")
+	hostname := "rt-test-host"
 	sourcePattern := filepath.ToSlash(filepath.Join(examples, "[a-p]*"))
 	backupSource, err := filepath.Glob(sourcePattern)
 	require.Greater(t, len(backupSource), 5)
 	require.NoError(t, err)
 
+	backupHost := ""
 	backupTags := []string{"one", "two"}
+	flatBackupTags := func() []string { return []string{strings.Join(backupTags, ",")} }
 
 	testProfileWithBase := func(t *testing.T, version Version, retention, baseDir string) *Profile {
 		prefix := ""
 		if version > Version01 {
 			prefix = "profiles."
 		}
 
+		host := ""
+		if len(backupHost) > 0 {
+			host = `host = "` + backupHost + `"`
+		}
 		tag := ""
 		if len(backupTags) > 0 {
 			tag = `tag = ["` + strings.Join(backupTags, `", "`) + `"]`
@@ -605,6 +613,7 @@ func TestPathAndTagInRetention(t *testing.T) {
             base-dir = "` + filepath.ToSlash(baseDir) + `"
             [` + prefix + `profile.backup]
             ` + tag + `
+            ` + host + `
             source = ["` + sourcePattern + `"]
 
             [` + prefix + `profile.retention]
@@ -614,6 +623,7 @@ func TestPathAndTagInRetention(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, profile)
 		profile.SetRootPath(examples) // ensure relative paths are converted to absolute paths
+		profile.SetHost(hostname)
 
 		return profile
 	}
@@ -622,12 +632,66 @@ func TestPathAndTagInRetention(t *testing.T) {
 		return testProfileWithBase(t, version, retention, "")
 	}
 
-	t.Run("Path", func(t *testing.T) {
-		pathFlag := func(t *testing.T, profile *Profile) interface{} {
+	flagGetter := func(flagName string) func(t *testing.T, profile *Profile) any {
+		return func(t *testing.T, profile *Profile) any {
 			flags := profile.GetRetentionFlags().ToMap()
 			assert.NotNil(t, flags)
-			return flags["path"]
+			return flags[flagName]
+		}
+	}
+
+	t.Run("AutoEnable", func(t *testing.T) {
+		retentionDisabled := func(t *testing.T, profile *Profile) {
+			assert.Nil(t, profile.Retention.BeforeBackup)
+			assert.Nil(t, profile.Retention.AfterBackup)
 		}
+		t.Run("EnableForAnyKeepInV2", func(t *testing.T) {
+			profile := testProfile(t, Version02, ``)
+			retentionDisabled(t, profile)
+			profile = testProfile(t, Version02, `keep-x = 1`)
+			assert.Nil(t, profile.Retention.BeforeBackup)
+			assert.Equal(t, bools.True(), profile.Retention.AfterBackup)
+		})
+		t.Run("NotEnabledInV1", func(t *testing.T) {
+			profile := testProfile(t, Version01, ``)
+			retentionDisabled(t, profile)
+			profile = testProfile(t, Version01, `keep-x = 1`)
+			retentionDisabled(t, profile)
+		})
+	})
+
+	t.Run("Host", func(t *testing.T) {
+		hostFlag := flagGetter(constants.ParameterHost)
+
+		t.Run("ImplicitCopyHostFromProfileInV2", func(t *testing.T) {
+			profile := testProfile(t, Version02, ``)
+			assert.Equal(t, []string{hostname}, hostFlag(t, profile))
+		})
+
+		t.Run("ImplicitCopyHostFromBackupInV2", func(t *testing.T) {
+			defer func() { backupHost = "" }()
+			backupHost = "custom-host-from-backup"
+
+			profile := testProfile(t, Version02, ``)
+			assert.Equal(t, []string{backupHost}, hostFlag(t, profile))
+		})
+
+		t.Run("NoImplicitCopyInV1", func(t *testing.T) {
+			profile := testProfile(t, Version01, ``)
+			assert.Nil(t, hostFlag(t, profile))
+		})
+
+		t.Run("ExplicitCopyHostInV1", func(t *testing.T) {
+			defer func() { backupHost = "" }()
+			backupHost = "custom-host-from-backup"
+
+			profile := testProfile(t, Version01, `host = true`)
+			assert.Equal(t, []string{hostname}, hostFlag(t, profile))
+		})
+	})
+
+	t.Run("Path", func(t *testing.T) {
+		pathFlag := flagGetter(constants.ParameterPath)
 
 		t.Run("ImplicitCopyPath", func(t *testing.T) {
 			profile := testProfile(t, Version01, ``)
@@ -680,11 +744,7 @@ func TestPathAndTagInRetention(t *testing.T) {
 	})
 
 	t.Run("Tag", func(t *testing.T) {
-		tagFlag := func(t *testing.T, profile *Profile) interface{} {
-			flags := profile.GetRetentionFlags().ToMap()
-			assert.NotNil(t, flags)
-			return flags["tag"]
-		}
+		tagFlag := flagGetter(constants.ParameterTag)
 
 		t.Run("NoImplicitCopyTagInV1", func(t *testing.T) {
 			profile := testProfile(t, Version01, ``)
@@ -693,12 +753,12 @@ func TestPathAndTagInRetention(t *testing.T) {
 
 		t.Run("ImplicitCopyTagInV2", func(t *testing.T) {
 			profile := testProfile(t, Version02, ``)
-			assert.Equal(t, backupTags, tagFlag(t, profile))
+			assert.Equal(t, flatBackupTags(), tagFlag(t, profile))
 		})
 
 		t.Run("CopyTag", func(t *testing.T) {
 			profile := testProfile(t, Version01, `tag = true`)
-			assert.Equal(t, backupTags, tagFlag(t, profile))
+			assert.Equal(t, flatBackupTags(), tagFlag(t, profile))
 		})
 
 		t.Run("ReplaceTag", func(t *testing.T) {

wrapper.go

@@ -22,6 +22,7 @@ import (
 	"github.com/creativeprojects/resticprofile/restic"
 	"github.com/creativeprojects/resticprofile/shell"
 	"github.com/creativeprojects/resticprofile/term"
+	"github.com/creativeprojects/resticprofile/util/bools"
 	"github.com/creativeprojects/resticprofile/util/collect"
 	"golang.org/x/exp/slices"
 )
@@ -162,7 +163,7 @@ func (r *resticWrapper) getBackupAction() func() error {
 		}
 
 		// Retention before
-		if err == nil && r.profile.Retention != nil && r.profile.Retention.BeforeBackup {
+		if err == nil && r.profile.Retention != nil && bools.IsTrue(r.profile.Retention.BeforeBackup) {
 			err = r.runRetention()
 		}
 
@@ -172,7 +173,7 @@ func (r *resticWrapper) getBackupAction() func() error {
 		}
 
 		// Retention after
-		if err == nil && r.profile.Retention != nil && r.profile.Retention.AfterBackup {
+		if err == nil && r.profile.Retention != nil && bools.IsTrue(r.profile.Retention.AfterBackup) {
 			err = r.runRetention()
 		}