Skip to content

Commit 16d1089

Browse files
authored
Make ClusterExtensionRevision .spec.phases optional (#2230)
Given that at the creation we can create a revision with no phases, we should declare `.spec.phases` as optional. Added initial set of unit tests to assert CER .spec immutability.
1 parent 31e8b60 commit 16d1089

File tree

6 files changed

+194
-9
lines changed

6 files changed

+194
-9
lines changed

api/v1/clusterextensionrevision_types.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,13 @@ type ClusterExtensionRevisionSpec struct {
5555
// +kubebuilder:validation:XValidation:rule="self == oldSelf", message="revision is immutable"
5656
Revision int64 `json:"revision"`
5757
// Phases are groups of objects that will be applied at the same time.
58-
// All objects in the a phase will have to pass their probes in order to progress to the next phase.
58+
// All objects in the phase will have to pass their probes in order to progress to the next phase.
5959
//
60-
// +kubebuilder:validation:Required
6160
// +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf.size() == 0", message="phases is immutable"
6261
// +listType=map
6362
// +listMapKey=name
64-
Phases []ClusterExtensionRevisionPhase `json:"phases"`
63+
// +optional
64+
Phases []ClusterExtensionRevisionPhase `json:"phases,omitempty"`
6565
// Previous references previous revisions that objects can be adopted from.
6666
//
6767
// +kubebuilder:validation:XValidation:rule="self == oldSelf", message="previous is immutable"
@@ -104,6 +104,7 @@ type ClusterExtensionRevisionObject struct {
104104
// already existing on the cluster or even owned by another controller.
105105
//
106106
// +kubebuilder:default="Prevent"
107+
// +optional
107108
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
108109
}
109110

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package v1
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
"k8s.io/apimachinery/pkg/api/errors"
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
)
12+
13+
func TestClusterExtensionRevisionImmutability(t *testing.T) {
14+
c := newClient(t)
15+
ctx := context.Background()
16+
i := 0
17+
for name, tc := range map[string]struct {
18+
spec ClusterExtensionRevisionSpec
19+
updateFunc func(*ClusterExtensionRevision)
20+
allowed bool
21+
}{
22+
"revision is immutable": {
23+
spec: ClusterExtensionRevisionSpec{
24+
Revision: 1,
25+
},
26+
updateFunc: func(cer *ClusterExtensionRevision) {
27+
cer.Spec.Revision = 2
28+
},
29+
},
30+
"phases may be initially empty": {
31+
spec: ClusterExtensionRevisionSpec{
32+
Phases: []ClusterExtensionRevisionPhase{},
33+
},
34+
updateFunc: func(cer *ClusterExtensionRevision) {
35+
cer.Spec.Phases = []ClusterExtensionRevisionPhase{
36+
{
37+
Name: "foo",
38+
Objects: []ClusterExtensionRevisionObject{},
39+
},
40+
}
41+
},
42+
allowed: true,
43+
},
44+
"phases may be initially unset": {
45+
spec: ClusterExtensionRevisionSpec{},
46+
updateFunc: func(cer *ClusterExtensionRevision) {
47+
cer.Spec.Phases = []ClusterExtensionRevisionPhase{
48+
{
49+
Name: "foo",
50+
Objects: []ClusterExtensionRevisionObject{},
51+
},
52+
}
53+
},
54+
allowed: true,
55+
},
56+
"phases are immutable if not empty": {
57+
spec: ClusterExtensionRevisionSpec{
58+
Phases: []ClusterExtensionRevisionPhase{
59+
{
60+
Name: "foo",
61+
Objects: []ClusterExtensionRevisionObject{},
62+
},
63+
},
64+
},
65+
updateFunc: func(cer *ClusterExtensionRevision) {
66+
cer.Spec.Phases = []ClusterExtensionRevisionPhase{
67+
{
68+
Name: "foo2",
69+
Objects: []ClusterExtensionRevisionObject{},
70+
},
71+
}
72+
},
73+
},
74+
} {
75+
t.Run(name, func(t *testing.T) {
76+
cer := &ClusterExtensionRevision{
77+
ObjectMeta: metav1.ObjectMeta{
78+
Name: fmt.Sprintf("foo%d", i),
79+
},
80+
Spec: tc.spec,
81+
}
82+
i = i + 1
83+
require.NoError(t, c.Create(ctx, cer))
84+
tc.updateFunc(cer)
85+
err := c.Update(ctx, cer)
86+
if tc.allowed && err != nil {
87+
t.Fatal("expected update to succeed, but got:", err)
88+
}
89+
if !tc.allowed && !errors.IsInvalid(err) {
90+
t.Fatal("expected update to fail due to invalid payload, but got:", err)
91+
}
92+
})
93+
}
94+
}

api/v1/suite_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
Copyright 2025.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v1
18+
19+
import (
20+
"log"
21+
"os"
22+
"path/filepath"
23+
"testing"
24+
25+
"github.com/stretchr/testify/require"
26+
apimachineryruntime "k8s.io/apimachinery/pkg/runtime"
27+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
28+
"k8s.io/client-go/rest"
29+
"sigs.k8s.io/controller-runtime/pkg/client"
30+
"sigs.k8s.io/controller-runtime/pkg/envtest"
31+
)
32+
33+
func newScheme(t *testing.T) *apimachineryruntime.Scheme {
34+
sch := apimachineryruntime.NewScheme()
35+
require.NoError(t, AddToScheme(sch))
36+
return sch
37+
}
38+
39+
func newClient(t *testing.T) client.Client {
40+
cl, err := client.New(config, client.Options{Scheme: newScheme(t)})
41+
require.NoError(t, err)
42+
require.NotNil(t, cl)
43+
return cl
44+
}
45+
46+
var config *rest.Config
47+
48+
func TestMain(m *testing.M) {
49+
testEnv := &envtest.Environment{
50+
CRDDirectoryPaths: []string{
51+
filepath.Join("..", "..", "helm", "olmv1", "base", "operator-controller", "crd", "experimental"),
52+
},
53+
ErrorIfCRDPathMissing: true,
54+
}
55+
56+
// ENVTEST-based tests require specific binaries. By default, these binaries are located
57+
// in paths defined by controller-runtime. However, the `BinaryAssetsDirectory` needs
58+
// to be explicitly set when running tests directly (e.g., debugging tests in an IDE)
59+
// without using the Makefile targets.
60+
//
61+
// This is equivalent to configuring your IDE to export the `KUBEBUILDER_ASSETS` environment
62+
// variable before each test execution. The following function simplifies this process
63+
// by handling the configuration for you.
64+
//
65+
// To ensure the binaries are in the expected path without manual configuration, run:
66+
// `make envtest-k8s-bins`
67+
if getFirstFoundEnvTestBinaryDir() != "" {
68+
testEnv.BinaryAssetsDirectory = getFirstFoundEnvTestBinaryDir()
69+
}
70+
71+
var err error
72+
config, err = testEnv.Start()
73+
utilruntime.Must(err)
74+
if config == nil {
75+
log.Panic("expected cfg to not be nil")
76+
}
77+
78+
code := m.Run()
79+
utilruntime.Must(testEnv.Stop())
80+
os.Exit(code)
81+
}
82+
83+
// getFirstFoundEnvTestBinaryDir finds and returns the first directory under the given path.
84+
func getFirstFoundEnvTestBinaryDir() string {
85+
basePath := filepath.Join("..", "..", "bin", "envtest-binaries", "k8s")
86+
entries, _ := os.ReadDir(basePath)
87+
for _, entry := range entries {
88+
if entry.IsDir() {
89+
return filepath.Join(basePath, entry.Name())
90+
}
91+
}
92+
return ""
93+
}

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ spec:
5757
phases:
5858
description: |-
5959
Phases are groups of objects that will be applied at the same time.
60-
All objects in the a phase will have to pass their probes in order to progress to the next phase.
60+
All objects in the phase will have to pass their probes in order to progress to the next phase.
6161
items:
6262
description: |-
6363
ClusterExtensionRevisionPhase are groups of objects that will be applied at the same time.
@@ -130,7 +130,6 @@ spec:
130130
- message: revision is immutable
131131
rule: self == oldSelf
132132
required:
133-
- phases
134133
- revision
135134
type: object
136135
status:

manifests/experimental-e2e.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ spec:
648648
phases:
649649
description: |-
650650
Phases are groups of objects that will be applied at the same time.
651-
All objects in the a phase will have to pass their probes in order to progress to the next phase.
651+
All objects in the phase will have to pass their probes in order to progress to the next phase.
652652
items:
653653
description: |-
654654
ClusterExtensionRevisionPhase are groups of objects that will be applied at the same time.
@@ -721,7 +721,6 @@ spec:
721721
- message: revision is immutable
722722
rule: self == oldSelf
723723
required:
724-
- phases
725724
- revision
726725
type: object
727726
status:

manifests/experimental.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,7 @@ spec:
613613
phases:
614614
description: |-
615615
Phases are groups of objects that will be applied at the same time.
616-
All objects in the a phase will have to pass their probes in order to progress to the next phase.
616+
All objects in the phase will have to pass their probes in order to progress to the next phase.
617617
items:
618618
description: |-
619619
ClusterExtensionRevisionPhase are groups of objects that will be applied at the same time.
@@ -686,7 +686,6 @@ spec:
686686
- message: revision is immutable
687687
rule: self == oldSelf
688688
required:
689-
- phases
690689
- revision
691690
type: object
692691
status:

0 commit comments

Comments
 (0)