From fa65c306ad7ba098c2454eba674197f6121f6f91 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Tue, 23 Nov 2021 09:38:26 -0500 Subject: [PATCH 1/2] 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 --- pkg/sqlite/load.go | 10 +++--- pkg/sqlite/load_test.go | 78 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/pkg/sqlite/load.go b/pkg/sqlite/load.go index 79824892d..10e4f7687 100644 --- a/pkg/sqlite/load.go +++ b/pkg/sqlite/load.go @@ -28,6 +28,8 @@ type MigratableLoader interface { var _ MigratableLoader = &sqlLoader{} +const startDepth = 0 + func newSQLLoader(db *sql.DB, opts ...DbOption) (*sqlLoader, error) { options := defaultDBOptions() for _, o := range opts { @@ -424,7 +426,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 +497,7 @@ 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 { + if len(channel.Nodes)+startDepth-1 != 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 +610,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 +622,7 @@ func (s *sqlLoader) addPackageChannels(tx *sql.Tx, manifest registry.PackageMani } channelEntryCSVName := c.CurrentCSVName - depth := 1 + depth := startDepth + 1 // Since this loop depends on following 'replaces', keep track of where it's been replaceCycle := map[string]bool{channelEntryCSVName: true} diff --git a/pkg/sqlite/load_test.go b/pkg/sqlite/load_test.go index 7af8112ec..af289ceae 100644 --- a/pkg/sqlite/load_test.go +++ b/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, ®istry.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 := ®istry.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, ®istry.Annotations{ PackageName: pkgName, From 5c0ca7575c6f0f744c336245c71146575eba350f Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Tue, 23 Nov 2021 21:06:39 -0500 Subject: [PATCH 2/2] pkg/sqlite/load.go: add descriptive comments for startDepth Signed-off-by: Joe Lanford --- pkg/sqlite/load.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/sqlite/load.go b/pkg/sqlite/load.go index 10e4f7687..47540b181 100644 --- a/pkg/sqlite/load.go +++ b/pkg/sqlite/load.go @@ -28,6 +28,10 @@ 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) { @@ -497,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)+startDepth-1 != depth { + // expectedDepth is: + // + - 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) } @@ -622,6 +631,9 @@ func (s *sqlLoader) addPackageChannels(tx *sql.Tx, manifest registry.PackageMani } channelEntryCSVName := c.CurrentCSVName + + // 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