Skip to content

Commit

Permalink
Move the GDPR default value validation into the GDPR validate method
Browse files Browse the repository at this point in the history
  • Loading branch information
@bsardo
bsardo committed May 21, 2021
1 parent bf8918d commit 424a442
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 23 deletions.
15 changes: 7 additions & 8 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ type HTTPClient struct {
IdleConnTimeout int `mapstructure:"idle_connection_timeout_seconds"`
}

func (cfg *Configuration) validate() []error {
func (cfg *Configuration) validate(v *viper.Viper) []error {
var errs []error
errs = cfg.AuctionTimeouts.validate(errs)
errs = cfg.StoredRequests.validate(errs)
Expand All @@ -105,7 +105,7 @@ func (cfg *Configuration) validate() []error {
if cfg.MaxRequestSize < 0 {
errs = append(errs, fmt.Errorf("cfg.max_request_size must be >= 0. Got %d", cfg.MaxRequestSize))
}
errs = cfg.GDPR.validate(errs)
errs = cfg.GDPR.validate(v, errs)
errs = cfg.CurrencyConverter.validate(errs)
errs = validateAdapters(cfg.Adapters, errs)
errs = cfg.Debug.validate(errs)
Expand Down Expand Up @@ -208,7 +208,10 @@ type GDPR struct {
EEACountriesMap map[string]struct{}
}

func (cfg *GDPR) validate(errs []error) []error {
func (cfg *GDPR) validate(v *viper.Viper, errs []error) []error {
if !v.IsSet("gdpr.default_value") {
errs = append(errs, fmt.Errorf("gdpr.default_value is required and must be specified"))
}
if cfg.HostVendorID < 0 || cfg.HostVendorID > 0xffff {
errs = append(errs, fmt.Errorf("gdpr.host_vendor_id must be in the range [0, %d]. Got %d", 0xffff, cfg.HostVendorID))
}
Expand Down Expand Up @@ -473,10 +476,6 @@ func (cfg *TimeoutNotification) validate(errs []error) []error {

// New uses viper to get our server configurations.
func New(v *viper.Viper) (*Configuration, error) {
if !v.IsSet("gdpr.default_value") {
return nil, fmt.Errorf("Required config flag gdpr.default_value not specified")
}

var c Configuration
if err := v.Unmarshal(&c); err != nil {
return nil, fmt.Errorf("viper failed to unmarshal app config: %v", err)
Expand Down Expand Up @@ -530,7 +529,7 @@ func New(v *viper.Viper) (*Configuration, error) {

glog.Info("Logging the resolved configuration:")
logGeneral(reflect.ValueOf(c), " \t")
if errs := c.validate(); len(errs) > 0 {
if errs := c.validate(v); len(errs) > 0 {
return &c, errortypes.NewAggregateError("validation errors", errs)
}

Expand Down
58 changes: 43 additions & 15 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,11 @@ func TestValidConfig(t *testing.T) {
},
}

v := viper.New()
v.Set("gdpr.default_value", false)

resolvedStoredRequestsConfig(&cfg)
err := cfg.validate()
err := cfg.validate(v)
assert.Nil(t, err, "OpenRTB filesystem config should work. %v", err)
}

Expand Down Expand Up @@ -534,16 +537,22 @@ func TestInvalidAdapterUserSyncURLConfig(t *testing.T) {
}

func TestNegativeRequestSize(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := newDefaultConfig(t)
cfg.MaxRequestSize = -1
assertOneError(t, cfg.validate(), "cfg.max_request_size must be >= 0. Got -1")
assertOneError(t, cfg.validate(v), "cfg.max_request_size must be >= 0. Got -1")
}

func TestNegativePrometheusTimeout(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := newDefaultConfig(t)
cfg.Metrics.Prometheus.Port = 8001
cfg.Metrics.Prometheus.TimeoutMillisRaw = 0
assertOneError(t, cfg.validate(), "metrics.prometheus.timeout_ms must be positive if metrics.prometheus.port is defined. Got timeout=0 and port=8001")
assertOneError(t, cfg.validate(v), "metrics.prometheus.timeout_ms must be positive if metrics.prometheus.port is defined. Got timeout=0 and port=8001")
}

func TestInvalidHostVendorID(t *testing.T) {
Expand All @@ -565,53 +574,66 @@ func TestInvalidHostVendorID(t *testing.T) {
}

for _, tt := range tests {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := newDefaultConfig(t)
cfg.GDPR.HostVendorID = tt.vendorID
errs := cfg.validate()
errs := cfg.validate(v)

assert.Equal(t, 1, len(errs), tt.description)
assert.EqualError(t, errs[0], tt.wantErrorMsg, tt.description)
}
}

func TestInvalidFetchGVL(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := newDefaultConfig(t)
cfg.GDPR.TCF1.FetchGVL = true
assertOneError(t, cfg.validate(), "gdpr.tcf1.fetch_gvl has been discontinued and must be removed from your config. TCF1 will always use the fallback GVL going forward")
assertOneError(t, cfg.validate(v), "gdpr.tcf1.fetch_gvl has been discontinued and must be removed from your config. TCF1 will always use the fallback GVL going forward")
}

func TestInvalidAMPException(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := newDefaultConfig(t)
cfg.GDPR.AMPException = true
assertOneError(t, cfg.validate(), "gdpr.amp_exception has been discontinued and must be removed from your config. If you need to disable GDPR for AMP, you may do so per-account (gdpr.integration_enabled.amp) or at the host level for the default account (account_defaults.gdpr.integration_enabled.amp)")
assertOneError(t, cfg.validate(v), "gdpr.amp_exception has been discontinued and must be removed from your config. If you need to disable GDPR for AMP, you may do so per-account (gdpr.integration_enabled.amp) or at the host level for the default account (account_defaults.gdpr.integration_enabled.amp)")
}

func TestMissingGDPRDefaultValue(t *testing.T) {
v := viper.New()
SetupViper(v, "")
cfg, err := New(v)
assert.Nil(t, cfg, "cfg is nil when gdpr.default_value is not specified")
assert.Error(t, err, "err is set when gdpr.default_value is not specified")
assert.Contains(t, err.Error(), "Required config flag gdpr.default_value not specified", "err msg indicates gdpr.default_value is required")

cfg := newDefaultConfig(t)
assertOneError(t, cfg.validate(v), "gdpr.default_value is required and must be specified")
}

func TestNegativeCurrencyConverterFetchInterval(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := Configuration{
CurrencyConverter: CurrencyConverter{
FetchIntervalSeconds: -1,
},
}
err := cfg.validate()
err := cfg.validate(v)
assert.NotNil(t, err, "cfg.currency_converter.fetch_interval_seconds should prevent negative values, but it doesn't")
}

func TestOverflowedCurrencyConverterFetchInterval(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := Configuration{
CurrencyConverter: CurrencyConverter{
FetchIntervalSeconds: (0xffff) + 1,
},
}
err := cfg.validate()
err := cfg.validate(v)
assert.NotNil(t, err, "cfg.currency_converter.fetch_interval_seconds prevent values over %d, but it doesn't", 0xffff)
}

Expand Down Expand Up @@ -687,20 +709,26 @@ func TestNewCallsRequestValidation(t *testing.T) {
}

func TestValidateDebug(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := newDefaultConfig(t)
cfg.Debug.TimeoutNotification.SamplingRate = 1.1

err := cfg.validate()
err := cfg.validate(v)
assert.NotNil(t, err, "cfg.debug.timeout_notification.sampling_rate should not be allowed to be greater than 1.0, but it was allowed")
}

func TestValidateAccountsConfigRestrictions(t *testing.T) {
v := viper.New()
v.Set("gdpr.default_value", false)

cfg := newDefaultConfig(t)
cfg.Accounts.Files.Enabled = true
cfg.Accounts.HTTP.Endpoint = "http://localhost"
cfg.Accounts.Postgres.ConnectionInfo.Database = "accounts"

errs := cfg.validate()
errs := cfg.validate(v)
assert.Len(t, errs, 1)
assert.Contains(t, errs, errors.New("accounts.postgres: retrieving accounts via postgres not available, use accounts.files"))
}
Expand Down

0 comments on commit 424a442

Please sign in to comment.