simplify feature flags (#1947)

Now checking for a feature flag is a one liner,
with no need to control errors.

if fflag.Crowdsec.CscliSetup.IsEnabled() {
   ...
}

cmd/crowdsec-cli/main.go

@@ -67,7 +67,7 @@ func initConfig() {
 	}
 
 	featurePath := filepath.Join(csConfig.ConfigPaths.ConfigDir, "feature.yaml")
-	if err = fflag.CrowdsecFeatures.SetFromYamlFile(featurePath, log.StandardLogger()); err != nil {
+	if err = fflag.Crowdsec.SetFromYamlFile(featurePath, log.StandardLogger()); err != nil {
 		log.Fatalf("File %s: %s", featurePath, err)
 	}
 
@@ -137,13 +137,13 @@ func main() {
 	logFormatter := &log.TextFormatter{TimestampFormat: "02-01-2006 15:04:05", FullTimestamp: true}
 	log.SetFormatter(logFormatter)
 
-	if err := fflag.InitCrowdsecFeatures(); err != nil {
-		log.Fatalf("failed to initialize features: %s", err)
+	if err := fflag.RegisterAllFeatures(); err != nil {
+		log.Fatalf("failed to register features: %s", err)
 	}
 
 	// some features can require configuration or command-line options,
 	// so we need to parse them asap. we'll load from feature.yaml later.
-	if err := fflag.CrowdsecFeatures.SetFromEnv("CROWDSEC_FEATURE_", log.StandardLogger()); err != nil {
+	if err := fflag.Crowdsec.SetFromEnv(log.StandardLogger()); err != nil {
 		log.Fatalf("failed to set features from environment: %s", err)
 	}
 

cmd/crowdsec-cli/support.go

@@ -93,7 +93,7 @@ func collectVersion() []byte {
 
 func collectFeatures() []byte {
 	log.Info("Collecting feature flags")
-	enabledFeatures := fflag.CrowdsecFeatures.GetEnabledFeatures()
+	enabledFeatures := fflag.Crowdsec.GetEnabledFeatures()
 
 	w := bytes.NewBuffer(nil)
 	for _, k := range enabledFeatures {

cmd/crowdsec/main.go

@@ -314,15 +314,11 @@ func LoadConfig(cConfig *csconfig.Config) error {
 func LoadFeatureFlags(cConfig *csconfig.Config, logger *log.Logger) error {
 	featurePath := filepath.Join(cConfig.ConfigPaths.ConfigDir, "feature.yaml")
 
-	if err := fflag.InitCrowdsecFeatures(); err != nil {
-		log.Fatalf("failed to initialize features: %s", err)
-	}
-
-	if err := fflag.CrowdsecFeatures.SetFromYamlFile(featurePath, logger); err != nil {
+	if err := fflag.Crowdsec.SetFromYamlFile(featurePath, logger); err != nil {
 		return fmt.Errorf("file %s: %s", featurePath, err)
 	}
 
-	enabledFeatures := fflag.CrowdsecFeatures.GetEnabledFeatures()
+	enabledFeatures := fflag.Crowdsec.GetEnabledFeatures()
 
 	msg := "<none>"
 	if len(enabledFeatures) > 0 {
@@ -358,9 +354,13 @@ func exitWithCode(exitCode int, err error) {
 var crowdsecT0 time.Time
 
 func main() {
+	if err := fflag.RegisterAllFeatures(); err != nil {
+		log.Fatalf("failed to register features: %s", err)
+	}
+
 	// some features can require configuration or command-line options,
 	// so wwe need to parse them asap. we'll load from feature.yaml later.
-	if err := fflag.CrowdsecFeatures.SetFromEnv("CROWDSEC_FEATURE_", log.StandardLogger()); err != nil {
+	if err := fflag.Crowdsec.SetFromEnv(log.StandardLogger()); err != nil {
 		log.Fatalf("failed set features from environment: %s", err)
 	}
 

pkg/fflag/crowdsec.go

@@ -1,9 +1,14 @@
 package fflag
 
-var CrowdsecFeatures FeatureMap
+var Crowdsec = FeatureRegister{EnvPrefix: "CROWDSEC_FEATURE_"}
 
-func InitCrowdsecFeatures() error {
-	var err error
-	CrowdsecFeatures, err = NewFeatureMap(map[string]FeatureFlag{"cscli_setup": {}})
-	return err
+var CscliSetup = &Feature{Name: "cscli_setup"}
+
+func RegisterAllFeatures() error {
+	err := Crowdsec.RegisterFeature(CscliSetup)
+	if err != nil {
+		return err
+	}
+
+	return nil
 }

pkg/fflag/features.go

@@ -23,6 +23,7 @@
 //
 // A specific deprecation message is used to inform the user of the behavior
 // that has been decided when the flag is/was finally retired.
+
 package fflag
 
 import (
@@ -38,74 +39,62 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+var (
+	ErrFeatureNameEmpty   = errors.New("name is empty")
+	ErrFeatureNameCase    = errors.New("name is not lowercase")
+	ErrFeatureNameInvalid = errors.New("invalid name (allowed a-z, 0-9, _, .)")
+	ErrFeatureUnknown     = errors.New("unknown feature")
+	ErrFeatureDeprecated  = errors.New("the flag is deprecated")
+	ErrFeatureRetired     = errors.New("the flag is retired")
+)
+
 const (
-	ActiveState = iota
-	DeprecatedState
-	RetiredState
+	ActiveState     = iota // the feature can be enabled, and its description is logged (Info)
+	DeprecatedState        // the feature can be enabled, and a deprecation message is logged (Warning)
+	RetiredState           // the feature is ignored and a deprecation message is logged (Error)
 )
 
-type FeatureFlag struct {
-	State          int    // active, deprecated, retired
-	DeprecationMsg string // Why was it deprecated? What happens next? What should the user do?
-}
+type Feature struct {
+	Name  string
+	State int // active, deprecated, retired
+
+	// Description should be a short sentence, explaining the feature.
+	Description string
+
+	// DeprecationMessage is used to inform the user of the behavior that has
+	// been decided when the flag is/was finally retired.
+	DeprecationMsg string
 
-type feature struct {
-	name	string
-	flag    FeatureFlag
 	enabled bool
-	fm      *FeatureMap
 }
 
-func (f *feature) IsEnabled() bool {
+func (f *Feature) IsEnabled() bool {
 	return f.enabled
 }
 
-func (f *feature) Set(value bool) error {
+// Set enables or disables a feature flag
+// It should not be called directly by the user, but by SetFromEnv or SetFromYaml
+func (f *Feature) Set(value bool) error {
 	// retired feature flags are ignored
-	if f.flag.State == RetiredState {
-		return FeatureRetiredError(*f)
+	if f.State == RetiredState {
+		return ErrFeatureRetired
 	}
 
 	f.enabled = value
-	(*f.fm)[f.name] = *f
 
 	// deprecated feature flags are still accepted, but a warning is triggered.
 	// We return an error but set the feature anyway.
-	if f.flag.State == DeprecatedState {
-		return FeatureDeprecatedError(*f)
+	if f.State == DeprecatedState {
+		return ErrFeatureDeprecated
 	}
 
 	return nil
 }
 
-type FeatureMap map[string]feature
-
-// These are returned by the constructor.
-var (
-	ErrFeatureNameEmpty   = errors.New("name is empty")
-	ErrFeatureNameCase    = errors.New("name is not lowercase")
-	ErrFeatureNameInvalid = errors.New("invalid name (allowed a-z, 0-9, _, .)")
-)
-
-var ErrFeatureUnknown = errors.New("unknown feature")
-var ErrFeatureDeprecated  = errors.New("the flag is deprecated")
-
-func FeatureDeprecatedError(feat feature) error {
-	if feat.flag.DeprecationMsg != "" {
-		return fmt.Errorf("%w: %s", ErrFeatureDeprecated, feat.flag.DeprecationMsg)
-	}
-
-	return ErrFeatureDeprecated
-}
-
-var ErrFeatureRetired = errors.New("the flag is retired")
-
-func FeatureRetiredError(feat feature) error {
-	if feat.flag.DeprecationMsg != "" {
-		return fmt.Errorf("%w: %s", ErrFeatureRetired, feat.flag.DeprecationMsg)
-	}
-
-	return ErrFeatureRetired
+// A register allows to enable features from the environment or a file
+type FeatureRegister struct {
+	EnvPrefix string
+	features  map[string]*Feature
 }
 
 var featureNameRexp = regexp.MustCompile(`^[a-z0-9_\.]+$`)
@@ -126,22 +115,22 @@ func validateFeatureName(featureName string) error {
 	return nil
 }
 
-func NewFeatureMap(flags map[string]FeatureFlag) (FeatureMap, error) {
-	fm := FeatureMap{}
-
-	for k, v := range flags {
-		if err := validateFeatureName(k); err != nil {
-			return nil, fmt.Errorf("feature flag '%s': %w", k, err)
-		}
+func (fr *FeatureRegister) RegisterFeature(feat *Feature) error {
+	if err := validateFeatureName(feat.Name); err != nil {
+		return fmt.Errorf("feature flag '%s': %w", feat.Name, err)
+	}
 
-		fm[k] = feature{name: k, flag: v, enabled: false, fm: &fm}
+	if fr.features == nil {
+		fr.features = make(map[string]*Feature)
 	}
 
-	return fm, nil
+	fr.features[feat.Name] = feat
+
+	return nil
 }
 
-func (fm *FeatureMap) GetFeature(featureName string) (feature, error) {
-	feat, ok := (*fm)[featureName]
+func (fr *FeatureRegister) GetFeature(featureName string) (*Feature, error) {
+	feat, ok := fr.features[featureName]
 	if !ok {
 		return feat, ErrFeatureUnknown
 	}
@@ -149,17 +138,17 @@ func (fm *FeatureMap) GetFeature(featureName string) (feature, error) {
 	return feat, nil
 }
 
-func (fm *FeatureMap) SetFromEnv(prefix string, logger *logrus.Logger) error {
+func (fr *FeatureRegister) SetFromEnv(logger *logrus.Logger) error {
 	for _, e := range os.Environ() {
 		// ignore non-feature variables
-		if !strings.HasPrefix(e, prefix) {
+		if !strings.HasPrefix(e, fr.EnvPrefix) {
 			continue
 		}
 
 		// extract feature name and value
 		pair := strings.SplitN(e, "=", 2)
 		varName := pair[0]
-		featureName := strings.ToLower(varName[len(prefix):])
+		featureName := strings.ToLower(varName[len(fr.EnvPrefix):])
 		value := pair[1]
 
 		var enable bool
@@ -174,9 +163,9 @@ func (fm *FeatureMap) SetFromEnv(prefix string, logger *logrus.Logger) error {
 			continue
 		}
 
-		feat, err := fm.GetFeature(featureName)
+		feat, err := fr.GetFeature(featureName)
 		if err != nil {
-			logger.Errorf("Ignored envvar '%s': %s", varName, err)
+			logger.Errorf("Ignored envvar '%s': %s.", varName, err)
 			continue
 		}
 
@@ -184,21 +173,21 @@ func (fm *FeatureMap) SetFromEnv(prefix string, logger *logrus.Logger) error {
 
 		switch {
 		case errors.Is(err, ErrFeatureRetired):
-			logger.Errorf("Ignored envvar '%s': %s", varName, err)
+			logger.Errorf("Ignored envvar '%s': %s. %s", varName, err, feat.DeprecationMsg)
 			continue
 		case errors.Is(err, ErrFeatureDeprecated):
-			logger.Warningf("Envvar '%s': %s", varName, err)
+			logger.Warningf("Envvar '%s': %s. %s", varName, err, feat.DeprecationMsg)
 		case err != nil:
 			return err
 		}
 
-		logger.Infof("Feature flag: %s=%t (from envvar)", featureName, enable)
+		logger.Infof("Feature flag: %s=%t (from envvar). %s", featureName, enable, feat.Description)
 	}
 
 	return nil
 }
 
-func (fm *FeatureMap) SetFromYaml(r io.Reader, logger *logrus.Logger) error {
+func (fr *FeatureRegister) SetFromYaml(r io.Reader, logger *logrus.Logger) error {
 	var cfg []string
 
 	bys, err := io.ReadAll(r)
@@ -217,7 +206,7 @@ func (fm *FeatureMap) SetFromYaml(r io.Reader, logger *logrus.Logger) error {
 
 	// set features
 	for _, k := range cfg {
-		feat, err := fm.GetFeature(k)
+		feat, err := fr.GetFeature(k)
 		if err != nil {
 			logger.Errorf("Ignored feature flag '%s': %s", k, err)
 			continue
@@ -227,21 +216,21 @@ func (fm *FeatureMap) SetFromYaml(r io.Reader, logger *logrus.Logger) error {
 
 		switch {
 		case errors.Is(err, ErrFeatureRetired):
-			logger.Errorf("Ignored feature flag '%s': %s", k, err)
+			logger.Errorf("Ignored feature flag '%s': %s. %s", k, err, feat.DeprecationMsg)
 			continue
 		case errors.Is(err, ErrFeatureDeprecated):
-			logger.Warningf("Feature '%s': %s", k, err)
+			logger.Warningf("Feature '%s': %s. %s", k, err, feat.DeprecationMsg)
 		case err != nil:
 			return err
 		}
 
-		logger.Infof("Feature flag: %s=true (from config file)", k)
+		logger.Infof("Feature flag: %s=true (from config file). %s", k, feat.Description)
 	}
 
 	return nil
 }
 
-func (fm *FeatureMap) SetFromYamlFile(path string, logger *logrus.Logger) error {
+func (fr *FeatureRegister) SetFromYamlFile(path string, logger *logrus.Logger) error {
 	f, err := os.Open(path)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -256,15 +245,14 @@ func (fm *FeatureMap) SetFromYamlFile(path string, logger *logrus.Logger) error
 
 	logger.Debugf("Reading feature flags from %s", path)
 
-	return fm.SetFromYaml(f, logger)
+	return fr.SetFromYaml(f, logger)
 }
 
 // GetEnabledFeatures returns the list of features that have been enabled by the user
-func (fm *FeatureMap) GetEnabledFeatures() []string {
+func (fr *FeatureRegister) GetEnabledFeatures() []string {
 	ret := make([]string, 0)
 
-	for k := range *fm {
-		feat := (*fm)[k]
+	for k, feat := range fr.features {
 		if feat.IsEnabled() {
 			ret = append(ret, k)
 		}

pkg/fflag/features_test.go

@@ -8,47 +8,41 @@ import (
 	"github.com/sirupsen/logrus"
 	logtest "github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/require"
-	
+
 	"github.com/crowdsecurity/crowdsec/pkg/cstest"
 	"github.com/crowdsecurity/crowdsec/pkg/fflag"
 )
 
-// Test the constructor, which is not required but useful for validation.
-func TestNewFeatureMap(t *testing.T) {
+func TestRegisterFeature(t *testing.T) {
 	tests := []struct {
 		name        string
-		flags       map[string]fflag.FeatureFlag
+		feature     fflag.Feature
 		expectedErr string
 	}{
 		{
-			name: "no feature at all",
-			flags: map[string]fflag.FeatureFlag{},
-		},
-		{
-			name: "a plain feature or two",
-			flags: map[string]fflag.FeatureFlag{
-				"plain":          {},
-				"plain_version2": {},
+			name: "a plain feature",
+			feature: fflag.Feature{
+				Name: "plain",
 			},
 		},
 		{
 			name: "capitalized feature name",
-			flags: map[string]fflag.FeatureFlag{
-				"Plain": {},
+			feature: fflag.Feature{
+				Name: "Plain",
 			},
 			expectedErr: "feature flag 'Plain': name is not lowercase",
 		},
 		{
 			name: "empty feature name",
-			flags: map[string]fflag.FeatureFlag{
-				"": {},
+			feature: fflag.Feature{
+				Name: "",
 			},
 			expectedErr: "feature flag '': name is empty",
 		},
 		{
 			name: "invalid feature name",
-			flags: map[string]fflag.FeatureFlag{
-				"meh!": {},
+			feature: fflag.Feature{
+				Name: "meh!",
 			},
 			expectedErr: "feature flag 'meh!': invalid name (allowed a-z, 0-9, _, .)",
 		},
@@ -58,29 +52,44 @@ func TestNewFeatureMap(t *testing.T) {
 		tc := tc
 
 		t.Run("", func(t *testing.T) {
-			_, err := fflag.NewFeatureMap(tc.flags)
+			fr := fflag.FeatureRegister{EnvPrefix: "FFLAG_TEST_"}
+			err := fr.RegisterFeature(&tc.feature)
 			cstest.RequireErrorContains(t, err, tc.expectedErr)
 		})
 	}
 }
 
-func setUp(t *testing.T) fflag.FeatureMap {
+func setUp(t *testing.T) fflag.FeatureRegister {
 	t.Helper()
 
-	fm, err := fflag.NewFeatureMap(map[string]fflag.FeatureFlag{
-		"experimental1":    {},
-		"new_standard": {
-			State:          fflag.DeprecatedState,
-			DeprecationMsg: "in 2.0 we'll do that by default",
-		},
-		"was_adopted": {
-			State:          fflag.RetiredState,
-			DeprecationMsg: "the trinket was implemented in 1.5",
-		},
+	fr := fflag.FeatureRegister{EnvPrefix: "FFLAG_TEST_"}
+
+	err := fr.RegisterFeature(&fflag.Feature{Name: "experimental1"})
+	require.NoError(t, err)
+
+	err = fr.RegisterFeature(&fflag.Feature{
+		Name:        "some_feature",
+		Description: "A feature that does something, with a description",
+	})
+	require.NoError(t, err)
+
+	err = fr.RegisterFeature(&fflag.Feature{
+		Name:           "new_standard",
+		State:          fflag.DeprecatedState,
+		Description:    "This implements the new standard T34.256w",
+		DeprecationMsg: "In 2.0 we'll do T34.256w by default",
+	})
+	require.NoError(t, err)
+
+	err = fr.RegisterFeature(&fflag.Feature{
+		Name:           "was_adopted",
+		State:          fflag.RetiredState,
+		Description:    "This implements a new tricket",
+		DeprecationMsg: "The trinket was implemented in 1.5",
 	})
 	require.NoError(t, err)
 
-	return fm
+	return fr
 }
 
 func TestGetFeature(t *testing.T) {
@@ -99,12 +108,12 @@ func TestGetFeature(t *testing.T) {
 		},
 	}
 
-	fm := setUp(t)
+	fr := setUp(t)
 
 	for _, tc := range tests {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
-			_, err := fm.GetFeature(tc.feature)
+			_, err := fr.GetFeature(tc.feature)
 			cstest.RequireErrorMessage(t, err, tc.expectedErr)
 			if tc.expectedErr != "" {
 				return
@@ -113,13 +122,12 @@ func TestGetFeature(t *testing.T) {
 	}
 }
 
-
 func TestIsEnabled(t *testing.T) {
 	tests := []struct {
-		name        string
-		feature     string
-		enable      bool
-		expected    bool
+		name     string
+		feature  string
+		enable   bool
+		expected bool
 	}{
 		{
 			name:     "feature that was not enabled",
@@ -133,12 +141,12 @@ func TestIsEnabled(t *testing.T) {
 		},
 	}
 
-	fm := setUp(t)
+	fr := setUp(t)
 
 	for _, tc := range tests {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
-			feat, err := fm.GetFeature(tc.feature)
+			feat, err := fr.GetFeature(tc.feature)
 			require.NoError(t, err)
 
 			err = feat.Set(tc.enable)
@@ -174,13 +182,13 @@ func TestFeatureSet(t *testing.T) {
 			feature:        "new_standard",
 			value:          true,
 			expected:       true,
-			expectedSetErr: "the flag is deprecated: in 2.0 we'll do that by default",
+			expectedSetErr: "the flag is deprecated",
 		}, {
-			name:     "enable a feature that was retired in v1.5",
-			feature:  "was_adopted",
-			value:    true,
-			expected: false,
-			expectedSetErr: "the flag is retired: the trinket was implemented in 1.5",
+			name:           "enable a feature that was retired in v1.5",
+			feature:        "was_adopted",
+			value:          true,
+			expected:       false,
+			expectedSetErr: "the flag is retired",
 		}, {
 			name:           "enable a feature that does not exist",
 			feature:        "will_never_exist",
@@ -192,12 +200,12 @@ func TestFeatureSet(t *testing.T) {
 
 	// the tests are not indepedent because we don't instantiate a feature
 	// map for each one, but it simplified the code
-	fm := setUp(t)
+	fr := setUp(t)
 
 	for _, tc := range tests {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
-			feat, err := fm.GetFeature(tc.feature)
+			feat, err := fr.GetFeature(tc.feature)
 			cstest.RequireErrorMessage(t, err, tc.expectedGetErr)
 			if tc.expectedGetErr != "" {
 				return
@@ -239,21 +247,28 @@ func TestSetFromEnv(t *testing.T) {
 			envvar:      "FFLAG_TEST_WILL_NEVER_EXIST",
 			value:       "true",
 			expectedLog: []string{"Ignored envvar 'FFLAG_TEST_WILL_NEVER_EXIST': unknown feature"},
+		}, {
+			name:   "enable a feature flag with a description",
+			envvar: "FFLAG_TEST_SOME_FEATURE",
+			value:  "true",
+			expectedLog: []string{
+				"Feature flag: some_feature=true (from envvar). A feature that does something, with a description",
+			},
 		}, {
 			name:   "enable a deprecated feature",
 			envvar: "FFLAG_TEST_NEW_STANDARD",
 			value:  "true",
 			expectedLog: []string{
-				"Envvar 'FFLAG_TEST_NEW_STANDARD': the flag is deprecated: in 2.0 we'll do that by default",
-				"Feature flag: new_standard=true (from envvar)",
+				"Envvar 'FFLAG_TEST_NEW_STANDARD': the flag is deprecated. In 2.0 we'll do T34.256w by default",
+				"Feature flag: new_standard=true (from envvar). This implements the new standard T34.256w",
 			},
 		}, {
 			name:   "enable a feature that was retired in v1.5",
 			envvar: "FFLAG_TEST_WAS_ADOPTED",
 			value:  "true",
 			expectedLog: []string{
-				"Ignored envvar 'FFLAG_TEST_WAS_ADOPTED': the flag is retired: " +
-				"the trinket was implemented in 1.5",
+				"Ignored envvar 'FFLAG_TEST_WAS_ADOPTED': the flag is retired. " +
+					"The trinket was implemented in 1.5",
 			},
 		}, {
 			// this could happen in theory, but only if environment variables
@@ -265,7 +280,7 @@ func TestSetFromEnv(t *testing.T) {
 		},
 	}
 
-	fm := setUp(t)
+	fr := setUp(t)
 
 	for _, tc := range tests {
 		tc := tc
@@ -273,7 +288,7 @@ func TestSetFromEnv(t *testing.T) {
 			logger, hook := logtest.NewNullLogger()
 			logger.SetLevel(logrus.InfoLevel)
 			t.Setenv(tc.envvar, tc.value)
-			err := fm.SetFromEnv("FFLAG_TEST_", logger)
+			err := fr.SetFromEnv(logger)
 			cstest.RequireErrorMessage(t, err, tc.expectedErr)
 			for _, expectedMessage := range tc.expectedLog {
 				cstest.RequireLogContains(t, hook, expectedMessage)
@@ -313,26 +328,26 @@ func TestSetFromYaml(t *testing.T) {
 			name: "enable a deprecated feature",
 			yml:  "- new_standard",
 			expectedLog: []string{
-				"Feature 'new_standard': the flag is deprecated: in 2.0 we'll do that by default",
-				"Feature flag: new_standard=true (from config file)",
+				"Feature 'new_standard': the flag is deprecated. In 2.0 we'll do T34.256w by default",
+				"Feature flag: new_standard=true (from config file). This implements the new standard T34.256w",
 			},
 		}, {
 			name: "enable a retired feature",
 			yml:  "- was_adopted",
 			expectedLog: []string{
-				"Ignored feature flag 'was_adopted': the flag is retired: the trinket was implemented in 1.5",
+				"Ignored feature flag 'was_adopted': the flag is retired. The trinket was implemented in 1.5",
 			},
 		},
 	}
 
-	fm := setUp(t)
+	fr := setUp(t)
 
 	for _, tc := range tests {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			logger, hook := logtest.NewNullLogger()
 			logger.SetLevel(logrus.InfoLevel)
-			err := fm.SetFromYaml(strings.NewReader(tc.yml), logger)
+			err := fr.SetFromYaml(strings.NewReader(tc.yml), logger)
 			cstest.RequireErrorMessage(t, err, tc.expectedErr)
 			for _, expectedMessage := range tc.expectedLog {
 				cstest.RequireLogContains(t, hook, expectedMessage)
@@ -352,24 +367,24 @@ func TestSetFromYamlFile(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, tmpfile.Close())
 
-	fm := setUp(t)
+	fr := setUp(t)
 	logger, hook := logtest.NewNullLogger()
 	logger.SetLevel(logrus.InfoLevel)
 
-	err = fm.SetFromYamlFile(tmpfile.Name(), logger)
+	err = fr.SetFromYamlFile(tmpfile.Name(), logger)
 	require.NoError(t, err)
 
 	cstest.RequireLogContains(t, hook, "Feature flag: experimental1=true (from config file)")
 }
 
 func TestGetEnabledFeatures(t *testing.T) {
-	fm := setUp(t)
+	fr := setUp(t)
 
-	feat1, err := fm.GetFeature("new_standard")
+	feat1, err := fr.GetFeature("new_standard")
 	require.NoError(t, err)
 	feat1.Set(true)
 
-	feat2, err := fm.GetFeature("experimental1")
+	feat2, err := fr.GetFeature("experimental1")
 	require.NoError(t, err)
 	feat2.Set(true)
 
@@ -378,5 +393,5 @@ func TestGetEnabledFeatures(t *testing.T) {
 		"new_standard",
 	}
 
-	require.Equal(t, expected, fm.GetEnabledFeatures())
+	require.Equal(t, expected, fr.GetEnabledFeatures())
 }