diff --git a/pkg/dto/backup_policy.go b/pkg/dto/backup_policy.go index 0d34e9ff..8887f0a3 100644 --- a/pkg/dto/backup_policy.go +++ b/pkg/dto/backup_policy.go @@ -72,10 +72,10 @@ func (p *BackupPolicy) Validate() error { return errValidationNonPositive("parallel", *p.Parallel) } if p.SocketTimeout != nil && *p.SocketTimeout < 0 { - return fmt.Errorf("socketTimeout %d invalid, should not be negative number", *p.SocketTimeout) + return errValidationNegative("socket-timeout", *p.SocketTimeout) } if p.TotalTimeout != nil && *p.TotalTimeout < 0 { - return fmt.Errorf("totalTimeout %d invalid, should not be negative number", *p.TotalTimeout) + return errValidationNegative("total-timeout", *p.TotalTimeout) } if err := p.RetryPolicy.Validate(); err != nil { return fmt.Errorf("retryPolicy validation failed: %w", err) @@ -87,7 +87,7 @@ func (p *BackupPolicy) Validate() error { return errValidationNonPositive("records-per-second", *p.RecordsPerSecond) } if p.FileLimit != nil && *p.FileLimit < 0 { - return fmt.Errorf("fileLimit %d invalid, should not be negative number", *p.FileLimit) + return errValidationNegative("file-limit", *p.FileLimit) } if err := p.RetentionPolicy.Validate(); err != nil { return fmt.Errorf("invalid retention policy: %w", err) @@ -196,7 +196,7 @@ func (rp *RetentionPolicy) Validate() error { if rp.IncrBackups != nil { if *rp.IncrBackups < 0 { - return fmt.Errorf("incremental backups retention %d is invalid, cannot be negative", *rp.IncrBackups) + return errValidationNegative("incremental", *rp.IncrBackups) } if rp.FullBackups != nil && *rp.IncrBackups > *rp.FullBackups { return fmt.Errorf("incremental backups retention %d cannot exceed full backups retention %d", diff --git a/pkg/dto/backup_policy_test.go b/pkg/dto/backup_policy_test.go index ac2f2911..e7048157 100644 --- a/pkg/dto/backup_policy_test.go +++ b/pkg/dto/backup_policy_test.go @@ -81,7 +81,7 @@ func TestRetentionPolicy_Validate(t *testing.T) { { name: "invalid incremental backups: negative value", policy: &RetentionPolicy{FullBackups: util.Ptr(3), IncrBackups: util.Ptr(-1)}, - expectedErr: "incremental backups retention -1 is invalid, cannot be negative", + expectedErr: "negative value validation error: \"incremental\" -1 invalid, should not be negative number", }, { name: "invalid incremental backups: exceeds full backups", diff --git a/pkg/dto/backup_routine.go b/pkg/dto/backup_routine.go index 6398715a..19f4679f 100644 --- a/pkg/dto/backup_routine.go +++ b/pkg/dto/backup_routine.go @@ -67,9 +67,9 @@ func (r *BackupRoutine) Validate() error { return fmt.Errorf("incremental backup interval string '%s' invalid: %w", r.IntervalCron, err) } } - for _, rack := range r.PreferRacks { + for i, rack := range r.PreferRacks { if rack < 0 { - return fmt.Errorf("rack id %d invalid, should be positive number", rack) + return errValidationNegative(fmt.Sprintf("prefer-racks[%d]", i), rack) } if rack > maxRack { return fmt.Errorf("rack id %d invalid, should not exceed %d", rack, maxRack) diff --git a/pkg/dto/http_server_config.go b/pkg/dto/http_server_config.go index cae4426f..e3a1ba41 100644 --- a/pkg/dto/http_server_config.go +++ b/pkg/dto/http_server_config.go @@ -33,7 +33,7 @@ func (s *HTTPServerConfig) Validate() error { return fmt.Errorf("context-path must start with a slash: %s", *s.ContextPath) } if s.Timeout != nil && *s.Timeout < 0 { - return fmt.Errorf("timeout cannot be negative: %d", *s.Timeout) + return errValidationNegative("timeout", *s.Timeout) } return nil diff --git a/pkg/dto/logger_config.go b/pkg/dto/logger_config.go index 309914c7..05a9642a 100644 --- a/pkg/dto/logger_config.go +++ b/pkg/dto/logger_config.go @@ -122,13 +122,13 @@ func (f *FileLoggerConfig) Validate() error { return errValidationEmptyField("logger file") } if f.MaxSize < 0 { - return fmt.Errorf("negative logger MaxSize: %d", f.MaxSize) + return errValidationNegative("maxsize", f.MaxSize) } if f.MaxAge < 0 { - return fmt.Errorf("negative logger MaxAge: %d", f.MaxAge) + return errValidationNegative("maxage", f.MaxAge) } if f.MaxBackups < 0 { - return fmt.Errorf("negative logger MaxBackups: %d", f.MaxBackups) + return errValidationNegative("maxbackups", f.MaxBackups) } return nil diff --git a/pkg/dto/restore_namespace.go b/pkg/dto/restore_namespace.go index d8ed1ee6..8fc81520 100644 --- a/pkg/dto/restore_namespace.go +++ b/pkg/dto/restore_namespace.go @@ -1,8 +1,6 @@ package dto import ( - "fmt" - "github.com/aerospike/aerospike-backup-service/v3/pkg/model" ) @@ -21,11 +19,11 @@ type RestoreNamespace struct { // Validate validates the restore namespace. func (n *RestoreNamespace) Validate() error { if n.Source == nil { - return fmt.Errorf("source namespace is not specified") + return errValidationEmptyField("source") } if n.Destination == nil { - return fmt.Errorf("destination namespace is not specified") + return errValidationEmptyField("destination") } return nil diff --git a/pkg/dto/restore_policy.go b/pkg/dto/restore_policy.go index f930cb95..83deed70 100644 --- a/pkg/dto/restore_policy.go +++ b/pkg/dto/restore_policy.go @@ -1,7 +1,6 @@ package dto import ( - "errors" "fmt" "github.com/aerospike/aerospike-backup-service/v3/pkg/model" @@ -97,12 +96,12 @@ func (p *RestorePolicy) Validate() error { return errValidationNonPositive("tps", *p.Tps) } if p.Replace != nil && *p.Replace && p.Unique != nil && *p.Unique { - return errors.New("replace and unique options are contradictory") + return errValidationMutuallyExclusive("replace", "unique") } if p.Namespace != nil { // namespace is optional. if err := p.Namespace.Validate(); err != nil { - return err + return fmt.Errorf("restore namespace invalid: %w", err) } } if err := p.EncryptionPolicy.Validate(); err != nil { diff --git a/pkg/dto/retry_policy.go b/pkg/dto/retry_policy.go index 53bb9a56..740d0cf2 100644 --- a/pkg/dto/retry_policy.go +++ b/pkg/dto/retry_policy.go @@ -37,7 +37,7 @@ func (r *RetryPolicy) Validate() error { } if r.MaxRetries < 0 { - return fmt.Errorf("max-retries %d invalid, should be non-negative number", r.MaxRetries) + return errValidationNegative("max-retries", r.MaxRetries) } return nil diff --git a/pkg/dto/secret_agent.go b/pkg/dto/secret_agent.go index 1d82f5e8..8cabb1fb 100644 --- a/pkg/dto/secret_agent.go +++ b/pkg/dto/secret_agent.go @@ -59,7 +59,7 @@ type SecretAgent struct { // Connection type: tcp, unix. ConnectionType string `yaml:"connection-type,omitempty" json:"connection-type,omitempty" example:"tcp"` // Address of the Secret Agent. - Address string `yaml:"address,omitempty" json:"address,omitempty" example:"localhost"` + Address string `yaml:"address" json:"address" example:"localhost" validate:"required"` // Port the Secret Agent is running on. Port *int `yaml:"port,omitempty" json:"port,omitempty" example:"8080"` // Timeout in milliseconds. @@ -124,11 +124,11 @@ func (s *SecretAgent) validate() error { } if s.Address == "" { - return fmt.Errorf("address is required") + return errValidationEmptyField("address") } - if s.Timeout != nil && *s.Timeout <= 0 { - return fmt.Errorf("invalid timeout: %d", *s.Timeout) + if s.Timeout != nil && *s.Timeout < 0 { + return errValidationNegative("timeout", *s.Timeout) } if s.ConnectionType != saClient.ConnectionTypeTCP && s.ConnectionType != saClient.ConnectionTypeUDS { diff --git a/pkg/dto/secret_agent_test.go b/pkg/dto/secret_agent_test.go index 1c7d37fe..350ef5f2 100644 --- a/pkg/dto/secret_agent_test.go +++ b/pkg/dto/secret_agent_test.go @@ -1,7 +1,6 @@ package dto import ( - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -85,7 +84,7 @@ func TestMissingAddress(t *testing.T) { err := agent.validate() assert.Error(t, err) - assert.EqualError(t, err, "address is required") + assert.EqualError(t, err, errValidationEmptyField("address").Error()) } func TestInvalidTimeout(t *testing.T) { @@ -98,7 +97,7 @@ func TestInvalidTimeout(t *testing.T) { err := agent.validate() assert.Error(t, err) - assert.EqualError(t, err, fmt.Sprintf("invalid timeout: %d", timeout)) + assert.EqualError(t, err, errValidationNegative("timeout", -100).Error()) } func TestSecretAgent_Nil(t *testing.T) { diff --git a/pkg/dto/storage_s3.go b/pkg/dto/storage_s3.go index 0a897c43..238837b9 100644 --- a/pkg/dto/storage_s3.go +++ b/pkg/dto/storage_s3.go @@ -1,7 +1,6 @@ package dto import ( - "errors" "fmt" "github.com/aerospike/aerospike-backup-service/v3/pkg/model" @@ -40,10 +39,10 @@ type S3Storage struct { // Validate checks if the S3Storage is valid. func (s *S3Storage) Validate() error { if s.Bucket == "" { - return errors.New("S3 bucket is not specified") + return errValidationEmptyField("bucket") } if s.S3Region == "" { - return errors.New("S3 region is not specified") + return errValidationEmptyField("s3-region") } if s.AccessKeyID != nil && s.SecretAccessKey == nil { diff --git a/pkg/dto/time_bounds.go b/pkg/dto/time_bounds.go index 4263d0be..d8390de8 100644 --- a/pkg/dto/time_bounds.go +++ b/pkg/dto/time_bounds.go @@ -2,7 +2,6 @@ package dto import ( "errors" - "fmt" "strconv" "time" @@ -34,12 +33,12 @@ func (t *TimeBounds) ToModel() model.TimeBounds { // NewTimeBoundsFromString creates a TimeBounds from the string representation of // time boundaries (string is given as epoch time millis). func NewTimeBoundsFromString(from, to string) (TimeBounds, error) { - fromTime, err := parseTimestamp(from) + fromTime, err := parseTimestamp(from, "from") if err != nil { return TimeBounds{}, err } - toTime, err := parseTimestamp(to) + toTime, err := parseTimestamp(to, "to") if err != nil { return TimeBounds{}, err } @@ -47,7 +46,7 @@ func NewTimeBoundsFromString(from, to string) (TimeBounds, error) { return NewTimeBounds(fromTime, toTime) } -func parseTimestamp(value string) (*time.Time, error) { +func parseTimestamp(value, field string) (*time.Time, error) { if len(value) == 0 { return nil, nil } @@ -58,7 +57,7 @@ func parseTimestamp(value string) (*time.Time, error) { } if intValue < 0 { - return nil, fmt.Errorf("timestamp should be positive or zero, got %d", intValue) + return nil, errValidationNegative(field, intValue) } return util.Ptr(time.UnixMilli(intValue)), nil diff --git a/pkg/dto/validate_errors.go b/pkg/dto/validate_errors.go index 51867a27..d97b12fc 100644 --- a/pkg/dto/validate_errors.go +++ b/pkg/dto/validate_errors.go @@ -11,6 +11,7 @@ var ( errEmpty = fmt.Errorf("empty field %w", errValidation) errNotFound = fmt.Errorf("not found %w", errValidation) errNonPositive = fmt.Errorf("non-positive value %w", errValidation) + errNegative = fmt.Errorf("negative value %w", errValidation) ) func errValidationRequiredEither(field1, field2 string) error { @@ -32,3 +33,7 @@ func errValidationNotFound(field, value string) error { func errValidationNonPositive[T ~int | ~int8 | ~int16 | ~int32 | ~int64](field string, value T) error { return fmt.Errorf("%w: %q %d invalid, should be positive number", errNonPositive, field, value) } + +func errValidationNegative[T ~int | ~int8 | ~int16 | ~int32 | ~int64](field string, value T) error { + return fmt.Errorf("%w: %q %d invalid, should not be negative number", errNegative, field, value) +}