From 8f6aff1da91f5ac804a377040628283ff5b66ea6 Mon Sep 17 00:00:00 2001
From: Sunjay Bhatia <5337253+sunjayBhatia@users.noreply.github.com>
Date: Tue, 10 Oct 2023 12:45:55 -0400
Subject: [PATCH] internal/xdscache: Generate uuid for snapshot version (#5819)

Snapshotter had a data race reading/writing the snapshot version between
threads. This version is not in practice used for the contour xDS server
DiscoveryResponse versions but is in the go-control-plane version.

Fixes: #5482

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
---
 go.mod                             |  2 +-
 internal/xdscache/snapshot.go      | 22 ++----------
 internal/xdscache/snapshot_test.go | 57 ------------------------------
 3 files changed, 3 insertions(+), 78 deletions(-)
 delete mode 100644 internal/xdscache/snapshot_test.go

diff --git a/go.mod b/go.mod
index 224bd72ef05..e088cb5e7ed 100644
--- a/go.mod
+++ b/go.mod
@@ -15,6 +15,7 @@ require (
 	github.com/go-logr/logr v1.2.4
 	github.com/google/go-cmp v0.5.9
 	github.com/google/go-github/v48 v48.2.0
+	github.com/google/uuid v1.3.1
 	github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
 	github.com/onsi/ginkgo/v2 v2.12.1
@@ -79,7 +80,6 @@ require (
 	github.com/google/go-querystring v1.1.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect
-	github.com/google/uuid v1.3.1 // indirect
 	github.com/hashicorp/hcl v1.0.1-vault-5 // indirect
 	github.com/huandu/xstrings v1.4.0 // indirect
 	github.com/iancoleman/strcase v0.2.0 // indirect
diff --git a/internal/xdscache/snapshot.go b/internal/xdscache/snapshot.go
index 5b05680f8d7..26fa69839ab 100644
--- a/internal/xdscache/snapshot.go
+++ b/internal/xdscache/snapshot.go
@@ -14,13 +14,12 @@
 package xdscache
 
 import (
-	"math"
 	"reflect"
-	"strconv"
 	"sync"
 
 	envoy_types "github.com/envoyproxy/go-control-plane/pkg/cache/types"
 	envoy_resource_v3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3"
+	"github.com/google/uuid"
 	"github.com/projectcontour/contour/internal/dag"
 	"github.com/sirupsen/logrus"
 )
@@ -36,9 +35,6 @@ type SnapshotHandler struct {
 	// resources holds the cache of xDS contents.
 	resources map[envoy_resource_v3.Type]ResourceCache
 
-	// snapshotVersion holds the current version of the snapshot.
-	snapshotVersion int64
-
 	snapshotters []Snapshotter
 	snapLock     sync.Mutex
 
@@ -75,7 +71,7 @@ func (s *SnapshotHandler) OnChange(root *dag.DAG) {
 // the Contour XDS caches.
 func (s *SnapshotHandler) generateNewSnapshot() {
 	// Generate new snapshot version.
-	version := s.newSnapshotVersion()
+	version := uuid.NewString()
 
 	// Convert caches to envoy xDS Resources.
 	resources := map[envoy_resource_v3.Type][]envoy_types.Resource{
@@ -97,20 +93,6 @@ func (s *SnapshotHandler) generateNewSnapshot() {
 	}
 }
 
-// newSnapshotVersion increments the current snapshotVersion
-// and returns as a string.
-func (s *SnapshotHandler) newSnapshotVersion() string {
-
-	// Reset the snapshotVersion if it ever hits max size.
-	if s.snapshotVersion == math.MaxInt64 {
-		s.snapshotVersion = 0
-	}
-
-	// Increment the snapshot version & return as string.
-	s.snapshotVersion++
-	return strconv.FormatInt(s.snapshotVersion, 10)
-}
-
 // asResources casts the given slice of values (that implement the envoy_types.Resource
 // interface) to a slice of envoy_types.Resource. If the length of the slice is 0, it
 // returns nil.
diff --git a/internal/xdscache/snapshot_test.go b/internal/xdscache/snapshot_test.go
deleted file mode 100644
index 632694b27bd..00000000000
--- a/internal/xdscache/snapshot_test.go
+++ /dev/null
@@ -1,57 +0,0 @@
-// Copyright Project Contour Authors
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package xdscache
-
-import (
-	"math"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestGetNewSnapshotVersion(t *testing.T) {
-	type testcase struct {
-		startingVersion int64
-		want            string
-	}
-
-	run := func(t *testing.T, name string, tc testcase) {
-		t.Helper()
-
-		t.Run(name, func(t *testing.T) {
-			t.Helper()
-
-			sh := SnapshotHandler{
-				snapshotVersion: tc.startingVersion,
-			}
-			got := sh.newSnapshotVersion()
-			assert.Equal(t, tc.want, got)
-		})
-	}
-
-	run(t, "simple", testcase{
-		startingVersion: 0,
-		want:            "1",
-	})
-
-	run(t, "big version", testcase{
-		startingVersion: math.MaxInt64 - 1,
-		want:            "9223372036854775807",
-	})
-
-	run(t, "resets if max hit", testcase{
-		startingVersion: math.MaxInt64,
-		want:            "1",
-	})
-}