Address recent comments

internal/mode/static/state/dataplane/configuration.go

@@ -607,7 +607,7 @@ func generateCertBundleID(configMap types.NamespacedName) CertBundleID {
 
 // buildTelemetry generates the Otel configuration.
 func buildTelemetry(g *graph.Graph) Telemetry {
-	if g.NginxProxy == nil ||
+	if g.NginxProxy == nil || !g.NginxProxy.Valid ||
 		g.NginxProxy.Source.Spec.Telemetry == nil ||
 		g.NginxProxy.Source.Spec.Telemetry.Exporter == nil {
 		return Telemetry{}
@@ -647,11 +647,9 @@ func buildTelemetry(g *graph.Graph) Telemetry {
 		}
 	}
 
-	if len(ratioMap) > 0 {
-		tel.Ratios = make([]Ratio, 0, len(ratioMap))
-		for name, ratio := range ratioMap {
-			tel.Ratios = append(tel.Ratios, Ratio{Name: name, Value: ratio})
-		}
+	tel.Ratios = make([]Ratio, 0, len(ratioMap))
+	for name, ratio := range ratioMap {
+		tel.Ratios = append(tel.Ratios, Ratio{Name: name, Value: ratio})
 	}
 
 	return tel

internal/mode/static/state/dataplane/configuration_test.go

@@ -634,6 +634,7 @@ func TestBuildConfiguration(t *testing.T) {
 				DisableHTTP2: true,
 			},
 		},
+		Valid: true,
 	}
 
 	tests := []struct {
@@ -2068,6 +2069,7 @@ func TestBuildConfiguration(t *testing.T) {
 					BatchSize:   512,
 					BatchCount:  4,
 					ServiceName: "ngf:ns:gw:my-svc",
+					Ratios:      []Ratio{},
 				},
 			},
 			msg: "NginxProxy with tracing config and http2 disabled",
@@ -2985,6 +2987,7 @@ func TestBuildTelemetry(t *testing.T) {
 				},
 			},
 		},
+		Valid: true,
 	}
 
 	createTelemetry := func() Telemetry {
@@ -2994,6 +2997,7 @@ func TestBuildTelemetry(t *testing.T) {
 			Interval:    "5s",
 			BatchSize:   512,
 			BatchCount:  4,
+			Ratios:      []Ratio{},
 		}
 	}
 
@@ -3015,6 +3019,22 @@ func TestBuildTelemetry(t *testing.T) {
 			expTelemetry: Telemetry{},
 			msg:          "No telemetry configured",
 		},
+		{
+			g: &graph.Graph{
+				NginxProxy: &graph.NginxProxy{
+					Source: &ngfAPI.NginxProxy{
+						Spec: ngfAPI.NginxProxySpec{
+							Telemetry: &ngfAPI.Telemetry{
+								Exporter: &ngfAPI.TelemetryExporter{},
+							},
+						},
+					},
+					Valid: false,
+				},
+			},
+			expTelemetry: Telemetry{},
+			msg:          "Invalid NginxProxy configured",
+		},
 		{
 			g: &graph.Graph{
 				Gateway: &graph.Gateway{

internal/mode/static/state/graph/gatewayclass.go

@@ -13,7 +13,6 @@ import (
 	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
 	staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation"
 )
 
 // GatewayClass represents the GatewayClass resource.
@@ -66,13 +65,12 @@ func buildGatewayClass(
 	gc *v1.GatewayClass,
 	npCfg *NginxProxy,
 	crdVersions map[types.NamespacedName]*metav1.PartialObjectMetadata,
-	validator validation.GenericValidator,
 ) *GatewayClass {
 	if gc == nil {
 		return nil
 	}
 
-	conds, valid := validateGatewayClass(gc, npCfg, crdVersions, validator)
+	conds, valid := validateGatewayClass(gc, npCfg, crdVersions)
 
 	return &GatewayClass{
 		Source:     gc,
@@ -85,7 +83,6 @@ func validateGatewayClass(
 	gc *v1.GatewayClass,
 	npCfg *NginxProxy,
 	crdVersions map[types.NamespacedName]*metav1.PartialObjectMetadata,
-	validator validation.GenericValidator,
 ) ([]conditions.Condition, bool) {
 	var conds []conditions.Condition
 
@@ -97,11 +94,8 @@ func validateGatewayClass(
 		} else if npCfg == nil {
 			err = field.NotFound(path.Child("name"), gc.Spec.ParametersRef.Name)
 			conds = append(conds, staticConds.NewGatewayClassRefNotFound())
-		} else {
-			nginxProxyErrs := validateNginxProxy(validator, npCfg)
-			if len(nginxProxyErrs) > 0 {
-				err = errors.New(nginxProxyErrs.ToAggregate().Error())
-			}
+		} else if !npCfg.Valid {
+			err = errors.New(npCfg.ErrMsgs.ToAggregate().Error())
 		}
 
 		if err != nil {

internal/mode/static/state/graph/gatewayclass_test.go

@@ -1,12 +1,12 @@
 package graph
 
 import (
-	"errors"
 	"testing"
 
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	v1 "sigs.k8s.io/gateway-api/apis/v1"
 
@@ -16,8 +16,6 @@ import (
 	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
 	staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation/validationfakes"
 )
 
 func TestProcessGatewayClasses(t *testing.T) {
@@ -166,24 +164,7 @@ func TestBuildGatewayClass(t *testing.T) {
 		},
 	}
 
-	createValidNPValidator := func() *validationfakes.FakeGenericValidator {
-		v := &validationfakes.FakeGenericValidator{}
-		v.ValidateServiceNameReturns(nil)
-		v.ValidateEndpointReturns(nil)
-
-		return v
-	}
-
-	createInvalidNPValidator := func() *validationfakes.FakeGenericValidator {
-		v := &validationfakes.FakeGenericValidator{}
-		v.ValidateServiceNameReturns(errors.New("error"))
-		v.ValidateEndpointReturns(errors.New("error"))
-
-		return v
-	}
-
 	tests := []struct {
-		validator    validation.GenericValidator
 		gc           *v1.GatewayClass
 		np           *NginxProxy
 		crdMetadata  map[types.NamespacedName]*metav1.PartialObjectMetadata
@@ -220,7 +201,6 @@ func TestBuildGatewayClass(t *testing.T) {
 				},
 				Valid: true,
 			},
-			validator: createValidNPValidator(),
 			expected: &GatewayClass{
 				Source:     gcWithParams,
 				Valid:      true,
@@ -267,22 +247,20 @@ func TestBuildGatewayClass(t *testing.T) {
 		{
 			gc: gcWithParams,
 			np: &NginxProxy{
-				Source: &ngfAPI.NginxProxy{
-					TypeMeta: metav1.TypeMeta{
-						Kind: kinds.NginxProxy,
-					},
-					Spec: ngfAPI.NginxProxySpec{
-						Telemetry: &ngfAPI.Telemetry{
-							ServiceName: helpers.GetPointer("my-svc"),
-							Exporter: &ngfAPI.TelemetryExporter{
-								Endpoint: "my-endpoint",
-							},
-						},
-					},
+				Valid: false,
+				ErrMsgs: field.ErrorList{
+					field.Invalid(
+						field.NewPath("spec", "telemetry", "serviceName"),
+						"my-svc",
+						"error",
+					),
+					field.Invalid(
+						field.NewPath("spec", "telemetry", "exporter", "endpoint"),
+						"my-endpoint",
+						"error",
+					),
 				},
-				Valid: true,
 			},
-			validator: createInvalidNPValidator(),
 			expected: &GatewayClass{
 				Source: gcWithParams,
 				Valid:  true,
@@ -312,7 +290,7 @@ func TestBuildGatewayClass(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			result := buildGatewayClass(test.gc, test.np, test.crdMetadata, test.validator)
+			result := buildGatewayClass(test.gc, test.np, test.crdMetadata)
 			g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty())
 			if test.np != nil {
 				g.Expect(test.np.Valid).ToNot(Equal(test.expNPInvalid))

internal/mode/static/state/graph/graph.go

@@ -185,8 +185,8 @@ func BuildGraph(
 		return &Graph{}
 	}
 
-	npCfg := getNginxProxy(state.NginxProxies, processedGwClasses.Winner)
-	gc := buildGatewayClass(processedGwClasses.Winner, npCfg, state.CRDMetadata, validators.GenericValidator)
+	npCfg := buildNginxProxy(state.NginxProxies, processedGwClasses.Winner, validators.GenericValidator)
+	gc := buildGatewayClass(processedGwClasses.Winner, npCfg, state.CRDMetadata)
 	if gc != nil && npCfg != nil && npCfg.Source != nil {
 		spec := npCfg.Source.Spec
 		globalSettings = &policies.GlobalPolicySettings{

internal/mode/static/state/graph/nginxproxy.go

@@ -14,21 +14,27 @@ import (
 type NginxProxy struct {
 	// Source is the source resource.
 	Source *ngfAPI.NginxProxy
+	// ErrMsgs contains the validation errors if they exist, to be included in the GatewayClass condition.
+	ErrMsgs field.ErrorList
 	// Valid shows whether the NginxProxy is valid.
 	Valid bool
 }
 
-// getNginxProxy returns the NginxProxy associated with the GatewayClass (if it exists).
-func getNginxProxy(
+// buildNginxProxy validates and returns the NginxProxy associated with the GatewayClass (if it exists).
+func buildNginxProxy(
 	nps map[types.NamespacedName]*ngfAPI.NginxProxy,
 	gc *v1.GatewayClass,
+	validator validation.GenericValidator,
 ) *NginxProxy {
 	if gcReferencesAnyNginxProxy(gc) {
 		npCfg := nps[types.NamespacedName{Name: gc.Spec.ParametersRef.Name}]
 		if npCfg != nil {
+			errs := validateNginxProxy(validator, npCfg)
+
 			return &NginxProxy{
-				Source: npCfg,
-				Valid:  true,
+				Source:  npCfg,
+				Valid:   len(errs) == 0,
+				ErrMsgs: errs,
 			}
 		}
 	}
@@ -54,12 +60,12 @@ func gcReferencesAnyNginxProxy(gc *v1.GatewayClass) bool {
 // validateNginxProxy performs re-validation on string values in the case of CRD validation failure.
 func validateNginxProxy(
 	validator validation.GenericValidator,
-	npCfg *NginxProxy,
+	npCfg *ngfAPI.NginxProxy,
 ) field.ErrorList {
 	var allErrs field.ErrorList
 	spec := field.NewPath("spec")
 
-	telemetry := npCfg.Source.Spec.Telemetry
+	telemetry := npCfg.Spec.Telemetry
 	if telemetry != nil {
 		telPath := spec.Child("telemetry")
 		if telemetry.ServiceName != nil {
@@ -102,9 +108,5 @@ func validateNginxProxy(
 		}
 	}
 
-	if len(allErrs) > 0 {
-		npCfg.Valid = false
-	}
-
 	return allErrs
 }