remove lb check from generic validator

Author: salonichf5

internal/controller/manager.go

@@ -124,7 +124,7 @@ func StartManager(cfg config.Config) error {
 	mustExtractGVK := kinds.NewMustExtractGKV(scheme)
 
 	genericValidator := ngxvalidation.GenericValidator{}
-	policyManager := createPolicyManager(mustExtractGVK, cfg.Plus, genericValidator)
+	policyManager := createPolicyManager(mustExtractGVK, genericValidator, cfg.Plus)
 
 	plusSecrets, err := createPlusSecretMetadata(cfg, mgr.GetAPIReader())
 	if err != nil {
@@ -322,8 +322,8 @@ func StartManager(cfg config.Config) error {
 
 func createPolicyManager(
 	mustExtractGVK kinds.MustExtractGVK,
-	plusEnabled bool,
 	validator validation.GenericValidator,
+	plusEnabled bool,
 ) *policies.CompositeValidator {
 	cfgs := []policies.ManagerConfig{
 		{

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

@@ -1,6 +1,7 @@
 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"
 )
 
@@ -168,3 +169,14 @@ type ServerConfig struct {
 	Plus                     bool
 	DisableSNIHostValidation bool
 }
+
+var 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/policies/upstreamsettings/validator.go

@@ -1,10 +1,14 @@
 package upstreamsettings
 
 import (
+	"fmt"
+	"strings"
+
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 
 	ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1"
+	httpConfig "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/http"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/validation"
@@ -123,7 +127,7 @@ func (v Validator) validateSettings(spec ngfAPI.UpstreamSettingsPolicySpec) erro
 		allErrs = append(allErrs, v.validateUpstreamKeepAlive(*spec.KeepAlive, fieldPath.Child("keepAlive"))...)
 	}
 
-	allErrs = append(allErrs, v.validateLoadBalancingMethod(spec, v.plusEnabled)...)
+	allErrs = append(allErrs, v.validateLoadBalancingMethod(spec)...)
 
 	return allErrs.ToAggregate()
 }
@@ -154,21 +158,26 @@ func (v Validator) validateUpstreamKeepAlive(
 }
 
 // ValidateLoadBalancingMethod validates the load balancing method for upstream servers.
-func (v Validator) validateLoadBalancingMethod(
-	spec ngfAPI.UpstreamSettingsPolicySpec,
-	plusEnabled bool,
-) field.ErrorList {
+func (v Validator) validateLoadBalancingMethod(spec ngfAPI.UpstreamSettingsPolicySpec) field.ErrorList {
 	var allErrs field.ErrorList
 	fieldPath := field.NewPath("spec")
 
 	if spec.LoadBalancingMethod == nil {
 		return nil
 	}
 
-	lbMethod := *spec.LoadBalancingMethod
-	if err := v.genericValidator.ValidateLoadBalancingMethod(string(lbMethod), plusEnabled); err != nil {
-		path := fieldPath.Child("loadBalancingMethod")
-		allErrs = append(allErrs, field.Invalid(path, lbMethod, err.Error()))
+	if !v.plusEnabled {
+		if _, ok := httpConfig.OSSAllowedLBMethods[*spec.LoadBalancingMethod]; !ok {
+			path := fieldPath.Child("loadBalancingMethod")
+			allErrs = append(allErrs, field.Invalid(
+				path,
+				*spec.LoadBalancingMethod,
+				fmt.Sprintf(
+					"NGINX OSS only supports the following load balancing methods: %s",
+					getLoadBalancingMethodList(httpConfig.OSSAllowedLBMethods),
+				),
+			))
+		}
 	}
 
 	if spec.HashMethodKey != nil {
@@ -181,3 +190,11 @@ func (v Validator) validateLoadBalancingMethod(
 
 	return allErrs
 }
+
+func getLoadBalancingMethodList(lbMethods map[ngfAPI.LoadBalancingType]struct{}) string {
+	methods := make([]string, 0, len(lbMethods))
+	for method := range lbMethods {
+		methods = append(methods, string(method))
+	}
+	return strings.Join(methods, ", ")
+}

internal/controller/nginx/config/validation/generic.go

@@ -2,13 +2,9 @@ package validation
 
 import (
 	"errors"
-	"fmt"
 	"regexp"
-	"strings"
 
 	k8svalidation "k8s.io/apimachinery/pkg/util/validation"
-
-	ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1"
 )
 
 // GenericValidator validates values for generic cases in the nginx conf.
@@ -131,54 +127,3 @@ func (GenericValidator) ValidateNginxVariableName(name string) error {
 
 	return nil
 }
-
-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: {},
-	}
-)
-
-func (GenericValidator) ValidateLoadBalancingMethod(method string, plusEnabled bool) error {
-	lbMethod := ngfAPI.LoadBalancingType(method)
-
-	if !plusEnabled {
-		if _, ok := OSSAllowedLBMethods[lbMethod]; ok {
-			return nil
-		}
-	}
-
-	return fmt.Errorf("NGINX OSS only supports the following load balancing methods: %s",
-		getLoadBalancingMethodList(OSSAllowedLBMethods))
-}
-
-func getLoadBalancingMethodList(lbMethods map[ngfAPI.LoadBalancingType]struct{}) string {
-	var methods []string
-	for method := range lbMethods {
-		methods = append(methods, string(method))
-	}
-	return strings.Join(methods, ", ")
-}

internal/controller/nginx/config/validation/generic_test.go

@@ -134,34 +134,3 @@ func TestValidateNginxVariableName(t *testing.T) {
 		`var$name`,
 	)
 }
-
-func makeValidator(plusEnabled bool) simpleValidatorFunc[string] {
-	return func(v string) error {
-		return (GenericValidator{}).ValidateLoadBalancingMethod(v, plusEnabled)
-	}
-}
-
-func TestValidateLoadBalancingMethod_OSS(t *testing.T) {
-	t.Helper()
-
-	ossValidator := makeValidator(false)
-	testValidValuesForSimpleValidator(t, ossValidator,
-		"round_robin",
-		"least_conn",
-		"ip_hash",
-		"hash",
-		"hash consistent",
-		"random",
-		"random two",
-		"random two least_conn",
-	)
-
-	testInvalidValuesForSimpleValidator(t, ossValidator,
-		"random two least_time=header",
-		"random two least_time=last_byte",
-		"least_time header",
-		"least_time last_byte",
-		"least_time header inflight",
-		"least_time last_byte inflight",
-	)
-}

internal/controller/state/validation/validationfakes/fake_generic_validator.go

@@ -50,7 +50,6 @@ type GenericValidator interface {
 	ValidateNginxSize(size string) error
 	ValidateEndpoint(endpoint string) error
 	ValidateNginxVariableName(name string) error
-	ValidateLoadBalancingMethod(method string, plusEnabled bool) error
 }
 
 // PolicyValidator validates an NGF Policy.