Merge pull request #528 from njhale/fix-default-chan

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

Author: openshift-merge-robot

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