diff --git a/internal/cmd/egctl/translate.go b/internal/cmd/egctl/translate.go index 62d6e0f5ee..0158b684cc 100644 --- a/internal/cmd/egctl/translate.go +++ b/internal/cmd/egctl/translate.go @@ -338,7 +338,10 @@ func translateGatewayAPIToGatewayAPI(resources *resource.Resources) (resource.Re msg := fmt.Sprintf("%s: %v", status.MsgGatewayClassInvalidParams, err) status.SetGatewayClassAccepted(resources.GatewayClass, false, string(gwapiv1.GatewayClassReasonInvalidParameters), msg) } - gRes.EnvoyProxyForGatewayClass = resources.EnvoyProxyForGatewayClass + // Don't overwrite gRes.EnvoyProxyForGatewayClass here: the translator already + // populated it with the status-isolated copy carrying the computed Accepted + // condition. resources.EnvoyProxyForGatewayClass is the original object, whose + // Status the translator no longer mutates in place (see Translate's StatusDeepCopy). } if !epInvalid { diff --git a/internal/gatewayapi/backend.go b/internal/gatewayapi/backend.go index 9f7d488c40..f8c29d4ea7 100644 --- a/internal/gatewayapi/backend.go +++ b/internal/gatewayapi/backend.go @@ -20,10 +20,8 @@ import ( ) func (t *Translator) ProcessBackends(backends []*egv1a1.Backend, backendTLSPolicies []*gwapiv1.BackendTLSPolicy) []*egv1a1.Backend { - backendCopies := backendCopiesWithStatusDeepCopy(backends) res := make([]*egv1a1.Backend, 0, len(backends)) - for i := range backends { - backend := backendCopies[i] + for _, backend := range backends { // Ensure Backends are enabled if !t.BackendEnabled { status.UpdateBackendStatusAcceptedCondition(backend, false, @@ -206,15 +204,3 @@ func validateIP(epIP *egv1a1.IPEndpoint, runningOnHost bool) status.Error { } return nil } - -// backendCopiesWithStatusDeepCopy returns shallow copies with deep-copied Status fields. -// Status is mutated during translation and shares a pointer with the watchable coalesce goroutine. -func backendCopiesWithStatusDeepCopy(backends []*egv1a1.Backend) []*egv1a1.Backend { - copies := make([]*egv1a1.Backend, len(backends)) - for i, b := range backends { - out := *b - b.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index 049cd15b7b..a520dbc212 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -259,8 +259,6 @@ func (t *Translator) ProcessBackendTrafficPolicies( SectionIndex: make(map[types.NamespacedName]sets.Set[string], gatewayMapSize), } - policyCopies := backendTrafficPolicyCopiesWithStatusDeepCopy(backendTrafficPolicies) - handledPolicies := make(map[types.NamespacedName]*egv1a1.BackendTrafficPolicy, policyMapSize) // Translate @@ -281,7 +279,7 @@ func (t *Translator) ProcessBackendTrafficPolicies( if isRouteRule(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = backendTrafficPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -307,7 +305,7 @@ func (t *Translator) ProcessBackendTrafficPolicies( if isRoute(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = backendTrafficPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -327,7 +325,7 @@ func (t *Translator) ProcessBackendTrafficPolicies( if isListener(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = backendTrafficPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -352,7 +350,7 @@ func (t *Translator) ProcessBackendTrafficPolicies( if isGateway(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = backendTrafficPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -2196,15 +2194,3 @@ func buildRouteStatName(routeStatName string, metadata *ir.ResourceMetadata) *st return &statName } - -// backendTrafficPolicyCopiesWithStatusDeepCopy returns shallow copies with deep-copied Status fields. -// Status is mutated during translation and shares a pointer with the watchable coalesce goroutine. -func backendTrafficPolicyCopiesWithStatusDeepCopy(policies []*egv1a1.BackendTrafficPolicy) []*egv1a1.BackendTrafficPolicy { - copies := make([]*egv1a1.BackendTrafficPolicy, len(policies)) - for i, p := range policies { - out := *p - p.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} diff --git a/internal/gatewayapi/clienttrafficpolicy.go b/internal/gatewayapi/clienttrafficpolicy.go index bca57a5a96..bf266fc222 100644 --- a/internal/gatewayapi/clienttrafficpolicy.go +++ b/internal/gatewayapi/clienttrafficpolicy.go @@ -96,8 +96,6 @@ func (t *Translator) ProcessClientTrafficPolicies( listenerSetMap[key] = ls } - policyCopies := clientTrafficPolicyCopiesWithStatusDeepCopy(clientTrafficPolicies) - handledPolicies := make(map[types.NamespacedName]*egv1a1.ClientTrafficPolicy) // Translate // 1. First translate Policies with a sectionName set @@ -112,7 +110,7 @@ func (t *Translator) ProcessClientTrafficPolicies( if hasSectionName(&targetRef) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = clientTrafficPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -258,7 +256,7 @@ func (t *Translator) ProcessClientTrafficPolicies( policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = clientTrafficPolicies[i] res = append(res, policy) handledPolicies[policyName] = policy } @@ -1510,15 +1508,3 @@ func translateHeaderModifier(headerModifier *egv1a1.HTTPHeaderFilter, modType st return addRequestHeaders, removeRequestHeaders, removeRequestHeadersOnMatch, errs } - -// clientTrafficPolicyCopiesWithStatusDeepCopy returns shallow copies with deep-copied Status fields. -// Status is mutated during translation and shares a pointer with the watchable coalesce goroutine. -func clientTrafficPolicyCopiesWithStatusDeepCopy(policies []*egv1a1.ClientTrafficPolicy) []*egv1a1.ClientTrafficPolicy { - copies := make([]*egv1a1.ClientTrafficPolicy, len(policies)) - for i, p := range policies { - out := *p - p.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} diff --git a/internal/gatewayapi/envoyextensionpolicy.go b/internal/gatewayapi/envoyextensionpolicy.go index 341252f8ed..aa26c8c279 100644 --- a/internal/gatewayapi/envoyextensionpolicy.go +++ b/internal/gatewayapi/envoyextensionpolicy.go @@ -103,8 +103,6 @@ func (t *Translator) ProcessEnvoyExtensionPolicies( // The routes are grouped by sectionNames of their targetRefs. gatewayRouteMap := make(map[string]map[string]sets.Set[string]) - policyCopies := envoyExtensionPolicyCopiesWithStatusDeepCopy(envoyExtensionPolicies) - handledPolicies := make(map[types.NamespacedName]*egv1a1.EnvoyExtensionPolicy) // Translate @@ -122,7 +120,7 @@ func (t *Translator) ProcessEnvoyExtensionPolicies( if isRouteRule(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = envoyExtensionPolicies[i] res = append(res, policy) handledPolicies[policyName] = policy } @@ -148,7 +146,7 @@ func (t *Translator) ProcessEnvoyExtensionPolicies( if isRoute(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = envoyExtensionPolicies[i] res = append(res, policy) handledPolicies[policyName] = policy } @@ -168,7 +166,7 @@ func (t *Translator) ProcessEnvoyExtensionPolicies( if isListener(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = envoyExtensionPolicies[i] res = append(res, policy) handledPolicies[policyName] = policy } @@ -194,7 +192,7 @@ func (t *Translator) ProcessEnvoyExtensionPolicies( if isGateway(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = envoyExtensionPolicies[i] res = append(res, policy) handledPolicies[policyName] = policy } @@ -1282,15 +1280,3 @@ func (t *Translator) buildDynamicModules( return dmIRList, errs } - -// envoyExtensionPolicyCopiesWithStatusDeepCopy returns shallow copies with deep-copied Status fields. -// Status is mutated during translation and shares a pointer with the watchable coalesce goroutine. -func envoyExtensionPolicyCopiesWithStatusDeepCopy(policies []*egv1a1.EnvoyExtensionPolicy) []*egv1a1.EnvoyExtensionPolicy { - copies := make([]*egv1a1.EnvoyExtensionPolicy, len(policies)) - for i, p := range policies { - out := *p - p.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} diff --git a/internal/gatewayapi/envoypatchpolicy.go b/internal/gatewayapi/envoypatchpolicy.go index 97b71a2e18..e184bbb069 100644 --- a/internal/gatewayapi/envoypatchpolicy.go +++ b/internal/gatewayapi/envoypatchpolicy.go @@ -19,11 +19,10 @@ import ( func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.EnvoyPatchPolicy, xdsIR resource.XdsIRMap) []*egv1a1.EnvoyPatchPolicy { // EnvoyPatchPolicies are already sorted by the provider layer (priority, then timestamp, then name) - policyCopies := envoyPatchPolicyCopiesWithStatusDeepCopy(envoyPatchPolicies) res := make([]*egv1a1.EnvoyPatchPolicy, 0, len(envoyPatchPolicies)) for i := range envoyPatchPolicies { - policy := policyCopies[i] + policy := envoyPatchPolicies[i] res = append(res, policy) var ( ancestorRef gwapiv1.ParentReference @@ -146,15 +145,3 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo return res } - -// envoyPatchPolicyCopiesWithStatusDeepCopy returns shallow copies with deep-copied Status fields. -// Status is mutated during translation and shares a pointer with the watchable coalesce goroutine. -func envoyPatchPolicyCopiesWithStatusDeepCopy(policies []*egv1a1.EnvoyPatchPolicy) []*egv1a1.EnvoyPatchPolicy { - copies := make([]*egv1a1.EnvoyPatchPolicy, len(policies)) - for i, p := range policies { - out := *p - p.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} diff --git a/internal/gatewayapi/extensionserverpolicy.go b/internal/gatewayapi/extensionserverpolicy.go index 12530c6f6f..fde7d80f32 100644 --- a/internal/gatewayapi/extensionserverpolicy.go +++ b/internal/gatewayapi/extensionserverpolicy.go @@ -9,11 +9,9 @@ import ( "encoding/json" "errors" "fmt" - "maps" "strings" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -38,13 +36,11 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst gatewayMap[key] = &policyGatewayTargetContext{GatewayContext: gw} } - policyCopies := extensionServerPolicyCopiesWithStatusDeepCopy(policies) - var errs error // 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 i := range policies { - policy := policyCopies[i] + policy := policies[i] var policyStatus gwapiv1.PolicyStatus accepted := false targetRefs, err := extractTargetRefs(&policy, gateways) @@ -192,17 +188,3 @@ func (t *Translator) translateExtServerPolicyForGateway( } return found } - -// extensionServerPolicyCopiesWithStatusDeepCopy returns shallow copies with deep-copied status entries. -// Status is mutated during translation and shares a pointer with the watchable coalesce goroutine. -func extensionServerPolicyCopiesWithStatusDeepCopy(policies []unstructured.Unstructured) []unstructured.Unstructured { - copies := make([]unstructured.Unstructured, len(policies)) - for i, p := range policies { - p.Object = maps.Clone(p.Object) // shallow copy map - no shared ref for "status" key - if statusObj, ok := policies[i].Object["status"].(map[string]any); ok { - p.Object["status"] = runtime.DeepCopyJSON(statusObj) - } - copies[i] = p - } - return copies -} diff --git a/internal/gatewayapi/resource/resource.go b/internal/gatewayapi/resource/resource.go index 6796ff7b4e..6d2c05a800 100644 --- a/internal/gatewayapi/resource/resource.go +++ b/internal/gatewayapi/resource/resource.go @@ -7,6 +7,7 @@ package resource import ( "context" + "maps" "reflect" "sort" @@ -14,6 +15,7 @@ import ( corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" gwapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" mcsapiv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" @@ -502,3 +504,83 @@ func (r *Resources) Sort() { return tsI.Before(&tsJ) }) } + +// StatusDeepCopy returns a shallow copy of Resources in which every status-bearing +// object is itself shallow-copied with only its Status field deep-copied. +// +// The translator mutates resource Status in place while building status updates, +// but the input tree is shared with the watchable coalesce goroutine, which walks it +// with reflect.DeepEqual. Deep-copying only the Status fields isolates those in-place +// mutations from the shared objects without the memory cost of a full DeepCopy of the +// (much larger, immutable-during-translation) Spec. +func (r *Resources) StatusDeepCopy() *Resources { + if r == nil { + return nil + } + + // Shallow copy of the struct: all fields (including the shared, non-status-bearing + // slices such as Namespaces, Services, Secrets, ...) are carried over as-is. + out := *r + + out.GatewayClass = statusDeepCopyObject(r.GatewayClass, func(dst, src *gwapiv1.GatewayClass) { src.Status.DeepCopyInto(&dst.Status) }) + out.EnvoyProxyForGatewayClass = statusDeepCopyObject(r.EnvoyProxyForGatewayClass, func(dst, src *egv1a1.EnvoyProxy) { src.Status.DeepCopyInto(&dst.Status) }) + out.EnvoyProxiesForGateways = statusDeepCopySlice(r.EnvoyProxiesForGateways, func(dst, src *egv1a1.EnvoyProxy) { src.Status.DeepCopyInto(&dst.Status) }) + out.Gateways = statusDeepCopySlice(r.Gateways, func(dst, src *gwapiv1.Gateway) { src.Status.DeepCopyInto(&dst.Status) }) + out.ListenerSets = statusDeepCopySlice(r.ListenerSets, func(dst, src *gwapiv1.ListenerSet) { src.Status.DeepCopyInto(&dst.Status) }) + out.HTTPRoutes = statusDeepCopySlice(r.HTTPRoutes, func(dst, src *gwapiv1.HTTPRoute) { src.Status.DeepCopyInto(&dst.Status) }) + out.GRPCRoutes = statusDeepCopySlice(r.GRPCRoutes, func(dst, src *gwapiv1.GRPCRoute) { src.Status.DeepCopyInto(&dst.Status) }) + out.TLSRoutes = statusDeepCopySlice(r.TLSRoutes, func(dst, src *gwapiv1.TLSRoute) { src.Status.DeepCopyInto(&dst.Status) }) + out.TCPRoutes = statusDeepCopySlice(r.TCPRoutes, func(dst, src *gwapiv1.TCPRoute) { src.Status.DeepCopyInto(&dst.Status) }) + out.UDPRoutes = statusDeepCopySlice(r.UDPRoutes, func(dst, src *gwapiv1.UDPRoute) { src.Status.DeepCopyInto(&dst.Status) }) + out.EnvoyPatchPolicies = statusDeepCopySlice(r.EnvoyPatchPolicies, func(dst, src *egv1a1.EnvoyPatchPolicy) { src.Status.DeepCopyInto(&dst.Status) }) + out.ClientTrafficPolicies = statusDeepCopySlice(r.ClientTrafficPolicies, func(dst, src *egv1a1.ClientTrafficPolicy) { src.Status.DeepCopyInto(&dst.Status) }) + out.BackendTrafficPolicies = statusDeepCopySlice(r.BackendTrafficPolicies, func(dst, src *egv1a1.BackendTrafficPolicy) { src.Status.DeepCopyInto(&dst.Status) }) + out.SecurityPolicies = statusDeepCopySlice(r.SecurityPolicies, func(dst, src *egv1a1.SecurityPolicy) { src.Status.DeepCopyInto(&dst.Status) }) + out.BackendTLSPolicies = statusDeepCopySlice(r.BackendTLSPolicies, func(dst, src *gwapiv1.BackendTLSPolicy) { src.Status.DeepCopyInto(&dst.Status) }) + out.EnvoyExtensionPolicies = statusDeepCopySlice(r.EnvoyExtensionPolicies, func(dst, src *egv1a1.EnvoyExtensionPolicy) { src.Status.DeepCopyInto(&dst.Status) }) + out.Backends = statusDeepCopySlice(r.Backends, func(dst, src *egv1a1.Backend) { src.Status.DeepCopyInto(&dst.Status) }) + out.ExtensionServerPolicies = statusDeepCopyUnstructured(r.ExtensionServerPolicies) + + return &out +} + +// statusDeepCopyObject returns a shallow copy of obj with copyStatus applied to +// deep-copy its Status field, or nil if obj is nil. +func statusDeepCopyObject[T any](obj *T, copyStatus func(dst, src *T)) *T { + if obj == nil { + return nil + } + cp := *obj + copyStatus(&cp, obj) + return &cp +} + +// statusDeepCopySlice returns a new slice of shallow copies, each with copyStatus +// applied to deep-copy its Status field, preserving the original length and nil-ness. +func statusDeepCopySlice[T any](in []*T, copyStatus func(dst, src *T)) []*T { + if in == nil { + return nil + } + out := make([]*T, len(in)) + for i, obj := range in { + out[i] = statusDeepCopyObject(obj, copyStatus) + } + return out +} + +// statusDeepCopyUnstructured returns shallow copies with deep-copied status entries +// for unstructured resources (ExtensionServerPolicies). +func statusDeepCopyUnstructured(in []unstructured.Unstructured) []unstructured.Unstructured { + if in == nil { + return nil + } + out := make([]unstructured.Unstructured, len(in)) + for i, p := range in { + p.Object = maps.Clone(p.Object) // shallow copy map - no shared ref for "status" key + if statusObj, ok := in[i].Object["status"].(map[string]any); ok { + p.Object["status"] = runtime.DeepCopyJSON(statusObj) + } + out[i] = p + } + return out +} diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 1539e5d180..18d6db8363 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -55,15 +55,14 @@ type RoutesTranslator interface { func (t *Translator) ProcessHTTPRoutes(httpRoutes []*gwapiv1.HTTPRoute, gateways []*GatewayContext, resources *resource.Resources, xdsIR resource.XdsIRMap) []*HTTPRouteContext { relevantHTTPRoutes := make([]*HTTPRouteContext, 0, len(httpRoutes)) - httpRouteCopies := httpRouteCopiesWithStatusDeepCopy(httpRoutes) // HTTPRoutes are already sorted by the provider layer - for i, h := range httpRoutes { + for _, h := range httpRoutes { if h == nil { panic("received nil httproute") } - httpRoute := &HTTPRouteContext{HTTPRoute: httpRouteCopies[i]} + 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 @@ -83,15 +82,14 @@ func (t *Translator) ProcessHTTPRoutes(httpRoutes []*gwapiv1.HTTPRoute, gateways func (t *Translator) ProcessGRPCRoutes(grpcRoutes []*gwapiv1.GRPCRoute, gateways []*GatewayContext, resources *resource.Resources, xdsIR resource.XdsIRMap) []*GRPCRouteContext { relevantGRPCRoutes := make([]*GRPCRouteContext, 0, len(grpcRoutes)) - grpcRouteCopies := grpcRouteCopiesWithStatusDeepCopy(grpcRoutes) // GRPCRoutes are already sorted by the provider layer - for i, g := range grpcRoutes { + for _, g := range grpcRoutes { if g == nil { panic("received nil grpcroute") } - grpcRoute := &GRPCRouteContext{GRPCRoute: grpcRouteCopies[i]} + 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 @@ -1427,14 +1425,13 @@ func filterEGPrefix(in map[string]string) map[string]string { func (t *Translator) ProcessTLSRoutes(tlsRoutes []*gwapiv1.TLSRoute, gateways []*GatewayContext, resources *resource.Resources, xdsIR resource.XdsIRMap) []*TLSRouteContext { relevantTLSRoutes := make([]*TLSRouteContext, 0, len(tlsRoutes)) - tlsRouteCopies := tlsRouteCopiesWithStatusDeepCopy(tlsRoutes) // TLSRoutes are already sorted by the provider layer - for i, tls := range tlsRoutes { + for _, tls := range tlsRoutes { if tls == nil { panic("received nil tlsroute") } - tlsRoute := &TLSRouteContext{TLSRoute: tlsRouteCopies[i]} + 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 @@ -1597,14 +1594,13 @@ func (t *Translator) ProcessUDPRoutes(udpRoutes []*gwapiv1.UDPRoute, gateways [] xdsIR resource.XdsIRMap, ) []*UDPRouteContext { relevantUDPRoutes := make([]*UDPRouteContext, 0, len(udpRoutes)) - udpRouteCopies := udpRouteCopiesWithStatusDeepCopy(udpRoutes) // UDPRoutes are already sorted by the provider layer - for i, u := range udpRoutes { + for _, u := range udpRoutes { if u == nil { panic("received nil udproute") } - udpRoute := &UDPRouteContext{UDPRoute: udpRouteCopies[i]} + 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 @@ -1749,14 +1745,13 @@ func (t *Translator) ProcessTCPRoutes(tcpRoutes []*gwapiv1.TCPRoute, gateways [] xdsIR resource.XdsIRMap, ) []*TCPRouteContext { relevantTCPRoutes := make([]*TCPRouteContext, 0, len(tcpRoutes)) - tcpRouteCopies := tcpRouteCopiesWithStatusDeepCopy(tcpRoutes) // TCPRoutes are already sorted by the provider layer - for i, tcp := range tcpRoutes { + for _, tcp := range tcpRoutes { if tcp == nil { panic("received nil tcproute") } - tcpRoute := &TCPRouteContext{TCPRoute: tcpRouteCopies[i]} + 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 @@ -2726,56 +2721,3 @@ func buildStatName(pattern string, route RouteContext, ruleName *gwapiv1.Section statName = strings.ReplaceAll(statName, egv1a1.StatFormatterBackendRefs, strings.Join(refs, "|")) return statName } - -// Shallow-copy helpers that deep-copy only the Status field. -// Status is mutated during translation and shares a pointer with the watchable coalesce goroutine. - -func httpRouteCopiesWithStatusDeepCopy(routes []*gwapiv1.HTTPRoute) []*gwapiv1.HTTPRoute { - copies := make([]*gwapiv1.HTTPRoute, len(routes)) - for i, r := range routes { - out := *r - r.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} - -func grpcRouteCopiesWithStatusDeepCopy(routes []*gwapiv1.GRPCRoute) []*gwapiv1.GRPCRoute { - copies := make([]*gwapiv1.GRPCRoute, len(routes)) - for i, r := range routes { - out := *r - r.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} - -func tlsRouteCopiesWithStatusDeepCopy(routes []*gwapiv1.TLSRoute) []*gwapiv1.TLSRoute { - copies := make([]*gwapiv1.TLSRoute, len(routes)) - for i, r := range routes { - out := *r - r.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} - -func udpRouteCopiesWithStatusDeepCopy(routes []*gwapiv1.UDPRoute) []*gwapiv1.UDPRoute { - copies := make([]*gwapiv1.UDPRoute, len(routes)) - for i, r := range routes { - out := *r - r.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} - -func tcpRouteCopiesWithStatusDeepCopy(routes []*gwapiv1.TCPRoute) []*gwapiv1.TCPRoute { - copies := make([]*gwapiv1.TCPRoute, len(routes)) - for i, r := range routes { - out := *r - r.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index d351489c6c..a6851dd39b 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -120,8 +120,6 @@ func (t *Translator) ProcessSecurityPolicies( listenerSetMap[key] = &policyListenerSetTargetContext{ListenerSet: ls} } - policyCopies := securityPolicyCopiesWithStatusDeepCopy(securityPolicies) - handledPolicies := make(map[types.NamespacedName]*egv1a1.SecurityPolicy, policyMapSize) // Map of attached Policy to Gateway. Used for policy merge process. @@ -160,7 +158,7 @@ func (t *Translator) ProcessSecurityPolicies( if isRouteRule(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = securityPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -184,7 +182,7 @@ func (t *Translator) ProcessSecurityPolicies( if isRoute(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = securityPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -202,7 +200,7 @@ func (t *Translator) ProcessSecurityPolicies( if isListenerSetListener(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = securityPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -227,7 +225,7 @@ func (t *Translator) ProcessSecurityPolicies( if isListenerSet(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = securityPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -245,7 +243,7 @@ func (t *Translator) ProcessSecurityPolicies( if isListener(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = securityPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -270,7 +268,7 @@ func (t *Translator) ProcessSecurityPolicies( if isGateway(currTarget) { policy, found := handledPolicies[policyName] if !found { - policy = policyCopies[i] + policy = securityPolicies[i] handledPolicies[policyName] = policy res = append(res, policy) } @@ -3159,15 +3157,3 @@ func buildExtAuthContextExtensionOwners(route, parent *egv1a1.SecurityPolicy) ma } return owners } - -// securityPolicyCopiesWithStatusDeepCopy returns shallow copies with deep-copied Status fields. -// Status is mutated during translation and shares a pointer with the watchable coalesce goroutine. -func securityPolicyCopiesWithStatusDeepCopy(policies []*egv1a1.SecurityPolicy) []*egv1a1.SecurityPolicy { - copies := make([]*egv1a1.SecurityPolicy, len(policies)) - for i, p := range policies { - out := *p - p.Status.DeepCopyInto(&out.Status) - copies[i] = &out - } - return copies -} diff --git a/internal/gatewayapi/translator.go b/internal/gatewayapi/translator.go index 56ff519b83..bc44d584bb 100644 --- a/internal/gatewayapi/translator.go +++ b/internal/gatewayapi/translator.go @@ -253,6 +253,13 @@ func newTranslateResult( func (t *Translator) Translate(resources *resource.Resources) (*TranslateResult, error) { var errs error + // The input resource tree is shared with the watchable coalesce goroutine, which + // walks it with reflect.DeepEqual. The translator mutates resource Status in place + // while computing status updates, which races with that walk. Work on a copy whose + // status-bearing objects are shallow-copied with only their Status fields deep-copied, + // isolating those mutations without the memory cost of a full DeepCopy. + resources = resources.StatusDeepCopy() + // Preprocessing to improve get resources operations performance. translatorContext := &TranslatorContext{} translatorContext.SetNamespaces(resources.Namespaces) diff --git a/release-notes/current/bug_fixes/9445-translator-status-mutation-data-race.md b/release-notes/current/bug_fixes/9445-translator-status-mutation-data-race.md new file mode 100644 index 0000000000..588dd604d8 --- /dev/null +++ b/release-notes/current/bug_fixes/9445-translator-status-mutation-data-race.md @@ -0,0 +1 @@ +Fixed a data race that could crash envoy-gateway with `panic: reflect: slice index out of range` when the watchable coalesce goroutine compared the xDS IR with `reflect.DeepEqual` while the translator concurrently mutated resource status in place; the translator now isolates status mutations by deep-copying only the status field of each resource at the start of translation.