Merge pull request #4666 from MichaelEischer/feature-flags

Implement feature flags

changelog/unreleased/issue-4601

@@ -0,0 +1,9 @@
+Enhancement: Add support for feature flags
+
+Restic now supports feature flags that can be used to enable and disable
+experimental features. The flags can be set using the environment variable
+`RESTIC_FEATURES`. To get a list of currently supported feature flags,
+run the `features` command.
+
+https://github.com/restic/restic/issues/4601
+https://github.com/restic/restic/pull/4666

cmd/restic/cmd_features.go

@@ -0,0 +1,58 @@
+package main
+
+import (
+	"fmt"
+
+	"github.com/restic/restic/internal/errors"
+	"github.com/restic/restic/internal/feature"
+	"github.com/restic/restic/internal/ui/table"
+
+	"github.com/spf13/cobra"
+)
+
+var featuresCmd = &cobra.Command{
+	Use:   "features",
+	Short: "Print list of feature flags",
+	Long: `
+The "features" command prints a list of supported feature flags.
+
+To pass feature flags to restic, set the RESTIC_FEATURES environment variable
+to "featureA=true,featureB=false". Specifying an unknown feature flag is an error.
+
+A feature can either be in alpha, beta, stable or deprecated state.
+An _alpha_ feature is disabled by default and may change in arbitrary ways between restic versions or be removed.
+A _beta_ feature is enabled by default, but still can change in minor ways or be removed.
+A _stable_ feature is always enabled and cannot be disabled. The flag will be removed in a future restic version.
+A _deprecated_ feature is always disabled and cannot be enabled. The flag will be removed in a future restic version.
+
+EXIT STATUS
+===========
+
+Exit status is 0 if the command was successful, and non-zero if there was any error.
+`,
+	Hidden:            true,
+	DisableAutoGenTag: true,
+	RunE: func(_ *cobra.Command, args []string) error {
+		if len(args) != 0 {
+			return errors.Fatal("the feature command expects no arguments")
+		}
+
+		fmt.Printf("All Feature Flags:\n")
+		flags := feature.Flag.List()
+
+		tab := table.New()
+		tab.AddColumn("Name", "{{ .Name }}")
+		tab.AddColumn("Type", "{{ .Type }}")
+		tab.AddColumn("Default", "{{ .Default }}")
+		tab.AddColumn("Description", "{{ .Description }}")
+
+		for _, flag := range flags {
+			tab.AddRow(flag)
+		}
+		return tab.Write(globalOptions.stdout)
+	},
+}
+
+func init() {
+	cmdRoot.AddCommand(featuresCmd)
+}

cmd/restic/main.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/restic/restic/internal/debug"
 	"github.com/restic/restic/internal/errors"
+	"github.com/restic/restic/internal/feature"
 	"github.com/restic/restic/internal/options"
 	"github.com/restic/restic/internal/restic"
 )
@@ -103,10 +104,18 @@ func main() {
 	// we can show the logs
 	log.SetOutput(logBuffer)
 
+	err := feature.Flag.Apply(os.Getenv("RESTIC_FEATURES"), func(s string) {
+		fmt.Fprintln(os.Stderr, s)
+	})
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		Exit(1)
+	}
+
 	debug.Log("main %#v", os.Args)
 	debug.Log("restic %s compiled with %v on %v/%v",
 		version, runtime.Version(), runtime.GOOS, runtime.GOARCH)
-	err := cmdRoot.ExecuteContext(internalGlobalCtx)
+	err = cmdRoot.ExecuteContext(internalGlobalCtx)
 
 	switch {
 	case restic.IsAlreadyLocked(err):

doc/047_tuning_backup_parameters.rst

@@ -26,7 +26,8 @@ When you start a backup, restic will concurrently count the number of files and
 their total size, which is used to estimate how long it will take. This will
 cause some extra I/O, which can slow down backups of network file systems or
 FUSE mounts. To avoid this overhead at the cost of not seeing a progress
-estimate, use the ``--no-scan`` option which disables this file scanning.
+estimate, use the ``--no-scan`` option of the ``backup`` command  which disables
+this file scanning.
 
 Backend Connections
 ===================
@@ -111,3 +112,28 @@ to disk. An operating system usually caches file write operations in memory and
 them to disk after a short delay. As larger pack files take longer to upload, this
 increases the chance of these files being written to disk. This can increase disk wear
 for SSDs.
+
+
+Feature Flags
+=============
+
+Feature flags allow disabling or enabling certain experimental restic features. The flags
+can be specified via the ``RESTIC_FEATURES`` environment variable. The variable expects a
+comma-separated list of ``key[=value],key2[=value2]`` pairs. The key is the name of a feature
+flag. The value is optional and can contain either the value ``true`` (default if omitted)
+or ``false``. The list of currently available feautre flags is shown by the ``features``
+command.
+
+Restic will return an error if an invalid feature flag is specified. No longer relevant
+feature flags may be removed in a future restic release. Thus, make sure to no longer
+specify these flags.
+
+A feature can either be in alpha, beta, stable or deprecated state.
+
+- An _alpha_ feature is disabled by default and may change in arbitrary ways between restic
+  versions or be removed.
+- A _beta_ feature is enabled by default, but still can change in minor ways or be removed.
+- A _stable_ feature is always enabled and cannot be disabled. This allows for a transition
+  period after which the flag will be removed in a future restic version.
+- A _deprecated_ feature is always disabled and cannot be enabled. The flag will be removed
+  in a future restic version.

internal/feature/features.go

@@ -0,0 +1,140 @@
+package feature
+
+import (
+	"fmt"
+	"sort"
+	"strconv"
+	"strings"
+)
+
+type state string
+type FlagName string
+
+const (
+	// Alpha features are disabled by default. They do not guarantee any backwards compatibility and may change in arbitrary ways between restic versions.
+	Alpha state = "alpha"
+	// Beta features are enabled by default. They may still change, but incompatible changes should be avoided.
+	Beta state = "beta"
+	// Stable features are always enabled
+	Stable state = "stable"
+	// Deprecated features are always disabled
+	Deprecated state = "deprecated"
+)
+
+type FlagDesc struct {
+	Type        state
+	Description string
+}
+
+type FlagSet struct {
+	flags   map[FlagName]*FlagDesc
+	enabled map[FlagName]bool
+}
+
+func New() *FlagSet {
+	return &FlagSet{}
+}
+
+func getDefault(phase state) bool {
+	switch phase {
+	case Alpha, Deprecated:
+		return false
+	case Beta, Stable:
+		return true
+	default:
+		panic("unknown feature phase")
+	}
+}
+
+func (f *FlagSet) SetFlags(flags map[FlagName]FlagDesc) {
+	f.flags = map[FlagName]*FlagDesc{}
+	f.enabled = map[FlagName]bool{}
+
+	for name, flag := range flags {
+		fcopy := flag
+		f.flags[name] = &fcopy
+		f.enabled[name] = getDefault(fcopy.Type)
+	}
+}
+
+func (f *FlagSet) Apply(flags string, logWarning func(string)) error {
+	if flags == "" {
+		return nil
+	}
+
+	selection := make(map[string]bool)
+
+	for _, flag := range strings.Split(flags, ",") {
+		parts := strings.SplitN(flag, "=", 2)
+
+		name := parts[0]
+		value := "true"
+		if len(parts) == 2 {
+			value = parts[1]
+		}
+
+		isEnabled, err := strconv.ParseBool(value)
+		if err != nil {
+			return fmt.Errorf("failed to parse value %q for feature flag %v: %w", value, name, err)
+		}
+
+		selection[name] = isEnabled
+	}
+
+	for name, value := range selection {
+		fname := FlagName(name)
+		flag := f.flags[fname]
+		if flag == nil {
+			return fmt.Errorf("unknown feature flag %q", name)
+		}
+
+		switch flag.Type {
+		case Alpha, Beta:
+			f.enabled[fname] = value
+		case Stable:
+			logWarning(fmt.Sprintf("feature flag %q is always enabled and will be removed in a future release", fname))
+		case Deprecated:
+			logWarning(fmt.Sprintf("feature flag %q is always disabled and will be removed in a future release", fname))
+		default:
+			panic("unknown feature phase")
+		}
+	}
+
+	return nil
+}
+
+func (f *FlagSet) Enabled(name FlagName) bool {
+	isEnabled, ok := f.enabled[name]
+	if !ok {
+		panic(fmt.Sprintf("unknown feature flag %v", name))
+	}
+
+	return isEnabled
+}
+
+// Help contains information about a feature.
+type Help struct {
+	Name        string
+	Type        string
+	Default     bool
+	Description string
+}
+
+func (f *FlagSet) List() []Help {
+	var help []Help
+
+	for name, flag := range f.flags {
+		help = append(help, Help{
+			Name:        string(name),
+			Type:        string(flag.Type),
+			Default:     getDefault(flag.Type),
+			Description: flag.Description,
+		})
+	}
+
+	sort.Slice(help, func(i, j int) bool {
+		return strings.Compare(help[i].Name, help[j].Name) < 0
+	})
+
+	return help
+}

internal/feature/features_test.go

@@ -0,0 +1,151 @@
+package feature_test
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/restic/restic/internal/feature"
+	rtest "github.com/restic/restic/internal/test"
+)
+
+var (
+	alpha      = feature.FlagName("alpha-feature")
+	beta       = feature.FlagName("beta-feature")
+	stable     = feature.FlagName("stable-feature")
+	deprecated = feature.FlagName("deprecated-feature")
+)
+
+var testFlags = map[feature.FlagName]feature.FlagDesc{
+	alpha: {
+		Type:        feature.Alpha,
+		Description: "alpha",
+	},
+	beta: {
+		Type:        feature.Beta,
+		Description: "beta",
+	},
+	stable: {
+		Type:        feature.Stable,
+		Description: "stable",
+	},
+	deprecated: {
+		Type:        feature.Deprecated,
+		Description: "deprecated",
+	},
+}
+
+func buildTestFlagSet() *feature.FlagSet {
+	flags := feature.New()
+	flags.SetFlags(testFlags)
+	return flags
+}
+
+func TestFeatureDefaults(t *testing.T) {
+	flags := buildTestFlagSet()
+	for _, exp := range []struct {
+		flag  feature.FlagName
+		value bool
+	}{
+		{alpha, false},
+		{beta, true},
+		{stable, true},
+		{deprecated, false},
+	} {
+		rtest.Assert(t, flags.Enabled(exp.flag) == exp.value, "expected flag %v to have value %v got %v", exp.flag, exp.value, flags.Enabled(exp.flag))
+	}
+}
+
+func panicIfCalled(msg string) {
+	panic(msg)
+}
+
+func TestEmptyApply(t *testing.T) {
+	flags := buildTestFlagSet()
+	rtest.OK(t, flags.Apply("", panicIfCalled))
+
+	rtest.Assert(t, !flags.Enabled(alpha), "expected alpha feature to be disabled")
+	rtest.Assert(t, flags.Enabled(beta), "expected beta feature to be enabled")
+}
+
+func TestFeatureApply(t *testing.T) {
+	flags := buildTestFlagSet()
+	rtest.OK(t, flags.Apply(string(alpha), panicIfCalled))
+	rtest.Assert(t, flags.Enabled(alpha), "expected alpha feature to be enabled")
+
+	rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", alpha), panicIfCalled))
+	rtest.Assert(t, !flags.Enabled(alpha), "expected alpha feature to be disabled")
+
+	rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true", alpha), panicIfCalled))
+	rtest.Assert(t, flags.Enabled(alpha), "expected alpha feature to be enabled again")
+
+	rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", beta), panicIfCalled))
+	rtest.Assert(t, !flags.Enabled(beta), "expected beta feature to be disabled")
+
+	logMsg := ""
+	log := func(msg string) {
+		logMsg = msg
+	}
+
+	rtest.OK(t, flags.Apply(fmt.Sprintf("%s=false", stable), log))
+	rtest.Assert(t, flags.Enabled(stable), "expected stable feature to remain enabled")
+	rtest.Assert(t, strings.Contains(logMsg, string(stable)), "unexpected log message for stable flag: %v", logMsg)
+
+	logMsg = ""
+	rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true", deprecated), log))
+	rtest.Assert(t, !flags.Enabled(deprecated), "expected deprecated feature to remain disabled")
+	rtest.Assert(t, strings.Contains(logMsg, string(deprecated)), "unexpected log message for deprecated flag: %v", logMsg)
+}
+
+func TestFeatureMultipleApply(t *testing.T) {
+	flags := buildTestFlagSet()
+
+	rtest.OK(t, flags.Apply(fmt.Sprintf("%s=true,%s=false", alpha, beta), panicIfCalled))
+	rtest.Assert(t, flags.Enabled(alpha), "expected alpha feature to be enabled")
+	rtest.Assert(t, !flags.Enabled(beta), "expected beta feature to be disabled")
+}
+
+func TestFeatureApplyInvalid(t *testing.T) {
+	flags := buildTestFlagSet()
+
+	err := flags.Apply("invalid-flag", panicIfCalled)
+	rtest.Assert(t, err != nil && strings.Contains(err.Error(), "unknown feature flag"), "expected unknown feature flag error, got: %v", err)
+
+	err = flags.Apply(fmt.Sprintf("%v=invalid", alpha), panicIfCalled)
+	rtest.Assert(t, err != nil && strings.Contains(err.Error(), "failed to parse value"), "expected parsing error, got: %v", err)
+}
+
+func assertPanic(t *testing.T) {
+	if r := recover(); r == nil {
+		t.Fatal("should have panicked")
+	}
+}
+
+func TestFeatureQueryInvalid(t *testing.T) {
+	defer assertPanic(t)
+
+	flags := buildTestFlagSet()
+	flags.Enabled("invalid-flag")
+}
+
+func TestFeatureSetInvalidPhase(t *testing.T) {
+	defer assertPanic(t)
+
+	flags := feature.New()
+	flags.SetFlags(map[feature.FlagName]feature.FlagDesc{
+		"invalid": {
+			Type: "invalid",
+		},
+	})
+}
+
+func TestFeatureList(t *testing.T) {
+	flags := buildTestFlagSet()
+
+	rtest.Equals(t, []feature.Help{
+		{string(alpha), string(feature.Alpha), false, "alpha"},
+		{string(beta), string(feature.Beta), true, "beta"},
+		{string(deprecated), string(feature.Deprecated), false, "deprecated"},
+		{string(stable), string(feature.Stable), true, "stable"},
+	}, flags.List())
+}

internal/feature/registry.go

@@ -0,0 +1,15 @@
+package feature
+
+// Flag is named such that checking for a feature uses `feature.Flag.Enabled(feature.ExampleFeature)`.
+var Flag = New()
+
+// flag names are written in kebab-case
+const (
+	ExampleFeature FlagName = "example-feature"
+)
+
+func init() {
+	Flag.SetFlags(map[FlagName]FlagDesc{
+		ExampleFeature: {Type: Alpha, Description: "just for testing"},
+	})
+}

internal/feature/testing.go

@@ -0,0 +1,33 @@
+package feature
+
+import (
+	"fmt"
+	"testing"
+)
+
+// TestSetFlag temporarily sets a feature flag to the given value until the
+// returned function is called.
+//
+// Usage
+// ```
+// defer TestSetFlag(t, features.Flags, features.ExampleFlag, true)()
+// ```
+func TestSetFlag(t *testing.T, f *FlagSet, flag FlagName, value bool) func() {
+	current := f.Enabled(flag)
+
+	panicIfCalled := func(msg string) {
+		panic(msg)
+	}
+
+	if err := f.Apply(fmt.Sprintf("%s=%v", flag, value), panicIfCalled); err != nil {
+		// not reachable
+		panic(err)
+	}
+
+	return func() {
+		if err := f.Apply(fmt.Sprintf("%s=%v", flag, current), panicIfCalled); err != nil {
+			// not reachable
+			panic(err)
+		}
+	}
+}

internal/feature/testing_test.go

@@ -0,0 +1,19 @@
+package feature_test
+
+import (
+	"testing"
+
+	"github.com/restic/restic/internal/feature"
+	rtest "github.com/restic/restic/internal/test"
+)
+
+func TestSetFeatureFlag(t *testing.T) {
+	flags := buildTestFlagSet()
+	rtest.Assert(t, !flags.Enabled(alpha), "expected alpha feature to be disabled")
+
+	restore := feature.TestSetFlag(t, flags, alpha, true)
+	rtest.Assert(t, flags.Enabled(alpha), "expected alpha feature to be enabled")
+
+	restore()
+	rtest.Assert(t, !flags.Enabled(alpha), "expected alpha feature to be disabled again")
+}