code review: replace mutable SetVersion() operation with an immutab…

…le `WithVersion()`

pkg/mads/cache/cache_suite_test.go

@@ -0,0 +1,13 @@
+package cache_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestCache(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Cache Suite")
+}

pkg/mads/cache/snapshot.go

@@ -10,9 +10,9 @@ import (
 )
 
 // NewSnapshot creates a snapshot from response types and a version.
-func NewSnapshot(version string, assignments []envoy_cache.Resource) Snapshot {
-	return Snapshot{
-		MonitoringAssignments: envoy_cache.NewResources(version, assignments),
+func NewSnapshot(version string, assignments map[string]envoy_cache.Resource) *Snapshot {
+	return &Snapshot{
+		MonitoringAssignments: envoy_cache.Resources{Version: version, Items: assignments},
 	}
 }
 
@@ -61,13 +61,19 @@ func (s *Snapshot) GetVersion(typ string) string {
 	return ""
 }
 
-// SetVersion sets the version for a resource type.
-func (s *Snapshot) SetVersion(typ string, version string) {
+// WithVersion creates a new snapshot with a different version for a given resource type.
+func (s *Snapshot) WithVersion(typ string, version string) util_xds.Snapshot {
 	if s == nil {
-		return
+		return nil
+	}
+	if s.GetVersion(typ) == version {
+		return s
 	}
 	switch typ {
 	case mads.MonitoringAssignmentType:
-		s.MonitoringAssignments.Version = version
+		return &Snapshot{
+			MonitoringAssignments: envoy_cache.Resources{Version: version, Items: s.MonitoringAssignments.Items},
+		}
 	}
+	return s
 }

pkg/mads/cache/snapshot_test.go

@@ -0,0 +1,185 @@
+package cache_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	envoy_cache "github.com/envoyproxy/go-control-plane/pkg/cache"
+
+	. "github.com/Kong/kuma/pkg/mads/cache"
+
+	observability_proto "github.com/Kong/kuma/api/observability/v1alpha1"
+)
+
+var _ = Describe("Snapshot", func() {
+
+	expectedType := "type.googleapis.com/kuma.observability.v1alpha1.MonitoringAssignment"
+
+	Describe("GetSupportedTypes()", func() {
+		It("should always return ['type.googleapis.com/kuma.observability.v1alpha1.MonitoringAssignment']", func() {
+			// when
+			var snapshot *Snapshot
+			// then
+			Expect(snapshot.GetSupportedTypes()).To(Equal([]string{expectedType}))
+
+			// when
+			snapshot = &Snapshot{}
+			// then
+			Expect(snapshot.GetSupportedTypes()).To(Equal([]string{expectedType}))
+		})
+	})
+
+	Describe("Consistent()", func() {
+		It("should handle `nil`", func() {
+			// when
+			var snapshot *Snapshot
+			// then
+			Expect(snapshot.Consistent()).To(MatchError("nil snapshot"))
+		})
+
+		It("non-`nil` snapshot should be always consistet", func() {
+			// when
+			snapshot := NewSnapshot("v1", nil)
+			// then
+			Expect(snapshot.Consistent()).To(BeNil())
+
+			// when
+			snapshot = NewSnapshot("v2", map[string]envoy_cache.Resource{
+				"backend": &observability_proto.MonitoringAssignment{
+					Name: "/meshes/default/dataplanes/backend",
+				},
+			})
+			// then
+			Expect(snapshot.Consistent()).To(BeNil())
+		})
+	})
+
+	Describe("GetResources()", func() {
+		It("should handle `nil`", func() {
+			// when
+			var snapshot *Snapshot
+			// then
+			Expect(snapshot.GetResources(expectedType)).To(BeNil())
+		})
+
+		It("should return MonitoringAssignments", func() {
+			// given
+			assignments := map[string]envoy_cache.Resource{
+				"backend": &observability_proto.MonitoringAssignment{
+					Name: "/meshes/default/dataplanes/backend",
+				},
+			}
+			// when
+			snapshot := NewSnapshot("v1", assignments)
+			// then
+			Expect(snapshot.GetResources(expectedType)).To(Equal(assignments))
+		})
+
+		It("should return `nil` for unsupported resource types", func() {
+			// given
+			assignments := map[string]envoy_cache.Resource{
+				"backend": &observability_proto.MonitoringAssignment{
+					Name: "/meshes/default/dataplanes/backend",
+				},
+			}
+			// when
+			snapshot := NewSnapshot("v1", assignments)
+			// then
+			Expect(snapshot.GetResources("unsupported type")).To(BeNil())
+		})
+	})
+
+	Describe("GetVersion()", func() {
+		It("should handle `nil`", func() {
+			// when
+			var snapshot *Snapshot
+			// then
+			Expect(snapshot.GetVersion(expectedType)).To(Equal(""))
+		})
+
+		It("should return proper version for a supported resource type", func() {
+			// given
+			assignments := map[string]envoy_cache.Resource{
+				"backend": &observability_proto.MonitoringAssignment{
+					Name: "/meshes/default/dataplanes/backend",
+				},
+			}
+			// when
+			snapshot := NewSnapshot("v1", assignments)
+			// then
+			Expect(snapshot.GetVersion(expectedType)).To(Equal("v1"))
+		})
+
+		It("should return an empty string for unsupported resource type", func() {
+			// given
+			assignments := map[string]envoy_cache.Resource{
+				"backend": &observability_proto.MonitoringAssignment{
+					Name: "/meshes/default/dataplanes/backend",
+				},
+			}
+			// when
+			snapshot := NewSnapshot("v1", assignments)
+			// then
+			Expect(snapshot.GetVersion("unsupported type")).To(Equal(""))
+		})
+	})
+
+	Describe("WithVersion()", func() {
+		It("should handle `nil`", func() {
+			// given
+			var snapshot *Snapshot
+			// when
+			actual := snapshot.WithVersion(expectedType, "v1")
+			// then
+			Expect(actual).To(BeNil())
+		})
+
+		It("should return a new snapshot if version has changed", func() {
+			// given
+			assignments := map[string]envoy_cache.Resource{
+				"backend": &observability_proto.MonitoringAssignment{
+					Name: "/meshes/default/dataplanes/backend",
+				},
+			}
+			snapshot := NewSnapshot("v1", assignments)
+			// when
+			actual := snapshot.WithVersion(expectedType, "v2")
+			// then
+			Expect(actual.GetVersion(expectedType)).To(Equal("v2"))
+			// and
+			Expect(actual).To(Equal(NewSnapshot("v2", assignments)))
+		})
+
+		It("should return the same snapshot if version has not changed", func() {
+			// given
+			assignments := map[string]envoy_cache.Resource{
+				"backend": &observability_proto.MonitoringAssignment{
+					Name: "/meshes/default/dataplanes/backend",
+				},
+			}
+			snapshot := NewSnapshot("v1", assignments)
+			// when
+			actual := snapshot.WithVersion(expectedType, "v1")
+			// then
+			Expect(actual.GetVersion(expectedType)).To(Equal("v1"))
+			// and
+			Expect(actual).To(BeIdenticalTo(snapshot))
+		})
+
+		It("should return the same snapshot if resource type is not supported", func() {
+			// given
+			assignments := map[string]envoy_cache.Resource{
+				"backend": &observability_proto.MonitoringAssignment{
+					Name: "/meshes/default/dataplanes/backend",
+				},
+			}
+			snapshot := NewSnapshot("v1", assignments)
+			// when
+			actual := snapshot.WithVersion("unsupported type", "v2")
+			// then
+			Expect(actual.GetVersion(expectedType)).To(Equal("v1"))
+			// and
+			Expect(actual).To(BeIdenticalTo(snapshot))
+		})
+	})
+})

pkg/util/xds/cache.go

@@ -59,8 +59,8 @@ type Snapshot interface {
 	// GetVersion returns the version for a resource type.
 	GetVersion(typ string) string
 
-	// SetVersion sets the version for a resource type.
-	SetVersion(typ string, version string)
+	// WithVersion creates a new snapshot with a different version for a given resource type.
+	WithVersion(typ string, version string) Snapshot
 }
 
 // SnapshotCache is a snapshot-based cache that maintains a single versioned