Skip to content

Commit

Permalink
fix: prevent overlapping remote writes
Browse files Browse the repository at this point in the history
By adding configuration validatation to prevent possibility of
overlapping end and start of two separate write attempts.

Also does additional basic validations:

- ensures that WriteRetryMinInt < WriteRetryMaxInt
- ensures that MetricsWALPath is defined
- ensures that WriteUrl is defined

Signed-off-by: Petr Vobornik <[email protected]>
  • Loading branch information
pvoborni committed Oct 18, 2023
1 parent 5bfd85f commit 0de885a
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 2 deletions.
40 changes: 40 additions & 0 deletions config/config_validator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package config

import (
"fmt"
"time"
)

type Validator interface {
Validate() error
}

type ConfigValidator struct {
config *Config
}

func NewConfigValidator(config *Config) *ConfigValidator {
return &ConfigValidator{config}
}

func (cv *ConfigValidator) Validate() error {
c := cv.config

if c.WriteUrl == "" {
return fmt.Errorf("WriteURL must be defined")
}

if c.WriteInterval <= time.Duration(c.WriteRetryAttempts)*(c.WriteRetryMaxInt+c.WriteTimeout) {
return fmt.Errorf("WriteInterval must be bigger than WriteRetryAttempts * ( WriteRetryMaxInt + WriteTimeout )")
}

if c.WriteRetryMinInt >= c.WriteRetryMaxInt {
return fmt.Errorf("WriteRetryMinInt must be smaller than WriteRetryMaxInt")
}

if c.MetricsWALPath == "" {
return fmt.Errorf("MetricsWALPath must be defined")
}

return nil
}
98 changes: 98 additions & 0 deletions config/config_validator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package config

import (
"strings"
"testing"
"time"
)

func TestConfigValidator(t *testing.T) {
t.Run("Validate Config", func(t *testing.T) {
t.Run("WriteURL must be defined", func(t *testing.T) {
// given
c := NewConfig()
c.WriteUrl = ""
cv := NewConfigValidator(c)

// when
err := cv.Validate()

// then
expectErrorContains(t, err, "WriteURL must be defined")
})

t.Run("overlapping requests", func(t *testing.T) {
// given
c := NewConfig()
c.WriteInterval = 80 * time.Second
c.WriteRetryAttempts = 8
c.WriteRetryMaxInt = 10 * time.Second
c.WriteTimeout = 60 * time.Second
cv := NewConfigValidator(c)

// when
err := cv.Validate()

// then
expectErrorContains(t, err, "WriteInterval must be bigger than WriteRetryAttempts * ( WriteRetryMaxInt + WriteTimeout )")
})

t.Run("WriteRetryMinInt to be smaller than WriteRetryMaxInt", func(t *testing.T) {
// given
c := NewConfig()
c.WriteRetryMinInt = 10 * time.Second
c.WriteRetryMaxInt = 1 * time.Second
cv := NewConfigValidator(c)

// when
err := cv.Validate()

// then
expectErrorContains(t, err, "WriteRetryMinInt must be smaller than WriteRetryMaxInt")
})

t.Run("MetricsWALPath must be defined", func(t *testing.T) {
// given
c := NewConfig()
c.MetricsWALPath = ""
cv := NewConfigValidator(c)

// when
err := cv.Validate()

// then
expectErrorContains(t, err, "MetricsWALPath must be defined")
})

t.Run("default config should be valid", func(t *testing.T) {
// given
c := NewConfig()
cv := NewConfigValidator(c)

// when
err := cv.Validate()

// then
if err != nil {
t.Fatalf("expected no error, got %v", err)
}
})
})
}

// Helpers

func expectError(t *testing.T, err error) {
t.Helper()
if err == nil {
t.Fatalf("expected error, got nil")
}
}

func expectErrorContains(t *testing.T, err error, expected string) {
t.Helper()
expectError(t, err)
if !strings.Contains(err.Error(), expected) {
t.Fatalf("expected error to contain '%s', got '%s'", expected, err.Error())
}
}
5 changes: 3 additions & 2 deletions hack/host-metering.conf
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ host_cert_path=./hack/test-cert.crt
host_cert_key_path=./hack/test-cert.key
collect_interval_sec=0
label_refresh_interval_sec=10
write_retry_attempts=4
write_retry_interval_sec=2
write_timeout_sec=1
write_retry_attempts=1
write_retry_max_int_sec=2
metrics_wal_path=./hack/cpumetrics
log_level=DEBUG
7 changes: 7 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ func main() {
//print out the configuration
logger.Infoln(cfg.String())

cv := config.NewConfigValidator(cfg)
err = cv.Validate()
if err != nil {
logger.Errorf("Invalid configuration: %v\n", err.Error())
os.Exit(2)
}

d, err := daemon.NewDaemon(cfg)
if err != nil {
logger.Errorf("Failed to create daemon: %v\n", err.Error())
Expand Down

0 comments on commit 0de885a

Please sign in to comment.