Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: refactor deprecations checking to own method #1226

Merged
merged 4 commits into from
Dec 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
4 changes: 1 addition & 3 deletions internal/config/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -59,8 +59,6 @@ func (c *AuthenticationConfig) setDefaults(v *viper.Viper) []string {
"token": token,
},
})

return nil
}

func (c *AuthenticationConfig) validate() error {
Expand Down
25 changes: 21 additions & 4 deletions internal/config/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand All @@ -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,
Expand All @@ -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
Expand Down
18 changes: 16 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++ {
Expand All @@ -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
Expand All @@ -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
Expand Down
9 changes: 6 additions & 3 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
},
Expand All @@ -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
},
},
Expand All @@ -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
},
},
Expand Down
4 changes: 1 addition & 3 deletions internal/config/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
13 changes: 10 additions & 3 deletions internal/config/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand All @@ -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) {
Expand Down
8 changes: 0 additions & 8 deletions internal/config/deprecate.go

This file was deleted.

25 changes: 25 additions & 0 deletions internal/config/deprecations.go
Original file line number Diff line number Diff line change
@@ -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))
}
4 changes: 1 addition & 3 deletions internal/config/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
4 changes: 1 addition & 3 deletions internal/config/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 1 addition & 3 deletions internal/config/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions internal/config/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 1 addition & 3 deletions internal/config/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}