diff --git a/config/info_customizer.go b/config/info_customizer.go index 2538eab4..a0e615bf 100644 --- a/config/info_customizer.go +++ b/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 != "" { diff --git a/config/info_customizer_test.go b/config/info_customizer_test.go index 232ec337..8956b7f0 100644 --- a/config/info_customizer_test.go +++ b/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 diff --git a/config/profile.go b/config/profile.go index d3b28b13..a487500e 100644 --- a/config/profile.go +++ b/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 } diff --git a/config/profile_test.go b/config/profile_test.go index 2b03669e..1f9ea705 100644 --- a/config/profile_test.go +++ b/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,12 +581,15 @@ 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 := "" @@ -593,6 +597,10 @@ func TestPathAndTagInRetention(t *testing.T) { 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) { diff --git a/wrapper.go b/wrapper.go index 6a19b9ed..867514b8 100644 --- a/wrapper.go +++ b/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() }