chore: refactor deprecations checking to own method (#1226)

* chore: refactor deprecations checking to own method * chore: add missing assertion * chore: trim * chore: trim fix
flipt-io · Dec 15, 2022 · f546cf8 · f546cf8
1 parent 21da213
commit f546cf8
Show file tree

Hide file tree

Showing 15 changed files with 85 additions and 48 deletions.
diff --git a/go.mod b/go.mod
@@ -25,7 +25,6 @@ require (
 	github.com/mattn/go-sqlite3 v1.14.16
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/patrickmn/go-cache v2.1.0+incompatible
-	github.com/phyber/negroni-gzip v1.0.0
 	github.com/prometheus/client_golang v1.14.0
 	github.com/santhosh-tekuri/jsonschema/v5 v5.1.1
 	github.com/spf13/cobra v1.6.1
@@ -111,7 +110,6 @@ require (
 	github.com/stretchr/objx v0.5.0 // indirect
 	github.com/subosito/gotenv v1.4.1 // indirect
 	github.com/uber/jaeger-lib v2.2.0+incompatible // indirect
-	github.com/urfave/negroni v1.0.1-0.20200608235619-7de0dfc1ff79 // indirect
 	github.com/vmihailenco/go-tinylfu v0.2.2 // indirect
 	github.com/vmihailenco/msgpack/v5 v5.3.4 // indirect
 	github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect

diff --git a/go.sum b/go.sum
@@ -1011,8 +1011,6 @@ github.com/pelletier/go-toml/v2 v2.0.5/go.mod h1:OMHamSCAODeSsVrwwvcJOaoN0LIUIaF
 github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
 github.com/phpdave11/gofpdf v1.4.2/go.mod h1:zpO6xFn9yxo3YLyMvW8HcKWVdbNqgIfOOp2dXMnm1mY=
 github.com/phpdave11/gofpdi v1.0.12/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI=
-github.com/phyber/negroni-gzip v1.0.0 h1:ru1uBeaUeoAXYgZRE7RsH7ftj/t5v/hkufXv1OYbNK8=
-github.com/phyber/negroni-gzip v1.0.0/go.mod h1:poOYjiFVKpeib8SnUpOgfQGStKNGLKsM8l09lOTNeyw=
 github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
 github.com/pierrec/lz4/v4 v4.1.8/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
 github.com/pkg/browser v0.0.0-20210706143420-7d21f8c997e2/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI=
@@ -1193,9 +1191,6 @@ github.com/urfave/cli v0.0.0-20171014202726-7bc6a0acffa5/go.mod h1:70zkFmudgCuE/
 github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA=
 github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
-github.com/urfave/negroni v1.0.0/go.mod h1:Meg73S6kFm/4PpbYdq35yYWoCZ9mS/YSx+lKnmiohz4=
-github.com/urfave/negroni v1.0.1-0.20200608235619-7de0dfc1ff79 h1:310aEUwMcbpcxq9wp7hSr9f+mRV6grcai3HxdAmVl9k=
-github.com/urfave/negroni v1.0.1-0.20200608235619-7de0dfc1ff79/go.mod h1:Meg73S6kFm/4PpbYdq35yYWoCZ9mS/YSx+lKnmiohz4=
 github.com/vishvananda/netlink v0.0.0-20181108222139-023a6dafdcdf/go.mod h1:+SR5DhBJrl6ZM7CoCKvpw5BKroDKQ+PJqOg65H/2ktk=
 github.com/vishvananda/netlink v1.1.0/go.mod h1:cTgwzPIzzgDAYoQrMm0EdrjRUBkTqKYppBueQtXaqoE=
 github.com/vishvananda/netlink v1.1.1-0.20201029203352-d40f9887b852/go.mod h1:twkDnbuQxJYemMlGd4JFIcuhgX83tXhKS2B/PRMpOho=

diff --git a/internal/config/authentication.go b/internal/config/authentication.go
@@ -41,7 +41,7 @@ func (c AuthenticationConfig) ShouldRunCleanup() bool {
 	return (c.Methods.Token.Enabled && c.Methods.Token.Cleanup != nil)
 }
 
-func (c *AuthenticationConfig) setDefaults(v *viper.Viper) []string {
+func (c *AuthenticationConfig) setDefaults(v *viper.Viper) {
 	token := map[string]any{
 		"enabled": false,
 	}
@@ -59,8 +59,6 @@ func (c *AuthenticationConfig) setDefaults(v *viper.Viper) []string {
 			"token": token,
 		},
 	})
-
-	return nil
 }
 
 func (c *AuthenticationConfig) validate() error {

diff --git a/internal/config/cache.go b/internal/config/cache.go
@@ -7,6 +7,9 @@ import (
 	"github.com/spf13/viper"
 )
 
+// cheers up the unparam linter
+var _ defaulter = (*CacheConfig)(nil)
+
 // CacheConfig contains fields, which enable and configure
 // Flipt's various caching mechanisms.
 //
@@ -19,7 +22,7 @@ type CacheConfig struct {
 	Redis   RedisCacheConfig  `json:"redis,omitempty" mapstructure:"redis"`
 }
 
-func (c *CacheConfig) setDefaults(v *viper.Viper) (warnings []string) {
+func (c *CacheConfig) setDefaults(v *viper.Viper) {
 	v.SetDefault("cache", map[string]any{
 		"enabled": false,
 		"backend": CacheMemory,
@@ -37,20 +40,34 @@ func (c *CacheConfig) setDefaults(v *viper.Viper) (warnings []string) {
 	})
 
 	if v.GetBool("cache.memory.enabled") {
-		warnings = append(warnings, deprecatedMsgMemoryEnabled)
 		// forcibly set top-level `enabled` to true
 		v.Set("cache.enabled", true)
 		// ensure ttl is mapped to the value at memory.expiration
 		v.RegisterAlias("cache.ttl", "cache.memory.expiration")
 		// ensure ttl default is set
 		v.SetDefault("cache.memory.expiration", 1*time.Minute)
 	}
+}
+
+func (c *CacheConfig) deprecations(v *viper.Viper) []deprecation {
+	var deprecations []deprecation
+
+	if v.GetBool("cache.memory.enabled") {
+		deprecations = append(deprecations, deprecation{
+
+			option:            "cache.memory.enabled",
+			additionalMessage: deprecatedMsgMemoryEnabled,
+		})
+	}
 
 	if v.IsSet("cache.memory.expiration") {
-		warnings = append(warnings, deprecatedMsgMemoryExpiration)
+		deprecations = append(deprecations, deprecation{
+			option:            "cache.memory.expiration",
+			additionalMessage: deprecatedMsgMemoryExpiration,
+		})
 	}
 
-	return
+	return deprecations
 }
 
 // CacheBackend is either memory or redis

diff --git a/internal/config/config.go b/internal/config/config.go
@@ -80,13 +80,17 @@ func Load(path string) (*Config, error) {
 }
 
 type defaulter interface {
-	setDefaults(v *viper.Viper) []string
+	setDefaults(v *viper.Viper)
 }
 
 type validator interface {
 	validate() error
 }
 
+type deprecator interface {
+	deprecations(v *viper.Viper) []deprecation
+}
+
 func (c *Config) prepare(v *viper.Viper) (validators []validator) {
 	val := reflect.ValueOf(c).Elem()
 	for i := 0; i < val.NumField(); i++ {
@@ -101,7 +105,7 @@ func (c *Config) prepare(v *viper.Viper) (validators []validator) {
 		// setting any defaults during this prepare stage
 		// on the supplied viper.
 		if defaulter, ok := field.(defaulter); ok {
-			c.Warnings = append(c.Warnings, defaulter.setDefaults(v)...)
+			defaulter.setDefaults(v)
 		}
 
 		// for-each validator implementing field we collect
@@ -110,6 +114,16 @@ func (c *Config) prepare(v *viper.Viper) (validators []validator) {
 		if validator, ok := field.(validator); ok {
 			validators = append(validators, validator)
 		}
+
+		// for-each deprecator implementing field we collect
+		// the messages as warnings.
+		if deprecator, ok := field.(deprecator); ok {
+			for _, d := range deprecator.deprecations(v) {
+				if msg := d.String(); msg != "" {
+					c.Warnings = append(c.Warnings, msg)
+				}
+			}
+		}
 	}
 
 	return

diff --git a/internal/config/config_test.go b/internal/config/config_test.go
@@ -246,7 +246,10 @@ func TestLoad(t *testing.T) {
 				cfg.Cache.Enabled = true
 				cfg.Cache.Backend = CacheMemory
 				cfg.Cache.TTL = -time.Second
-				cfg.Warnings = append(cfg.Warnings, deprecatedMsgMemoryEnabled, deprecatedMsgMemoryExpiration)
+				cfg.Warnings = []string{
+					"\"cache.memory.enabled\" is deprecated and will be removed in a future version. Please use 'cache.backend' and 'cache.enabled' instead.",
+					"\"cache.memory.expiration\" is deprecated and will be removed in a future version. Please use 'cache.ttl' instead.",
+				}
 				return cfg
 			},
 		},
@@ -255,7 +258,7 @@ func TestLoad(t *testing.T) {
 			path: "./testdata/deprecated/database_migrations_path.yml",
 			expected: func() *Config {
 				cfg := defaultConfig()
-				cfg.Warnings = append(cfg.Warnings, deprecatedMsgDatabaseMigrations)
+				cfg.Warnings = []string{"\"db.migrations.path\" is deprecated and will be removed in a future version. Migrations are now embedded within Flipt and are no longer required on disk."}
 				return cfg
 			},
 		},
@@ -264,7 +267,7 @@ func TestLoad(t *testing.T) {
 			path: "./testdata/deprecated/database_migrations_path_legacy.yml",
 			expected: func() *Config {
 				cfg := defaultConfig()
-				cfg.Warnings = append(cfg.Warnings, deprecatedMsgDatabaseMigrations)
+				cfg.Warnings = []string{"\"db.migrations.path\" is deprecated and will be removed in a future version. Migrations are now embedded within Flipt and are no longer required on disk."}
 				return cfg
 			},
 		},

diff --git a/internal/config/cors.go b/internal/config/cors.go
@@ -12,11 +12,9 @@ type CorsConfig struct {
 	AllowedOrigins []string `json:"allowedOrigins,omitempty" mapstructure:"allowed_origins"`
 }
 
-func (c *CorsConfig) setDefaults(v *viper.Viper) []string {
+func (c *CorsConfig) setDefaults(v *viper.Viper) {
 	v.SetDefault("cors", map[string]any{
 		"enabled":         false,
 		"allowed_origins": "*",
 	})
-
-	return nil
 }
diff --git a/internal/config/database.go b/internal/config/database.go
@@ -39,7 +39,7 @@ type DatabaseConfig struct {
 	Protocol        DatabaseProtocol `json:"protocol,omitempty" mapstructure:"protocol"`
 }
 
-func (c *DatabaseConfig) setDefaults(v *viper.Viper) []string {
+func (c *DatabaseConfig) setDefaults(v *viper.Viper) {
 	v.SetDefault("db", map[string]any{
 		"max_idle_conn": 2,
 	})
@@ -54,12 +54,19 @@ func (c *DatabaseConfig) setDefaults(v *viper.Viper) []string {
 	if setDefaultURL {
 		v.SetDefault("db.url", "file:/var/opt/flipt/flipt.db")
 	}
+}
+
+func (c *DatabaseConfig) deprecations(v *viper.Viper) []deprecation {
+	var deprecations []deprecation
 
 	if v.IsSet("db.migrations.path") || v.IsSet("db.migrations_path") {
-		return []string{deprecatedMsgDatabaseMigrations}
+		deprecations = append(deprecations, deprecation{
+			option:            "db.migrations.path",
+			additionalMessage: deprecatedMsgDatabaseMigrations,
+		})
 	}
 
-	return nil
+	return deprecations
 }
 
 func (c *DatabaseConfig) validate() (err error) {

diff --git a/internal/config/deprecate.go b/internal/config/deprecate.go
diff --git a/internal/config/deprecations.go b/internal/config/deprecations.go
@@ -0,0 +1,25 @@
+package config
+
+import (
+	"fmt"
+	"strings"
+)
+
+const (
+	// additional deprecation messages
+	deprecatedMsgMemoryEnabled      = `Please use 'cache.backend' and 'cache.enabled' instead.`
+	deprecatedMsgMemoryExpiration   = `Please use 'cache.ttl' instead.`
+	deprecatedMsgDatabaseMigrations = `Migrations are now embedded within Flipt and are no longer required on disk.`
+)
+
+// deprecation represents a deprecated configuration option
+type deprecation struct {
+	// the deprecated option
+	option string
+	// the (optional) additionalMessage to display
+	additionalMessage string
+}
+
+func (d deprecation) String() string {
+	return strings.TrimSpace(fmt.Sprintf("%q is deprecated and will be removed in a future version. %s", d.option, d.additionalMessage))
+}
diff --git a/internal/config/log.go b/internal/config/log.go
@@ -18,14 +18,12 @@ type LogConfig struct {
 	GRPCLevel string      `json:"grpcLevel,omitempty" mapstructure:"grpc_level"`
 }
 
-func (c *LogConfig) setDefaults(v *viper.Viper) []string {
+func (c *LogConfig) setDefaults(v *viper.Viper) {
 	v.SetDefault("log", map[string]any{
 		"level":      "INFO",
 		"encoding":   "console",
 		"grpc_level": "ERROR",
 	})
-
-	return nil
 }
 
 var (

diff --git a/internal/config/meta.go b/internal/config/meta.go
@@ -12,11 +12,9 @@ type MetaConfig struct {
 	StateDirectory   string `json:"stateDirectory" mapstructure:"state_directory"`
 }
 
-func (c *MetaConfig) setDefaults(v *viper.Viper) []string {
+func (c *MetaConfig) setDefaults(v *viper.Viper) {
 	v.SetDefault("meta", map[string]any{
 		"check_for_updates": true,
 		"telemetry_enabled": true,
 	})
-
-	return nil
 }
diff --git a/internal/config/server.go b/internal/config/server.go
@@ -22,16 +22,14 @@ type ServerConfig struct {
 	CertKey   string `json:"certKey,omitempty" mapstructure:"cert_key"`
 }
 
-func (c *ServerConfig) setDefaults(v *viper.Viper) []string {
+func (c *ServerConfig) setDefaults(v *viper.Viper) {
 	v.SetDefault("server", map[string]any{
 		"host":       "0.0.0.0",
 		"protocol":   HTTP,
 		"http_port":  8080,
 		"https_port": 443,
 		"grpc_port":  9000,
 	})
-
-	return nil
 }
 
 func (c *ServerConfig) validate() (err error) {

diff --git a/internal/config/tracing.go b/internal/config/tracing.go
@@ -19,14 +19,12 @@ type TracingConfig struct {
 	Jaeger JaegerTracingConfig `json:"jaeger,omitempty" mapstructure:"jaeger"`
 }
 
-func (c *TracingConfig) setDefaults(v *viper.Viper) []string {
+func (c *TracingConfig) setDefaults(v *viper.Viper) {
 	v.SetDefault("tracing", map[string]any{
 		"jaeger": map[string]any{
 			"enabled": false,
 			"host":    "localhost",
 			"port":    6831,
 		},
 	})
-
-	return nil
 }
diff --git a/internal/config/ui.go b/internal/config/ui.go
@@ -11,10 +11,8 @@ type UIConfig struct {
 	Enabled bool `json:"enabled" mapstructure:"enabled"`
 }
 
-func (c *UIConfig) setDefaults(v *viper.Viper) []string {
+func (c *UIConfig) setDefaults(v *viper.Viper) {
 	v.SetDefault("ui", map[string]any{
 		"enabled": true,
 	})
-
-	return nil
 }