Skip to content

reduce DeepCopy in gateway-api layer #6940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zirain merged 2 commits into from
Sep 12, 2025
Merged

reduce DeepCopy in gateway-api layer #6940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e2f81c
reduce deep copy in gateway-api layer
Sep 11, 2025
e4a08a4
fix lint
Sep 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions internal/gatewayapi/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
func (t *Translator) ProcessBackends(backends []*egv1a1.Backend, backendTLSPolicies []*gwapiv1a3.BackendTLSPolicy) []*egv1a1.Backend {
var res []*egv1a1.Backend
for _, backend := range backends {
backend := backend.DeepCopy()

// Ensure Backends are enabled
if !t.BackendEnabled {
status.UpdateBackendStatusAcceptedCondition(backend, false,
Expand Down
6 changes: 3 additions & 3 deletions internal/gatewayapi/backendtrafficpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (t *Translator) ProcessBackendTrafficPolicies(resources *resource.Resources
if currTarget.Kind != resource.KindGateway {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
Copy link
Copy Markdown
Member

@rudrakhp rudrakhp Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why needed a DeepCopy before, and why even do the shallow copy now? Are we modifying policy during the process?

Copy link
Copy Markdown
Contributor Author

@arkodg arkodg Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like its been there from the beginning
we dont need a deepCopy here because the gateway-api layer is free to update the object received from watchable and modify it, which it does ( modifies status)
note, there was a bug in DeepCopy implementation of ControllerResources that I fixed in this PR too which was keeping some shallow copy references around, that was needed to be changed to a pure DeepCopy for isolation between the provider layer and gateway-api layer

Copy link
Copy Markdown
Member

@rudrakhp rudrakhp Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the gateway-api layer is free to update the object received from watchable and modify it

Why not use currPolicy everywhere in that case? Can we remove the shallow copy policy = currPolicy? Unless this is intentional to avoid multiple changes. Approved as this is minor.

Copy link
Copy Markdown
Contributor Author

@arkodg arkodg Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure will try doing another round of clean up

handledPolicies[policyName] = policy
res = append(res, policy)
}
Expand Down Expand Up @@ -237,7 +237,7 @@ func (t *Translator) ProcessBackendTrafficPolicies(resources *resource.Resources
if currTarget.Kind == resource.KindGateway {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
handledPolicies[policyName] = policy
res = append(res, policy)
}
Expand Down Expand Up @@ -545,7 +545,7 @@ func applyTrafficFeatureToRoute(route RouteContext,

func mergeBackendTrafficPolicy(routePolicy, gwPolicy *egv1a1.BackendTrafficPolicy) (*egv1a1.BackendTrafficPolicy, error) {
if routePolicy.Spec.MergeType == nil || gwPolicy == nil {
return routePolicy.DeepCopy(), nil
return routePolicy, nil
}

return utils.Merge[*egv1a1.BackendTrafficPolicy](gwPolicy, routePolicy, *routePolicy.Spec.MergeType)
Expand Down
4 changes: 2 additions & 2 deletions internal/gatewayapi/clienttrafficpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (t *Translator) ProcessClientTrafficPolicies(
if hasSectionName(&currTarget) {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
handledPolicies[policyName] = policy
res = append(res, policy)
}
Expand Down Expand Up @@ -169,7 +169,7 @@ func (t *Translator) ProcessClientTrafficPolicies(

policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
res = append(res, policy)
handledPolicies[policyName] = policy
}
Expand Down
8 changes: 4 additions & 4 deletions internal/gatewayapi/envoyextensionpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (t *Translator) ProcessEnvoyExtensionPolicies(envoyExtensionPolicies []*egv
if currTarget.Kind != resource.KindGateway && currTarget.SectionName != nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
res = append(res, policy)
handledPolicies[policyName] = policy
}
Expand All @@ -99,7 +99,7 @@ func (t *Translator) ProcessEnvoyExtensionPolicies(envoyExtensionPolicies []*egv
if currTarget.Kind != resource.KindGateway && currTarget.SectionName == nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
res = append(res, policy)
handledPolicies[policyName] = policy
}
Expand All @@ -118,7 +118,7 @@ func (t *Translator) ProcessEnvoyExtensionPolicies(envoyExtensionPolicies []*egv
if currTarget.Kind == resource.KindGateway && currTarget.SectionName != nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
res = append(res, policy)
handledPolicies[policyName] = policy
}
Expand All @@ -137,7 +137,7 @@ func (t *Translator) ProcessEnvoyExtensionPolicies(envoyExtensionPolicies []*egv
if currTarget.Kind == resource.KindGateway && currTarget.SectionName == nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
res = append(res, policy)
handledPolicies[policyName] = policy
}
Expand Down
2 changes: 1 addition & 1 deletion internal/gatewayapi/envoypatchpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo

for _, policy := range envoyPatchPolicies {
var (
policy = policy.DeepCopy()
policy = policy
ancestorRefs []gwapiv1a2.ParentReference
resolveErr *status.PolicyResolveError
targetKind string
Expand Down
2 changes: 1 addition & 1 deletion internal/gatewayapi/extensionserverpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst
// Process the policies targeting Gateways. Only update the policy status if it was accepted.
// A policy is considered accepted if at least one targetRef contained inside matched a listener.
for policyIndex, policy := range policies {
policy := policy.DeepCopy()
policy := &policy
var policyStatus gwapiv1a2.PolicyStatus
accepted := false
targetRefs, err := extractTargetRefs(policy, gateways)
Expand Down
6 changes: 5 additions & 1 deletion internal/gatewayapi/resource/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,11 @@ func (c *ControllerResources) DeepCopy() *ControllerResources {
return nil
}
out := make(ControllerResources, len(*c))
copy(out, *c)
for i, res := range *c {
if res != nil {
out[i] = res.DeepCopy()
}
}
return &out
}

Expand Down
10 changes: 5 additions & 5 deletions internal/gatewayapi/route.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (t *Translator) ProcessHTTPRoutes(httpRoutes []*gwapiv1.HTTPRoute, gateways
if h == nil {
panic("received nil httproute")
}
httpRoute := &HTTPRouteContext{HTTPRoute: h.DeepCopy()}
httpRoute := &HTTPRouteContext{HTTPRoute: h}

// Find out if this route attaches to one of our Gateway's listeners,
// and if so, get the list of listeners that allow it to attach for each
Expand All @@ -87,7 +87,7 @@ func (t *Translator) ProcessGRPCRoutes(grpcRoutes []*gwapiv1.GRPCRoute, gateways
if g == nil {
panic("received nil grpcroute")
}
grpcRoute := &GRPCRouteContext{GRPCRoute: g.DeepCopy()}
grpcRoute := &GRPCRouteContext{GRPCRoute: g}

// Find out if this route attaches to one of our Gateway's listeners,
// and if so, get the list of listeners that allow it to attach for each
Expand Down Expand Up @@ -916,7 +916,7 @@ func (t *Translator) ProcessTLSRoutes(tlsRoutes []*gwapiv1a2.TLSRoute, gateways
if tls == nil {
panic("received nil tlsroute")
}
tlsRoute := &TLSRouteContext{TLSRoute: tls.DeepCopy()}
tlsRoute := &TLSRouteContext{TLSRoute: tls}

// Find out if this route attaches to one of our Gateway's listeners,
// and if so, get the list of listeners that allow it to attach for each
Expand Down Expand Up @@ -1060,7 +1060,7 @@ func (t *Translator) ProcessUDPRoutes(udpRoutes []*gwapiv1a2.UDPRoute, gateways
if u == nil {
panic("received nil udproute")
}
udpRoute := &UDPRouteContext{UDPRoute: u.DeepCopy()}
udpRoute := &UDPRouteContext{UDPRoute: u}

// Find out if this route attaches to one of our Gateway's listeners,
// and if so, get the list of listeners that allow it to attach for each
Expand Down Expand Up @@ -1208,7 +1208,7 @@ func (t *Translator) ProcessTCPRoutes(tcpRoutes []*gwapiv1a2.TCPRoute, gateways
if tcp == nil {
panic("received nil tcproute")
}
tcpRoute := &TCPRouteContext{TCPRoute: tcp.DeepCopy()}
tcpRoute := &TCPRouteContext{TCPRoute: tcp}

// Find out if this route attaches to one of our Gateway's listeners,
// and if so, get the list of listeners that allow it to attach for each
Expand Down
8 changes: 4 additions & 4 deletions internal/gatewayapi/securitypolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security
if currTarget.Kind != resource.KindGateway && currTarget.SectionName != nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
handledPolicies[policyName] = policy
res = append(res, policy)
}
Expand All @@ -114,7 +114,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security
if currTarget.Kind != resource.KindGateway && currTarget.SectionName == nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
handledPolicies[policyName] = policy
res = append(res, policy)
}
Expand All @@ -132,7 +132,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security
if currTarget.Kind == resource.KindGateway && currTarget.SectionName != nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
handledPolicies[policyName] = policy
res = append(res, policy)
}
Expand All @@ -150,7 +150,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security
if currTarget.Kind == resource.KindGateway && currTarget.SectionName == nil {
policy, found := handledPolicies[policyName]
if !found {
policy = currPolicy.DeepCopy()
policy = currPolicy
handledPolicies[policyName] = policy
res = append(res, policy)
}
Expand Down
29 changes: 29 additions & 0 deletions internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,20 @@ xdsIR:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
- object:
apiVersion: foo.example.io/v1alpha1
kind: Bar
Expand All @@ -175,6 +189,21 @@ xdsIR:
kind: Gateway
name: gateway-1
sectionName: tcp1
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
sectionName: tcp1
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
globalResources:
proxyServiceCluster:
name: envoy-gateway/gateway-1
Expand Down
29 changes: 29 additions & 0 deletions internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,20 @@ xdsIR:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
- object:
apiVersion: foo.example.io/v1alpha1
kind: Bar
Expand All @@ -175,6 +189,21 @@ xdsIR:
kind: Gateway
name: gateway-1
sectionName: udp1
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
sectionName: udp1
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
globalResources:
proxyServiceCluster:
name: envoy-gateway/gateway-1
Expand Down
52 changes: 52 additions & 0 deletions internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,32 @@ xdsIR:
- group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-2
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
http:
- address: 0.0.0.0
extensionRefs:
Expand Down Expand Up @@ -263,6 +289,32 @@ xdsIR:
- group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-2
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
globalResources:
proxyServiceCluster:
name: envoy-gateway/gateway-2
Expand Down
29 changes: 29 additions & 0 deletions internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,20 @@ xdsIR:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
- object:
apiVersion: foo.example.io/v1alpha1
kind: Bar
Expand All @@ -179,6 +193,21 @@ xdsIR:
kind: Gateway
name: gateway-1
sectionName: http2
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
sectionName: http2
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
globalResources:
proxyServiceCluster:
name: envoy-gateway/gateway-1
Expand Down
Loading