improve validation of leader election values

- take into account partial setting of params and default values
- fix md file format
- fix linter issues

Signed-off-by: Nader Ziada <[email protected]>

Author: nader-ziada

docs/operate.md

@@ -72,7 +72,7 @@ Optional variables
 
 ### Configuring Service Failover
 
-# Configuring the KEDA HTTP Add-on Operator
+## Configuring the KEDA HTTP Add-on Operator
 
 ## Leader Election Timing
 
@@ -92,3 +92,23 @@ env:
 - name: KEDA_HTTP_OPERATOR_LEADER_ELECTION_RETRY_PERIOD
   value: "5s"
 ```
+
+### Timing Parameter Constraints
+
+**Important:** These parameters have strict ordering requirements to prevent leadership conflicts and unnecessary failover:
+
+```
+LeaseDuration > RenewDeadline > RetryPeriod
+```
+
+**Why this matters:**
+- **LeaseDuration > RenewDeadline**: Ensures the leader finishes renewal attempts before the lease expires, preventing multiple active leaders (split-brain scenarios)
+- **RenewDeadline > RetryPeriod**: Allows multiple retry attempts during the renewal window, preventing unnecessary leadership changes due to transient failures
+
+**Configuration Guidelines:**
+
+1. **Configure all three together**: When overriding any parameter, it's recommended to set all three values to avoid invalid combinations with defaults. Setting only one or two parameters can result in invalid configurations when mixed with default values.
+
+2. **All values must be positive**: Each duration must be greater than 0.
+
+3. **Validation failure**: If the operator detects an invalid configuration at startup, it will log an error and exit immediately before attempting leader election.

operator/main_test.go

@@ -20,8 +20,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/kedacore/http-add-on/pkg/util"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/kedacore/http-add-on/pkg/util"
 )
 
 func TestLeaderElectionEnvVarsIntegration(t *testing.T) {
@@ -78,7 +79,6 @@ func TestLeaderElectionEnvVarsIntegration(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-
 			for key, value := range tt.envVars {
 				t.Setenv(key, value)
 			}

pkg/util/env_resolver.go

@@ -54,32 +54,55 @@ func ResolveOsEnvDuration(envName string) (*time.Duration, error) {
 	return nil, nil
 }
 
-// ValidateLeaderElectionDurations ensures LeaseDuration > RenewDeadline > RetryPeriod
+// Controller-runtime default values for leader election
+// Reference: https://github.com/kubernetes-sigs/controller-runtime/blob/main/pkg/manager/manager.go
+const (
+	defaultLeaseDuration = 15 * time.Second
+	defaultRenewDeadline = 10 * time.Second
+	defaultRetryPeriod   = 2 * time.Second
+)
+
+// ValidateLeaderElectionConfig ensures LeaseDuration > RenewDeadline > RetryPeriod
 // to prevent multiple active leaders and unnecessary leadership churn.
+// This validation checks against the actual runtime values (user-provided or defaults)
+// to catch invalid partial configurations.
 func ValidateLeaderElectionConfig(leaseDuration, renewDeadline, retryPeriod *time.Duration) error {
-	if leaseDuration == nil && renewDeadline == nil && retryPeriod == nil {
-		return nil
+	// Resolve actual values that will be used at runtime (user-provided or defaults)
+	lease := defaultLeaseDuration
+	if leaseDuration != nil {
+		lease = *leaseDuration
 	}
 
-	// If any are set, validate relationships
-	if leaseDuration != nil && *leaseDuration <= 0 {
-		return fmt.Errorf("lease duration must be greater than 0, got %v", *leaseDuration)
+	renew := defaultRenewDeadline
+	if renewDeadline != nil {
+		renew = *renewDeadline
 	}
-	if renewDeadline != nil && *renewDeadline <= 0 {
-		return fmt.Errorf("renew deadline must be greater than 0, got %v", *renewDeadline)
+
+	retry := defaultRetryPeriod
+	if retryPeriod != nil {
+		retry = *retryPeriod
 	}
-	if retryPeriod != nil && *retryPeriod <= 0 {
-		return fmt.Errorf("retry period must be greater than 0, got %v", *retryPeriod)
+
+	// Validate all values are positive
+	if lease <= 0 {
+		return fmt.Errorf("lease duration must be greater than 0, got %v", lease)
+	}
+	if renew <= 0 {
+		return fmt.Errorf("renew deadline must be greater than 0, got %v", renew)
+	}
+	if retry <= 0 {
+		return fmt.Errorf("retry period must be greater than 0, got %v", retry)
 	}
 
-	// Validate relationships when multiple values are set
-	if leaseDuration != nil && renewDeadline != nil && *leaseDuration <= *renewDeadline {
-		return fmt.Errorf("lease duration (%v) must be greater than renew deadline (%v)",
-			*leaseDuration, *renewDeadline)
+	// Validate relationships: LeaseDuration > RenewDeadline > RetryPeriod
+	if lease <= renew {
+		return fmt.Errorf("lease duration (%v) must be greater than renew deadline (%v)", lease, renew)
+	}
+	if renew <= retry {
+		return fmt.Errorf("renew deadline (%v) must be greater than retry period (%v)", renew, retry)
 	}
-	if renewDeadline != nil && retryPeriod != nil && *renewDeadline <= *retryPeriod {
-		return fmt.Errorf("renew deadline (%v) must be greater than retry period (%v)",
-			*renewDeadline, *retryPeriod)
+	if lease <= retry {
+		return fmt.Errorf("lease duration (%v) must be greater than retry period (%v)", lease, retry)
 	}
 
 	return nil

pkg/util/env_resolver_test.go

@@ -170,6 +170,60 @@ func TestValidateLeaderElectionDurations(t *testing.T) {
 			wantErr:       true,
 			errContains:   "renew deadline",
 		},
+		{
+			name:          "lease duration <= retry period",
+			leaseDuration: ptr(5 * time.Second),
+			renewDeadline: ptr(3 * time.Second),
+			retryPeriod:   ptr(5 * time.Second),
+			wantErr:       true,
+			errContains:   "retry period",
+		},
+		// Partial configuration tests - validating against defaults
+		{
+			name:          "partial config: only lease duration set (valid)",
+			leaseDuration: ptr(20 * time.Second),
+			renewDeadline: nil, // defaults to 10s
+			retryPeriod:   nil, // defaults to 2s
+			wantErr:       false,
+		},
+		{
+			name:          "partial config: only lease duration set (invalid - too low)",
+			leaseDuration: ptr(5 * time.Second),
+			renewDeadline: nil, // defaults to 10s
+			retryPeriod:   nil, // defaults to 2s
+			wantErr:       true,
+			errContains:   "lease duration (5s) must be greater than renew deadline (10s)",
+		},
+		{
+			name:          "partial config: only retry period set (invalid - too high)",
+			leaseDuration: nil, // defaults to 15s
+			renewDeadline: nil, // defaults to 10s
+			retryPeriod:   ptr(12 * time.Second),
+			wantErr:       true,
+			errContains:   "renew deadline (10s) must be greater than retry period (12s)",
+		},
+		{
+			name:          "partial config: only retry period set (valid)",
+			leaseDuration: nil, // defaults to 15s
+			renewDeadline: nil, // defaults to 10s
+			retryPeriod:   ptr(1 * time.Second),
+			wantErr:       false,
+		},
+		{
+			name:          "partial config: lease and renew set (valid)",
+			leaseDuration: ptr(30 * time.Second),
+			renewDeadline: ptr(20 * time.Second),
+			retryPeriod:   nil, // defaults to 2s
+			wantErr:       false,
+		},
+		{
+			name:          "partial config: lease and renew set (invalid - renew conflicts with default retry)",
+			leaseDuration: ptr(30 * time.Second),
+			renewDeadline: ptr(1 * time.Second), // less than default retry (2s)
+			retryPeriod:   nil,                  // defaults to 2s
+			wantErr:       true,
+			errContains:   "renew deadline (1s) must be greater than retry period (2s)",
+		},
 	}
 
 	for _, tt := range tests {