pkg/sqlite: use consistent starting depth when building channel_entry table (#868)

* pkg/sqlite: use consistent starting depth when building channel_entry table

When building the channel_entry table, always using a starting depth of
0. This is important because the ListBundles query makes an assumption
that all packages in the database will have the same minimum depth.

Signed-off-by: Joe Lanford <[email protected]>

* pkg/sqlite/load.go: add descriptive comments for startDepth

Signed-off-by: Joe Lanford <[email protected]>
Upstream-repository: operator-registry
Upstream-commit: f478fddfe36153536fdef0e1f30f673fd85a94d7

Author: joelanford

staging/operator-registry/pkg/sqlite/load.go

@@ -28,6 +28,12 @@ type MigratableLoader interface {
 
 var _ MigratableLoader = &sqlLoader{}
 
+// startDepth is the depth that channel heads should be assigned
+// in the channel_entry table. This const exists so that all
+// add modes (replaces, semver, and semver-skippatch) are
+// consistent.
+const startDepth = 0
+
 func newSQLLoader(db *sql.DB, opts ...DbOption) (*sqlLoader, error) {
 	options := defaultDBOptions()
 	for _, o := range opts {
@@ -424,7 +430,7 @@ func (s *sqlLoader) AddPackageChannelsFromGraph(graph *registry.Package) error {
 	// update each channel's graph
 	for channelName, channel := range graph.Channels {
 		currentNode := channel.Head
-		depth := 1
+		depth := startDepth
 
 		var previousNodeID int64
 
@@ -495,7 +501,12 @@ func (s *sqlLoader) AddPackageChannelsFromGraph(graph *registry.Package) error {
 
 			// we got to the end of the channel graph
 			if nextNode.IsEmpty() {
-				if len(channel.Nodes) != depth {
+				// expectedDepth is:
+				//   <number-of-nodes> + <start-depth> - 1
+				// For example, if the number of nodes is 3 and the startDepth is 0, the expected depth is 2 (0, 1, 2)
+				// If the number of nodes is 5 and the startDepth is 3, the expected depth is 7 (3, 4, 5, 6, 7)
+				expectedDepth := len(channel.Nodes) + startDepth - 1
+				if expectedDepth != depth {
 					err := fmt.Errorf("Invalid graph: some (non-bottom) nodes defined in the graph were not mentioned as replacements of any node")
 					errs = append(errs, err)
 				}
@@ -608,7 +619,7 @@ func (s *sqlLoader) addPackageChannels(tx *sql.Tx, manifest registry.PackageMani
 	}
 
 	for _, c := range channels {
-		res, err := addChannelEntry.Exec(c.Name, manifest.PackageName, c.CurrentCSVName, 0)
+		res, err := addChannelEntry.Exec(c.Name, manifest.PackageName, c.CurrentCSVName, startDepth)
 		if err != nil {
 			errs = append(errs, fmt.Errorf("failed to add channel %q in package %q: %s", c.Name, manifest.PackageName, err.Error()))
 			continue
@@ -620,7 +631,10 @@ func (s *sqlLoader) addPackageChannels(tx *sql.Tx, manifest registry.PackageMani
 		}
 
 		channelEntryCSVName := c.CurrentCSVName
-		depth := 1
+
+		// depth is set to `startDepth + 1` here because we already added the channel head
+		// with depth `startDepth` above.
+		depth := startDepth + 1
 
 		// Since this loop depends on following 'replaces', keep track of where it's been
 		replaceCycle := map[string]bool{channelEntryCSVName: true}

staging/operator-registry/pkg/sqlite/load_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strings"
 	"testing"
@@ -154,6 +155,71 @@ func TestAddPackageChannels(t *testing.T) {
 	}
 }
 
+func TestAddBundleSemver(t *testing.T) {
+	// Create a test DB
+	db, cleanup := CreateTestDb(t)
+	defer cleanup()
+	store, err := NewSQLLiteLoader(db)
+	require.NoError(t, err)
+	err = store.Migrate(context.TODO())
+	require.NoError(t, err)
+	graphLoader, err := NewSQLGraphLoaderFromDB(db)
+	require.NoError(t, err)
+
+	// Seed the db with a replaces-mode bundle/package
+	replacesBundle := newBundle(t, "csv-a", "pkg-foo", []string{"stable"}, newUnstructuredCSV(t, "csv-a", ""))
+	err = store.AddOperatorBundle(replacesBundle)
+	require.NoError(t, err)
+
+	err = store.AddPackageChannels(registry.PackageManifest{
+		PackageName: "pkg-foo",
+		Channels: []registry.PackageChannel{
+			{
+				Name:           "stable",
+				CurrentCSVName: "csv-a",
+			},
+		},
+		DefaultChannelName: "stable",
+	})
+	require.NoError(t, err)
+
+	// Add semver bundles in non-semver order.
+	bundles := []*registry.Bundle{
+		newBundle(t, "csv-3", "pkg-0", []string{"stable"}, newUnstructuredCSVWithVersion(t, "csv-3", "0.3.0")),
+		newBundle(t, "csv-1", "pkg-0", []string{"stable"}, newUnstructuredCSVWithVersion(t, "csv-1", "0.1.0")),
+		newBundle(t, "csv-2", "pkg-0", []string{"stable"}, newUnstructuredCSVWithVersion(t, "csv-2", "0.2.0")),
+	}
+	for _, b := range bundles {
+		graph, err := graphLoader.Generate(b.Package)
+		require.Conditionf(t, func() bool {
+			return err == nil || errors.Is(err, registry.ErrPackageNotInDatabase)
+		}, "got unexpected error: %v", err)
+		bundleLoader := registry.BundleGraphLoader{}
+		updatedGraph, err := bundleLoader.AddBundleToGraph(b, graph, &registry.AnnotationsFile{Annotations: *b.Annotations}, false)
+		require.NoError(t, err)
+		err = store.AddBundleSemver(updatedGraph, b)
+		require.NoError(t, err)
+	}
+
+	// Ensure bundles can be queried with expected replaces and skips values.
+	querier := NewSQLLiteQuerierFromDb(db)
+	gotBundles, err := querier.ListBundles(context.Background())
+	require.NoError(t, err)
+	replaces := map[string]string{}
+	for _, b := range gotBundles {
+		if b.PackageName != "pkg-0" {
+			continue
+		}
+		require.Len(t, b.Skips, 0, "unexpected skips value(s) for bundle %q", b.CsvName)
+		replaces[b.CsvName] = b.Replaces
+	}
+	require.Equal(t, map[string]string{
+		"csv-3": "csv-2",
+		"csv-2": "csv-1",
+		"csv-1": "",
+	}, replaces)
+}
+
 func TestClearNonHeadBundles(t *testing.T) {
 	db, cleanup := CreateTestDb(t)
 	defer cleanup()
@@ -237,6 +303,18 @@ func newUnstructuredCSVWithSkips(t *testing.T, name, replaces string, skips ...s
 	return &unstructured.Unstructured{Object: out}
 }
 
+func newUnstructuredCSVWithVersion(t *testing.T, name, version string) *unstructured.Unstructured {
+	csv := &registry.ClusterServiceVersion{}
+	csv.TypeMeta.Kind = "ClusterServiceVersion"
+	csv.SetName(name)
+	versionJson := fmt.Sprintf(`{"version": "%s"}`, version)
+	csv.Spec = json.RawMessage(versionJson)
+
+	out, err := runtime.DefaultUnstructuredConverter.ToUnstructured(csv)
+	require.NoError(t, err)
+	return &unstructured.Unstructured{Object: out}
+}
+
 func newBundle(t *testing.T, name, pkgName string, channels []string, objs ...*unstructured.Unstructured) *registry.Bundle {
 	bundle := registry.NewBundle(name, &registry.Annotations{
 		PackageName: pkgName,