Fixes BackendTLSPolicy Status Update data race (#6187)

DAG cache objects were being written to when StatusUpdater fetched resource to perform update on since status updates held a reference to them. Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
projectcontour · Feb 12, 2024 · 3164039 · 3164039
1 parent 26a6532
commit 3164039
Show file tree

Hide file tree

Showing 4 changed files with 3 additions and 6 deletions.
diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go
@@ -2098,7 +2098,6 @@ func (p *GatewayAPIProcessor) computeBackendTLSPolicies(routeNamespace string, b
 		backendTLSPolicyAccessor, commit := p.dag.StatusCache.BackendTLSPolicyConditionsAccessor(
 			k8s.NamespacedNameOf(backendTLSPolicy),
 			backendTLSPolicy.GetGeneration(),
-			backendTLSPolicy,
 		)
 		defer commit()
 		backendTLSPolicyAncestorStatus := backendTLSPolicyAccessor.StatusUpdateFor(routeParentRef)

diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go
@@ -11471,7 +11471,6 @@ func TestGatewayAPIBackendTLSPolicyDAGStatus(t *testing.T) {
 				cmpopts.IgnoreFields(status.BackendTLSPolicyStatusUpdate{}, "GatewayRef"),
 				cmpopts.IgnoreFields(status.BackendTLSPolicyStatusUpdate{}, "Generation"),
 				cmpopts.IgnoreFields(status.BackendTLSPolicyStatusUpdate{}, "TransitionTime"),
-				cmpopts.IgnoreFields(status.BackendTLSPolicyStatusUpdate{}, "Resource"),
 				cmpopts.SortSlices(func(i, j metav1.Condition) bool {
 					return i.Message < j.Message
 				}),

diff --git a/internal/status/backendtlspolicyconditions.go b/internal/status/backendtlspolicyconditions.go
@@ -32,7 +32,6 @@ type BackendTLSPolicyStatusUpdate struct {
 	PolicyAncestorStatuses []*gatewayapi_v1alpha2.PolicyAncestorStatus
 	GatewayRef             types.NamespacedName
 	GatewayController      gatewayapi_v1beta1.GatewayController
-	Resource               client.Object
 	Generation             int64
 	TransitionTime         metav1.Time
 }

diff --git a/internal/status/cache.go b/internal/status/cache.go
@@ -23,6 +23,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	gatewayapi_v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 	gatewayapi_v1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 )
 
@@ -101,7 +102,7 @@ func (c *Cache) GetStatusUpdates() []k8s.StatusUpdate {
 	for fullname, backendTLSPolicyUpdate := range c.backendTLSPolicyUpdates {
 		update := k8s.StatusUpdate{
 			NamespacedName: fullname,
-			Resource:       backendTLSPolicyUpdate.Resource,
+			Resource:       &gatewayapi_v1alpha2.BackendTLSPolicy{},
 			Mutator:        backendTLSPolicyUpdate,
 		}
 
@@ -268,14 +269,13 @@ func (c *Cache) RouteConditionsAccessor(nsName types.NamespacedName, generation
 // to build up a list of metav1.Conditions as well as a function to commit the change back to the
 // cache when everything is done. The commit function pattern is used so that the
 // BackendTLSPolicyStatusUpdate does not need to know anything the cache internals.
-func (c *Cache) BackendTLSPolicyConditionsAccessor(nsName types.NamespacedName, generation int64, resource client.Object) (*BackendTLSPolicyStatusUpdate, func()) {
+func (c *Cache) BackendTLSPolicyConditionsAccessor(nsName types.NamespacedName, generation int64) (*BackendTLSPolicyStatusUpdate, func()) {
 	pu := &BackendTLSPolicyStatusUpdate{
 		FullName:          nsName,
 		GatewayRef:        c.gatewayRef,
 		GatewayController: c.gatewayController,
 		Generation:        generation,
 		TransitionTime:    metav1.NewTime(time.Now()),
-		Resource:          resource,
 	}
 
 	return pu, func() {