Skip to content

Commit dd93fc8

Browse files
committed
resolve the linting errors
1 parent b3427b6 commit dd93fc8

File tree

9 files changed

+128
-180
lines changed

9 files changed

+128
-180
lines changed

internal/controller/nginx/agent/agent.go

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -136,36 +136,9 @@ func (n *NginxUpdaterImpl) UpdateUpstreamServers(
136136
actions = append(actions, action)
137137
}
138138

139-
// TLS Passthrough Upstreams
140-
for _, upstream := range conf.StreamUpstreams {
141-
// Skip upstreams that have resolve servers to avoid "UpstreamServerImmutable" errors
142-
if upstreamHasResolveServers(upstream) {
143-
continue
144-
}
145-
action := &pb.NGINXPlusAction{
146-
Action: &pb.NGINXPlusAction_UpdateStreamServers{
147-
UpdateStreamServers: buildStreamUpstreamServers(upstream),
148-
},
149-
}
150-
actions = append(actions, action)
151-
}
152-
153-
// TCP Upstreams
154-
for _, upstream := range conf.TCPUpstreams {
155-
// Skip upstreams that have resolve servers to avoid "UpstreamServerImmutable" errors
156-
if upstreamHasResolveServers(upstream) {
157-
continue
158-
}
159-
action := &pb.NGINXPlusAction{
160-
Action: &pb.NGINXPlusAction_UpdateStreamServers{
161-
UpdateStreamServers: buildStreamUpstreamServers(upstream),
162-
},
163-
}
164-
actions = append(actions, action)
165-
}
166-
167-
// UDP Upstreams
168-
for _, upstream := range conf.UDPUpstreams {
139+
// TLS Passthrough Upstreams, TCP Upstreams, UDP Upstreams
140+
allStreamUpstreams := append(append(conf.StreamUpstreams, conf.TCPUpstreams...), conf.UDPUpstreams...)
141+
for _, upstream := range allStreamUpstreams {
169142
// Skip upstreams that have resolve servers to avoid "UpstreamServerImmutable" errors
170143
if upstreamHasResolveServers(upstream) {
171144
continue

internal/controller/nginx/config/stream/config.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ import (
77

88
// Server holds all configuration for a stream server.
99
type Server struct {
10+
UDPConfig *UDPConfig
1011
Listen string
1112
StatusZone string
1213
ProxyPass string
1314
Pass string
15+
Protocol string
1416
RewriteClientIP shared.RewriteClientIPSettings
1517
SSLPreread bool
1618
IsSocket bool
17-
Protocol string
18-
UDPConfig *UDPConfig
1919
}
2020

2121
type UDPConfig struct {

internal/controller/nginx/config/stream_servers.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
gotemplate "text/template"
66

77
"github.com/go-logr/logr"
8+
89
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared"
910
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/stream"
1011
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane"

internal/controller/nginx/config/upstreams_template.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ upstream {{ $u.Name }} {
4949
state {{ $u.StateFile }};
5050
{{- else }}
5151
{{ range $server := $u.Servers }}
52-
server {{ $server.Address }}{{ if ne $server.Weight 0 }}{{ if ne $server.Weight 1 }} weight={{ $server.Weight }}{{ end }}{{ end }}{{ if $server.Resolve }} resolve{{ end }};
52+
server {{ $server.Address }}
53+
{{- if ne $server.Weight 0 }}{{ if ne $server.Weight 1 }} weight={{ $server.Weight }}{{ end }}{{ end }}
54+
{{- if $server.Resolve }} resolve{{ end }};
5355
{{- end }}
5456
{{- end }}
5557
}

internal/controller/state/dataplane/configuration.go

Lines changed: 73 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -210,20 +210,12 @@ func buildL4Servers(logger logr.Logger, gateway *graph.Gateway, protocol v1.Prot
210210
continue
211211
}
212212

213-
// For single backend, use direct upstream name
214-
// For multiple backends, we'll create a combined upstream name based on the route
215-
var upstreamName string
216-
if len(backendRefs) == 1 {
217-
upstreamName = backendRefs[0].ServicePortReference()
218-
} else {
219-
// For multiple backends, create a group upstream name
220-
// Format: protocol_namespace_routename
221-
upstreamName = fmt.Sprintf("%s_%s_%s",
222-
protocolName,
223-
r.Source.GetNamespace(),
224-
r.Source.GetName(),
225-
)
226-
}
213+
// Use unified upstream naming: protocol_namespace_routename
214+
upstreamName := fmt.Sprintf("%s_%s_%s",
215+
protocolName,
216+
r.Source.GetNamespace(),
217+
r.Source.GetName(),
218+
)
227219

228220
server := Layer4VirtualServer{
229221
Hostname: "", // Layer4 doesn't use hostnames
@@ -324,122 +316,96 @@ func buildL4Upstreams(
324316
protocol v1.ProtocolType,
325317
) []Upstream {
326318
uniqueUpstreams := make(map[string]Upstream)
327-
328319
protocolName := string(protocol)
329320
gatewayNSName := client.ObjectKeyFromObject(gateway.Source)
321+
allowedAddressType := getAllowedAddressType(ipFamily)
330322

331323
for _, l := range gateway.Listeners {
332324
if !l.Valid || l.Source.Protocol != protocol {
333325
continue
334326
}
327+
processListenerRoutes(ctx, logger, l.L4Routes, protocolName, gatewayNSName,
328+
allowedAddressType, serviceResolver, uniqueUpstreams)
329+
}
335330

336-
for _, route := range l.L4Routes {
337-
if !route.Valid {
338-
continue
339-
}
340-
341-
// Use helper method to get all backend references
342-
backendRefs := route.Spec.GetBackendRefs()
343-
344-
if len(backendRefs) == 0 {
345-
continue
346-
}
347-
348-
// For single backend: create one upstream with service name
349-
// For multiple backends: create individual upstreams + one combined upstream with weighted endpoints
350-
if len(backendRefs) == 1 {
351-
br := backendRefs[0]
352-
if !br.Valid {
353-
continue
354-
}
355-
356-
if _, ok := br.InvalidForGateways[gatewayNSName]; ok {
357-
continue
358-
}
359-
360-
upstreamName := br.ServicePortReference()
361-
362-
if _, exist := uniqueUpstreams[upstreamName]; exist {
363-
continue
364-
}
365-
366-
var errMsg string
367-
allowedAddressType := getAllowedAddressType(ipFamily)
331+
if len(uniqueUpstreams) == 0 {
332+
return nil
333+
}
334+
upstreams := make([]Upstream, 0, len(uniqueUpstreams))
335+
for _, up := range uniqueUpstreams {
336+
upstreams = append(upstreams, up)
337+
}
368338

369-
eps, err := serviceResolver.Resolve(ctx, logger, br.SvcNsName, br.ServicePort, allowedAddressType)
370-
if err != nil {
371-
errMsg = err.Error()
372-
}
339+
return upstreams
340+
}
373341

374-
uniqueUpstreams[upstreamName] = Upstream{
375-
Name: upstreamName,
376-
Endpoints: eps,
377-
ErrorMsg: errMsg,
378-
}
379-
} else {
380-
// Multiple backends: create a combined upstream with weighted endpoints
381-
combinedUpstreamName := fmt.Sprintf("%s_%s_%s",
382-
protocolName,
383-
route.Source.GetNamespace(),
384-
route.Source.GetName(),
385-
)
342+
func processListenerRoutes(
343+
ctx context.Context,
344+
logger logr.Logger,
345+
routes map[graph.L4RouteKey]*graph.L4Route,
346+
protocolName string,
347+
gatewayNSName types.NamespacedName,
348+
allowedAddressType []discoveryV1.AddressType,
349+
serviceResolver resolver.ServiceResolver,
350+
uniqueUpstreams map[string]Upstream,
351+
) {
352+
for _, route := range routes {
353+
if !route.Valid {
354+
continue
355+
}
386356

387-
if _, exist := uniqueUpstreams[combinedUpstreamName]; exist {
388-
continue
389-
}
357+
backendRefs := route.Spec.GetBackendRefs()
358+
if len(backendRefs) == 0 {
359+
continue
360+
}
390361

391-
var combinedEndpoints []resolver.Endpoint
392-
var errMsgs []string
393-
allowedAddressType := getAllowedAddressType(ipFamily)
362+
upstreamName := fmt.Sprintf("%s_%s_%s",
363+
protocolName,
364+
route.Source.GetNamespace(),
365+
route.Source.GetName(),
366+
)
394367

395-
// Collect endpoints from all backends with their weights
396-
for _, br := range backendRefs {
397-
if !br.Valid {
398-
continue
399-
}
368+
if _, exist := uniqueUpstreams[upstreamName]; exist {
369+
continue
370+
}
400371

401-
if _, ok := br.InvalidForGateways[gatewayNSName]; ok {
402-
continue
403-
}
372+
var endpoints []resolver.Endpoint
373+
var errMsgs []string
404374

405-
eps, err := serviceResolver.Resolve(ctx, logger, br.SvcNsName, br.ServicePort, allowedAddressType)
406-
if err != nil {
407-
errMsgs = append(errMsgs, err.Error())
408-
continue
409-
}
375+
// Collect endpoints from all backends with their weights
376+
for _, br := range backendRefs {
377+
if !br.Valid {
378+
continue
379+
}
410380

411-
// Add weight to each endpoint
412-
for _, ep := range eps {
413-
ep.Weight = br.Weight
414-
combinedEndpoints = append(combinedEndpoints, ep)
415-
}
416-
}
381+
if _, ok := br.InvalidForGateways[gatewayNSName]; ok {
382+
continue
383+
}
417384

418-
var errMsg string
419-
if len(errMsgs) > 0 {
420-
errMsg = fmt.Sprintf("some backends failed: %v", errMsgs)
421-
}
385+
eps, err := serviceResolver.Resolve(ctx, logger, br.SvcNsName, br.ServicePort, allowedAddressType)
386+
if err != nil {
387+
errMsgs = append(errMsgs, err.Error())
388+
continue
389+
}
422390

423-
uniqueUpstreams[combinedUpstreamName] = Upstream{
424-
Name: combinedUpstreamName,
425-
Endpoints: combinedEndpoints,
426-
ErrorMsg: errMsg,
427-
}
391+
// Add weight to each endpoint
392+
for _, ep := range eps {
393+
ep.Weight = br.Weight
394+
endpoints = append(endpoints, ep)
428395
}
429396
}
430-
}
431-
432-
if len(uniqueUpstreams) == 0 {
433-
return nil
434-
}
435397

436-
upstreams := make([]Upstream, 0, len(uniqueUpstreams))
398+
var errMsg string
399+
if len(errMsgs) > 0 {
400+
errMsg = fmt.Sprintf("some backends failed: %v", errMsgs)
401+
}
437402

438-
for _, up := range uniqueUpstreams {
439-
upstreams = append(upstreams, up)
403+
uniqueUpstreams[upstreamName] = Upstream{
404+
Name: upstreamName,
405+
Endpoints: endpoints,
406+
ErrorMsg: errMsg,
407+
}
440408
}
441-
442-
return upstreams
443409
}
444410

445411
// buildSSLKeyPairs builds the SSLKeyPairs from the Secrets. It will only include Secrets that are referenced by

internal/controller/state/graph/gateway_listener.go

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ func newListenerConfiguratorFactory(
102102
valErr := field.NotSupported(
103103
field.NewPath("protocol"),
104104
listener.Protocol,
105-
[]string{string(v1.HTTPProtocolType), string(v1.HTTPSProtocolType), string(v1.TLSProtocolType), string(v1.TCPProtocolType), string(v1.UDPProtocolType)},
105+
[]string{
106+
string(v1.HTTPProtocolType), string(v1.HTTPSProtocolType), string(v1.TLSProtocolType),
107+
string(v1.TCPProtocolType), string(v1.UDPProtocolType),
108+
},
106109
)
107110
return conditions.NewListenerUnsupportedProtocol(valErr.Error()), false /* not attachable */
108111
},
@@ -283,27 +286,7 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) (
283286
var conds []conditions.Condition
284287
var supportedKinds []v1.RouteGroupKind
285288

286-
var validKinds []v1.RouteGroupKind
287-
288-
switch listener.Protocol {
289-
case v1.HTTPProtocolType, v1.HTTPSProtocolType:
290-
validKinds = []v1.RouteGroupKind{
291-
{Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
292-
{Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
293-
}
294-
case v1.TLSProtocolType:
295-
validKinds = []v1.RouteGroupKind{
296-
{Kind: v1.Kind(kinds.TLSRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
297-
}
298-
case v1.TCPProtocolType:
299-
validKinds = []v1.RouteGroupKind{
300-
{Kind: v1.Kind(kinds.TCPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
301-
}
302-
case v1.UDPProtocolType:
303-
validKinds = []v1.RouteGroupKind{
304-
{Kind: v1.Kind(kinds.UDPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
305-
}
306-
}
289+
validKinds := getValidKindsForProtocol(listener.Protocol)
307290

308291
validProtocolRouteKind := func(kind v1.RouteGroupKind) bool {
309292
if kind.Group != nil && *kind.Group != v1.GroupName {
@@ -347,6 +330,31 @@ func getAndValidateListenerSupportedKinds(listener v1.Listener) (
347330
return conds, validKinds
348331
}
349332

333+
// getValidKindsForProtocol returns the valid route kinds for a given protocol.
334+
func getValidKindsForProtocol(protocol v1.ProtocolType) []v1.RouteGroupKind {
335+
switch protocol {
336+
case v1.HTTPProtocolType, v1.HTTPSProtocolType:
337+
return []v1.RouteGroupKind{
338+
{Kind: v1.Kind(kinds.HTTPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
339+
{Kind: v1.Kind(kinds.GRPCRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
340+
}
341+
case v1.TLSProtocolType:
342+
return []v1.RouteGroupKind{
343+
{Kind: v1.Kind(kinds.TLSRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
344+
}
345+
case v1.TCPProtocolType:
346+
return []v1.RouteGroupKind{
347+
{Kind: v1.Kind(kinds.TCPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
348+
}
349+
case v1.UDPProtocolType:
350+
return []v1.RouteGroupKind{
351+
{Kind: v1.Kind(kinds.UDPRoute), Group: helpers.GetPointer[v1.Group](v1.GroupName)},
352+
}
353+
default:
354+
return nil
355+
}
356+
}
357+
350358
func validateListenerAllowedRouteKind(listener v1.Listener) (conds []conditions.Condition, attachable bool) {
351359
conds, _ = getAndValidateListenerSupportedKinds(listener)
352360
return conds, len(conds) == 0
@@ -478,7 +486,7 @@ func createHTTPSListenerValidator(protectedPorts ProtectedPorts) listenerValidat
478486
}
479487
}
480488

481-
// isL4Protocol checks if the protocol is a Layer 4 protocol (TCP or UDP)
489+
// isL4Protocol checks if the protocol is a Layer 4 protocol (TCP or UDP).
482490
func isL4Protocol(protocol v1.ProtocolType) bool {
483491
return protocol == v1.TCPProtocolType || protocol == v1.UDPProtocolType
484492
}

0 commit comments

Comments
 (0)