fix(indexing): respect strict mode

pkg/registry/input_stream.go

@@ -90,7 +90,7 @@ func (r *ReplacesInputStream) canAdd(bundle *Bundle, packageGraph *Package) erro
 
 	if replaces != "" && !packageGraph.HasCsv(replaces) {
 		// We can't add this until a replacement exists
-		return fmt.Errorf("Invalid bundle %s, bundle specifies a non-existent replacement %s", bundle.Name, replaces)
+		return fmt.Errorf("Invalid bundle %s, replaces nonexistent bundle %s", bundle.Name, replaces)
 	}
 
 	images, ok := r.packages[packageGraph.Name]
@@ -145,11 +145,8 @@ func (r *ReplacesInputStream) Next() (*ImageInput, error) {
 			return image, nil
 		}
 
-		// No viable bundle found in the package, can't parse it any further
-		if len(packageErrs) > 0 {
-			delete(r.packages, pkg)
-			errs = append(errs, packageErrs...)
-		}
+		// No viable bundle found in the package, can't parse it any further, so return any errors
+		errs = append(errs, packageErrs...)
 	}
 
 	// We've exhausted all valid input bundles, any errors here indicate invalid input of some kind

pkg/registry/input_stream_test.go

@@ -280,35 +280,102 @@ func TestReplacesTakesPrecedence(t *testing.T) {
 	}
 }
 
-func TestMissingReplacesReturnsError(t *testing.T) {
-	input := []*registry.ImageInput{
-		newImageInput(&inputConfig{
-			name:     genName(""),
-			replaces: genName(""), // This won't exist and should cause a failure
-		}),
-	}
+type missingReplaces struct {
+	input []*registry.ImageInput
+	valid int
+}
 
-	loader := &registryfakes.FakeGraphLoader{
-		GenerateStub: func(pkg string) (*registry.Package, error) {
-			return &registry.Package{
-				Name: pkg,
-			}, nil
-		},
+func (missingReplaces) Generate(r *rand.Rand, max int) reflect.Value {
+	if max < 1 {
+		max++
+	}
+	size := r.Intn(max + 1) // [1, max] inputs total
+	if size < 1 {
+		size++
 	}
+	valid := r.Intn(size)       // [0, size) valid inputs (at least one invalid)
+	pkgSize := r.Intn(size + 1) // Each distinct package contains [1, size] inputs
+	if pkgSize == 0 {
+		pkgSize++
+	}
+	var pkg string
 
-	stream, err := registry.NewReplacesInputStream(loader, input)
-	if err != nil {
-		t.Error(err)
-		return
+	input := make([]*registry.ImageInput, size)
+	var version semver.Version
+	for i := 0; i < size; i++ {
+		if size%pkgSize == 0 {
+			pkg = genName("")
+		}
+
+		version = bumpVersion(r, version)
+		config := inputConfig{
+			pkg:     pkg,
+			name:    genName(""),
+			version: version.String(), // Be sure each bundle has a valid sort order via semver for tests using this generator
+		}
+
+		if i >= valid {
+			config.replaces = genName("missing-")
+		}
+
+		input[i] = newImageInput(&config)
 	}
 
-	next, err := stream.Next()
-	if err == nil {
-		t.Errorf("expected next to return an error")
-		return
+	// Ensure we're not depending on any ordering
+	r.Shuffle(len(input), func(i, j int) { input[i], input[j] = input[j], input[i] })
+
+	return reflect.ValueOf(missingReplaces{
+		input: input,
+		valid: valid,
+	})
+}
+
+func TestMissingReplacesReturnsError(t *testing.T) {
+	f := func(missing missingReplaces) bool {
+		if len(missing.input) < 1 {
+			return true
+		}
+
+		loader := &registryfakes.FakeGraphLoader{
+			GenerateStub: func(pkg string) (*registry.Package, error) {
+				return &registry.Package{
+					Name: pkg,
+				}, nil
+			},
+		}
+
+		stream, err := registry.NewReplacesInputStream(loader, missing.input)
+		if err != nil {
+			t.Error(err)
+			return false
+		}
+
+		for i := 0; i < missing.valid; i++ {
+			next, err := stream.Next()
+			if err != nil {
+				t.Errorf("next returned unexpected error: %s", err)
+				return false
+			}
+			if next == nil {
+				t.Errorf("next returned unexpected nil")
+				return false
+			}
+		}
+
+		// The next call should result in an error
+		next, err := stream.Next()
+		if err == nil {
+			t.Errorf("expected next to return an error")
+			return false
+		}
+		if next != nil {
+			t.Errorf("expected next to return nil on error, got %v instead", next)
+			return false
+		}
+
+		return true
 	}
-	if next != nil {
-		t.Errorf("expected next to return nil on error, got %v instead", next)
-		return
+	if err := quick.Check(f, nil); err != nil {
+		t.Error(err)
 	}
 }

pkg/registry/populator.go

@@ -196,7 +196,6 @@ func (i *DirectoryPopulator) loadManifests(imagesToAdd []*ImageInput, imagesToRe
 				errs = append(errs, err)
 				break
 			}
-
 		}
 
 		if len(errs) > 0 {

pkg/registry/populator_test.go

@@ -714,8 +714,8 @@ func TestDirectoryPopulator(t *testing.T) {
 
 	err = populate(add)
 	require.NotNil(t, err)
-	require.Contains(t, err.Error(), fmt.Sprintf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "etcdoperator.v0.9.2", "etcdoperator.v0.9.0"))
-	require.Contains(t, err.Error(), fmt.Sprintf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "prometheusoperator.0.22.2", "prometheusoperator.0.15.0"))
+	require.Contains(t, err.Error(), fmt.Sprintf("Invalid bundle %s, replaces nonexistent bundle %s", "etcdoperator.v0.9.2", "etcdoperator.v0.9.0"))
+	require.Contains(t, err.Error(), fmt.Sprintf("Invalid bundle %s, replaces nonexistent bundle %s", "prometheusoperator.0.22.2", "prometheusoperator.0.15.0"))
 }
 
 func TestDeprecateBundle(t *testing.T) {
@@ -943,8 +943,8 @@ func TestOverwrite(t *testing.T) {
 			},
 			expected: expected{
 				errs: []error{
-					fmt.Errorf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "etcdoperator.v0.9.2", "etcdoperator.v0.9.0"),
-					fmt.Errorf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "prometheusoperator.0.22.2", "prometheusoperator.0.15.0"),
+					fmt.Errorf("Invalid bundle %s, replaces nonexistent bundle %s", "etcdoperator.v0.9.2", "etcdoperator.v0.9.0"),
+					fmt.Errorf("Invalid bundle %s, replaces nonexistent bundle %s", "prometheusoperator.0.22.2", "prometheusoperator.0.15.0"),
 				},
 				remainingBundles: []string{
 					"quay.io/test/prometheus.0.14.0/preview",