Skip to content

Commit 40493e6

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #169 from ankitathomas/maxocperror
Bug 1992677: validate maxocpversion to have major.minor format
2 parents 5bd4cad + a5bcfe0 commit 40493e6

File tree

7 files changed

+108
-78
lines changed

7 files changed

+108
-78
lines changed

‎staging/api/pkg/validation/internal/community.go

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,20 @@ func checkMaxOpenShiftVersion(checks CommunityOperatorChecks, v1beta1MsgForResou
149149

150150
semVerVersionMaxOcp, err := semver.ParseTolerant(olmMaxOpenShiftVersionValue)
151151
if err != nil {
152-
checks.errs = append(checks.errs, fmt.Errorf("csv.Annotations.%s has an invalid value."+
152+
checks.errs = append(checks.errs, fmt.Errorf("csv.Annotations.%s has an invalid value. "+
153153
"Unable to parse (%s) using semver : %s",
154154
olmproperties, olmMaxOpenShiftVersionValue, err))
155155
return checks
156156
}
157157

158+
truncatedMaxOcp := semver.Version{Major: semVerVersionMaxOcp.Major, Minor: semVerVersionMaxOcp.Minor}
159+
if !semVerVersionMaxOcp.EQ(truncatedMaxOcp) {
160+
checks.warns = append(checks.warns, fmt.Errorf("csv.Annotations.%s has an invalid value. "+
161+
"%s must specify only major.minor versions, %s will be truncated to %s",
162+
olmproperties, olmmaxOpenShiftVersion, semVerVersionMaxOcp, truncatedMaxOcp))
163+
return checks
164+
}
165+
158166
if semVerVersionMaxOcp.GE(semVerOCPV1beta1Unsupported) {
159167
checks.errs = append(checks.errs, fmt.Errorf("csv.Annotations.%s with the "+
160168
"key and value for %s has the OCP version value %s which is >= of %s. This bundle is using APIs which were deprecated and removed in v1.22. More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. "+
@@ -248,12 +256,12 @@ func validateImageFile(checks CommunityOperatorChecks, deprecatedAPImsg string)
248256
}
249257

250258
if verParsed.GE(semVerOCPV1beta1Unsupported) {
251-
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were " +
252-
"deprecated and removed in v1.22. " +
253-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
259+
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were "+
260+
"deprecated and removed in v1.22. "+
261+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. "+
254262
"Migrate the API(s) for "+
255-
"%s or provide compatible version(s) by using the %s annotation in " +
256-
"`metadata/annotations.yaml` to ensure that the index image will be geneared " +
263+
"%s or provide compatible version(s) by using the %s annotation in "+
264+
"`metadata/annotations.yaml` to ensure that the index image will be geneared "+
257265
"with its label. (e.g. LABEL %s='4.6-4.8')",
258266
deprecatedAPImsg,
259267
ocpLabelindex,
@@ -263,12 +271,12 @@ func validateImageFile(checks CommunityOperatorChecks, deprecatedAPImsg string)
263271
} else {
264272
// if not has not the = then the value needs contains - value less < 4.9
265273
if !strings.Contains(indexRange, "-") {
266-
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were " +
267-
"deprecated and removed in v1.22. " +
274+
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were "+
275+
"deprecated and removed in v1.22. "+
268276
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22 "+
269277
"The %s allows to distribute it on >= %s. Migrate the API(s) for "+
270-
"%s or provide compatible version(s) by using the %s annotation in " +
271-
"`metadata/annotations.yaml` to ensure that the index image will be generated " +
278+
"%s or provide compatible version(s) by using the %s annotation in "+
279+
"`metadata/annotations.yaml` to ensure that the index image will be generated "+
272280
"with its label. (e.g. LABEL %s='4.6-4.8')",
273281
indexRange,
274282
ocpVerV1beta1Unsupported,
@@ -287,12 +295,12 @@ func validateImageFile(checks CommunityOperatorChecks, deprecatedAPImsg string)
287295
}
288296

289297
if verParsed.GE(semVerOCPV1beta1Unsupported) {
290-
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were " +
291-
"deprecated and removed in v1.22. " +
292-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
298+
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were "+
299+
"deprecated and removed in v1.22. "+
300+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. "+
293301
"Upgrade the APIs from "+
294-
"for %s or provide compatible distribution version(s) by using the %s " +
295-
"annotation in `metadata/annotations.yaml` to ensure that the index image will " +
302+
"for %s or provide compatible distribution version(s) by using the %s "+
303+
"annotation in `metadata/annotations.yaml` to ensure that the index image will "+
296304
"be generated with its label. (e.g. LABEL %s='4.6-4.8')",
297305
deprecatedAPImsg,
298306
ocpLabelindex,
@@ -310,8 +318,8 @@ func validateImageFile(checks CommunityOperatorChecks, deprecatedAPImsg string)
310318
}
311319
}
312320
} else {
313-
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were deprecated and " +
314-
"removed in v1.22. More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
321+
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were deprecated and "+
322+
"removed in v1.22. More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. "+
315323
"Migrate the APIs "+
316324
"for %s or provide compatible version(s) via the labels. (e.g. LABEL %s='4.6-4.8')",
317325
deprecatedAPImsg,

‎staging/api/pkg/validation/internal/community_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,31 @@ func Test_communityValidator(t *testing.T) {
104104
"This bundle is using APIs which were deprecated and removed in v1.22. " +
105105
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22 "},
106106
},
107+
{
108+
name: "should warn on patch version in maxOpenShiftVersion",
109+
wantWarning: true,
110+
args: args{
111+
bundleDir: "./testdata/valid_bundle_v1beta1",
112+
imageIndexPath: "./testdata/dockerfile/valid_bundle.Dockerfile",
113+
annotations: map[string]string{
114+
"olm.properties": fmt.Sprintf(`[{"type": "olm.maxOpenShiftVersion", "value": "4.8.1"}]`),
115+
},
116+
},
117+
warnStrings: []string{
118+
"Warning: Value : (etcdoperator.v0.9.4) csv.Annotations.olm.properties has an invalid value. olm.maxOpenShiftVersion must specify only major.minor versions, 4.8.1 will be truncated to 4.8.0",
119+
},
120+
},
121+
{
122+
name: "should pass when the maxOpenShiftVersion is semantically equivalent to <major>.<minor>.0",
123+
wantError: false,
124+
args: args{
125+
bundleDir: "./testdata/valid_bundle_v1beta1",
126+
imageIndexPath: "./testdata/dockerfile/valid_bundle.Dockerfile",
127+
annotations: map[string]string{
128+
"olm.properties": fmt.Sprintf(`[{"type": "olm.maxOpenShiftVersion", "value": "4.8.0+build"}]`),
129+
},
130+
},
131+
},
107132
}
108133

109134
for _, tt := range tests {

‎staging/operator-lifecycle-manager/pkg/controller/operators/openshift/clusteroperator_controller_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package openshift
22

33
import (
44
"fmt"
5-
65
semver "github.com/blang/semver/v4"
76
. "github.com/onsi/ginkgo"
87
. "github.com/onsi/gomega"
@@ -213,6 +212,7 @@ var _ = Describe("ClusterOperator controller", func() {
213212
}).Should(Succeed())
214213
}()
215214

215+
parsedVersion := semver.MustParse(clusterVersion)
216216
Eventually(func() ([]configv1.ClusterOperatorStatusCondition, error) {
217217
err := k8sClient.Get(ctx, clusterOperatorName, co)
218218
return co.Status.Conditions, err
@@ -224,7 +224,7 @@ var _ = Describe("ClusterOperator controller", func() {
224224
{
225225
namespace: ns.GetName(),
226226
name: incompatible.GetName(),
227-
maxOpenShiftVersion: clusterVersion,
227+
maxOpenShiftVersion: fmt.Sprintf("%d.%d", parsedVersion.Major, parsedVersion.Minor),
228228
},
229229
}.String(),
230230
LastTransitionTime: fixedNow(),
@@ -270,7 +270,7 @@ var _ = Describe("ClusterOperator controller", func() {
270270
{
271271
namespace: ns.GetName(),
272272
name: incompatible.GetName(),
273-
maxOpenShiftVersion: short + ".0",
273+
maxOpenShiftVersion: short,
274274
},
275275
}.String(),
276276
LastTransitionTime: fixedNow(),

‎staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func (s skew) String() string {
110110
return fmt.Sprintf("%s/%s has invalid %s properties: %s", s.namespace, s.name, MaxOpenShiftVersionProperty, s.err)
111111
}
112112

113-
return fmt.Sprintf("%s/%s is incompatible with OpenShift versions greater than %s", s.namespace, s.name, s.maxOpenShiftVersion)
113+
return fmt.Sprintf("%s/%s is incompatible with OpenShift minor versions greater than %s", s.namespace, s.name, s.maxOpenShiftVersion)
114114
}
115115

116116
type transientError struct {
@@ -165,7 +165,8 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
165165
if max == nil || max.GTE(next) {
166166
continue
167167
}
168-
s.maxOpenShiftVersion = max.String()
168+
169+
s.maxOpenShiftVersion = fmt.Sprintf("%d.%d", max.Major, max.Minor)
169170

170171
incompatible = append(incompatible, s)
171172
}
@@ -252,7 +253,11 @@ func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) (*semver.
252253
return nil, fmt.Errorf(`Failed to parse "%s" as semver: %w`, value, err)
253254
}
254255

255-
return &version, nil
256+
truncatedVersion := semver.Version{Major: version.Major, Minor: version.Minor}
257+
if !version.EQ(truncatedVersion) {
258+
return nil, fmt.Errorf("property %s must specify only <major>.<minor> version, got invalid value %s", MaxOpenShiftVersionProperty, version)
259+
}
260+
return &truncatedVersion, nil
256261
}
257262

258263
func notCopiedSelector() (labels.Selector, error) {

‎staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers_test.go

Lines changed: 14 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,6 @@ func TestIncompatibleOperators(t *testing.T) {
251251
{
252252
name: "chestnut",
253253
namespace: "default",
254-
maxOpenShiftVersion: "1.2.0-pre+build",
255-
},
256-
{
257-
name: "drupe",
258-
namespace: "default",
259254
maxOpenShiftVersion: "2.0.0",
260255
},
261256
},
@@ -309,27 +304,27 @@ func TestIncompatibleOperators(t *testing.T) {
309304
{
310305
name: "almond",
311306
namespace: "default",
312-
maxOpenShiftVersion: "1.0.0",
307+
maxOpenShiftVersion: "1.0",
313308
},
314309
{
315310
name: "beech",
316311
namespace: "default",
317-
maxOpenShiftVersion: "1.0.0+build",
312+
maxOpenShiftVersion: "1.0",
318313
},
319314
{
320-
name: "chestnut",
321-
namespace: "default",
322-
maxOpenShiftVersion: "1.1.0-pre",
315+
name: "chestnut",
316+
namespace: "default",
317+
err: fmt.Errorf("property olm.maxOpenShiftVersion must specify only <major>.<minor> version, got invalid value 1.1.0-pre"),
323318
},
324319
{
325-
name: "drupe",
326-
namespace: "default",
327-
maxOpenShiftVersion: "1.1.0-pre+build",
320+
name: "drupe",
321+
namespace: "default",
322+
err: fmt.Errorf("property olm.maxOpenShiftVersion must specify only <major>.<minor> version, got invalid value 1.1.0-pre+build"),
328323
},
329324
{
330325
name: "european-hazelnut",
331326
namespace: "default",
332-
maxOpenShiftVersion: "0.1.0",
327+
maxOpenShiftVersion: "0.1",
333328
},
334329
},
335330
},
@@ -369,12 +364,12 @@ func TestIncompatibleOperators(t *testing.T) {
369364
{
370365
name: "beech",
371366
namespace: "default",
372-
maxOpenShiftVersion: "1.0.0",
367+
maxOpenShiftVersion: "1.0",
373368
},
374369
{
375370
name: "chestnut",
376371
namespace: "default",
377-
maxOpenShiftVersion: "1.0.0",
372+
maxOpenShiftVersion: "1.0",
378373
},
379374
},
380375
},
@@ -414,7 +409,7 @@ func TestIncompatibleOperators(t *testing.T) {
414409
{
415410
name: "beech",
416411
namespace: "default",
417-
maxOpenShiftVersion: "1.0.0",
412+
maxOpenShiftVersion: "1.0",
418413
},
419414
{
420415
name: "chestnut",
@@ -469,11 +464,6 @@ func TestIncompatibleOperators(t *testing.T) {
469464
},
470465
},
471466
in: skews{
472-
{
473-
name: "almond",
474-
namespace: "default",
475-
maxOpenShiftVersion: "1.1.2",
476-
},
477467
{
478468
name: "beech",
479469
namespace: "default",
@@ -503,21 +493,10 @@ func TestIncompatibleOperators(t *testing.T) {
503493
namespace: "default",
504494
maxOpenShiftVersion: "1.1.0",
505495
},
506-
{
507-
name: "beech",
508-
namespace: "default",
509-
maxOpenShiftVersion: "1.1.0-pre",
510-
},
511496
},
512497
expect: expect{
513-
err: false,
514-
incompatible: skews{
515-
{
516-
name: "beech",
517-
namespace: "default",
518-
maxOpenShiftVersion: "1.1.0-pre",
519-
},
520-
},
498+
err: false,
499+
incompatible: nil,
521500
},
522501
},
523502
} {

‎vendor/github.com/operator-framework/api/pkg/validation/internal/community.go

Lines changed: 25 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)