opm validate: skips stranding is commutative

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

Author: joelanford

alpha/model/model.go

@@ -264,10 +264,12 @@ func (c *Channel) Validate() error {
 // validateReplacesChain checks the replaces chain of a channel.
 // Specifically the following rules must be followed:
 //  1. There must be exactly 1 channel head.
-//  2. Beginning at the head, the replaces chain must reach all non-skipped entries.
-//     Non-skipped entries are defined as entries that are not skipped by any other entry in the channel.
-//  3. There must be no cycles in the replaces chain.
-//  4. The tail entry in the replaces chain is permitted to replace a non-existent entry.
+//  2. Beginning at the head, the replaces chain traversal must reach all entries.
+//     Unreached entries are considered "stranded" and cause a channel to be invalid.
+//  3. Skipped entries are always leaf nodes. We never follow replaces or skips edges
+//     of skipped entries during replaces chain traversal.
+//  4. There must be no cycles in the replaces chain.
+//  5. The tail entry in the replaces chain is permitted to replace a non-existent entry.
 func (c *Channel) validateReplacesChain() error {
 	head, err := c.Head()
 	if err != nil {
@@ -289,12 +291,12 @@ func (c *Channel) validateReplacesChain() error {
 		if _, ok := chainFrom[cur.Name]; !ok {
 			chainFrom[cur.Name] = []string{cur.Name}
 		}
-		if skippedBundles.Has(cur.Replaces) {
+
+		skippedInChain = skippedInChain.Insert(cur.Skips...)
+		if skippedInChain.Has(cur.Replaces) {
 			break
 		}
-		for _, skip := range cur.Skips {
-			skippedInChain = skippedInChain.Insert(skip)
-		}
+
 		for k := range chainFrom {
 			chainFrom[k] = append(chainFrom[k], cur.Replaces)
 		}

alpha/model/model_test.go

@@ -132,7 +132,7 @@ func TestValidReplacesChain(t *testing.T) {
 			ch: Channel{Bundles: map[string]*Bundle{
 				"anakin.v0.0.1": {Name: "anakin.v0.0.1"},
 				"anakin.v0.0.2": {Name: "anakin.v0.0.2", Skips: []string{"anakin.v0.0.1"}},
-				"anakin.v0.0.3": {Name: "anakin.v0.0.3", Skips: []string{"anakin.v0.0.2"}},
+				"anakin.v0.0.3": {Name: "anakin.v0.0.3", Replaces: "anakin.v0.0.2"},
 				"anakin.v0.0.4": {Name: "anakin.v0.0.4", Replaces: "anakin.v0.0.3"},
 			}},
 			assertion: require.NoError,
@@ -181,8 +181,8 @@ func TestValidReplacesChain(t *testing.T) {
 			name: "Error/StrandedSkipsAfterReplaces",
 			ch: Channel{Bundles: map[string]*Bundle{
 				"anakin.v0.0.1": {Name: "anakin.v0.0.1"},
-				"anakin.v0.0.2": {Name: "anakin.v0.0.2", Replaces: "anakin.v0.0.1"},
-				"anakin.v0.0.3": {Name: "anakin.v0.0.3", Replaces: "anakin.v0.0.2", Skips: []string{"anakin.v0.0.3"}},
+				"anakin.v0.0.2": {Name: "anakin.v0.0.2", Replaces: "anakin.v0.0.1", Skips: []string{"anakin.v0.0.3"}},
+				"anakin.v0.0.3": {Name: "anakin.v0.0.3", Replaces: "anakin.v0.0.2"},
 				"anakin.v0.0.4": {Name: "anakin.v0.0.4", Replaces: "anakin.v0.0.3"},
 				"anakin.v0.0.5": {Name: "anakin.v0.0.5", Replaces: "anakin.v0.0.4"},
 			}},