Skip to content

Commit 8e568ba

Browse files
committed
Fix infinite loop when a CSV replacement chain contains a cycle.
Signed-off-by: Ben Luddy <[email protected]>
1 parent 5e982c5 commit 8e568ba

File tree

2 files changed

+63
-0
lines changed

2 files changed

+63
-0
lines changed

pkg/controller/operators/olm/operator.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1912,6 +1912,9 @@ func (a *Operator) getReplacementChain(in *v1alpha1.ClusterServiceVersion, csvsI
19121912

19131913
next := replacement(current)
19141914
for next != nil {
1915+
if _, ok := csvsInChain[*next]; ok {
1916+
break // cycle
1917+
}
19151918
csvsInChain[*next] = struct{}{}
19161919
current = *next
19171920
next = replacement(current)
@@ -1923,6 +1926,9 @@ func (a *Operator) getReplacementChain(in *v1alpha1.ClusterServiceVersion, csvsI
19231926
csvsInChain[current] = struct{}{}
19241927
}
19251928
for prev != nil && *prev != "" {
1929+
if _, ok := csvsInChain[*prev]; ok {
1930+
break // cycle
1931+
}
19261932
current = *prev
19271933
csvsInChain[current] = struct{}{}
19281934
prev = replaces(current)

pkg/controller/operators/olm/operator_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4834,3 +4834,60 @@ func csvWithWebhook(csv *v1alpha1.ClusterServiceVersion, deploymentName string,
48344834
}
48354835
return csv
48364836
}
4837+
4838+
func TestGetReplacementChain(t *testing.T) {
4839+
CSV := func(name, replaces string) *v1alpha1.ClusterServiceVersion {
4840+
return &v1alpha1.ClusterServiceVersion{
4841+
ObjectMeta: metav1.ObjectMeta{
4842+
Name: name,
4843+
},
4844+
Spec: v1alpha1.ClusterServiceVersionSpec{
4845+
Replaces: replaces,
4846+
},
4847+
}
4848+
}
4849+
4850+
for _, tc := range []struct {
4851+
Name string
4852+
From *v1alpha1.ClusterServiceVersion
4853+
All map[string]*v1alpha1.ClusterServiceVersion
4854+
Expected []string
4855+
}{
4856+
{
4857+
Name: "csv replaces itself",
4858+
From: CSV("itself", "itself"),
4859+
All: map[string]*v1alpha1.ClusterServiceVersion{
4860+
"itself": CSV("itself", "itself"),
4861+
},
4862+
Expected: []string{"itself"},
4863+
},
4864+
{
4865+
Name: "two csvs replace each other",
4866+
From: CSV("a", "b"),
4867+
All: map[string]*v1alpha1.ClusterServiceVersion{
4868+
"a": CSV("a", "b"),
4869+
"b": CSV("b", "a"),
4870+
},
4871+
Expected: []string{"a", "b"},
4872+
},
4873+
{
4874+
Name: "starting from head of chain without cycles",
4875+
From: CSV("a", "b"),
4876+
All: map[string]*v1alpha1.ClusterServiceVersion{
4877+
"a": CSV("a", "b"),
4878+
"b": CSV("b", "c"),
4879+
"c": CSV("c", ""),
4880+
},
4881+
Expected: []string{"a", "b", "c"},
4882+
},
4883+
} {
4884+
t.Run(tc.Name, func(t *testing.T) {
4885+
assert := assert.New(t)
4886+
var actual []string
4887+
for name := range (&Operator{}).getReplacementChain(tc.From, tc.All) {
4888+
actual = append(actual, name)
4889+
}
4890+
assert.ElementsMatch(tc.Expected, actual)
4891+
})
4892+
}
4893+
}

0 commit comments

Comments
 (0)