Skip to content

Commit f27ad17

Browse files
SkceySkcey
committed
optimize the code
1 parent c64e34c commit f27ad17

File tree

6 files changed

+69
-59
lines changed

6 files changed

+69
-59
lines changed

‎internal/controller/manager_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
9393
partialObjectMetadataList,
9494
&gatewayv1alpha3.BackendTLSPolicyList{},
9595
&gatewayv1alpha2.TLSRouteList{},
96+
&gatewayv1alpha2.TCPRouteList{},
97+
&gatewayv1alpha2.UDPRouteList{},
9698
&gatewayv1.GRPCRouteList{},
9799
&ngfAPIv1alpha1.ClientSettingsPolicyList{},
98100
&ngfAPIv1alpha2.ObservabilityPolicyList{},
@@ -149,6 +151,8 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {
149151
partialObjectMetadataList,
150152
&gatewayv1alpha3.BackendTLSPolicyList{},
151153
&gatewayv1alpha2.TLSRouteList{},
154+
&gatewayv1alpha2.TCPRouteList{},
155+
&gatewayv1alpha2.UDPRouteList{},
152156
&gatewayv1.GRPCRouteList{},
153157
&ngfAPIv1alpha1.ClientSettingsPolicyList{},
154158
&ngfAPIv1alpha2.ObservabilityPolicyList{},

‎internal/controller/nginx/agent/agent.go‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,10 @@ func (n *NginxUpdaterImpl) UpdateUpstreamServers(
119119

120120
var errs []error
121121
var applied bool
122-
actions := make([]*pb.NGINXPlusAction, 0, len(conf.Upstreams)+len(conf.StreamUpstreams))
122+
actions := make([]*pb.NGINXPlusAction, 0,
123+
len(conf.Upstreams)+len(conf.StreamUpstreams)+len(conf.TCPUpstreams)+len(conf.UDPUpstreams))
124+
125+
// HTTP/GRPC Upstreams
123126
for _, upstream := range conf.Upstreams {
124127
// Skip upstreams that have resolve servers to avoid "UpstreamServerImmutable" errors
125128
if upstreamHasResolveServers(upstream) {
@@ -133,6 +136,7 @@ func (n *NginxUpdaterImpl) UpdateUpstreamServers(
133136
actions = append(actions, action)
134137
}
135138

139+
// TLS Passthrough Upstreams
136140
for _, upstream := range conf.StreamUpstreams {
137141
// Skip upstreams that have resolve servers to avoid "UpstreamServerImmutable" errors
138142
if upstreamHasResolveServers(upstream) {
@@ -146,6 +150,34 @@ func (n *NginxUpdaterImpl) UpdateUpstreamServers(
146150
actions = append(actions, action)
147151
}
148152

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 {
169+
// Skip upstreams that have resolve servers to avoid "UpstreamServerImmutable" errors
170+
if upstreamHasResolveServers(upstream) {
171+
continue
172+
}
173+
action := &pb.NGINXPlusAction{
174+
Action: &pb.NGINXPlusAction_UpdateStreamServers{
175+
UpdateStreamServers: buildStreamUpstreamServers(upstream),
176+
},
177+
}
178+
actions = append(actions, action)
179+
}
180+
149181
if actionsEqual(deployment.GetNGINXPlusActions(), actions) {
150182
return
151183
}

‎internal/controller/nginx/config/upstreams.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,13 @@ func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstre
115115
if ep.IPv6 {
116116
format = "[%s]:%d"
117117
}
118-
// Default weight to 1 if not specified
119-
weight := ep.Weight
120-
if weight == 0 {
121-
weight = 1
122-
}
118+
// Keep the original weight from endpoint
119+
// For single backend: Weight is 0 (template won't output weight directive)
120+
// For multi-backend: Weight is set from BackendRef.Weight (template outputs weight=X if > 1)
123121
upstreamServers[idx] = stream.UpstreamServer{
124122
Address: fmt.Sprintf(format, ep.Address, ep.Port),
125123
Resolve: ep.Resolve,
126-
Weight: weight,
124+
Weight: ep.Weight,
127125
}
128126
}
129127

‎internal/controller/state/dataplane/configuration.go‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ func BuildConfiguration(
7474
HTTPServers: httpServers,
7575
SSLServers: sslServers,
7676
TLSPassthroughServers: buildPassthroughServers(gateway),
77-
TCPServers: buildTCPServers(logger, gateway),
78-
UDPServers: buildUDPServers(logger, gateway),
77+
TCPServers: buildL4Servers(logger, gateway, v1.TCPProtocolType),
78+
UDPServers: buildL4Servers(logger, gateway, v1.UDPProtocolType),
7979
Upstreams: upstreams,
8080
StreamUpstreams: buildStreamUpstreams(
8181
ctx,
@@ -84,8 +84,8 @@ func BuildConfiguration(
8484
serviceResolver,
8585
g.ReferencedServices,
8686
baseHTTPConfig.IPFamily),
87-
TCPUpstreams: buildTCPUpstreams(ctx, logger, gateway, serviceResolver, baseHTTPConfig.IPFamily),
88-
UDPUpstreams: buildUDPUpstreams(ctx, logger, gateway, serviceResolver, baseHTTPConfig.IPFamily),
87+
TCPUpstreams: buildL4Upstreams(ctx, logger, gateway, serviceResolver, baseHTTPConfig.IPFamily, v1.TCPProtocolType),
88+
UDPUpstreams: buildL4Upstreams(ctx, logger, gateway, serviceResolver, baseHTTPConfig.IPFamily, v1.UDPProtocolType),
8989
BackendGroups: backendGroups,
9090
SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, gateway.Listeners),
9191
CertBundles: buildCertBundles(
@@ -238,16 +238,6 @@ func buildL4Servers(logger logr.Logger, gateway *graph.Gateway, protocol v1.Prot
238238
return servers
239239
}
240240

241-
// buildTCPServers builds TCPServers from TCPRoutes attached to listeners.
242-
func buildTCPServers(logger logr.Logger, gateway *graph.Gateway) []Layer4VirtualServer {
243-
return buildL4Servers(logger, gateway, v1.TCPProtocolType)
244-
}
245-
246-
// buildUDPServers builds UDPServers from UDPRoutes attached to listeners.
247-
func buildUDPServers(logger logr.Logger, gateway *graph.Gateway) []Layer4VirtualServer {
248-
return buildL4Servers(logger, gateway, v1.UDPProtocolType)
249-
}
250-
251241
// buildStreamUpstreams builds all stream upstreams.
252242
func buildStreamUpstreams(
253243
ctx context.Context,
@@ -452,28 +442,6 @@ func buildL4Upstreams(
452442
return upstreams
453443
}
454444

455-
// buildTCPUpstreams builds all TCP upstreams.
456-
func buildTCPUpstreams(
457-
ctx context.Context,
458-
logger logr.Logger,
459-
gateway *graph.Gateway,
460-
serviceResolver resolver.ServiceResolver,
461-
ipFamily IPFamilyType,
462-
) []Upstream {
463-
return buildL4Upstreams(ctx, logger, gateway, serviceResolver, ipFamily, v1.TCPProtocolType)
464-
}
465-
466-
// buildUDPUpstreams builds all UDP upstreams.
467-
func buildUDPUpstreams(
468-
ctx context.Context,
469-
logger logr.Logger,
470-
gateway *graph.Gateway,
471-
serviceResolver resolver.ServiceResolver,
472-
ipFamily IPFamilyType,
473-
) []Upstream {
474-
return buildL4Upstreams(ctx, logger, gateway, serviceResolver, ipFamily, v1.UDPProtocolType)
475-
}
476-
477445
// buildSSLKeyPairs builds the SSLKeyPairs from the Secrets. It will only include Secrets that are referenced by
478446
// valid listeners, so that we don't include unused Secrets in the configuration of the data plane.
479447
func buildSSLKeyPairs(

‎internal/controller/state/graph/gateway_listener.go‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func newListenerConfiguratorFactory(
151151
validators: []listenerValidator{
152152
validateListenerAllowedRouteKind,
153153
validateListenerLabelSelector,
154-
createTCPListenerValidator(protectedPorts),
154+
createL4ListenerValidator(v1.TCPProtocolType, protectedPorts),
155155
},
156156
conflictResolvers: []listenerConflictResolver{
157157
sharedPortConflictResolver,
@@ -161,7 +161,7 @@ func newListenerConfiguratorFactory(
161161
validators: []listenerValidator{
162162
validateListenerAllowedRouteKind,
163163
validateListenerLabelSelector,
164-
createUDPListenerValidator(protectedPorts),
164+
createL4ListenerValidator(v1.UDPProtocolType, protectedPorts),
165165
},
166166
conflictResolvers: []listenerConflictResolver{
167167
sharedPortConflictResolver,
@@ -487,13 +487,14 @@ func createPortConflictResolver() listenerConflictResolver {
487487
const (
488488
secureProtocolGroup int = 0
489489
insecureProtocolGroup int = 1
490+
l4ProtocolGroup int = 2
490491
)
491492
protocolGroups := map[v1.ProtocolType]int{
492493
v1.TLSProtocolType: secureProtocolGroup,
493494
v1.HTTPProtocolType: insecureProtocolGroup,
494495
v1.HTTPSProtocolType: secureProtocolGroup,
495-
v1.TCPProtocolType: insecureProtocolGroup,
496-
v1.UDPProtocolType: insecureProtocolGroup,
496+
v1.TCPProtocolType: l4ProtocolGroup,
497+
v1.UDPProtocolType: l4ProtocolGroup,
497498
}
498499
conflictedPorts := make(map[v1.PortNumber]bool)
499500
portProtocolOwner := make(map[v1.PortNumber]int)
@@ -505,6 +506,8 @@ func createPortConflictResolver() listenerConflictResolver {
505506
formatHostname := "HTTPS and TLS listeners for the same port %d specify overlapping hostnames; " +
506507
"ensure no overlapping hostnames for HTTPS and TLS listeners for the same port"
507508

509+
formatL4SameProtocol := "Multiple %s listeners cannot share the same port %d"
510+
508511
return func(l *Listener) {
509512
port := l.Source.Port
510513

@@ -542,6 +545,14 @@ func createPortConflictResolver() listenerConflictResolver {
542545
} else {
543546
foundConflict := false
544547
for _, listener := range listenersByPort[port] {
548+
if isL4Protocol(l.Source.Protocol) &&
549+
listener.Source.Protocol == l.Source.Protocol {
550+
listener.Valid = false
551+
conflictedConds := conditions.NewListenerProtocolConflict(
552+
fmt.Sprintf(formatL4SameProtocol, l.Source.Protocol, port))
553+
listener.Conditions = append(listener.Conditions, conflictedConds...)
554+
foundConflict = true
555+
}
545556
if listener.Source.Protocol != l.Source.Protocol &&
546557
!isL4Protocol(listener.Source.Protocol) && !isL4Protocol(l.Source.Protocol) &&
547558
haveOverlap(l.Source.Hostname, listener.Source.Hostname) {
@@ -554,8 +565,14 @@ func createPortConflictResolver() listenerConflictResolver {
554565

555566
if foundConflict {
556567
l.Valid = false
557-
conflictedConds := conditions.NewListenerHostnameConflict(fmt.Sprintf(formatHostname, port))
558-
l.Conditions = append(l.Conditions, conflictedConds...)
568+
if isL4Protocol(l.Source.Protocol) {
569+
conflictedConds := conditions.NewListenerProtocolConflict(
570+
fmt.Sprintf(formatL4SameProtocol, l.Source.Protocol, port))
571+
l.Conditions = append(l.Conditions, conflictedConds...)
572+
} else {
573+
conflictedConds := conditions.NewListenerHostnameConflict(fmt.Sprintf(formatHostname, port))
574+
l.Conditions = append(l.Conditions, conflictedConds...)
575+
}
559576
}
560577
}
561578

@@ -668,14 +685,6 @@ func createL4ListenerValidator(protocol v1.ProtocolType, protectedPorts Protecte
668685
}
669686
}
670687

671-
func createTCPListenerValidator(protectedPorts ProtectedPorts) listenerValidator {
672-
return createL4ListenerValidator(v1.TCPProtocolType, protectedPorts)
673-
}
674-
675-
func createUDPListenerValidator(protectedPorts ProtectedPorts) listenerValidator {
676-
return createL4ListenerValidator(v1.UDPProtocolType, protectedPorts)
677-
}
678-
679688
func createOverlappingTLSConfigResolver() listenerConflictResolver {
680689
listenersByPort := make(map[v1.PortNumber][]*Listener)
681690

‎internal/controller/state/graph/route_common.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,7 +1283,7 @@ func buildGenericL4Route(
12831283

12841284
for refIdx, ref := range rule.backendRefs {
12851285
br, conds := validateBackendRefL4RouteMulti(
1286-
config.namespace, config.routeType, ref, services, r.ParentRefs,
1286+
config.namespace, ref, services, r.ParentRefs,
12871287
config.refGrantResolver, ruleIdx, refIdx,
12881288
)
12891289
allBackendRefs = append(allBackendRefs, br)
@@ -1307,7 +1307,6 @@ func buildGenericL4Route(
13071307
// This eliminates code duplication between validateBackendRefTCPRouteMulti and validateBackendRefUDPRouteMulti.
13081308
func validateBackendRefL4RouteMulti(
13091309
namespace string,
1310-
routeType string, // "TCP" or "UDP"
13111310
ref v1alpha.BackendRef,
13121311
services map[types.NamespacedName]*apiv1.Service,
13131312
parentRefs []ParentRef,

0 commit comments

Comments
 (0)