move load balancing checks to generic validator

Author: salonichf5

apis/v1alpha1/upstreamsettingspolicy_types.go

@@ -36,7 +36,7 @@ type UpstreamSettingsPolicyList struct {
 }
 
 // UpstreamSettingsPolicySpec defines the desired state of the UpstreamSettingsPolicy.
-// +kubebuilder:validation:XValidation:rule="!(has(self.loadBalancingMethod) && (self.loadBalancingMethod == 'hash' || self.loadBalancingMethod == 'hash consistent')) || has(self.hashKey)",message="hashKey is required when loadBalancingMethod is 'hash' or 'hash consistent'"
+// +kubebuilder:validation:XValidation:rule="!(has(self.loadBalancingMethod) && (self.loadBalancingMethod == 'hash' || self.loadBalancingMethod == 'hash consistent')) || has(self.hashMethodKey)",message="hashMethodKey is required when loadBalancingMethod is 'hash' or 'hash consistent'"
 //
 //nolint:lll
 type UpstreamSettingsPolicySpec struct {
@@ -61,11 +61,11 @@ type UpstreamSettingsPolicySpec struct {
 	// +optional
 	LoadBalancingMethod *LoadBalancingType `json:"loadBalancingMethod,omitempty"`
 
-	// HashKey defines the key used for hash-based load balancing methods.
+	// HashMethodKey defines the key used for hash-based load balancing methods.
 	// This field is required when `LoadBalancingMethod` is set to `hash` or `hash consistent`.
 	//
 	// +optional
-	HashKey *HashMethodKey `json:"hashKey,omitempty"`
+	HashMethodKey *HashMethodKey `json:"hashMethodKey,omitempty"`
 
 	// TargetRefs identifies API object(s) to apply the policy to.
 	// Objects must be in the same namespace as the policy.
@@ -135,7 +135,6 @@ const (
 
 	// LoadBalancingTypeRoundRobin enables round-robin load balancing,
 	// distributing requests evenly across all upstream servers.
-	// NGINX defaults to this method if no load balancing method is specified.
 	LoadBalancingTypeRoundRobin LoadBalancingType = "round_robin"
 
 	// LoadBalancingTypeLeastConn enables least-connections load balancing,
@@ -148,13 +147,13 @@ const (
 
 	// LoadBalancingTypeHash enables generic hash-based load balancing,
 	// routing requests to upstream servers based on a hash of a specified key
-	// HashKey field must be set when this method is selected.
+	// HashMethodKey field must be set when this method is selected.
 	// Example configuration: hash $binary_remote_addr;.
 	LoadBalancingTypeHash LoadBalancingType = "hash"
 
 	// LoadBalancingTypeHashConsistent enables consistent hash-based load balancing,
 	// which minimizes the number of keys remapped when a server is added or removed.
-	// HashKey field must be set when this method is selected.
+	// HashMethodKey field must be set when this method is selected.
 	// Example configuration: hash $binary_remote_addr consistent;.
 	LoadBalancingTypeHashConsistent LoadBalancingType = "hash consistent"
 

apis/v1alpha1/zz_generated.deepcopy.go

@@ -51,9 +51,9 @@ spec:
           spec:
             description: Spec defines the desired state of the UpstreamSettingsPolicy.
             properties:
-              hashKey:
+              hashMethodKey:
                 description: |-
-                  HashKey defines the key used for hash-based load balancing methods.
+                  HashMethodKey defines the key used for hash-based load balancing methods.
                   This field is required when `LoadBalancingMethod` is set to `hash` or `hash consistent`.
                 pattern: ^\$[a-z_]+$
                 type: string
@@ -171,11 +171,11 @@ spec:
             - targetRefs
             type: object
             x-kubernetes-validations:
-            - message: hashKey is required when loadBalancingMethod is 'hash' or 'hash
-                consistent'
+            - message: hashMethodKey is required when loadBalancingMethod is 'hash'
+                or 'hash consistent'
               rule: '!(has(self.loadBalancingMethod) && (self.loadBalancingMethod
                 == ''hash'' || self.loadBalancingMethod == ''hash consistent'')) ||
-                has(self.hashKey)'
+                has(self.hashMethodKey)'
           status:
             description: Status defines the state of the UpstreamSettingsPolicy.
             properties:

config/crd/bases/gateway.nginx.org_upstreamsettingspolicies.yaml

@@ -9578,9 +9578,9 @@ spec:
           spec:
             description: Spec defines the desired state of the UpstreamSettingsPolicy.
             properties:
-              hashKey:
+              hashMethodKey:
                 description: |-
-                  HashKey defines the key used for hash-based load balancing methods.
+                  HashMethodKey defines the key used for hash-based load balancing methods.
                   This field is required when `LoadBalancingMethod` is set to `hash` or `hash consistent`.
                 pattern: ^\$[a-z_]+$
                 type: string
@@ -9698,11 +9698,11 @@ spec:
             - targetRefs
             type: object
             x-kubernetes-validations:
-            - message: hashKey is required when loadBalancingMethod is 'hash' or 'hash
-                consistent'
+            - message: hashMethodKey is required when loadBalancingMethod is 'hash'
+                or 'hash consistent'
               rule: '!(has(self.loadBalancingMethod) && (self.loadBalancingMethod
                 == ''hash'' || self.loadBalancingMethod == ''hash consistent'')) ||
-                has(self.hashKey)'
+                has(self.hashMethodKey)'
           status:
             description: Status defines the state of the UpstreamSettingsPolicy.
             properties:

deploy/crds.yaml

@@ -124,18 +124,13 @@ func StartManager(cfg config.Config) error {
 	mustExtractGVK := kinds.NewMustExtractGKV(scheme)
 
 	genericValidator := ngxvalidation.GenericValidator{}
-	policyManager := createPolicyManager(mustExtractGVK, genericValidator)
+	policyManager := createPolicyManager(mustExtractGVK, cfg.Plus, genericValidator)
 
 	plusSecrets, err := createPlusSecretMetadata(cfg, mgr.GetAPIReader())
 	if err != nil {
 		return err
 	}
 
-	flags := graph.Flags{
-		Plus:         cfg.Plus,
-		Experimental: cfg.ExperimentalFeatures,
-	}
-
 	processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
 		GatewayCtlrName:  cfg.GatewayCtlrName,
 		GatewayClassName: cfg.GatewayClassName,
@@ -145,10 +140,10 @@ func StartManager(cfg config.Config) error {
 			GenericValidator:    genericValidator,
 			PolicyValidator:     policyManager,
 		},
-		EventRecorder:  recorder,
-		MustExtractGVK: mustExtractGVK,
-		PlusSecrets:    plusSecrets,
-		Flags:          flags,
+		EventRecorder:        recorder,
+		MustExtractGVK:       mustExtractGVK,
+		PlusSecrets:          plusSecrets,
+		ExperimentalFeatures: cfg.ExperimentalFeatures,
 	})
 
 	var handlerCollector handlerMetricsCollector = collectors.NewControllerNoopCollector()
@@ -327,6 +322,7 @@ func StartManager(cfg config.Config) error {
 
 func createPolicyManager(
 	mustExtractGVK kinds.MustExtractGVK,
+	plusEnabled bool,
 	validator validation.GenericValidator,
 ) *policies.CompositeValidator {
 	cfgs := []policies.ManagerConfig{
@@ -340,7 +336,7 @@ func createPolicyManager(
 		},
 		{
 			GVK:       mustExtractGVK(&ngfAPIv1alpha1.UpstreamSettingsPolicy{}),
-			Validator: upstreamsettings.NewValidator(validator),
+			Validator: upstreamsettings.NewValidator(validator, plusEnabled),
 		},
 	}
 

internal/controller/manager.go

@@ -1,7 +1,6 @@
 package http
 
 import (
-	ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared"
 )
 
@@ -124,7 +123,7 @@ type Upstream struct {
 	ZoneSize            string // format: 512k, 1m
 	StateFile           string
 	LoadBalancingMethod string
-	HashKey             string
+	HashMethodKey       string
 	KeepAlive           UpstreamKeepAlive
 	Servers             []UpstreamServer
 }
@@ -169,33 +168,3 @@ type ServerConfig struct {
 	Plus                     bool
 	DisableSNIHostValidation bool
 }
-
-var (
-	PlusAllowedLBMethods = map[ngfAPI.LoadBalancingType]struct{}{
-		ngfAPI.LoadBalancingTypeRoundRobin:                 {},
-		ngfAPI.LoadBalancingTypeLeastConnection:            {},
-		ngfAPI.LoadBalancingTypeIPHash:                     {},
-		ngfAPI.LoadBalancingTypeRandom:                     {},
-		ngfAPI.LoadBalancingTypeHash:                       {},
-		ngfAPI.LoadBalancingTypeHashConsistent:             {},
-		ngfAPI.LoadBalancingTypeRandomTwo:                  {},
-		ngfAPI.LoadBalancingTypeRandomTwoLeastConnection:   {},
-		ngfAPI.LoadBalancingTypeLeastTimeHeader:            {},
-		ngfAPI.LoadBalancingTypeLeastTimeLastByte:          {},
-		ngfAPI.LoadBalancingTypeLeastTimeHeaderInflight:    {},
-		ngfAPI.LoadBalancingTypeLeastTimeLastByteInflight:  {},
-		ngfAPI.LoadBalancingTypeRandomTwoLeastTimeHeader:   {},
-		ngfAPI.LoadBalancingTypeRandomTwoLeastTimeLastByte: {},
-	}
-
-	OSSAllowedLBMethods = map[ngfAPI.LoadBalancingType]struct{}{
-		ngfAPI.LoadBalancingTypeRoundRobin:               {},
-		ngfAPI.LoadBalancingTypeLeastConnection:          {},
-		ngfAPI.LoadBalancingTypeIPHash:                   {},
-		ngfAPI.LoadBalancingTypeRandom:                   {},
-		ngfAPI.LoadBalancingTypeHash:                     {},
-		ngfAPI.LoadBalancingTypeHashConsistent:           {},
-		ngfAPI.LoadBalancingTypeRandomTwo:                {},
-		ngfAPI.LoadBalancingTypeRandomTwoLeastConnection: {},
-	}
-)

internal/controller/nginx/config/http/config.go

@@ -50,14 +50,6 @@ func (v *Validator) ValidateGlobalSettings(
 	return nil
 }
 
-// ValidateLoadBalancingMethod validates the load balancing method for upstream servers.
-func (v *Validator) ValidateLoadBalancingMethod(
-	_ policies.Policy,
-	_ bool,
-) []conditions.Condition {
-	return nil
-}
-
 // Conflicts returns true if the two ClientSettingsPolicies conflict.
 func (v *Validator) Conflicts(polA, polB policies.Policy) bool {
 	cspA := helpers.MustCastObject[*ngfAPI.ClientSettingsPolicy](polA)

internal/controller/nginx/config/policies/clientsettings/validator.go

@@ -141,11 +141,3 @@ func (v *Validator) validateSettings(spec ngfAPIv1alpha2.ObservabilityPolicySpec
 
 	return allErrs.ToAggregate()
 }
-
-// ValidateLoadBalancingMethod validates the load balancing method for upstream servers.
-func (v *Validator) ValidateLoadBalancingMethod(
-	_ policies.Policy,
-	_ bool,
-) []conditions.Condition {
-	return nil
-}

internal/controller/nginx/config/policies/observability/validator.go

@@ -15,8 +15,8 @@ type UpstreamSettings struct {
 	ZoneSize string
 	// LoadBalancingMethod is the load balancing method setting.
 	LoadBalancingMethod string
-	// HashKey is the key to be used for hash-based load balancing methods.
-	HashKey string
+	// HashMethodKey is the key to be used for hash-based load balancing methods.
+	HashMethodKey string
 	// KeepAlive contains the keepalive settings.
 	KeepAlive http.UpstreamKeepAlive
 }
@@ -70,8 +70,8 @@ func processPolicies(pols []policies.Policy) UpstreamSettings {
 			upstreamSettings.LoadBalancingMethod = string(*usp.Spec.LoadBalancingMethod)
 		}
 
-		if usp.Spec.HashKey != nil {
-			upstreamSettings.HashKey = string(*usp.Spec.HashKey)
+		if usp.Spec.HashMethodKey != nil {
+			upstreamSettings.HashMethodKey = string(*usp.Spec.HashMethodKey)
 		}
 	}