Allow reuse of cache indexes (#20562)

Previously calling `index.New` would return an object with the index information such as the Indexer, whether it was required, and the name of the index as well as a radix tree to store indexed data.

Now the main `Index` type doesn’t contain the radix tree for indexed data. Instead the `IndexedData` method can be used to combine the main `Index` with a radix tree in the `IndexedData` structure.

The cache still only allows configuring the `Index` type and will invoke the `IndexedData` method on the provided indexes to get the structure that the cache can use for actual data management.

All of this makes it now safe to reuse the `index.Index` types.

internal/controller/cache/index/convenience_test.go

@@ -243,6 +243,55 @@ func TestPrefixReferenceOrIDFromArgs(t *testing.T) {
 	})
 }
 
+func TestMaybePrefixReferenceOrIDFromArgs(t *testing.T) {
+	ref := &pbresource.Reference{
+		Type: &pbresource.Type{
+			Group:        "test",
+			GroupVersion: "v1",
+			Kind:         "fake",
+		},
+		Tenancy: &pbresource.Tenancy{
+			Partition: "default",
+		},
+	}
+	t.Run("invalid length", func(t *testing.T) {
+		// the second arg will cause a validation error
+		val, err := MaybePrefixReferenceOrIDFromArgs(ref, IndexQueryOptions{Prefix: false}, 3)
+		require.Nil(t, val)
+		require.Error(t, err)
+	})
+
+	t.Run("invalid type arg 1", func(t *testing.T) {
+		val, err := MaybePrefixReferenceOrIDFromArgs("string type unexpected", IndexQueryOptions{Prefix: true})
+		require.Nil(t, val)
+		require.Error(t, err)
+	})
+
+	t.Run("invalid type arg 2", func(t *testing.T) {
+		val, err := MaybePrefixReferenceOrIDFromArgs(ref, "string type unexpected")
+		require.Nil(t, val)
+		require.Error(t, err)
+	})
+
+	t.Run("ok single arg", func(t *testing.T) {
+		val, err := MaybePrefixReferenceOrIDFromArgs(ref)
+		require.NoError(t, err)
+		require.Equal(t, IndexFromRefOrID(ref), val)
+	})
+
+	t.Run("ok two args no prefix", func(t *testing.T) {
+		val, err := MaybePrefixReferenceOrIDFromArgs(ref, IndexQueryOptions{Prefix: false})
+		require.NoError(t, err)
+		require.Equal(t, IndexFromRefOrID(ref), val)
+	})
+
+	t.Run("ok two args with prefix", func(t *testing.T) {
+		val, err := MaybePrefixReferenceOrIDFromArgs(ref, IndexQueryOptions{Prefix: true})
+		require.NoError(t, err)
+		require.Equal(t, PrefixIndexFromRefOrID(ref), val)
+	})
+}
+
 func TestSingleValueFromArgs(t *testing.T) {
 	injectedError := errors.New("injected test error")
 

internal/controller/cache/index/index.go

@@ -12,7 +12,6 @@ type Index struct {
 	name     string
 	required bool
 	indexer  MultiIndexer
-	tree     *iradix.Tree[[]*pbresource.Resource]
 }
 
 func New(name string, i Indexer, opts ...IndexOption) *Index {
@@ -36,7 +35,6 @@ func New(name string, i Indexer, opts ...IndexOption) *Index {
 	idx := &Index{
 		name:    name,
 		indexer: multiIndexer,
-		tree:    iradix.New[[]*pbresource.Resource](),
 	}
 
 	for _, opt := range opts {
@@ -50,7 +48,21 @@ func (i *Index) Name() string {
 	return i.name
 }
 
-func (i *Index) Txn() Txn {
+// IndexedData combines the Index with an radix tree for index and resource storage.
+func (i *Index) IndexedData() *IndexedData {
+	return &IndexedData{
+		Index: i,
+		tree:  iradix.New[[]*pbresource.Resource](),
+	}
+}
+
+// IndexedData is a wrapper around an Index and an radix tree for index and resource storage.
+type IndexedData struct {
+	*Index
+	tree *iradix.Tree[[]*pbresource.Resource]
+}
+
+func (i *IndexedData) Txn() Txn {
 	return &txn{
 		inner: i.tree.Txn(),
 		index: i,

internal/controller/cache/index/index_test.go

@@ -10,23 +10,24 @@ import (
 	"github.com/hashicorp/consul/internal/controller/cache/index/indexmock"
 	"github.com/hashicorp/consul/internal/resource/resourcetest"
 	"github.com/hashicorp/consul/proto-public/pbresource"
+	"github.com/hashicorp/consul/proto/private/prototest"
 	"github.com/stretchr/testify/require"
 )
 
 type testSingleIndexer struct{}
 
 func (testSingleIndexer) FromArgs(args ...any) ([]byte, error) {
-	return ReferenceOrIDFromArgs(args)
+	return ReferenceOrIDFromArgs(args...)
 }
 
-func (testSingleIndexer) FromResource(*pbresource.Resource) (bool, []byte, error) {
-	return false, nil, nil
+func (testSingleIndexer) FromResource(res *pbresource.Resource) (bool, []byte, error) {
+	return true, IndexFromRefOrID(res.Id), nil
 }
 
 type testMultiIndexer struct{}
 
 func (testMultiIndexer) FromArgs(args ...any) ([]byte, error) {
-	return ReferenceOrIDFromArgs(args)
+	return ReferenceOrIDFromArgs(args...)
 }
 
 func (testMultiIndexer) FromResource(*pbresource.Resource) (bool, [][]byte, error) {
@@ -135,3 +136,54 @@ func TestSingleIndexWrapper(t *testing.T) {
 		require.Equal(t, []byte{1, 2, 3}, vals[0])
 	})
 }
+
+func TestIndexReuse(t *testing.T) {
+	rtype := &pbresource.Type{
+		Group:        "test",
+		GroupVersion: "v1",
+		Kind:         "fake",
+	}
+	id := resourcetest.Resource(rtype, "foo").ID()
+
+	res1 := resourcetest.ResourceID(id).Build()
+	res2 := resourcetest.ResourceID(id).WithStatus("foo", &pbresource.Status{
+		ObservedGeneration: "woo",
+	}).Build()
+
+	indexer := testSingleIndexer{}
+
+	// Verify that the indexer produces an identical index for both resources. If this
+	// isn't true then the rest of the checks we do don't actually prove that the
+	// two IndexedData objects have independent resource storage.
+	_, idx1, _ := indexer.FromResource(res1)
+	_, idx2, _ := indexer.FromResource(res2)
+	require.Equal(t, idx1, idx2)
+
+	// Create the index and two indepent indexed data storage objects
+	idx := New("test", indexer)
+	data1 := idx.IndexedData()
+	data2 := idx.IndexedData()
+
+	// Push 1 resource into each
+	txn := data1.Txn()
+	txn.Insert(res1)
+	txn.Commit()
+
+	txn = data2.Txn()
+	txn.Insert(res2)
+	txn.Commit()
+
+	// Verify that querying the first indexed data can only return the first resource
+	iter, err := data1.Txn().ListIterator(id)
+	require.NoError(t, err)
+	res := iter.Next()
+	prototest.AssertDeepEqual(t, res1, res)
+	require.Nil(t, iter.Next())
+
+	// Verify that querying the second indexed data can only return the second resource
+	iter, err = data2.Txn().ListIterator(id)
+	require.NoError(t, err)
+	res = iter.Next()
+	prototest.AssertDeepEqual(t, res2, res)
+	require.Nil(t, iter.Next())
+}

internal/controller/cache/index/txn.go

@@ -12,7 +12,7 @@ import (
 
 type txn struct {
 	inner *iradix.Txn[[]*pbresource.Resource]
-	index *Index
+	index *IndexedData
 	dirty bool
 }
 

internal/controller/cache/index/txn_test.go

@@ -24,7 +24,7 @@ type txnSuite struct {
 	suite.Suite
 
 	indexer *indexmock.SingleIndexer
-	index   *Index
+	index   *IndexedData
 
 	r1   *pbresource.Resource
 	r2   *pbresource.Resource
@@ -34,7 +34,7 @@ type txnSuite struct {
 
 func (suite *txnSuite) SetupTest() {
 	suite.indexer = indexmock.NewSingleIndexer(suite.T())
-	suite.index = New("test", suite.indexer, IndexRequired)
+	suite.index = New("test", suite.indexer, IndexRequired).IndexedData()
 
 	suite.r1 = testResource("r1")
 	suite.r2 = testResource("r2")

internal/controller/cache/indexers/decoded_indexer_test.go

@@ -131,7 +131,7 @@ func (suite *decodedSingleIndexerSuite) TestIntegration() {
 
 	dec := resourcetest.MustDecode[*pbdemo.Album](suite.T(), res)
 
-	idx := DecodedSingleIndexer("test", suite.args.Execute, suite.indexer.Execute)
+	idx := DecodedSingleIndexer("test", suite.args.Execute, suite.indexer.Execute).IndexedData()
 
 	suite.indexer.EXPECT().
 		Execute(dec).
@@ -263,7 +263,7 @@ func (suite *decodedMultiIndexerSuite) TestIntegration() {
 
 	dec := resourcetest.MustDecode[*pbdemo.Album](suite.T(), res)
 
-	idx := DecodedMultiIndexer("test", suite.args.Execute, suite.indexer.Execute)
+	idx := DecodedMultiIndexer("test", suite.args.Execute, suite.indexer.Execute).IndexedData()
 
 	suite.indexer.EXPECT().
 		Execute(dec).

internal/controller/cache/indexers/id_indexer_test.go

@@ -17,7 +17,7 @@ import (
 )
 
 func TestIDIndex(t *testing.T) {
-	idx := IDIndex("test", index.IndexRequired)
+	idx := IDIndex("test", index.IndexRequired).IndexedData()
 
 	r1 := resourcetest.Resource(pbdemo.AlbumType, "foo").
 		WithTenancy(&pbresource.Tenancy{
@@ -39,7 +39,7 @@ func TestIDIndex(t *testing.T) {
 }
 
 func TestOwnerIndex(t *testing.T) {
-	idx := OwnerIndex("test", index.IndexRequired)
+	idx := OwnerIndex("test", index.IndexRequired).IndexedData()
 
 	r1 := resourcetest.Resource(pbdemo.AlbumType, "foo").
 		WithTenancy(&pbresource.Tenancy{
@@ -70,7 +70,7 @@ func TestOwnerIndex(t *testing.T) {
 func TestSingleIDOrRefIndex(t *testing.T) {
 	getRef := indexersmock.NewGetSingleRefOrID(t)
 
-	idx := SingleIDOrRefIndex("test", getRef.Execute)
+	idx := SingleIDOrRefIndex("test", getRef.Execute).IndexedData()
 
 	r1 := resourcetest.Resource(pbdemo.AlbumType, "foo").Build()
 	ref := &pbresource.Reference{

internal/controller/cache/indexers/ref_indexer_test.go

@@ -43,7 +43,7 @@ func TestRefOrIDIndex(t *testing.T) {
 
 	refs := indexersmock.NewRefOrIDFetcher[*pbdemo.Album, *pbresource.Reference](t)
 
-	idx := RefOrIDIndex("test", refs.Execute)
+	idx := RefOrIDIndex("test", refs.Execute).IndexedData()
 
 	refs.EXPECT().Execute(dec).
 		Return([]*pbresource.Reference{ref1, ref2}).
@@ -90,7 +90,7 @@ func TestBoundRefsIndex(t *testing.T) {
 		}).
 		Build()
 
-	idx := BoundRefsIndex[*pbmesh.ComputedRoutes]("test")
+	idx := BoundRefsIndex[*pbmesh.ComputedRoutes]("test").IndexedData()
 
 	txn := idx.Txn()
 	require.NoError(t, txn.Insert(r1))

internal/controller/cache/kind.go

@@ -18,16 +18,16 @@ type kindIndices struct {
 
 	it unversionedType
 
-	indices map[string]*index.Index
+	indices map[string]*index.IndexedData
 }
 
 func newKindIndices() *kindIndices {
 	kind := &kindIndices{
-		indices: make(map[string]*index.Index),
+		indices: make(map[string]*index.IndexedData),
 	}
 
 	// add the id index
-	kind.indices[IDIndex] = indexers.IDIndex(IDIndex, index.IndexRequired)
+	kind.indices[IDIndex] = indexers.IDIndex(IDIndex, index.IndexRequired).IndexedData()
 
 	return kind
 }
@@ -38,7 +38,7 @@ func (k *kindIndices) addIndex(i *index.Index) error {
 		return DuplicateIndexError{name: i.Name()}
 	}
 
-	k.indices[i.Name()] = i
+	k.indices[i.Name()] = i.IndexedData()
 	return nil
 }
 
@@ -178,7 +178,7 @@ func (k *kindIndices) delete(r *pbresource.Resource) error {
 	return nil
 }
 
-func (k *kindIndices) getIndex(name string) (*index.Index, error) {
+func (k *kindIndices) getIndex(name string) (*index.IndexedData, error) {
 	idx, ok := k.indices[name]
 	if !ok {
 		return nil, CacheTypeError{err: IndexNotFoundError{name: name}, it: k.it}