From 127180bce89b74ef8b8c706f6d33e8294b9ed40f Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Wed, 10 Sep 2025 16:56:41 -0600 Subject: [PATCH 1/5] add tls.backend field to specify gateway cert --- internal/controller/manager.go | 7 +- .../nginx/config/base_http_config.go | 2 + .../nginx/config/base_http_config_template.go | 6 + .../nginx/config/base_http_config_test.go | 46 + internal/controller/state/change_processor.go | 3 + .../controller/state/conditions/conditions.go | 29 + .../state/dataplane/configuration.go | 43 +- .../state/dataplane/configuration_test.go | 1762 +++++++++-------- internal/controller/state/dataplane/types.go | 20 +- internal/controller/state/graph/gateway.go | 63 +- .../controller/state/graph/gateway_test.go | 285 ++- internal/controller/state/graph/graph.go | 2 + internal/controller/state/graph/graph_test.go | 22 +- .../state/graph/multiple_gateways_test.go | 2 + 14 files changed, 1440 insertions(+), 852 deletions(-) diff --git a/internal/controller/manager.go b/internal/controller/manager.go index a4e9fd9cf0..6a55088e55 100644 --- a/internal/controller/manager.go +++ b/internal/controller/manager.go @@ -139,9 +139,10 @@ func StartManager(cfg config.Config) error { GenericValidator: genericValidator, PolicyValidator: policyManager, }, - EventRecorder: recorder, - MustExtractGVK: mustExtractGVK, - PlusSecrets: plusSecrets, + EventRecorder: recorder, + MustExtractGVK: mustExtractGVK, + PlusSecrets: plusSecrets, + ExperimentalFeatures: cfg.ExperimentalFeatures, }) var handlerCollector handlerMetricsCollector = collectors.NewControllerNoopCollector() diff --git a/internal/controller/nginx/config/base_http_config.go b/internal/controller/nginx/config/base_http_config.go index 4bce95a0bc..d01536dd8d 100644 --- a/internal/controller/nginx/config/base_http_config.go +++ b/internal/controller/nginx/config/base_http_config.go @@ -12,6 +12,7 @@ var baseHTTPTemplate = gotemplate.Must(gotemplate.New("baseHttp").Parse(baseHTTP type httpConfig struct { DNSResolver *dataplane.DNSResolverConfig + GatewaySecretID dataplane.SSLKeyPairID Includes []shared.Include NginxReadinessProbePort int32 IPFamily shared.IPFamily @@ -27,6 +28,7 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult { NginxReadinessProbePort: conf.BaseHTTPConfig.NginxReadinessProbePort, IPFamily: getIPFamily(conf.BaseHTTPConfig), DNSResolver: conf.BaseHTTPConfig.DNSResolver, + GatewaySecretID: conf.BaseHTTPConfig.GatewaySecretID, } results := make([]executeResult, 0, len(includes)+1) diff --git a/internal/controller/nginx/config/base_http_config_template.go b/internal/controller/nginx/config/base_http_config_template.go index e50a83a65f..b6e9b6f6ee 100644 --- a/internal/controller/nginx/config/base_http_config_template.go +++ b/internal/controller/nginx/config/base_http_config_template.go @@ -48,6 +48,12 @@ server { } } +{{- if $.GatewaySecretID }} +# Gateway Certificate +proxy_ssl_certificate /etc/nginx/secrets/{{ $.GatewaySecretID }}.pem; +proxy_ssl_certificate_key /etc/nginx/secrets/{{ $.GatewaySecretID }}.pem; +{{- end }} + {{ range $i := .Includes -}} include {{ $i.Name }}; {{ end -}} diff --git a/internal/controller/nginx/config/base_http_config_test.go b/internal/controller/nginx/config/base_http_config_test.go index fe7ad3a58d..90238f1f16 100644 --- a/internal/controller/nginx/config/base_http_config_test.go +++ b/internal/controller/nginx/config/base_http_config_test.go @@ -284,3 +284,49 @@ func TestExecuteBaseHttp_DNSResolver(t *testing.T) { }) } } + +func TestExecuteBaseHttp_GatewaySecretID(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + expectedConfig string + conf dataplane.Configuration + }{ + { + name: "with GatewaySecretID", + conf: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + GatewaySecretID: "client-secret", + }, + }, + expectedConfig: "proxy_ssl_certificate /etc/nginx/secrets/client-secret.pem;" + + "\nproxy_ssl_certificate_key /etc/nginx/secrets/client-secret.pem;", + }, + { + name: "without GatewaySecretID", + conf: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + GatewaySecretID: "", + }, + }, + expectedConfig: "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + res := executeBaseHTTPConfig(test.conf) + g.Expect(res).To(HaveLen(1)) + + httpConfig := string(res[0].data) + + if test.expectedConfig != "" { + g.Expect(httpConfig).To(ContainSubstring(test.expectedConfig)) + } + }) + } +} diff --git a/internal/controller/state/change_processor.go b/internal/controller/state/change_processor.go index f3184adde8..27ba386192 100644 --- a/internal/controller/state/change_processor.go +++ b/internal/controller/state/change_processor.go @@ -63,6 +63,8 @@ type ChangeProcessorConfig struct { GatewayCtlrName string // GatewayClassName is the name of the GatewayClass resource. GatewayClassName string + // ExperimentalFeatures indicates whether experimental features are enabled. + ExperimentalFeatures bool } // ChangeProcessorImpl is an implementation of ChangeProcessor. @@ -269,6 +271,7 @@ func (c *ChangeProcessorImpl) Process() *graph.Graph { c.cfg.PlusSecrets, c.cfg.Validators, c.cfg.Logger, + c.cfg.ExperimentalFeatures, ) return c.latestGraph diff --git a/internal/controller/state/conditions/conditions.go b/internal/controller/state/conditions/conditions.go index ad5d00a0dc..44f4232a2d 100644 --- a/internal/controller/state/conditions/conditions.go +++ b/internal/controller/state/conditions/conditions.go @@ -135,6 +135,14 @@ const ( // parametersRef resource is invalid. GatewayReasonParamsRefInvalid v1.GatewayConditionReason = "ParametersRefInvalid" + // GatewayReasonSecretRefInvalid is used with the "GatewayResolvedRefs" condition when the + // secretRef resource is invalid. + GatewayReasonSecretRefInvalid v1.GatewayConditionReason = "SecretRefInvalid" + + // GatewayReasonSecretRefNotPermitted is used with the "GatewayResolvedRefs" condition when the + // secretRef resource is not permitted by any ReferenceGrant. + GatewayReasonSecretRefNotPermitted v1.GatewayConditionReason = "SecretRefNotPermitted" + // PolicyReasonAncestorLimitReached is used with the "PolicyAccepted" condition when a policy // cannot be applied because the ancestor status list has reached the maximum size of 16. PolicyReasonAncestorLimitReached v1alpha2.PolicyConditionReason = "AncestorLimitReached" @@ -293,6 +301,27 @@ func NewGatewayClassUnsupportedVersion(recommendedVersion string) []Condition { } } +// NewGatewaySecretRefNotPermitted returns Condition that indicates that the Gateway references a TLS secret that is not +// permitted by any ReferenceGrant. +func NewGatewaySecretRefNotPermitted(msg string) Condition { + return Condition{ + Type: string(GatewayReasonResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(GatewayReasonSecretRefNotPermitted), + Message: msg, + } +} + +// NewGatewaySecretRefInvalid returns Condition that indicates that the Gateway references a TLS secret that is invalid. +func NewGatewaySecretRefInvalid(msg string) Condition { + return Condition{ + Type: string(GatewayReasonResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(GatewayReasonSecretRefInvalid), + Message: msg, + } +} + // NewGatewayClassConflict returns a Condition that indicates that the GatewayClass is not accepted // due to a conflict with another GatewayClass. func NewGatewayClassConflict() Condition { diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 52306f4e0b..daedc0ec77 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -81,9 +81,10 @@ func BuildConfiguration( gateway, serviceResolver, g.ReferencedServices, - baseHTTPConfig.IPFamily), + baseHTTPConfig.IPFamily, + ), BackendGroups: backendGroups, - SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, gateway.Listeners), + SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, gateway), CertBundles: buildCertBundles( buildRefCertificateBundles(g.ReferencedSecrets, g.ReferencedCaCertConfigMaps), backendGroups, @@ -248,14 +249,14 @@ func buildStreamUpstreams( } // buildSSLKeyPairs builds the SSLKeyPairs from the Secrets. It will only include Secrets that are referenced by -// valid listeners, so that we don't include unused Secrets in the configuration of the data plane. +// valid gateway and its listeners, so that we don't include unused Secrets in the configuration of the data plane. func buildSSLKeyPairs( secrets map[types.NamespacedName]*graph.Secret, - listeners []*graph.Listener, + gateway *graph.Gateway, ) map[SSLKeyPairID]SSLKeyPair { keyPairs := make(map[SSLKeyPairID]SSLKeyPair) - for _, l := range listeners { + for _, l := range gateway.Listeners { if l.Valid && l.ResolvedSecret != nil { id := generateSSLKeyPairID(*l.ResolvedSecret) secret := secrets[*l.ResolvedSecret] @@ -268,6 +269,15 @@ func buildSSLKeyPairs( } } + if gateway.Valid && gateway.SecretRef != nil { + id := generateSSLKeyPairID(*gateway.SecretRef) + secret := secrets[*gateway.SecretRef] + keyPairs[id] = SSLKeyPair{ + Cert: secret.CertBundle.Cert.TLSCert, + Key: secret.CertBundle.Cert.TLSPrivateKey, + } + } + return keyPairs } @@ -1019,6 +1029,10 @@ func buildBaseHTTPConfig( NginxReadinessProbePort: DefaultNginxReadinessProbePort, } + if gateway.Valid && gateway.SecretRef != nil { + baseConfig.GatewaySecretID = generateSSLKeyPairID(*gateway.SecretRef) + } + // safe to access EffectiveNginxProxy since we only call this function when the Gateway is not nil. np := gateway.EffectiveNginxProxy if np == nil { @@ -1042,8 +1056,20 @@ func buildBaseHTTPConfig( } } + if port := getNginxReadinessProbePort(np); port != 0 { + baseConfig.NginxReadinessProbePort = port + } + baseConfig.RewriteClientIPSettings = buildRewriteClientIPConfig(np.RewriteClientIP) + baseConfig.DNSResolver = buildDNSResolverConfig(np.DNSResolver) + + return baseConfig +} + +func getNginxReadinessProbePort(np *graph.EffectiveNginxProxy) int32 { + var port int32 + if np.Kubernetes != nil { var containerSpec *ngfAPIv1alpha2.ContainerSpec if np.Kubernetes.Deployment != nil { @@ -1052,13 +1078,10 @@ func buildBaseHTTPConfig( containerSpec = &np.Kubernetes.DaemonSet.Container } if containerSpec != nil && containerSpec.ReadinessProbe != nil && containerSpec.ReadinessProbe.Port != nil { - baseConfig.NginxReadinessProbePort = *containerSpec.ReadinessProbe.Port + port = *containerSpec.ReadinessProbe.Port } } - - baseConfig.DNSResolver = buildDNSResolverConfig(np.DNSResolver) - - return baseConfig + return port } // buildBaseStreamConfig generates the base stream context config that should be applied to all stream servers. diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index b329b9d46a..edbd2997a0 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -32,6 +32,150 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" ) +const ( + invalidMatchesPath = "/not-valid-matches" + invalidFiltersPath = "/not-valid-filters" + prefix = v1.PathMatchPathPrefix +) + +var ( + + // backends. + validBackendRef = getNormalBackendRef() + + fooUpstreamName = "test_foo_80" + expValidBackend = Backend{ + UpstreamName: fooUpstreamName, + Weight: 1, + Valid: true, + } + fooEndpoints = []resolver.Endpoint{ + { + Address: "10.0.0.0", + Port: 8080, + }, + } + + fooUpstream = Upstream{ + Name: fooUpstreamName, + Endpoints: fooEndpoints, + } + + // routes. + + httpsHR1, expHTTPSHR1Groups, httpsRouteHR1 = createTestResources( + "https-hr-1", + "foo.example.com", + "listener-443-1", + pathAndType{path: "/", pathType: prefix}, + ) + + httpsHR2, expHTTPSHR2Groups, httpsRouteHR2 = createTestResources( + "https-hr-2", + "bar.example.com", + "listener-443-1", + pathAndType{path: "/", pathType: prefix}, + ) + + // secrets. + secret2NsName = types.NamespacedName{Namespace: "test", Name: "secret-2"} + secret2 = &graph.Secret{ + Source: &apiv1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secret2NsName.Name, + Namespace: secret2NsName.Namespace, + }, + Data: map[string][]byte{ + apiv1.TLSCertKey: []byte("cert-2"), + apiv1.TLSPrivateKeyKey: []byte("privateKey-2"), + }, + }, + CertBundle: graph.NewCertificateBundle( + secret2NsName, + "Secret", + &graph.Certificate{ + TLSCert: []byte("cert-2"), + TLSPrivateKey: []byte("privateKey-2"), + }, + ), + } + secret1NsName = types.NamespacedName{Namespace: "test", Name: "secret-1"} + secret1 = &graph.Secret{ + Source: &apiv1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secret1NsName.Name, + Namespace: secret1NsName.Namespace, + }, + Data: map[string][]byte{ + apiv1.TLSCertKey: []byte("cert-1"), + apiv1.TLSPrivateKeyKey: []byte("privateKey-1"), + }, + }, + CertBundle: graph.NewCertificateBundle( + secret1NsName, + "Secret", + &graph.Certificate{ + TLSCert: []byte("cert-1"), + TLSPrivateKey: []byte("privateKey-1"), + }, + ), + } + + defaultConfig = Configuration{ + Logging: Logging{ErrorLevel: defaultErrorLogLevel}, + NginxPlus: NginxPlus{}, + } + + // listeners. + listener80 = v1.Listener{ + Name: "listener-80-1", + Hostname: nil, + Port: 80, + Protocol: v1.HTTPProtocolType, + } + + listener443 = v1.Listener{ + Name: "listener-443-1", + Hostname: nil, + Port: 443, + Protocol: v1.HTTPSProtocolType, + TLS: &v1.GatewayTLSConfig{ + Mode: helpers.GetPointer(v1.TLSModeTerminate), + CertificateRefs: []v1.SecretObjectReference{ + { + Kind: (*v1.Kind)(helpers.GetPointer("Secret")), + Namespace: helpers.GetPointer(v1.Namespace(secret1NsName.Namespace)), + Name: v1.ObjectName(secret1NsName.Name), + }, + }, + }, + } + + hostname = v1.Hostname("example.com") + listener443WithHostname = v1.Listener{ + Name: "listener-443-with-hostname", + Hostname: &hostname, + Port: 443, + Protocol: v1.HTTPSProtocolType, + TLS: &v1.GatewayTLSConfig{ + Mode: helpers.GetPointer(v1.TLSModeTerminate), + CertificateRefs: []v1.SecretObjectReference{ + { + Kind: (*v1.Kind)(helpers.GetPointer("Secret")), + Namespace: helpers.GetPointer(v1.Namespace(secret2NsName.Namespace)), + Name: v1.ObjectName(secret2NsName.Name), + }, + }, + }, + } +) + +type commonTestCase struct { + msg string + graph *graph.Graph + expConf Configuration +} + var defaultBaseHTTPConfig = BaseHTTPConfig{ NginxReadinessProbePort: DefaultNginxReadinessProbePort, HTTP2: true, @@ -135,189 +279,176 @@ func createFakePolicy(name string, kind string) policies.Policy { } } -func TestBuildConfiguration(t *testing.T) { - t.Parallel() - const ( - invalidMatchesPath = "/not-valid-matches" - invalidFiltersPath = "/not-valid-filters" - ) - - gwPolicy1 := &graph.Policy{ - Source: createFakePolicy("attach-gw", "ApplePolicy"), - Valid: true, +func createRoute(name string) *v1.HTTPRoute { + return &v1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: name, + }, + Spec: v1.HTTPRouteSpec{}, } +} - gwPolicy2 := &graph.Policy{ - Source: createFakePolicy("attach-gw", "OrangePolicy"), - Valid: true, +func createGRPCRoute(name string) *v1.GRPCRoute { + return &v1.GRPCRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: name, + }, + Spec: v1.GRPCRouteSpec{}, } +} - hrPolicy1 := &graph.Policy{ - Source: createFakePolicy("attach-hr", "LemonPolicy"), - Valid: true, +func addFilters(hr *graph.L7Route, filters []graph.Filter) { + for i := range hr.Spec.Rules { + hr.Spec.Rules[i].Filters = graph.RouteRuleFilters{ + Filters: filters, + Valid: *hr.Spec.Rules[i].Matches[0].Path.Value != invalidFiltersPath, + } } +} - hrPolicy2 := &graph.Policy{ - Source: createFakePolicy("attach-hr", "LimePolicy"), - Valid: true, +func createBackendRefs(validRule bool) []graph.BackendRef { + if !validRule { + return nil } - invalidPolicy := &graph.Policy{ - Source: createFakePolicy("invalid", "LimePolicy"), - Valid: false, - } + return []graph.BackendRef{validBackendRef} +} - createRoute := func(name string) *v1.HTTPRoute { - return &v1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: name, - }, - Spec: v1.HTTPRouteSpec{}, - } - } +func createRules(paths []pathAndType) []graph.RouteRule { + rules := make([]graph.RouteRule, len(paths)) - createGRPCRoute := func(name string) *v1.GRPCRoute { - return &v1.GRPCRoute{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: name, + for i := range paths { + validMatches := paths[i].path != invalidMatchesPath + validFilters := paths[i].path != invalidFiltersPath + validRule := validMatches && validFilters + + m := []v1.HTTPRouteMatch{ + { + Path: &v1.HTTPPathMatch{ + Value: &paths[i].path, + Type: &paths[i].pathType, + }, }, - Spec: v1.GRPCRouteSpec{}, } - } - addFilters := func(hr *graph.L7Route, filters []graph.Filter) { - for i := range hr.Spec.Rules { - hr.Spec.Rules[i].Filters = graph.RouteRuleFilters{ - Filters: filters, - Valid: *hr.Spec.Rules[i].Matches[0].Path.Value != invalidFiltersPath, - } + rules[i] = graph.RouteRule{ + Matches: m, + Filters: graph.RouteRuleFilters{ + Valid: validFilters, + }, + BackendRefs: createBackendRefs(validRule), + ValidMatches: validMatches, } } - fooUpstreamName := "test_foo_80" + return rules +} - fooEndpoints := []resolver.Endpoint{ - { - Address: "10.0.0.0", - Port: 8080, +func createInternalRoute( + source client.Object, + routeType graph.RouteType, + hostnames []string, + listenerName string, + paths []pathAndType, +) *graph.L7Route { + r := &graph.L7Route{ + RouteType: routeType, + Source: source, + Spec: graph.L7RouteSpec{ + Rules: createRules(paths), + }, + Valid: true, + ParentRefs: []graph.ParentRef{ + { + Gateway: &graph.ParentRefGateway{ + NamespacedName: gatewayNsName, + }, + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + graph.CreateGatewayListenerKey(gatewayNsName, listenerName): hostnames, + }, + }, + }, }, } + return r +} - fooUpstream := Upstream{ - Name: fooUpstreamName, - Endpoints: fooEndpoints, - } - - fakeResolver := &resolverfakes.FakeServiceResolver{} - fakeResolver.ResolveReturns(fooEndpoints, nil) - - validBackendRef := getNormalBackendRef() - - expValidBackend := Backend{ - UpstreamName: fooUpstreamName, - Weight: 1, - Valid: true, - } +func createExpBackendGroupsForRoute(route *graph.L7Route) []BackendGroup { + groups := make([]BackendGroup, 0) - createBackendRefs := func(validRule bool) []graph.BackendRef { - if !validRule { - return nil + for idx, r := range route.Spec.Rules { + var backends []Backend + if r.Filters.Valid && r.ValidMatches { + backends = []Backend{expValidBackend} } - return []graph.BackendRef{validBackendRef} + groups = append(groups, BackendGroup{ + Backends: backends, + Source: client.ObjectKeyFromObject(route.Source), + RuleIdx: idx, + }) } - createRules := func(paths []pathAndType) []graph.RouteRule { - rules := make([]graph.RouteRule, len(paths)) + return groups +} - for i := range paths { - validMatches := paths[i].path != invalidMatchesPath - validFilters := paths[i].path != invalidFiltersPath - validRule := validMatches && validFilters +func createTestResources(name, hostname, listenerName string, paths ...pathAndType) ( + *v1.HTTPRoute, []BackendGroup, *graph.L7Route, +) { + hr := createRoute(name) + route := createInternalRoute(hr, graph.RouteTypeHTTP, []string{hostname}, listenerName, paths) + groups := createExpBackendGroupsForRoute(route) + return hr, groups, route +} - m := []v1.HTTPRouteMatch{ - { - Path: &v1.HTTPPathMatch{ - Value: &paths[i].path, - Type: &paths[i].pathType, - }, - }, - } +// common function to assert the generated configuration. +func assertBuildConfiguration(g *WithT, result, expected Configuration) { + g.Expect(result.BackendGroups).To(ConsistOf(expected.BackendGroups)) + g.Expect(result.Upstreams).To(ConsistOf(expected.Upstreams)) + g.Expect(result.HTTPServers).To(ConsistOf(expected.HTTPServers)) + g.Expect(result.SSLServers).To(ConsistOf(expected.SSLServers)) + g.Expect(result.TLSPassthroughServers).To(ConsistOf(expected.TLSPassthroughServers)) + g.Expect(result.SSLKeyPairs).To(Equal(expected.SSLKeyPairs)) + g.Expect(result.CertBundles).To(Equal(expected.CertBundles)) + g.Expect(result.Telemetry).To(Equal(expected.Telemetry)) + g.Expect(result.BaseHTTPConfig).To(Equal(expected.BaseHTTPConfig)) + g.Expect(result.Logging).To(Equal(expected.Logging)) + g.Expect(result.NginxPlus).To(Equal(expected.NginxPlus)) +} - rules[i] = graph.RouteRule{ - Matches: m, - Filters: graph.RouteRuleFilters{ - Valid: validFilters, - }, - BackendRefs: createBackendRefs(validRule), - ValidMatches: validMatches, - } - } +func TestBuildConfiguration(t *testing.T) { + t.Parallel() - return rules - } + fakeResolver := &resolverfakes.FakeServiceResolver{} + fakeResolver.ResolveReturns(fooEndpoints, nil) - createInternalRoute := func( - source client.Object, - routeType graph.RouteType, - hostnames []string, - listenerName string, - paths []pathAndType, - ) *graph.L7Route { - r := &graph.L7Route{ - RouteType: routeType, - Source: source, - Spec: graph.L7RouteSpec{ - Rules: createRules(paths), - }, - Valid: true, - ParentRefs: []graph.ParentRef{ - { - Gateway: &graph.ParentRefGateway{ - NamespacedName: gatewayNsName, - }, - Attachment: &graph.ParentRefAttachmentStatus{ - AcceptedHostnames: map[string][]string{ - graph.CreateGatewayListenerKey(gatewayNsName, listenerName): hostnames, - }, - }, - }, - }, - } - return r + gwPolicy1 := &graph.Policy{ + Source: createFakePolicy("attach-gw", "ApplePolicy"), + Valid: true, } - createExpBackendGroupsForRoute := func(route *graph.L7Route) []BackendGroup { - groups := make([]BackendGroup, 0) - - for idx, r := range route.Spec.Rules { - var backends []Backend - if r.Filters.Valid && r.ValidMatches { - backends = []Backend{expValidBackend} - } - - groups = append(groups, BackendGroup{ - Backends: backends, - Source: client.ObjectKeyFromObject(route.Source), - RuleIdx: idx, - }) - } + gwPolicy2 := &graph.Policy{ + Source: createFakePolicy("attach-gw", "OrangePolicy"), + Valid: true, + } - return groups + hrPolicy1 := &graph.Policy{ + Source: createFakePolicy("attach-hr", "LemonPolicy"), + Valid: true, } - createTestResources := func(name, hostname, listenerName string, paths ...pathAndType) ( - *v1.HTTPRoute, []BackendGroup, *graph.L7Route, - ) { - hr := createRoute(name) - route := createInternalRoute(hr, graph.RouteTypeHTTP, []string{hostname}, listenerName, paths) - groups := createExpBackendGroupsForRoute(route) - return hr, groups, route + hrPolicy2 := &graph.Policy{ + Source: createFakePolicy("attach-hr", "LimePolicy"), + Valid: true, } - prefix := v1.PathMatchPathPrefix + invalidPolicy := &graph.Policy{ + Source: createFakePolicy("invalid", "LimePolicy"), + Valid: false, + } hr1, expHR1Groups, routeHR1 := createTestResources( "hr-1", @@ -325,7 +456,8 @@ func TestBuildConfiguration(t *testing.T) { "listener-80-1", pathAndType{path: "/", pathType: prefix}, ) - hr1Invalid, _, routeHR1Invalid := createTestResources( + + _, _, routeHR1Invalid := createTestResources( "hr-1", "foo.example.com", "listener-80-1", @@ -454,13 +586,7 @@ func TestBuildConfiguration(t *testing.T) { pathAndType{path: "/third", pathType: prefix}, ) - httpsHR1, expHTTPSHR1Groups, httpsRouteHR1 := createTestResources( - "https-hr-1", - "foo.example.com", - "listener-443-1", - pathAndType{path: "/", pathType: prefix}, - ) - httpsHR1Invalid, _, httpsRouteHR1Invalid := createTestResources( + _, _, httpsRouteHR1Invalid := createTestResources( "https-hr-1", "foo.example.com", "listener-443-1", @@ -468,13 +594,6 @@ func TestBuildConfiguration(t *testing.T) { ) httpsRouteHR1Invalid.Valid = false - httpsHR2, expHTTPSHR2Groups, httpsRouteHR2 := createTestResources( - "https-hr-2", - "bar.example.com", - "listener-443-1", - pathAndType{path: "/", pathType: prefix}, - ) - httpsHR3, expHTTPSHR3Groups, httpsRouteHR3 := createTestResources( "https-hr-3", "foo.example.com", @@ -772,57 +891,6 @@ func TestBuildConfiguration(t *testing.T) { }, }) - secret1NsName := types.NamespacedName{Namespace: "test", Name: "secret-1"} - secret1 := &graph.Secret{ - Source: &apiv1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secret1NsName.Name, - Namespace: secret1NsName.Namespace, - }, - Data: map[string][]byte{ - apiv1.TLSCertKey: []byte("cert-1"), - apiv1.TLSPrivateKeyKey: []byte("privateKey-1"), - }, - }, - CertBundle: graph.NewCertificateBundle( - secret1NsName, - "Secret", - &graph.Certificate{ - TLSCert: []byte("cert-1"), - TLSPrivateKey: []byte("privateKey-1"), - }, - ), - } - - secret2NsName := types.NamespacedName{Namespace: "test", Name: "secret-2"} - secret2 := &graph.Secret{ - Source: &apiv1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secret2NsName.Name, - Namespace: secret2NsName.Namespace, - }, - Data: map[string][]byte{ - apiv1.TLSCertKey: []byte("cert-2"), - apiv1.TLSPrivateKeyKey: []byte("privateKey-2"), - }, - }, - CertBundle: graph.NewCertificateBundle( - secret2NsName, - "Secret", - &graph.Certificate{ - TLSCert: []byte("cert-2"), - TLSPrivateKey: []byte("privateKey-2"), - }, - ), - } - - listener80 := v1.Listener{ - Name: "listener-80-1", - Hostname: nil, - Port: 80, - Protocol: v1.HTTPProtocolType, - } - listener8080 := v1.Listener{ Name: "listener-8080", Hostname: nil, @@ -830,23 +898,6 @@ func TestBuildConfiguration(t *testing.T) { Protocol: v1.HTTPProtocolType, } - listener443 := v1.Listener{ - Name: "listener-443-1", - Hostname: nil, - Port: 443, - Protocol: v1.HTTPSProtocolType, - TLS: &v1.GatewayTLSConfig{ - Mode: helpers.GetPointer(v1.TLSModeTerminate), - CertificateRefs: []v1.SecretObjectReference{ - { - Kind: (*v1.Kind)(helpers.GetPointer("Secret")), - Namespace: helpers.GetPointer(v1.Namespace(secret1NsName.Namespace)), - Name: v1.ObjectName(secret1NsName.Name), - }, - }, - }, - } - listener443_2 := v1.Listener{ Name: "listener-443-2", Hostname: (*v1.Hostname)(helpers.GetPointer("*.example.com")), @@ -884,42 +935,6 @@ func TestBuildConfiguration(t *testing.T) { }, } - hostname := v1.Hostname("example.com") - - listener443WithHostname := v1.Listener{ - Name: "listener-443-with-hostname", - Hostname: &hostname, - Port: 443, - Protocol: v1.HTTPSProtocolType, - TLS: &v1.GatewayTLSConfig{ - Mode: helpers.GetPointer(v1.TLSModeTerminate), - CertificateRefs: []v1.SecretObjectReference{ - { - Kind: (*v1.Kind)(helpers.GetPointer("Secret")), - Namespace: helpers.GetPointer(v1.Namespace(secret2NsName.Namespace)), - Name: v1.ObjectName(secret2NsName.Name), - }, - }, - }, - } - - invalidListener := v1.Listener{ - Name: "invalid-listener", - Hostname: nil, - Port: 443, - Protocol: v1.HTTPSProtocolType, - TLS: &v1.GatewayTLSConfig{ - // Mode is missing, that's why invalid - CertificateRefs: []v1.SecretObjectReference{ - { - Kind: helpers.GetPointer[v1.Kind]("Secret"), - Namespace: helpers.GetPointer(v1.Namespace(secret1NsName.Namespace)), - Name: v1.ObjectName(secret1NsName.Name), - }, - }, - }, - } - referencedConfigMaps := map[types.NamespacedName]*graph.CaCertConfigMap{ {Namespace: "test", Name: "configmap-1"}: { Source: &apiv1.ConfigMap{ @@ -959,48 +974,69 @@ func TestBuildConfiguration(t *testing.T) { }, } - nginxProxy := &graph.EffectiveNginxProxy{ - Telemetry: &ngfAPIv1alpha2.Telemetry{ - Exporter: &ngfAPIv1alpha2.TelemetryExporter{ - Endpoint: helpers.GetPointer("my-otel.svc:4563"), - BatchSize: helpers.GetPointer(int32(512)), - BatchCount: helpers.GetPointer(int32(4)), - Interval: helpers.GetPointer(ngfAPIv1alpha1.Duration("5s")), - }, - ServiceName: helpers.GetPointer("my-svc"), - }, - DisableHTTP2: helpers.GetPointer(true), - IPFamily: helpers.GetPointer(ngfAPIv1alpha2.Dual), - DisableSNIHostValidation: helpers.GetPointer(true), - } - - nginxProxyIPv4 := &graph.EffectiveNginxProxy{ - IPFamily: helpers.GetPointer(ngfAPIv1alpha2.IPv4), - } - - nginxProxyIPv6 := &graph.EffectiveNginxProxy{ - IPFamily: helpers.GetPointer(ngfAPIv1alpha2.IPv6), - } - - defaultConfig := Configuration{ - Logging: Logging{ErrorLevel: defaultErrorLogLevel}, - NginxPlus: NginxPlus{}, - } - - tests := []struct { - graph *graph.Graph - msg string - expConf Configuration - }{ + tests := []commonTestCase{ { - graph: getNormalGraph(), + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): routeHR1, + graph.CreateRouteKey(hr2): routeHR2, + }, + }) + g.Routes = map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): routeHR1, + graph.CreateRouteKey(hr2): routeHR2, + } + return g + }), expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.HTTPServers = []VirtualServer{} + conf.HTTPServers = append(conf.HTTPServers, []VirtualServer{ + { + Hostname: "bar.example.com", + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + BackendGroup: expHR2Groups[0], + Source: &hr2.ObjectMeta, + }, + }, + }, + }, + Port: 80, + }, + { + Hostname: "foo.example.com", + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + BackendGroup: expHR1Groups[0], + Source: &hr1.ObjectMeta, + }, + }, + }, + }, + Port: 80, + }, + }...) conf.SSLServers = []VirtualServer{} + conf.Upstreams = []Upstream{fooUpstream} + conf.BackendGroups = []BackendGroup{expHR1Groups[0], expHR2Groups[0]} conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf }), - msg: "no listeners and routes", + msg: "one http listener with two routes for different hostnames", }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { @@ -1010,277 +1046,80 @@ func TestBuildConfiguration(t *testing.T) { GatewayName: gatewayNsName, Source: listener80, Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(gr): routeGR, + }, }) + g.Routes[graph.CreateRouteKey(gr)] = routeGR return g }), expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = append(conf.HTTPServers, VirtualServer{ + Hostname: "foo.example.com", + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + GRPC: true, + MatchRules: []MatchRule{ + { + BackendGroup: expGRGroups[0], + Source: &gr.ObjectMeta, + }, + }, + }, + }, + Port: 80, + }, + ) conf.SSLServers = []VirtualServer{} + conf.Upstreams = append(conf.Upstreams, fooUpstream) + conf.BackendGroups = []BackendGroup{expGRGroups[0]} conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} return conf }), - msg: "http listener with no routes", + msg: "one http listener with one grpc route", }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { gw := g.Gateways[gatewayNsName] gw.Listeners = append(gw.Listeners, []*graph.Listener{ { - Name: "listener-80-1", + Name: "listener-443-1", GatewayName: gatewayNsName, - Source: listener80, + Source: listener443, Valid: true, Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1Invalid): routeHR1Invalid, + graph.CreateRouteKey(httpsHR1): httpsRouteHR1, + graph.CreateRouteKey(httpsHR2): httpsRouteHR2, }, + ResolvedSecret: &secret1NsName, }, { - Name: "listener-443-1", + Name: "listener-443-with-hostname", GatewayName: gatewayNsName, - Source: listener443, // nil hostname + Source: listener443WithHostname, Valid: true, Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR1Invalid): httpsRouteHR1Invalid, + graph.CreateRouteKey(httpsHR5): httpsRouteHR5, }, - ResolvedSecret: &secret1NsName, + ResolvedSecret: &secret2NsName, }, }...) - g.Routes[graph.CreateRouteKey(hr1Invalid)] = routeHR1Invalid - g.ReferencedSecrets[secret1NsName] = secret1 + g.Routes = map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1): httpsRouteHR1, + graph.CreateRouteKey(hr2): httpsRouteHR2, + graph.CreateRouteKey(httpsHR5): httpsRouteHR5, + } + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ + secret1NsName: secret1, + secret2NsName: secret2, + } return g }), expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.HTTPServers = []VirtualServer{{ - IsDefault: true, - Port: 80, - }} - conf.SSLServers = append(conf.SSLServers, VirtualServer{ - Hostname: wildcardHostname, - SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, - Port: 443, - }) - return conf - }), - msg: "http and https listeners with no valid routes", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Listeners = append(gw.Listeners, []*graph.Listener{ - { - Name: "listener-443-1", - GatewayName: gatewayNsName, - Source: listener443, // nil hostname - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - ResolvedSecret: &secret1NsName, - }, - { - Name: "listener-443-with-hostname", - GatewayName: gatewayNsName, - Source: listener443WithHostname, // non-nil hostname - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - ResolvedSecret: &secret2NsName, - }, - }...) - g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ - secret1NsName: secret1, - secret2NsName: secret2, - } - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.HTTPServers = []VirtualServer{} - conf.SSLServers = append(conf.SSLServers, []VirtualServer{ - { - Hostname: string(hostname), - SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-2"}, - Port: 443, - }, - { - Hostname: wildcardHostname, - SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, - Port: 443, - }, - }...) - conf.SSLKeyPairs["ssl_keypair_test_secret-2"] = SSLKeyPair{ - Cert: []byte("cert-2"), - Key: []byte("privateKey-2"), - } - return conf - }), - msg: "https listeners with no routes", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "invalid-listener", - GatewayName: gatewayNsName, - Source: invalidListener, - Valid: false, - ResolvedSecret: &secret1NsName, - }) - g.Routes = map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR1): httpsRouteHR1, - graph.CreateRouteKey(httpsHR2): httpsRouteHR2, - } - g.ReferencedSecrets[secret1NsName] = secret1 - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.HTTPServers = []VirtualServer{} - conf.SSLServers = []VirtualServer{} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - return conf - }), - msg: "invalid https listener with resolved secret", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - graph.CreateRouteKey(hr2): routeHR2, - }, - }) - g.Routes = map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - graph.CreateRouteKey(hr2): routeHR2, - } - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.HTTPServers = append(conf.HTTPServers, []VirtualServer{ - { - Hostname: "bar.example.com", - PathRules: []PathRule{ - { - Path: "/", - PathType: PathTypePrefix, - MatchRules: []MatchRule{ - { - BackendGroup: expHR2Groups[0], - Source: &hr2.ObjectMeta, - }, - }, - }, - }, - Port: 80, - }, - { - Hostname: "foo.example.com", - PathRules: []PathRule{ - { - Path: "/", - PathType: PathTypePrefix, - MatchRules: []MatchRule{ - { - BackendGroup: expHR1Groups[0], - Source: &hr1.ObjectMeta, - }, - }, - }, - }, - Port: 80, - }, - }...) - conf.SSLServers = []VirtualServer{} - conf.Upstreams = []Upstream{fooUpstream} - conf.BackendGroups = []BackendGroup{expHR1Groups[0], expHR2Groups[0]} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - - return conf - }), - msg: "one http listener with two routes for different hostnames", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(gr): routeGR, - }, - }) - g.Routes[graph.CreateRouteKey(gr)] = routeGR - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.HTTPServers = append(conf.HTTPServers, VirtualServer{ - Hostname: "foo.example.com", - PathRules: []PathRule{ - { - Path: "/", - PathType: PathTypePrefix, - GRPC: true, - MatchRules: []MatchRule{ - { - BackendGroup: expGRGroups[0], - Source: &gr.ObjectMeta, - }, - }, - }, - }, - Port: 80, - }, - ) - conf.SSLServers = []VirtualServer{} - conf.Upstreams = append(conf.Upstreams, fooUpstream) - conf.BackendGroups = []BackendGroup{expGRGroups[0]} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - return conf - }), - msg: "one http listener with one grpc route", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Listeners = append(gw.Listeners, []*graph.Listener{ - { - Name: "listener-443-1", - GatewayName: gatewayNsName, - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR1): httpsRouteHR1, - graph.CreateRouteKey(httpsHR2): httpsRouteHR2, - }, - ResolvedSecret: &secret1NsName, - }, - { - Name: "listener-443-with-hostname", - GatewayName: gatewayNsName, - Source: listener443WithHostname, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHR5): httpsRouteHR5, - }, - ResolvedSecret: &secret2NsName, - }, - }...) - g.Routes = map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): httpsRouteHR1, - graph.CreateRouteKey(hr2): httpsRouteHR2, - graph.CreateRouteKey(httpsHR5): httpsRouteHR5, - } - g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ - secret1NsName: secret1, - secret2NsName: secret2, - } - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.HTTPServers = []VirtualServer{} - conf.SSLServers = append(conf.SSLServers, []VirtualServer{ + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ { Hostname: "bar.example.com", PathRules: []PathRule{ @@ -1695,14 +1534,6 @@ func TestBuildConfiguration(t *testing.T) { }), msg: "multiple http and https listener; different ports", }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.GatewayClass.Valid = false - return g - }), - expConf: defaultConfig, - msg: "invalid gatewayclass", - }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { g.GatewayClass = nil @@ -2243,70 +2074,31 @@ func TestBuildConfiguration(t *testing.T) { { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { gw := g.Gateways[gatewayNsName] - gw.Source.ObjectMeta = metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - } - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }) - gw.EffectiveNginxProxy = nginxProxy - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.SSLServers = []VirtualServer{} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - conf.Telemetry = Telemetry{ - Endpoint: "my-otel.svc:4563", - Interval: "5s", - BatchSize: 512, - BatchCount: 4, - ServiceName: "ngf:ns:gw:my-svc", - Ratios: []Ratio{}, - SpanAttributes: []SpanAttribute{}, - } - conf.BaseHTTPConfig = BaseHTTPConfig{ - HTTP2: false, - IPFamily: Dual, - NginxReadinessProbePort: DefaultNginxReadinessProbePort, - DisableSNIHostValidation: true, - } - return conf - }), - msg: "EffectiveNginxProxy with tracing config and http2 disabled", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Listeners = append(gw.Listeners, []*graph.Listener{ - { - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hrWithPolicy): l7RouteWithPolicy, - }, - }, - { - Name: "listener-443-1", - GatewayName: gatewayNsName, - Source: listener443, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(httpsHRWithPolicy): l7HTTPSRouteWithPolicy, - }, - ResolvedSecret: &secret1NsName, - }, - }...) - gw.Policies = []*graph.Policy{gwPolicy1, gwPolicy2} - g.Routes = map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hrWithPolicy): l7RouteWithPolicy, - graph.CreateRouteKey(httpsHRWithPolicy): l7HTTPSRouteWithPolicy, + gw.Listeners = append(gw.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hrWithPolicy): l7RouteWithPolicy, + }, + }, + { + Name: "listener-443-1", + GatewayName: gatewayNsName, + Source: listener443, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHRWithPolicy): l7HTTPSRouteWithPolicy, + }, + ResolvedSecret: &secret1NsName, + }, + }...) + gw.Policies = []*graph.Policy{gwPolicy1, gwPolicy2} + g.Routes = map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hrWithPolicy): l7RouteWithPolicy, + graph.CreateRouteKey(httpsHRWithPolicy): l7HTTPSRouteWithPolicy, } g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ secret1NsName: secret1, @@ -2445,138 +2237,6 @@ func TestBuildConfiguration(t *testing.T) { }), msg: "Gateway and HTTPRoute with policies attached with advanced routing", }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Source.ObjectMeta = metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - } - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }) - gw.EffectiveNginxProxy = nginxProxyIPv4 - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.SSLServers = []VirtualServer{} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - conf.BaseHTTPConfig = BaseHTTPConfig{ - HTTP2: true, - IPFamily: IPv4, - NginxReadinessProbePort: DefaultNginxReadinessProbePort, - } - return conf - }), - msg: "GatewayClass has NginxProxy with IPv4 IPFamily and no routes", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Source.ObjectMeta = metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - } - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }) - gw.EffectiveNginxProxy = nginxProxyIPv6 - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.SSLServers = []VirtualServer{} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - conf.BaseHTTPConfig = BaseHTTPConfig{ - HTTP2: true, - IPFamily: IPv6, - NginxReadinessProbePort: DefaultNginxReadinessProbePort, - } - return conf - }), - msg: "GatewayClass has NginxProxy with IPv6 IPFamily and no routes", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Source.ObjectMeta = metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - } - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }) - gw.EffectiveNginxProxy = &graph.EffectiveNginxProxy{ - RewriteClientIP: &ngfAPIv1alpha2.RewriteClientIP{ - SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPIv1alpha2.RewriteClientIPAddress{ - { - Type: ngfAPIv1alpha2.RewriteClientIPCIDRAddressType, - Value: "1.1.1.1/32", - }, - }, - Mode: helpers.GetPointer(ngfAPIv1alpha2.RewriteClientIPModeProxyProtocol), - }, - } - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.SSLServers = []VirtualServer{} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - conf.BaseHTTPConfig = BaseHTTPConfig{ - HTTP2: true, - IPFamily: Dual, - RewriteClientIPSettings: RewriteClientIPSettings{ - IPRecursive: true, - TrustedAddresses: []string{"1.1.1.1/32"}, - Mode: RewriteIPModeProxyProtocol, - }, - NginxReadinessProbePort: DefaultNginxReadinessProbePort, - } - return conf - }), - msg: "GatewayClass has NginxProxy with rewriteClientIP details set", - }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Source.ObjectMeta = metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - } - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }) - gw.EffectiveNginxProxy = &graph.EffectiveNginxProxy{ - Logging: &ngfAPIv1alpha2.NginxLogging{ - ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelDebug), - }, - } - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.SSLServers = []VirtualServer{} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - conf.Logging = Logging{ErrorLevel: "debug"} - return conf - }), - msg: "GatewayClass has NginxProxy with error log level set to debug", - }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { g.SnippetsFilters = map[types.NamespacedName]*graph.SnippetsFilter{ @@ -2599,37 +2259,6 @@ func TestBuildConfiguration(t *testing.T) { }), msg: "SnippetsFilters scoped per gateway - no routes reference SnippetsFilters", }, - { - graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - gw := g.Gateways[gatewayNsName] - gw.Source.ObjectMeta = metav1.ObjectMeta{ - Name: "gw", - Namespace: "ns", - } - gw.Listeners = append(gw.Listeners, &graph.Listener{ - Name: "listener-80-1", - GatewayName: gatewayNsName, - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{}, - }) - gw.EffectiveNginxProxy = &graph.EffectiveNginxProxy{ - NginxPlus: &ngfAPIv1alpha2.NginxPlus{ - AllowedAddresses: []ngfAPIv1alpha2.NginxPlusAllowAddress{ - {Type: ngfAPIv1alpha2.NginxPlusAllowIPAddressType, Value: "127.0.0.3"}, - {Type: ngfAPIv1alpha2.NginxPlusAllowIPAddressType, Value: "25.0.0.3"}, - }, - }, - } - return g - }), - expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { - conf.SSLServers = []VirtualServer{} - conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} - return conf - }), - msg: "NginxProxy with NginxPlus allowed addresses configured but running on nginx oss", - }, } for _, test := range tests { @@ -2646,17 +2275,7 @@ func TestBuildConfiguration(t *testing.T) { false, ) - g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups)) - g.Expect(result.Upstreams).To(ConsistOf(test.expConf.Upstreams)) - g.Expect(result.HTTPServers).To(ConsistOf(test.expConf.HTTPServers)) - g.Expect(result.SSLServers).To(ConsistOf(test.expConf.SSLServers)) - g.Expect(result.TLSPassthroughServers).To(ConsistOf(test.expConf.TLSPassthroughServers)) - g.Expect(result.SSLKeyPairs).To(Equal(test.expConf.SSLKeyPairs)) - g.Expect(result.CertBundles).To(Equal(test.expConf.CertBundles)) - g.Expect(result.Telemetry).To(Equal(test.expConf.Telemetry)) - g.Expect(result.BaseHTTPConfig).To(Equal(test.expConf.BaseHTTPConfig)) - g.Expect(result.Logging).To(Equal(test.expConf.Logging)) - g.Expect(result.NginxPlus).To(Equal(test.expConf.NginxPlus)) + assertBuildConfiguration(g, result, test.expConf) }) } } @@ -5303,3 +4922,568 @@ func TestBuildDNSResolverConfig(t *testing.T) { }) } } + +func TestBuildConfiguration_GatewaysAndListeners(t *testing.T) { + t.Parallel() + + fakeResolver := &resolverfakes.FakeServiceResolver{} + fakeResolver.ResolveReturns(fooEndpoints, nil) + + secret1NsName := types.NamespacedName{Namespace: "test", Name: "secret-1"} + secret1 := &graph.Secret{ + Source: &apiv1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secret1NsName.Name, + Namespace: secret1NsName.Namespace, + }, + Data: map[string][]byte{ + apiv1.TLSCertKey: []byte("cert-1"), + apiv1.TLSPrivateKeyKey: []byte("privateKey-1"), + }, + }, + CertBundle: graph.NewCertificateBundle( + secret1NsName, + "Secret", + &graph.Certificate{ + TLSCert: []byte("cert-1"), + TLSPrivateKey: []byte("privateKey-1"), + }, + ), + } + + listener80 := v1.Listener{ + Name: "listener-80-1", + Hostname: nil, + Port: 80, + Protocol: v1.HTTPProtocolType, + } + + listener443 := v1.Listener{ + Name: "listener-443-1", + Hostname: nil, + Port: 443, + Protocol: v1.HTTPSProtocolType, + TLS: &v1.GatewayTLSConfig{ + Mode: helpers.GetPointer(v1.TLSModeTerminate), + CertificateRefs: []v1.SecretObjectReference{ + { + Kind: (*v1.Kind)(helpers.GetPointer("Secret")), + Namespace: helpers.GetPointer(v1.Namespace(secret1NsName.Namespace)), + Name: v1.ObjectName(secret1NsName.Name), + }, + }, + }, + } + + invalidListener := v1.Listener{ + Name: "invalid-listener", + Hostname: nil, + Port: 443, + Protocol: v1.HTTPSProtocolType, + TLS: &v1.GatewayTLSConfig{ + // Mode is missing, that's why invalid + CertificateRefs: []v1.SecretObjectReference{ + { + Kind: helpers.GetPointer[v1.Kind]("Secret"), + Namespace: helpers.GetPointer(v1.Namespace(secret1NsName.Namespace)), + Name: v1.ObjectName(secret1NsName.Name), + }, + }, + }, + } + + hr1Invalid, _, routeHR1Invalid := createTestResources( + "hr-1", + "foo.example.com", + "listener-80-1", + pathAndType{path: "/", pathType: prefix}, + ) + + routeHR1Invalid.Valid = false + + httpsHR1Invalid, _, httpsRouteHR1Invalid := createTestResources( + "https-hr-1", + "foo.example.com", + "listener-443-1", + pathAndType{path: "/", pathType: prefix}, + ) + httpsRouteHR1Invalid.Valid = false + + tests := []commonTestCase{ + { + graph: getNormalGraph(), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), + msg: "no listeners and routes", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + }) + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), + msg: "http listener with no routes", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Listeners = append(gw.Listeners, []*graph.Listener{ + { + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(hr1Invalid): routeHR1Invalid, + }, + }, + { + Name: "listener-443-1", + GatewayName: gatewayNsName, + Source: listener443, // nil hostname + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR1Invalid): httpsRouteHR1Invalid, + }, + ResolvedSecret: &secret1NsName, + }, + }...) + g.Routes[graph.CreateRouteKey(hr1Invalid)] = routeHR1Invalid + g.ReferencedSecrets[secret1NsName] = secret1 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{{ + IsDefault: true, + Port: 80, + }} + conf.SSLServers = append(conf.SSLServers, VirtualServer{ + Hostname: wildcardHostname, + SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, + Port: 443, + }) + return conf + }), + msg: "http and https listeners with no valid routes", + }, + + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Listeners = append(gw.Listeners, []*graph.Listener{ + { + Name: "listener-443-1", + GatewayName: gatewayNsName, + Source: listener443, // nil hostname + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + ResolvedSecret: &secret1NsName, + }, + { + Name: "listener-443-with-hostname", + GatewayName: gatewayNsName, + Source: listener443WithHostname, // non-nil hostname + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + ResolvedSecret: &secret2NsName, + }, + }...) + g.ReferencedSecrets = map[types.NamespacedName]*graph.Secret{ + secret1NsName: secret1, + secret2NsName: secret2, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = append(conf.SSLServers, []VirtualServer{ + { + Hostname: string(hostname), + SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-2"}, + Port: 443, + }, + { + Hostname: wildcardHostname, + SSL: &SSL{KeyPairID: "ssl_keypair_test_secret-1"}, + Port: 443, + }, + }...) + conf.SSLKeyPairs["ssl_keypair_test_secret-2"] = SSLKeyPair{ + Cert: []byte("cert-2"), + Key: []byte("privateKey-2"), + } + return conf + }), + msg: "https listeners with no routes", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "invalid-listener", + GatewayName: gatewayNsName, + Source: invalidListener, + Valid: false, + ResolvedSecret: &secret1NsName, + }) + g.Routes = map[graph.RouteKey]*graph.L7Route{ + graph.CreateRouteKey(httpsHR1): httpsRouteHR1, + graph.CreateRouteKey(httpsHR2): httpsRouteHR2, + } + g.ReferencedSecrets[secret1NsName] = secret1 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), + msg: "invalid https listener with resolved secret", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.GatewayClass.Valid = false + return g + }), + expConf: defaultConfig, + msg: "invalid gatewayclass", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Valid = true + gw.SecretRef = &types.NamespacedName{ + Namespace: secret1NsName.Namespace, + Name: secret1NsName.Name, + } + g.ReferencedSecrets[secret1NsName] = secret1 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{ + "ssl_keypair_test_secret-1": { + Cert: []byte("cert-1"), + Key: []byte("privateKey-1"), + }, + } + conf.BaseHTTPConfig = BaseHTTPConfig{ + HTTP2: true, + IPFamily: Dual, + NginxReadinessProbePort: DefaultNginxReadinessProbePort, + GatewaySecretID: "ssl_keypair_test_secret-1", + } + return conf + }), + msg: "gateway is valid and client certificate is set -- " + + "secret should be part of SSLKeyPairs and config", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.SecretRef = &types.NamespacedName{ + Namespace: secret1NsName.Namespace, + Name: secret1NsName.Name, + } + g.ReferencedSecrets[secret1NsName] = secret1 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.HTTPServers = []VirtualServer{} + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = defaultBaseHTTPConfig + return conf + }), + msg: "gateway is invalid and client certificate is set -- " + + "secret will be ignored", + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + result := BuildConfiguration( + t.Context(), + logr.Discard(), + test.graph, + test.graph.Gateways[gatewayNsName], + fakeResolver, + false, + ) + + assertBuildConfiguration(g, result, test.expConf) + }) + } +} + +func TestBuildConfiguration_NginxProxy(t *testing.T) { + t.Parallel() + + fakeResolver := &resolverfakes.FakeServiceResolver{} + fakeResolver.ResolveReturns(fooEndpoints, nil) + + nginxProxy := &graph.EffectiveNginxProxy{ + Telemetry: &ngfAPIv1alpha2.Telemetry{ + Exporter: &ngfAPIv1alpha2.TelemetryExporter{ + Endpoint: helpers.GetPointer("my-otel.svc:4563"), + BatchSize: helpers.GetPointer(int32(512)), + BatchCount: helpers.GetPointer(int32(4)), + Interval: helpers.GetPointer(ngfAPIv1alpha1.Duration("5s")), + }, + ServiceName: helpers.GetPointer("my-svc"), + }, + DisableHTTP2: helpers.GetPointer(true), + IPFamily: helpers.GetPointer(ngfAPIv1alpha2.Dual), + DisableSNIHostValidation: helpers.GetPointer(true), + } + + nginxProxyIPv4 := &graph.EffectiveNginxProxy{ + IPFamily: helpers.GetPointer(ngfAPIv1alpha2.IPv4), + } + + nginxProxyIPv6 := &graph.EffectiveNginxProxy{ + IPFamily: helpers.GetPointer(ngfAPIv1alpha2.IPv6), + } + + tests := []commonTestCase{ + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + gw.EffectiveNginxProxy = nginxProxy + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.Telemetry = Telemetry{ + Endpoint: "my-otel.svc:4563", + Interval: "5s", + BatchSize: 512, + BatchCount: 4, + ServiceName: "ngf:ns:gw:my-svc", + Ratios: []Ratio{}, + SpanAttributes: []SpanAttribute{}, + } + conf.BaseHTTPConfig = BaseHTTPConfig{ + HTTP2: false, + IPFamily: Dual, + NginxReadinessProbePort: DefaultNginxReadinessProbePort, + DisableSNIHostValidation: true, + } + return conf + }), + msg: "EffectiveNginxProxy with tracing config and http2 disabled", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + gw.EffectiveNginxProxy = nginxProxyIPv4 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = BaseHTTPConfig{ + HTTP2: true, + IPFamily: IPv4, + NginxReadinessProbePort: DefaultNginxReadinessProbePort, + } + return conf + }), + msg: "GatewayClass has NginxProxy with IPv4 IPFamily and no routes", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + gw.EffectiveNginxProxy = nginxProxyIPv6 + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = BaseHTTPConfig{ + HTTP2: true, + IPFamily: IPv6, + NginxReadinessProbePort: DefaultNginxReadinessProbePort, + } + return conf + }), + msg: "GatewayClass has NginxProxy with IPv6 IPFamily and no routes", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + gw.EffectiveNginxProxy = &graph.EffectiveNginxProxy{ + RewriteClientIP: &ngfAPIv1alpha2.RewriteClientIP{ + SetIPRecursively: helpers.GetPointer(true), + TrustedAddresses: []ngfAPIv1alpha2.RewriteClientIPAddress{ + { + Type: ngfAPIv1alpha2.RewriteClientIPCIDRAddressType, + Value: "1.1.1.1/32", + }, + }, + Mode: helpers.GetPointer(ngfAPIv1alpha2.RewriteClientIPModeProxyProtocol), + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.BaseHTTPConfig = BaseHTTPConfig{ + HTTP2: true, + IPFamily: Dual, + RewriteClientIPSettings: RewriteClientIPSettings{ + IPRecursive: true, + TrustedAddresses: []string{"1.1.1.1/32"}, + Mode: RewriteIPModeProxyProtocol, + }, + NginxReadinessProbePort: DefaultNginxReadinessProbePort, + } + return conf + }), + msg: "GatewayClass has NginxProxy with rewriteClientIP details set", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + gw.EffectiveNginxProxy = &graph.EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelDebug), + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.Logging = Logging{ErrorLevel: "debug"} + return conf + }), + msg: "GatewayClass has NginxProxy with error log level set to debug", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + gw := g.Gateways[gatewayNsName] + gw.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + gw.Listeners = append(gw.Listeners, &graph.Listener{ + Name: "listener-80-1", + GatewayName: gatewayNsName, + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + gw.EffectiveNginxProxy = &graph.EffectiveNginxProxy{ + NginxPlus: &ngfAPIv1alpha2.NginxPlus{ + AllowedAddresses: []ngfAPIv1alpha2.NginxPlusAllowAddress{ + {Type: ngfAPIv1alpha2.NginxPlusAllowIPAddressType, Value: "127.0.0.3"}, + {Type: ngfAPIv1alpha2.NginxPlusAllowIPAddressType, Value: "25.0.0.3"}, + }, + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), + msg: "NginxProxy with NginxPlus allowed addresses configured but running on nginx oss", + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + result := BuildConfiguration( + t.Context(), + logr.Discard(), + test.graph, + test.graph.Gateways[gatewayNsName], + fakeResolver, + false, + ) + + assertBuildConfiguration(g, result, test.expConf) + }) + } +} diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 08e7e0867b..3166f73dcf 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -23,23 +23,19 @@ const ( // Configuration is an intermediate representation of dataplane configuration. type Configuration struct { - // AuxiliarySecrets contains additional secret data, like certificates/keys/tokens that are not related to - // Gateway API resources. - AuxiliarySecrets map[graph.SecretFileType][]byte // CertBundles holds all unique Certificate Bundles. CertBundles map[CertBundleID]CertBundle // BaseStreamConfig holds the configuration options at the stream context. BaseStreamConfig BaseStreamConfig // SSLKeyPairs holds all unique SSLKeyPairs. SSLKeyPairs map[SSLKeyPairID]SSLKeyPair + // AuxiliarySecrets contains additional secret data, like certificates/keys/tokens that are not related to + // Gateway API resources. + AuxiliarySecrets map[graph.SecretFileType][]byte // DeploymentContext contains metadata about NGF and the cluster. DeploymentContext DeploymentContext // Logging defines logging related settings for NGINX. Logging Logging - // StreamUpstreams holds all unique stream Upstreams - StreamUpstreams []Upstream - // TLSPassthroughServers hold all TLSPassthroughServers - TLSPassthroughServers []Layer4VirtualServer // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup // MainSnippets holds all the snippets that apply to the main context. @@ -48,10 +44,14 @@ type Configuration struct { Upstreams []Upstream // NginxPlus specifies NGINX Plus additional settings. NginxPlus NginxPlus - // SSLServers holds all SSLServers. - SSLServers []VirtualServer // HTTPServers holds all HTTPServers. HTTPServers []VirtualServer + // StreamUpstreams holds all unique stream Upstreams + StreamUpstreams []Upstream + // SSLServers holds all SSLServers. + SSLServers []VirtualServer + // TLSPassthroughServers hold all TLSPassthroughServers + TLSPassthroughServers []Layer4VirtualServer // Telemetry holds the Otel configuration. Telemetry Telemetry // BaseHTTPConfig holds the configuration options at the http context. @@ -372,6 +372,8 @@ type BaseHTTPConfig struct { DNSResolver *DNSResolverConfig // IPFamily specifies the IP family for all servers. IPFamily IPFamilyType + // GatewaySecretID is the ID of the secret that contains the gateway backend TLS certificate. + GatewaySecretID SSLKeyPairID // Snippets contain the snippets that apply to the http context. Snippets []Snippet // RewriteIPSettings defines configuration for rewriting the client IP to the original client's IP. diff --git a/internal/controller/state/graph/gateway.go b/internal/controller/state/graph/gateway.go index 103634871e..459f6165fd 100644 --- a/internal/controller/state/graph/gateway.go +++ b/internal/controller/state/graph/gateway.go @@ -1,6 +1,8 @@ package graph import ( + "fmt" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" @@ -24,6 +26,8 @@ type Gateway struct { // the GatewayClass resource. This is the effective set of config that should be applied to the Gateway. // If non-nil, then this config is valid. EffectiveNginxProxy *EffectiveNginxProxy + // SecretRef is the namespaced name of the secret referenced by the Gateway for backend TLS. + SecretRef *types.NamespacedName // DeploymentName is the name of the nginx Deployment associated with this Gateway. DeploymentName types.NamespacedName // Listeners include the listeners of the Gateway. @@ -64,6 +68,7 @@ func buildGateways( gc *GatewayClass, refGrantResolver *referenceGrantResolver, nps map[types.NamespacedName]*NginxProxy, + experimentalFeatures bool, ) map[types.NamespacedName]*Gateway { if len(gws) == 0 { return nil @@ -86,7 +91,7 @@ func buildGateways( effectiveNginxProxy := buildEffectiveNginxProxy(gcNp, np) - conds, valid := validateGateway(gw, gc, np) + conds, valid, secretRefNsName := validateGateway(gw, gc, np, experimentalFeatures, secretResolver, refGrantResolver) protectedPorts := make(ProtectedPorts) if port, enabled := MetricsEnabledForNginxProxy(effectiveNginxProxy); enabled { @@ -110,6 +115,7 @@ func buildGateways( EffectiveNginxProxy: effectiveNginxProxy, Conditions: conds, DeploymentName: deploymentName, + SecretRef: secretRefNsName, } } else { builtGateways[gwNsName] = &Gateway{ @@ -120,6 +126,7 @@ func buildGateways( Valid: true, Conditions: conds, DeploymentName: deploymentName, + SecretRef: secretRefNsName, } } } @@ -170,7 +177,14 @@ func validateGatewayParametersRef(npCfg *NginxProxy, ref v1.LocalParametersRefer return conds } -func validateGateway(gw *v1.Gateway, gc *GatewayClass, npCfg *NginxProxy) ([]conditions.Condition, bool) { +func validateGateway( + gw *v1.Gateway, + gc *GatewayClass, + npCfg *NginxProxy, + experimentalFeatures bool, + secretResolver *secretResolver, + refGrantResolver *referenceGrantResolver, +) ([]conditions.Condition, bool, *types.NamespacedName) { var conds []conditions.Condition if gc == nil { @@ -189,8 +203,35 @@ func validateGateway(gw *v1.Gateway, gc *GatewayClass, npCfg *NginxProxy) ([]con } } - // we evaluate validity before validating parametersRef because an invalid parametersRef/NginxProxy does not - // invalidate the entire Gateway. + // this path will be updated to spec.tls.backend.clientCertificateRef + // after gateway API v1.4 release. + var secretRefNsName *types.NamespacedName + if gw.Spec.BackendTLS != nil && gw.Spec.BackendTLS.ClientCertificateRef != nil { + if !experimentalFeatures { + path := field.NewPath("spec", "tls") + valErr := field.Forbidden(path, "tls is not supported when experimental features are disabled") + conds = append(conds, conditions.NewGatewayUnsupportedValue(valErr.Error())...) + } else { + secretNsName, secretNs := getGatewayCertSecretNsName(gw) + err := secretResolver.resolve(*secretNsName) + if err != nil { + path := field.NewPath("backend.clientCertificateRef") + valErr := field.Invalid(path, secretNsName, err.Error()) + conds = append(conds, conditions.NewGatewaySecretRefInvalid(valErr.Error())) + } + + if secretNs != gw.Namespace { + if !refGrantResolver.refAllowed(toSecret(*secretNsName), fromGateway(gw.Namespace)) { + msg := fmt.Sprintf("secret ref %s not permitted by any ReferenceGrant", secretNsName) + conds = append(conds, conditions.NewGatewaySecretRefNotPermitted(msg)) + } + } + + secretRefNsName = secretNsName + } + } + + // Evaluate validity before validating parametersRef valid := len(conds) == 0 if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil { @@ -198,7 +239,19 @@ func validateGateway(gw *v1.Gateway, gc *GatewayClass, npCfg *NginxProxy) ([]con conds = append(conds, paramConds...) } - return conds, valid + return conds, valid, secretRefNsName +} + +func getGatewayCertSecretNsName(gw *v1.Gateway) (*types.NamespacedName, string) { + gatewayCert := gw.Spec.BackendTLS.ClientCertificateRef + secretRefNs := gw.Namespace + if gatewayCert.Namespace != nil { + secretRefNs = string(*gatewayCert.Namespace) + } + return &types.NamespacedName{ + Namespace: secretRefNs, + Name: string(gatewayCert.Name), + }, secretRefNs } // GetReferencedSnippetsFilters returns all SnippetsFilters that are referenced by routes attached to this Gateway. diff --git a/internal/controller/state/graph/gateway_test.go b/internal/controller/state/graph/gateway_test.go index f31fa1dd3c..3c915844f3 100644 --- a/internal/controller/state/graph/gateway_test.go +++ b/internal/controller/state/graph/gateway_test.go @@ -1,6 +1,7 @@ package graph import ( + "fmt" "testing" . "github.com/onsi/gomega" @@ -21,6 +22,32 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds" ) +var ( + secretSameNs = &apiv1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "secret", + }, + Data: map[string][]byte{ + apiv1.TLSCertKey: cert, + apiv1.TLSPrivateKeyKey: key, + }, + Type: apiv1.SecretTypeTLS, + } + + secretDiffNamespace = &apiv1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "diff-ns", + Name: "secret-diff-ns", + }, + Data: map[string][]byte{ + apiv1.TLSCertKey: cert, + apiv1.TLSPrivateKeyKey: key, + }, + Type: apiv1.SecretTypeTLS, + } +) + func TestProcessGateways(t *testing.T) { t.Parallel() const gcName = "test-gc" @@ -124,18 +151,6 @@ func TestBuildGateway(t *testing.T) { }, } - secretSameNs := &apiv1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "secret", - }, - Data: map[string][]byte{ - apiv1.TLSCertKey: cert, - apiv1.TLSPrivateKeyKey: key, - }, - Type: apiv1.SecretTypeTLS, - } - gatewayTLSConfigSameNs := &v1.GatewayTLSConfig{ Mode: helpers.GetPointer(v1.TLSModeTerminate), CertificateRefs: []v1.SecretObjectReference{ @@ -158,18 +173,6 @@ func TestBuildGateway(t *testing.T) { }, } - secretDiffNamespace := &apiv1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "diff-ns", - Name: "secret", - }, - Data: map[string][]byte{ - apiv1.TLSCertKey: cert, - apiv1.TLSPrivateKeyKey: key, - }, - Type: apiv1.SecretTypeTLS, - } - gatewayTLSConfigDiffNs := &v1.GatewayTLSConfig{ Mode: helpers.GetPointer(v1.TLSModeTerminate), CertificateRefs: []v1.SecretObjectReference{ @@ -530,7 +533,7 @@ func TestBuildGateway(t *testing.T) { { Group: "core", Kind: "Secret", - Name: helpers.GetPointer[v1.ObjectName]("secret"), + Name: helpers.GetPointer[v1.ObjectName]("secret-diff-ns"), }, }, }, @@ -696,7 +699,7 @@ func TestBuildGateway(t *testing.T) { Valid: false, Attachable: true, Conditions: conditions.NewListenerRefNotPermitted( - `Certificate ref to secret diff-ns/secret not permitted by any ReferenceGrant`, + `Certificate ref to secret diff-ns/secret-diff-ns not permitted by any ReferenceGrant`, ), Routes: map[RouteKey]*L7Route{}, L4Routes: map[L4RouteKey]*L4Route{}, @@ -1535,7 +1538,7 @@ func TestBuildGateway(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) resolver := newReferenceGrantResolver(test.refGrants) - result := buildGateways(test.gateway, secretResolver, test.gatewayClass, resolver, nginxProxies) + result := buildGateways(test.gateway, secretResolver, test.gatewayClass, resolver, nginxProxies, false) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) } @@ -1821,3 +1824,231 @@ func TestGetReferencedSnippetsFilters(t *testing.T) { emptyFilterResult := gw.GetReferencedSnippetsFilters(routes, map[types.NamespacedName]*SnippetsFilter{}) g.Expect(emptyFilterResult).To(BeEmpty()) } + +func TestGateway_BackendTLSConfig(t *testing.T) { + t.Parallel() + + secretSameNsKey := client.ObjectKeyFromObject(secretSameNs) + secretDiffNsKey := client.ObjectKeyFromObject(secretDiffNamespace) + + secrets := map[types.NamespacedName]*apiv1.Secret{ + secretSameNsKey: secretSameNs, + secretDiffNsKey: secretDiffNamespace, + } + + gcName := "nginx" + deploymentName := types.NamespacedName{ + Namespace: "test", + Name: "test-gateway-nginx", + } + + invalidSecret := &apiv1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "invalid-secret", + }, + Data: map[string][]byte{ + apiv1.TLSCertKey: []byte("invalid-cert"), + apiv1.TLSPrivateKeyKey: []byte("invalid-key"), + }, + Type: apiv1.SecretTypeTLS, + } + invalidSecretNsKey := client.ObjectKeyFromObject(invalidSecret) + + createNewGatewayMap := func(secretRef types.NamespacedName) map[types.NamespacedName]*v1.Gateway { + return map[types.NamespacedName]*v1.Gateway{ + {Namespace: "test", Name: "test-gateway"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "test", + }, + Spec: v1.GatewaySpec{ + GatewayClassName: v1.ObjectName(gcName), + BackendTLS: &v1.GatewayBackendTLS{ + ClientCertificateRef: &v1.SecretObjectReference{ + Name: v1.ObjectName(secretRef.Name), + Namespace: (*v1.Namespace)(&secretRef.Namespace), + }, + }, + }, + }, + } + } + + expectedGatewayMap := func( + secretRef types.NamespacedName, + cond []conditions.Condition, + valid, experimental bool, + ) map[types.NamespacedName]*Gateway { + gwNsName := types.NamespacedName{Namespace: "test", Name: "test-gateway"} + gatewayMap := map[types.NamespacedName]*Gateway{ + gwNsName: { + Source: &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "test", + }, + Spec: v1.GatewaySpec{ + GatewayClassName: v1.ObjectName(gcName), + BackendTLS: &v1.GatewayBackendTLS{ + ClientCertificateRef: &v1.SecretObjectReference{ + Name: v1.ObjectName(secretRef.Name), + Namespace: (*v1.Namespace)(&secretRef.Namespace), + }, + }, + }, + }, + DeploymentName: deploymentName, + Conditions: cond, + Valid: valid, + }, + } + + if experimental { + gatewayMap[gwNsName].SecretRef = &secretRef + } + + if valid { + gatewayMap[gwNsName].Listeners = []*Listener{} + } + + return gatewayMap + } + + tests := []struct { + gw map[types.NamespacedName]*v1.Gateway + refGrants map[types.NamespacedName]*v1beta1.ReferenceGrant + secretResolver *secretResolver + expected map[types.NamespacedName]*Gateway + name string + experimental bool + }{ + { + name: "gateway with experimental enabled and tls.backend is not specified", + gw: map[types.NamespacedName]*v1.Gateway{ + {Namespace: "test", Name: "test-gateway"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "test", + }, + Spec: v1.GatewaySpec{ + GatewayClassName: v1.ObjectName(gcName), + }, + }, + }, + expected: map[types.NamespacedName]*Gateway{ + {Namespace: "test", Name: "test-gateway"}: { + Source: &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "test", + }, + Spec: v1.GatewaySpec{ + GatewayClassName: v1.ObjectName(gcName), + }, + }, + DeploymentName: deploymentName, + Listeners: []*Listener{}, + Valid: true, + }, + }, + experimental: false, + secretResolver: newSecretResolver(secrets), + }, + { + name: "gateway with experimental disabled and tls.backend is specified", + gw: createNewGatewayMap(secretSameNsKey), + expected: expectedGatewayMap( + secretSameNsKey, + conditions.NewGatewayUnsupportedValue( + "spec.tls: Forbidden: tls is not supported when experimental features are disabled", + ), + false, + false, + ), + experimental: false, + secretResolver: newSecretResolver(secrets), + }, + { + name: "gateway with experimental enabled, tls.backend is specified but secret reference is invalid", + gw: createNewGatewayMap(invalidSecretNsKey), + expected: expectedGatewayMap( + invalidSecretNsKey, + []conditions.Condition{conditions.NewGatewaySecretRefInvalid( + "backend.clientCertificateRef: " + + "Invalid value: {\"Namespace\":\"test\",\"Name\":\"invalid-secret\"}: secret does not exist", + )}, + false, + true, + ), + experimental: true, + secretResolver: newSecretResolver(secrets), + }, + { + name: "gateway with experimental enabled, tls.backend is specified but secret is not permitted by reference grant", + gw: createNewGatewayMap(secretDiffNsKey), + expected: expectedGatewayMap( + secretDiffNsKey, + []conditions.Condition{ + conditions.NewGatewaySecretRefNotPermitted( + fmt.Sprintf("secret ref %s not permitted by any ReferenceGrant", secretDiffNsKey), + ), + }, + false, + true, + ), + experimental: true, + secretResolver: newSecretResolver(secrets), + }, + { + name: "gateway with experimental enabled, tls.backend is specified secret in" + + "different namespace is permitted by reference grant", + gw: createNewGatewayMap(secretDiffNsKey), + expected: expectedGatewayMap( + secretDiffNsKey, + nil, + true, + true, + ), + refGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{ + {Namespace: "diff-ns", Name: "allow-secret-diff-ns"}: { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-gateway", + Name: "allow-secret-diff-ns", + }, + Spec: v1beta1.ReferenceGrantSpec{ + From: []v1beta1.ReferenceGrantFrom{ + { + Group: v1.GroupName, + Kind: kinds.Gateway, + Namespace: "test", + }, + }, + To: []v1beta1.ReferenceGrantTo{ + { + Group: "", + Kind: "Secret", + Name: helpers.GetPointer(v1.ObjectName(secretDiffNsKey.Name)), + }, + }, + }, + }, + }, + experimental: true, + secretResolver: newSecretResolver(secrets), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + validGC := &GatewayClass{ + Valid: true, + } + resolver := newReferenceGrantResolver(test.refGrants) + gateways := buildGateways(test.gw, test.secretResolver, validGC, resolver, nil, test.experimental) + g.Expect(helpers.Diff(test.expected, gateways)).To(BeEmpty()) + }) + } +} diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index e556c798ba..4ff69e83dc 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -200,6 +200,7 @@ func BuildGraph( plusSecrets map[types.NamespacedName][]PlusSecretFile, validators validation.Validators, logger logr.Logger, + experimentalFeatures bool, ) *Graph { processedGwClasses, gcExists := processGatewayClasses(state.GatewayClasses, gcName, controllerName) if gcExists && processedGwClasses.Winner == nil { @@ -232,6 +233,7 @@ func BuildGraph( gc, refGrantResolver, processedNginxProxies, + experimentalFeatures, ) processedBackendTLSPolicies := processBackendTLSPolicies( diff --git a/internal/controller/state/graph/graph_test.go b/internal/controller/state/graph/graph_test.go index ac5cfff3a2..b10448718b 100644 --- a/internal/controller/state/graph/graph_test.go +++ b/internal/controller/state/graph/graph_test.go @@ -1269,19 +1269,22 @@ func TestBuildGraph(t *testing.T) { } tests := []struct { - store ClusterState - expected *Graph - name string + store ClusterState + expected *Graph + name string + experimental bool }{ { - store: createStateWithGatewayClass(normalGC), - expected: createExpectedGraphWithGatewayClass(normalGC), - name: "normal case", + store: createStateWithGatewayClass(normalGC), + expected: createExpectedGraphWithGatewayClass(normalGC), + experimental: false, + name: "normal case", }, { - store: createStateWithGatewayClass(differentControllerGC), - expected: &Graph{}, - name: "gatewayclass belongs to a different controller", + store: createStateWithGatewayClass(differentControllerGC), + expected: &Graph{}, + experimental: true, + name: "gatewayclass belongs to a different controller", }, } @@ -1312,6 +1315,7 @@ func TestBuildGraph(t *testing.T) { PolicyValidator: fakePolicyValidator, }, logr.Discard(), + test.experimental, ) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) diff --git a/internal/controller/state/graph/multiple_gateways_test.go b/internal/controller/state/graph/multiple_gateways_test.go index 80f4a10b04..667ae850bc 100644 --- a/internal/controller/state/graph/multiple_gateways_test.go +++ b/internal/controller/state/graph/multiple_gateways_test.go @@ -400,6 +400,7 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) { PolicyValidator: fakePolicyValidator, }, logr.Discard(), + false, ) g.Expect(helpers.Diff(test.expGraph, result)).To(BeEmpty()) @@ -889,6 +890,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) { PolicyValidator: fakePolicyValidator, }, logr.Discard(), + false, ) g.Expect(helpers.Diff(test.expGraph, result)).To(BeEmpty()) From ac47d399bfd21f9dc9ff5339a9e21d4bdf654229 Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Fri, 12 Sep 2025 10:22:20 -0600 Subject: [PATCH 2/5] update unit tests --- internal/controller/state/graph/gateway_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/state/graph/gateway_test.go b/internal/controller/state/graph/gateway_test.go index 3c915844f3..8db42a65e8 100644 --- a/internal/controller/state/graph/gateway_test.go +++ b/internal/controller/state/graph/gateway_test.go @@ -2013,7 +2013,7 @@ func TestGateway_BackendTLSConfig(t *testing.T) { refGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{ {Namespace: "diff-ns", Name: "allow-secret-diff-ns"}: { ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-gateway", + Namespace: "diff-ns", Name: "allow-secret-diff-ns", }, Spec: v1beta1.ReferenceGrantSpec{ From f627dae2194136c4588584ba899f5ed3c230baed Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Tue, 16 Sep 2025 11:02:13 -0600 Subject: [PATCH 3/5] code reviews --- internal/controller/state/graph/gateway.go | 3 +-- internal/controller/state/graph/multiple_gateways_test.go | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/controller/state/graph/gateway.go b/internal/controller/state/graph/gateway.go index 459f6165fd..33448763f3 100644 --- a/internal/controller/state/graph/gateway.go +++ b/internal/controller/state/graph/gateway.go @@ -213,8 +213,7 @@ func validateGateway( conds = append(conds, conditions.NewGatewayUnsupportedValue(valErr.Error())...) } else { secretNsName, secretNs := getGatewayCertSecretNsName(gw) - err := secretResolver.resolve(*secretNsName) - if err != nil { + if err := secretResolver.resolve(*secretNsName); err != nil { path := field.NewPath("backend.clientCertificateRef") valErr := field.Invalid(path, secretNsName, err.Error()) conds = append(conds, conditions.NewGatewaySecretRefInvalid(valErr.Error())) diff --git a/internal/controller/state/graph/multiple_gateways_test.go b/internal/controller/state/graph/multiple_gateways_test.go index 667ae850bc..d9ebd8c97b 100644 --- a/internal/controller/state/graph/multiple_gateways_test.go +++ b/internal/controller/state/graph/multiple_gateways_test.go @@ -65,6 +65,8 @@ var ( {Kind: kinds.TLSRoute, Group: helpers.GetPointer[gatewayv1.Group](gatewayv1.GroupName)}, }, } + + experimentalFeaturesEnabled = false ) func createGateway(name, namespace, nginxProxyName string, listeners []gatewayv1.Listener) *gatewayv1.Gateway { @@ -400,7 +402,7 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) { PolicyValidator: fakePolicyValidator, }, logr.Discard(), - false, + experimentalFeaturesEnabled, ) g.Expect(helpers.Diff(test.expGraph, result)).To(BeEmpty()) @@ -890,7 +892,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) { PolicyValidator: fakePolicyValidator, }, logr.Discard(), - false, + experimentalFeaturesEnabled, ) g.Expect(helpers.Diff(test.expGraph, result)).To(BeEmpty()) From ad0583d54a5080060333040fc8eb5355b128aadd Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Thu, 18 Sep 2025 10:19:00 -0600 Subject: [PATCH 4/5] update test paramaters name --- internal/controller/state/graph/graph_test.go | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/controller/state/graph/graph_test.go b/internal/controller/state/graph/graph_test.go index b10448718b..e2c8684064 100644 --- a/internal/controller/state/graph/graph_test.go +++ b/internal/controller/state/graph/graph_test.go @@ -1269,22 +1269,22 @@ func TestBuildGraph(t *testing.T) { } tests := []struct { - store ClusterState - expected *Graph - name string - experimental bool + store ClusterState + expected *Graph + name string + experimentalEnabled bool }{ { - store: createStateWithGatewayClass(normalGC), - expected: createExpectedGraphWithGatewayClass(normalGC), - experimental: false, - name: "normal case", + store: createStateWithGatewayClass(normalGC), + expected: createExpectedGraphWithGatewayClass(normalGC), + experimentalEnabled: false, + name: "normal case", }, { - store: createStateWithGatewayClass(differentControllerGC), - expected: &Graph{}, - experimental: true, - name: "gatewayclass belongs to a different controller", + store: createStateWithGatewayClass(differentControllerGC), + expected: &Graph{}, + experimentalEnabled: true, + name: "gatewayclass belongs to a different controller", }, } @@ -1315,7 +1315,7 @@ func TestBuildGraph(t *testing.T) { PolicyValidator: fakePolicyValidator, }, logr.Discard(), - test.experimental, + test.experimentalEnabled, ) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) From 6971026eb475b4b396f8985affc32b678204049f Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Mon, 22 Sep 2025 14:17:29 -0600 Subject: [PATCH 5/5] add condition back and update graph tests --- .../controller/state/conditions/conditions.go | 19 ++++++++ internal/controller/state/graph/graph_test.go | 45 ++++++++++++++++--- 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/internal/controller/state/conditions/conditions.go b/internal/controller/state/conditions/conditions.go index 44f4232a2d..148418f589 100644 --- a/internal/controller/state/conditions/conditions.go +++ b/internal/controller/state/conditions/conditions.go @@ -854,6 +854,25 @@ func NewGatewayInvalid(msg string) []Condition { } } +// NewGatewayUnsupportedValue returns Conditions that indicate that a field of the Gateway has an unsupported value. +// Unsupported means that the value is not supported by the implementation or invalid. +func NewGatewayUnsupportedValue(msg string) []Condition { + return []Condition{ + { + Type: string(v1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(GatewayReasonUnsupportedValue), + Message: msg, + }, + { + Type: string(v1.GatewayConditionProgrammed), + Status: metav1.ConditionFalse, + Reason: string(GatewayReasonUnsupportedValue), + Message: msg, + }, + } +} + // NewGatewayUnsupportedAddress returns a Condition that indicates the Gateway is not accepted because it // contains an address type that is not supported. func NewGatewayUnsupportedAddress(msg string) Condition { diff --git a/internal/controller/state/graph/graph_test.go b/internal/controller/state/graph/graph_test.go index e2c8684064..6b09b98b62 100644 --- a/internal/controller/state/graph/graph_test.go +++ b/internal/controller/state/graph/graph_test.go @@ -363,6 +363,21 @@ func TestBuildGraph(t *testing.T) { }, } + gatewaySecret := &v1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNs, + Name: "gateway-secret", + }, + Data: map[string][]byte{ + v1.TLSCertKey: cert, + v1.TLSPrivateKeyKey: key, + }, + Type: v1.SecretTypeTLS, + } + ns := &v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: testNs, @@ -442,6 +457,13 @@ func TestBuildGraph(t *testing.T) { TLS: &gatewayv1.GatewayTLSConfig{Mode: helpers.GetPointer(gatewayv1.TLSModePassthrough)}, }, }, + BackendTLS: &gatewayv1.GatewayBackendTLS{ + ClientCertificateRef: &gatewayv1.SecretObjectReference{ + Kind: helpers.GetPointer[gatewayv1.Kind]("Secret"), + Name: gatewayv1.ObjectName(gatewaySecret.Name), + Namespace: helpers.GetPointer(gatewayv1.Namespace(gatewaySecret.Namespace)), + }, + }, }, }, } @@ -712,8 +734,9 @@ func TestBuildGraph(t *testing.T) { client.ObjectKeyFromObject(grToServiceNsRefGrant): grToServiceNsRefGrant, }, Secrets: map[types.NamespacedName]*v1.Secret{ - client.ObjectKeyFromObject(secret): secret, - client.ObjectKeyFromObject(plusSecret): plusSecret, + client.ObjectKeyFromObject(secret): secret, + client.ObjectKeyFromObject(plusSecret): plusSecret, + client.ObjectKeyFromObject(gatewaySecret): gatewaySecret, }, BackendTLSPolicies: map[types.NamespacedName]*v1alpha3.BackendTLSPolicy{ client.ObjectKeyFromObject(btp.Source): btp.Source, @@ -1093,6 +1116,7 @@ func TestBuildGraph(t *testing.T) { Namespace: "test", Name: "gateway-1-my-class", }, + SecretRef: helpers.GetPointer(client.ObjectKeyFromObject(gatewaySecret)), }, {Namespace: testNs, Name: "gateway-2"}: { Source: gw2.Source, @@ -1169,6 +1193,7 @@ func TestBuildGraph(t *testing.T) { Namespace: "test", Name: "gateway-2-my-class", }, + SecretRef: helpers.GetPointer(client.ObjectKeyFromObject(gatewaySecret)), }, }, Routes: map[RouteKey]*L7Route{ @@ -1188,6 +1213,13 @@ func TestBuildGraph(t *testing.T) { TLSPrivateKey: key, }), }, + client.ObjectKeyFromObject(gatewaySecret): { + Source: gatewaySecret, + CertBundle: NewCertificateBundle(client.ObjectKeyFromObject(gatewaySecret), "Secret", &Certificate{ + TLSCert: cert, + TLSPrivateKey: key, + }), + }, }, ReferencedNamespaces: map[types.NamespacedName]*v1.Namespace{ client.ObjectKeyFromObject(ns): ns, @@ -1277,14 +1309,13 @@ func TestBuildGraph(t *testing.T) { { store: createStateWithGatewayClass(normalGC), expected: createExpectedGraphWithGatewayClass(normalGC), - experimentalEnabled: false, + experimentalEnabled: true, name: "normal case", }, { - store: createStateWithGatewayClass(differentControllerGC), - expected: &Graph{}, - experimentalEnabled: true, - name: "gatewayclass belongs to a different controller", + store: createStateWithGatewayClass(differentControllerGC), + expected: &Graph{}, + name: "gatewayclass belongs to a different controller", }, }