Skip to content

Commit 1d1230f

Browse files
committed
opm validate: channel validation improvements
1. improve detection of bundles stranded due to usage of ancestor skips 2. find and report all cycles Signed-off-by: Joe Lanford <[email protected]>
1 parent 2629279 commit 1d1230f

File tree

4 files changed

+491
-89
lines changed

4 files changed

+491
-89
lines changed

alpha/model/graph.go

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
package model
2+
3+
import (
4+
"cmp"
5+
"errors"
6+
"fmt"
7+
"slices"
8+
"sort"
9+
"strings"
10+
11+
"github.com/blang/semver/v4"
12+
"golang.org/x/exp/maps"
13+
"k8s.io/apimachinery/pkg/util/sets"
14+
)
15+
16+
type graph struct {
17+
nodes map[string]*node
18+
}
19+
20+
func newGraph(c *Channel) *graph {
21+
nodes := map[string]*node{}
22+
23+
// Add all nodes (without edges)
24+
for _, b := range c.Bundles {
25+
nodes[b.Name] = &node{
26+
name: b.Name,
27+
version: b.Version,
28+
skipRange: b.SkipRange,
29+
replacedBy: make(map[string]*node),
30+
skippedBy: make(map[string]*node),
31+
skips: make(map[string]*node),
32+
}
33+
}
34+
35+
// Populate edges between nodes
36+
for _, b := range c.Bundles {
37+
n := nodes[b.Name]
38+
39+
if b.Replaces != "" {
40+
replaces, ok := nodes[b.Replaces]
41+
if !ok {
42+
// the "replaces" edge points to a node outside the channel
43+
replaces = &node{
44+
name: b.Replaces,
45+
replacedBy: make(map[string]*node),
46+
externalToChannel: true,
47+
}
48+
nodes[b.Replaces] = replaces
49+
}
50+
n.replaces = replaces
51+
n.replaces.replacedBy[n.name] = n
52+
}
53+
54+
for _, skipName := range b.Skips {
55+
skip, ok := nodes[skipName]
56+
if !ok {
57+
// the "skips" edge points to a node outside the channel
58+
skip = &node{
59+
name: skipName,
60+
skippedBy: make(map[string]*node),
61+
skips: make(map[string]*node),
62+
externalToChannel: true,
63+
}
64+
}
65+
skip.skippedBy[b.Name] = n
66+
n.skips[skipName] = skip
67+
}
68+
}
69+
70+
return &graph{
71+
nodes: nodes,
72+
}
73+
}
74+
75+
type node struct {
76+
name string
77+
version semver.Version
78+
replacedBy map[string]*node
79+
replaces *node
80+
skippedBy map[string]*node
81+
skips map[string]*node
82+
skipRange string
83+
externalToChannel bool
84+
}
85+
86+
func (n *node) pathsTo(other *node) [][]*node {
87+
var pathsToInternal func(existingPath []*node, froms map[string]*node, to *node) [][]*node
88+
pathsToInternal = func(existingPath []*node, froms map[string]*node, to *node) [][]*node {
89+
if len(froms) == 0 {
90+
// we never found a path to "to"
91+
return nil
92+
}
93+
var allPaths [][]*node
94+
for _, f := range froms {
95+
path := append(slices.Clone(existingPath), f)
96+
if f == to {
97+
// we found "to"!
98+
allPaths = append(allPaths, path)
99+
} else {
100+
// From an intermediate node, look only in replacedBy, so that we don't stray off the replaces chain.
101+
allPaths = append(allPaths, pathsToInternal(path, f.replacedBy, to)...)
102+
}
103+
}
104+
return allPaths
105+
}
106+
107+
// From the starting node, look in all ancestors (replacedBy and skippedBy).
108+
ancestors := map[string]*node{}
109+
maps.Copy(ancestors, n.replacedBy)
110+
maps.Copy(ancestors, n.skippedBy)
111+
return pathsToInternal(nil, ancestors, other)
112+
}
113+
114+
func (g *graph) validate() error {
115+
result := newValidationError("invalid upgrade graph")
116+
if err := g.validateNoCycles(); err != nil {
117+
result.subErrors = append(result.subErrors, err)
118+
}
119+
if err := g.validateNoStranded(); err != nil {
120+
result.subErrors = append(result.subErrors, err)
121+
}
122+
return result.orNil()
123+
}
124+
125+
func (g *graph) validateNoCycles() error {
126+
result := newValidationError("cycles found in graph")
127+
allCycles := [][]*node{}
128+
for _, n := range g.nodes {
129+
allCycles = append(allCycles, n.pathsTo(n)...)
130+
}
131+
dedupSameRotations(&allCycles)
132+
for _, cycle := range allCycles {
133+
cycleStr := nodeCycleString(append(cycle, cycle[0]))
134+
result.subErrors = append(result.subErrors, errors.New(cycleStr))
135+
}
136+
137+
return result.orNil()
138+
}
139+
140+
// dedupSameRotations removes rotations of the same cycle.
141+
// dedupSameRotations sorts the cycles so that shorter paths
142+
// and paths with lower versions appear earlier in the list.
143+
func dedupSameRotations(paths *[][]*node) {
144+
slices.SortFunc(*paths, func(a, b []*node) int {
145+
if len(a) == 0 && len(b) == 0 {
146+
return 0
147+
}
148+
if v := cmp.Compare(len(a), len(b)); v != 0 {
149+
return v
150+
}
151+
return a[0].version.Compare(b[0].version)
152+
})
153+
seen := map[string]struct{}{}
154+
tmp := (*paths)[:0]
155+
for _, path := range *paths {
156+
rotate(&path)
157+
k := nodeCycleString(path)
158+
if _, ok := seen[k]; ok {
159+
continue
160+
}
161+
seen[k] = struct{}{}
162+
tmp = append(tmp, path)
163+
}
164+
*paths = tmp
165+
}
166+
167+
func rotate(in *[]*node) {
168+
if len(*in) == 0 {
169+
return
170+
}
171+
maxIndex := 0
172+
for i, n := range (*in)[1:] {
173+
if n.version.GT((*in)[maxIndex].version) {
174+
maxIndex = i + 1
175+
}
176+
}
177+
slices.Reverse((*in)[:maxIndex])
178+
slices.Reverse((*in)[maxIndex:])
179+
slices.Reverse((*in))
180+
}
181+
182+
func nodeCycleString(nodes []*node) string {
183+
return strings.Join(mapSlice(nodes, nodeName), " -> ")
184+
}
185+
186+
func (g *graph) validateNoStranded() error {
187+
head, err := g.head()
188+
if err != nil {
189+
return err
190+
}
191+
all := sets.New[*node](maps.Values(g.nodes)...)
192+
chain := sets.New[*node]()
193+
skipped := sets.New[*node]()
194+
195+
cur := head
196+
for cur != nil && !skipped.Has(cur) && !chain.Has(cur) {
197+
chain.Insert(cur)
198+
skipped.Insert(maps.Values(cur.skips)...)
199+
cur = cur.replaces
200+
}
201+
202+
stranded := all.Difference(chain).Difference(skipped)
203+
if stranded.Len() > 0 {
204+
strandedNames := mapSlice(stranded.UnsortedList(), nodeName)
205+
slices.Sort(strandedNames)
206+
return fmt.Errorf("channel contains one or more stranded bundles: %s", strings.Join(strandedNames, ", "))
207+
}
208+
209+
return nil
210+
}
211+
212+
func (g *graph) head() (*node, error) {
213+
heads := []*node{}
214+
for _, n := range g.nodes {
215+
if len(n.replacedBy) == 0 && len(n.skippedBy) == 0 {
216+
heads = append(heads, n)
217+
}
218+
}
219+
if len(heads) == 0 {
220+
return nil, fmt.Errorf("no channel head found in graph")
221+
}
222+
if len(heads) > 1 {
223+
headNames := mapSlice(heads, nodeName)
224+
sort.Strings(headNames)
225+
return nil, fmt.Errorf("multiple channel heads found in graph: %s", strings.Join(headNames, ", "))
226+
}
227+
return heads[0], nil
228+
}
229+
230+
func nodeName(n *node) string {
231+
return n.name
232+
}
233+
234+
func mapSlice[I, O any](s []I, fn func(I) O) []O {
235+
result := make([]O, 0, len(s))
236+
for _, i := range s {
237+
result = append(result, fn(i))
238+
}
239+
return result
240+
}

alpha/model/graph_test.go

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package model
2+
3+
import (
4+
"testing"
5+
6+
"github.com/blang/semver/v4"
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func Test_dedupSameRotations(t *testing.T) {
11+
type spec struct {
12+
name string
13+
paths [][]*node
14+
expect [][]*node
15+
}
16+
for _, s := range []spec{
17+
{
18+
name: "Empty",
19+
paths: [][]*node{},
20+
expect: [][]*node{},
21+
},
22+
{
23+
name: "One",
24+
paths: [][]*node{{{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")}}},
25+
expect: [][]*node{{{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")}}},
26+
},
27+
{
28+
name: "Two",
29+
paths: [][]*node{
30+
{
31+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
32+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
33+
},
34+
{
35+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
36+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
37+
},
38+
},
39+
expect: [][]*node{{
40+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
41+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
42+
}},
43+
},
44+
{
45+
name: "Three",
46+
paths: [][]*node{
47+
{
48+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
49+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
50+
{name: "anakin.v0.0.3", version: semver.MustParse("0.0.3")},
51+
},
52+
{
53+
{name: "anakin.v0.0.3", version: semver.MustParse("0.0.3")},
54+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
55+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
56+
},
57+
{
58+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
59+
{name: "anakin.v0.0.3", version: semver.MustParse("0.0.3")},
60+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
61+
},
62+
},
63+
expect: [][]*node{{
64+
{name: "anakin.v0.0.3", version: semver.MustParse("0.0.3")},
65+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
66+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
67+
}},
68+
},
69+
{
70+
name: "Multiple",
71+
paths: [][]*node{
72+
{
73+
{name: "anakin.v0.0.4", version: semver.MustParse("0.0.4")},
74+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
75+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
76+
},
77+
{
78+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
79+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
80+
{name: "anakin.v0.0.3", version: semver.MustParse("0.0.3")},
81+
},
82+
{
83+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
84+
{name: "anakin.v0.0.3", version: semver.MustParse("0.0.3")},
85+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
86+
},
87+
{
88+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
89+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
90+
{name: "anakin.v0.0.4", version: semver.MustParse("0.0.4")},
91+
},
92+
{
93+
{name: "anakin.v0.0.3", version: semver.MustParse("0.0.3")},
94+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
95+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
96+
},
97+
{
98+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
99+
{name: "anakin.v0.0.4", version: semver.MustParse("0.0.4")},
100+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
101+
},
102+
},
103+
expect: [][]*node{
104+
{
105+
{name: "anakin.v0.0.3", version: semver.MustParse("0.0.3")},
106+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
107+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
108+
},
109+
{
110+
{name: "anakin.v0.0.4", version: semver.MustParse("0.0.4")},
111+
{name: "anakin.v0.0.1", version: semver.MustParse("0.0.1")},
112+
{name: "anakin.v0.0.2", version: semver.MustParse("0.0.2")},
113+
},
114+
},
115+
},
116+
} {
117+
t.Run(s.name, func(t *testing.T) {
118+
dedupSameRotations(&s.paths)
119+
assert.Equal(t, s.expect, s.paths)
120+
})
121+
}
122+
}

0 commit comments

Comments
 (0)