Skip to content

Commit aef073d

Browse files
exdxexdx
committed
fix: add check for storage version changes when installing CRDs. This
prevents data loss when a CRD is upgraded and no longer supports existing stored versions of CRs on the cluster.
1 parent 1849f65 commit aef073d

File tree

4 files changed

+376
-21
lines changed

4 files changed

+376
-21
lines changed

‎pkg/controller/operators/catalog/step.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,14 @@ func (b *builder) NewCRDV1Step(client apiextensionsv1client.ApiextensionsV1Inter
139139
}
140140
}
141141

142-
// TODO ensure stored version compatibility
142+
// check to see if stored versions changed and whether the upgrade could cause potential data loss
143+
err = crdlib.SafeStorageVersionUpgrade(currentCRD, crd)
144+
if err != nil {
145+
b.logger.Errorf("potential for data loss updating %s: %s", step.Resource.Name, err)
146+
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "potential for data loss updating %s", step.Resource.Name)
147+
}
148+
149+
143150
// Update CRD to new version
144151
_, err = client.CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{})
145152
if err != nil {
@@ -219,7 +226,13 @@ func (b *builder) NewCRDV1Beta1Step(client apiextensionsv1beta1client.Apiextensi
219226
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "error validating existing CRs agains new CRD's schema: %s", step.Resource.Name)
220227
}
221228
}
222-
// TODO ensure stored version compatibility
229+
230+
// check to see if stored versions changed and whether the upgrade could cause potential data loss
231+
err = crdlib.SafeStorageVersionUpgrade(currentCRD, crd)
232+
if err != nil {
233+
b.logger.Errorf("potential for data loss updating %s: %s", step.Resource.Name, err)
234+
return v1alpha1.StepStatusUnknown, errorwrap.Wrapf(err, "potential for data loss updating %s", step.Resource.Name)
235+
}
223236

224237
// Update CRD to new version
225238
_, err = client.CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{})

‎pkg/lib/crd/storage.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package crd
2+
3+
import (
4+
"fmt"
5+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
6+
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
7+
"k8s.io/apimachinery/pkg/runtime"
8+
)
9+
10+
// SafeStorageVersionUpgrade determines whether the new CRD spec includes all the storage versions of the existing on-cluster CRD.
11+
// For each stored version in the status of the CRD on the cluster (there will be at least one) - each version must exist in the spec of the new CRD that is being installed.
12+
// See https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definition-versioning/#upgrade-existing-objects-to-a-new-stored-version.
13+
func SafeStorageVersionUpgrade(existingCRD runtime.Object, newCRD runtime.Object) error {
14+
newSpecVersions, existingStatusVersions := getStoredVersions(existingCRD, newCRD)
15+
if newSpecVersions == nil {
16+
return fmt.Errorf("could not find a stored version in the spec of the new CRD: found %#v", newSpecVersions)
17+
}
18+
if existingStatusVersions == nil {
19+
return fmt.Errorf("could not find a stored version in the status of the existing CRD: found %#v", existingStatusVersions)
20+
}
21+
22+
for name := range existingStatusVersions {
23+
if _, ok := newSpecVersions[name]; !ok {
24+
// a storage version in the status of the old CRD is not present in the spec of the new CRD
25+
// potential data loss of CRs without a storage migration - throw error and block the CRD upgrade
26+
return fmt.Errorf("new CRD removes version %s that is listed as a stored version on the existing CRD", name)
27+
}
28+
}
29+
30+
return nil
31+
}
32+
33+
// getStoredVersions returns the storage versions listed in the status of the old on-cluster CRD
34+
// and the storage versions listed in the spec of the new CRD.
35+
func getStoredVersions(oldCRD runtime.Object, newCRD runtime.Object) (newSpecVersions map[string]struct{}, existingStatusVersions map[string]struct{}) {
36+
existingStatusVersions = make(map[string]struct{})
37+
newSpecVersions = make(map[string]struct{})
38+
39+
// find old storage versions by inspect the status field of the existing on-cluster CRD
40+
switch crd := oldCRD.(type) {
41+
case *apiextensionsv1.CustomResourceDefinition:
42+
for _, version := range crd.Status.StoredVersions {
43+
existingStatusVersions[version] = struct{}{}
44+
}
45+
case *apiextensionsv1beta1.CustomResourceDefinition:
46+
for _, version := range crd.Status.StoredVersions {
47+
existingStatusVersions[version] = struct{}{}
48+
}
49+
}
50+
51+
switch crd := newCRD.(type) {
52+
case *apiextensionsv1.CustomResourceDefinition:
53+
for _, version := range crd.Spec.Versions {
54+
newSpecVersions[version.Name] = struct{}{}
55+
}
56+
case *apiextensionsv1beta1.CustomResourceDefinition:
57+
if crd.Spec.Version != "" {
58+
newSpecVersions[crd.Spec.Version] = struct{}{}
59+
}
60+
for _, version := range crd.Spec.Versions {
61+
newSpecVersions[version.Name] = struct{}{}
62+
}
63+
}
64+
65+
return newSpecVersions, existingStatusVersions
66+
}

‎pkg/lib/crd/storage_test.go

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package crd
2+
3+
import (
4+
"github.com/stretchr/testify/require"
5+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
6+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
7+
"testing"
8+
)
9+
10+
const crdName = "test"
11+
12+
func TestSafeStorageVersionUpgradeFailure(t *testing.T) {
13+
existingCRD := &apiextensionsv1.CustomResourceDefinition{
14+
ObjectMeta: metav1.ObjectMeta{
15+
Name: crdName,
16+
},
17+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
18+
Group: "cluster.com",
19+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
20+
{
21+
Name: "v1alpha2",
22+
Served: true,
23+
Storage: true, //stored at v1alpha1
24+
Schema: &apiextensionsv1.CustomResourceValidation{
25+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
26+
Type: "object",
27+
Description: "my crd schema",
28+
},
29+
},
30+
},
31+
},
32+
},
33+
Status: apiextensionsv1.CustomResourceDefinitionStatus{
34+
StoredVersions: []string{"v1alpha2"},
35+
},
36+
}
37+
38+
newCRD := &apiextensionsv1.CustomResourceDefinition{
39+
ObjectMeta: metav1.ObjectMeta{
40+
Name: crdName,
41+
},
42+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
43+
Group: "cluster.com",
44+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
45+
{
46+
Name: "v1alpha3",
47+
Served: true,
48+
Storage: true,
49+
Schema: &apiextensionsv1.CustomResourceValidation{
50+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
51+
Type: "object",
52+
Description: "my crd schema",
53+
},
54+
},
55+
},
56+
},
57+
},
58+
}
59+
60+
err := SafeStorageVersionUpgrade(existingCRD, newCRD)
61+
// expect error, since crd upgrade is not safe
62+
require.Error(t, err, "expected error for unsafe CRD upgrade")
63+
64+
// error should be related to the storage upgrade removing a version
65+
require.Contains(t, err.Error(), "new CRD removes version", "expected storage version error")
66+
}
67+
68+
func TestSafeStorageVersionUpgradeSuccess(t *testing.T) {
69+
existingCRD := &apiextensionsv1.CustomResourceDefinition{
70+
ObjectMeta: metav1.ObjectMeta{
71+
Name: crdName,
72+
},
73+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
74+
Group: "cluster.com",
75+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
76+
{
77+
Name: "v1alpha2",
78+
Served: true,
79+
Storage: false,
80+
Schema: &apiextensionsv1.CustomResourceValidation{
81+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
82+
Type: "object",
83+
Description: "my crd schema",
84+
},
85+
},
86+
},
87+
{
88+
Name: "v1alpha3",
89+
Served: true,
90+
Storage: true, //stored at v1alpha1
91+
Schema: &apiextensionsv1.CustomResourceValidation{
92+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
93+
Type: "object",
94+
Description: "my crd schema",
95+
},
96+
},
97+
},
98+
},
99+
},
100+
Status: apiextensionsv1.CustomResourceDefinitionStatus{
101+
StoredVersions: []string{"v1alpha2", "v1alpha3"},
102+
},
103+
}
104+
105+
newCRD := &apiextensionsv1.CustomResourceDefinition{
106+
ObjectMeta: metav1.ObjectMeta{
107+
Name: crdName,
108+
},
109+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
110+
Group: "cluster.com",
111+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
112+
{
113+
Name: "v1alpha2",
114+
Served: true,
115+
Storage: false,
116+
Schema: &apiextensionsv1.CustomResourceValidation{
117+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
118+
Type: "object",
119+
Description: "my crd schema",
120+
},
121+
},
122+
},
123+
{
124+
Name: "v1alpha3",
125+
Served: true,
126+
Storage: true,
127+
Schema: &apiextensionsv1.CustomResourceValidation{
128+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
129+
Type: "object",
130+
Description: "my crd schema",
131+
},
132+
},
133+
},
134+
},
135+
},
136+
}
137+
138+
err := SafeStorageVersionUpgrade(existingCRD, newCRD)
139+
// expect error, since crd upgrade is not safe
140+
require.NoError(t, err, "did not expect error for safe CRD upgrade")
141+
}

0 commit comments

Comments
 (0)