Praefect: Failover.Validate method

The new 'Validate' method validates values of the 'Failover' type and
returns all found errors together. If 'Enabled' field is set to
'false' no validation done as it won't be used and doesn't affect
the service anyhow. It will be used in the later changes.

The cfgerror now has a new 'IsSupportedValue' check.

4 files changed, 164 insertions, 1 deletions

diff --git a/internal/errors/cfgerror/validate.go b/internal/errors/cfgerror/validate.go
index 9d32f2eac..00e45fa15 100644
--- a/internal/errors/cfgerror/validate.go
+++ b/internal/errors/cfgerror/validate.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 
 	"golang.org/x/exp/constraints"
@@ -31,6 +32,8 @@ var (
 	ErrBadOrder = errors.New("bad order")
 	// ErrNotInRange should be used when the value is not in expected range of values.
 	ErrNotInRange = errors.New("not in range")
+	// ErrUnsupportedValue should be used when the value is not supported.
+	ErrUnsupportedValue = errors.New("not supported")
 )
 
 // ValidationError represents an issue with provided configuration.
@@ -247,3 +250,18 @@ func InRange[T Numeric](min, max, val T, opts ...InRangeOpt) error {
 
 	return nil
 }
+
+// IsSupportedValue ensures the provided 'value' is one listed as 'supportedValues'.
+func IsSupportedValue[T comparable](value T, supportedValues ...T) error {
+	for _, supportedValue := range supportedValues {
+		if value == supportedValue {
+			return nil
+		}
+	}
+
+	if reflect.TypeOf(value).Kind() == reflect.String {
+		return NewValidationError(fmt.Errorf(`%w: "%v"`, ErrUnsupportedValue, value))
+	}
+
+	return NewValidationError(fmt.Errorf("%w: %v", ErrUnsupportedValue, value))
+}
diff --git a/internal/errors/cfgerror/validate_test.go b/internal/errors/cfgerror/validate_test.go
index c4084eb10..023bf1355 100644
--- a/internal/errors/cfgerror/validate_test.go
+++ b/internal/errors/cfgerror/validate_test.go
@@ -237,3 +237,11 @@ func TestInRange(t *testing.T) {
 		})
 	}
 }
+
+func TestIsSupportedValue(t *testing.T) {
+	t.Parallel()
+	require.NoError(t, IsSupportedValue(1, 1, 2, 3))
+	require.Equal(t, NewValidationError(fmt.Errorf("%w: 0", ErrUnsupportedValue)), IsSupportedValue(0))
+	require.Equal(t, NewValidationError(fmt.Errorf("%w: 1", ErrUnsupportedValue)), IsSupportedValue(1, 0, 10))
+	require.Equal(t, NewValidationError(fmt.Errorf(`%w: "c"`, ErrUnsupportedValue)), IsSupportedValue("c", "a", "b"))
+}
diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go
index 79aff30e3..edbe5c88f 100644
--- a/internal/praefect/config/config.go
+++ b/internal/praefect/config/config.go
@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/yamux"
 	"github.com/pelletier/go-toml/v2"
 	promclient "github.com/prometheus/client_golang/prometheus"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/errors/cfgerror"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/auth"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/log"
@@ -44,8 +45,9 @@ const (
 	minimalSyncRunInterval   = time.Minute
 )
 
-//nolint:revive // This is unintentionally missing documentation.
+// Failover contains configuration for the mechanism that tracks healthiness of the cluster nodes.
 type Failover struct {
+	// Enabled is a trigger used to check if failover is enabled or not.
 	Enabled bool `toml:"enabled,omitempty"`
 	// ElectionStrategy is the strategy to use for electing primaries nodes.
 	ElectionStrategy         ElectionStrategy  `toml:"election_strategy,omitempty"`
@@ -82,6 +84,39 @@ func (f Failover) ErrorThresholdsConfigured() (bool, error) {
 	return true, nil
 }
 
+// Validate returns a list of failed checks.
+func (f Failover) Validate() error {
+	if !f.Enabled {
+		// If it is not enabled we shouldn't care about provided values
+		// as they won't be used.
+		return nil
+	}
+
+	errs := cfgerror.New().
+		Append(cfgerror.IsSupportedValue(f.ElectionStrategy, ElectionStrategyLocal, ElectionStrategySQL, ElectionStrategyPerRepository), "election_strategy").
+		Append(cfgerror.IsPositive(f.BootstrapInterval.Duration()), "bootstrap_interval").
+		Append(cfgerror.IsPositive(f.MonitorInterval.Duration()), "monitor_interval").
+		Append(cfgerror.IsPositive(f.ErrorThresholdWindow.Duration()), "error_threshold_window")
+
+	if f.ErrorThresholdWindow == 0 && f.WriteErrorThresholdCount == 0 && f.ReadErrorThresholdCount == 0 {
+		return errs.AsError()
+	}
+
+	if f.ErrorThresholdWindow == 0 {
+		errs = errs.Append(cfgerror.ErrNotSet, "error_threshold_window")
+	}
+
+	if f.WriteErrorThresholdCount == 0 {
+		errs = errs.Append(cfgerror.ErrNotSet, "write_error_threshold_count")
+	}
+
+	if f.ReadErrorThresholdCount == 0 {
+		errs = errs.Append(cfgerror.ErrNotSet, "read_error_threshold_count")
+	}
+
+	return errs.AsError()
+}
+
 // BackgroundVerification contains configuration options for the repository background verification.
 type BackgroundVerification struct {
 	// VerificationInterval determines the duration after a replica due for reverification.
diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go
index 1ed5e0cd9..0f2334a0a 100644
--- a/internal/praefect/config/config_test.go
+++ b/internal/praefect/config/config_test.go
@@ -5,12 +5,14 @@ package config
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"os"
 	"testing"
 	"time"
 
 	"github.com/pelletier/go-toml/v2"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/errors/cfgerror"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/log"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/prometheus"
@@ -536,3 +538,103 @@ func TestSerialization(t *testing.T) {
 		require.Equal(t, "listen_addr = 'localhost:5640'\n", out.String())
 	})
 }
+
+func TestFailover_Validate(t *testing.T) {
+	t.Parallel()
+	for _, tc := range []struct {
+		name        string
+		failover    Failover
+		expectedErr error
+	}{
+		{
+			name:     "empty disabled config",
+			failover: Failover{},
+		},
+		{
+			name: "all set valid",
+			failover: Failover{
+				Enabled:                  true,
+				ElectionStrategy:         ElectionStrategySQL,
+				ErrorThresholdWindow:     duration.Duration(1),
+				WriteErrorThresholdCount: 1,
+				ReadErrorThresholdCount:  1,
+				BootstrapInterval:        duration.Duration(1),
+				MonitorInterval:          duration.Duration(1),
+			},
+		},
+		{
+			name: "all valid with disabled error threshold",
+			failover: Failover{
+				ElectionStrategy:  ElectionStrategySQL,
+				BootstrapInterval: duration.Duration(1),
+				MonitorInterval:   duration.Duration(1),
+			},
+		},
+		{
+			name: "all set invalid except ErrorThresholdWindow",
+			failover: Failover{
+				Enabled:                  true,
+				ElectionStrategy:         ElectionStrategy("bad"),
+				ErrorThresholdWindow:     duration.Duration(-1),
+				WriteErrorThresholdCount: 0,
+				ReadErrorThresholdCount:  0,
+				BootstrapInterval:        duration.Duration(-1),
+				MonitorInterval:          duration.Duration(-1),
+			},
+			expectedErr: cfgerror.ValidationErrors{
+				{
+					Key:   []string{"election_strategy"},
+					Cause: fmt.Errorf(`%w: "bad"`, cfgerror.ErrUnsupportedValue),
+				},
+				{
+					Key:   []string{"bootstrap_interval"},
+					Cause: fmt.Errorf("%w: -1ns", cfgerror.ErrIsNegative),
+				},
+				{
+					Key:   []string{"monitor_interval"},
+					Cause: fmt.Errorf("%w: -1ns", cfgerror.ErrIsNegative),
+				},
+				{
+					Key:   []string{"error_threshold_window"},
+					Cause: fmt.Errorf("%w: -1ns", cfgerror.ErrIsNegative),
+				},
+				{
+					Key:   []string{"write_error_threshold_count"},
+					Cause: cfgerror.ErrNotSet,
+				},
+				{
+					Key:   []string{"read_error_threshold_count"},
+					Cause: cfgerror.ErrNotSet,
+				},
+			},
+		},
+		{
+			name: "invalid error threshold",
+			failover: Failover{
+				Enabled:                  true,
+				ElectionStrategy:         ElectionStrategySQL,
+				ErrorThresholdWindow:     duration.Duration(0),
+				WriteErrorThresholdCount: 1,
+				ReadErrorThresholdCount:  1,
+			},
+			expectedErr: cfgerror.ValidationErrors{
+				{
+					Key:   []string{"error_threshold_window"},
+					Cause: cfgerror.ErrNotSet,
+				},
+			},
+		},
+		{
+			name: "set invalid but disabled",
+			failover: Failover{
+				Enabled:          false,
+				ElectionStrategy: ElectionStrategy("bad"),
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			errs := tc.failover.Validate()
+			require.Equal(t, tc.expectedErr, errs)
+		})
+	}
+}