fix(index): account for default channel in index add order

Ensure the default channel referenced by a bundle exists, either by that
bundle or in the package, before returning it as the next element of an
input stream. Adding this check prevents smaller versions from breaking
index add when the `--overwrite-latest` flag is set; e.g. if a z-stream
bundle was originally added _after_ a larger y-stream and references a
default channel only defined in that y-stream.

Author: njhale

pkg/registry/input_stream.go

@@ -93,6 +93,23 @@ func (r *ReplacesInputStream) canAdd(bundle *Bundle, packageGraph *Package) erro
 		return fmt.Errorf("Invalid bundle %s, replaces nonexistent bundle %s", bundle.Name, replaces)
 	}
 
+	defaultChannel := bundle.Annotations.DefaultChannelName
+	if defaultChannel != "" && !packageGraph.HasChannel(defaultChannel) {
+		// We also can't add if the bundle isn't in the default channel it specifies or it doesn't already exist in the package
+		var defaultFound bool
+		for _, channel := range bundle.Channels {
+			if channel != defaultChannel {
+				continue
+			}
+			defaultFound = true
+			break
+		}
+
+		if !defaultFound {
+			return fmt.Errorf("Invalid bundle %s, references nonexistent default channel %s", bundle.Name, defaultChannel)
+		}
+	}
+
 	images, ok := r.packages[packageGraph.Name]
 	if !ok || images == nil {
 		// This shouldn't happen unless canAdd is being called without the correct setup

pkg/registry/input_stream_test.go

@@ -4,6 +4,7 @@ import (
 	"math"
 	"math/rand"
 	"reflect"
+	"strings"
 	"testing"
 	"testing/quick"
 
@@ -31,17 +32,25 @@ func unstructuredCSV() *unstructured.Unstructured {
 }
 
 type inputConfig struct {
-	pkg      string
-	name     string
-	version  string
-	replaces string
+	pkg            string
+	name           string
+	version        string
+	replaces       string
+	defaultChannel string
+	channels       []string
 }
 
 func newImageInput(config *inputConfig) *registry.ImageInput {
 	image := &registry.ImageInput{
 		Bundle: &registry.Bundle{
-			Package: config.pkg,
-			Name:    config.name,
+			Package:  config.pkg,
+			Name:     config.name,
+			Channels: config.channels,
+			Annotations: &registry.Annotations{
+				PackageName:        config.pkg,
+				Channels:           strings.Join(config.channels, ","),
+				DefaultChannelName: config.defaultChannel,
+			},
 		},
 	}
 
@@ -280,6 +289,125 @@ func TestReplacesTakesPrecedence(t *testing.T) {
 	}
 }
 
+func TestDefaultChannelAffectsOrder(t *testing.T) {
+	type args struct {
+		input []*registry.ImageInput
+	}
+	type expect struct {
+		err     bool
+		ordered []string
+	}
+	for _, tt := range []struct {
+		description string
+		args        args
+		expect      expect
+	}{
+		{
+			description: "BundleDefiningDefaultChannelIsFirst",
+			args: args{
+				input: []*registry.ImageInput{
+					newImageInput(&inputConfig{
+						name:           "a",
+						version:        "1.0.0",
+						channels:       []string{"stable"},
+						defaultChannel: "alpha",
+					}),
+					newImageInput(&inputConfig{
+						name:           "b",
+						version:        "1.1.0",
+						channels:       []string{"alpha", "stable"},
+						defaultChannel: "alpha",
+					}),
+				},
+			},
+			expect: expect{
+				ordered: []string{"b", "a"},
+			},
+		},
+		{
+			description: "ConflictingReplacesAndDefaultChannelReturnsError",
+			args: args{
+				input: []*registry.ImageInput{
+					newImageInput(&inputConfig{
+						name:           "a",
+						version:        "1.0.0",
+						channels:       []string{"stable"},
+						defaultChannel: "alpha",
+					}),
+					newImageInput(&inputConfig{
+						name:           "b",
+						version:        "1.1.0",
+						replaces:       "a",
+						channels:       []string{"alpha", "stable"},
+						defaultChannel: "alpha",
+					}),
+				},
+			},
+			expect: expect{
+				err: true,
+			},
+		},
+	} {
+		t.Run(tt.description, func(t *testing.T) {
+			addedChannels := map[string]registry.Channel{}
+			loader := &registryfakes.FakeGraphLoader{
+				GenerateStub: func(pkg string) (*registry.Package, error) {
+					return &registry.Package{
+						Name:     pkg,
+						Channels: addedChannels,
+					}, nil
+				},
+			}
+
+			stream, err := registry.NewReplacesInputStream(loader, tt.args.input)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+
+			if tt.expect.err {
+				_, err := stream.Next()
+				if err == nil {
+					t.Error("expected next to return error, returned nil instead")
+				}
+				return
+			}
+
+			for _, expected := range tt.expect.ordered {
+				next, err := stream.Next()
+				if err != nil {
+					t.Errorf("next returned unexpected error %s", err)
+					return
+				}
+				if next == nil {
+					t.Errorf("next returned unexpected nil, expecting %s", expected)
+					return
+				}
+
+				name := next.Bundle.Name
+				if name != expected {
+					t.Errorf("next returned unexpected bundle %s, expecting %s", name, expected)
+					return
+				}
+
+				// Simulate an add
+				for _, channel := range next.Bundle.Channels {
+					added := addedChannels[channel]
+					if added.Nodes == nil {
+						added.Nodes = map[registry.BundleKey]map[registry.BundleKey]struct{}{}
+					}
+					added.Nodes[registry.BundleKey{CsvName: name}] = nil
+					addedChannels[channel] = added
+				}
+			}
+
+			if !stream.Empty() {
+				t.Errorf("stream still contains content, expected end of content")
+			}
+		})
+	}
+}
+
 type missingReplaces struct {
 	input []*registry.ImageInput
 	valid int

pkg/registry/populator.go

@@ -487,7 +487,7 @@ func SemverPackageManifest(bundles []*Bundle) (*PackageManifest, error) {
 	}
 
 	if !defaultFound {
-		return nil, fmt.Errorf("unable to determine default channel")
+		return nil, fmt.Errorf("unable to determine default channel among channel heads: %+v", heads)
 	}
 
 	return pkg, nil