diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/clusteroperator_controller_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/clusteroperator_controller_test.go index 72a0552ff2..9f3e9b6d47 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/clusteroperator_controller_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/clusteroperator_controller_test.go @@ -195,7 +195,6 @@ var _ = Describe("ClusterOperator controller", func() { }).Should(Succeed()) }() - parsedVersion := semver.MustParse(clusterVersion) Eventually(func() ([]configv1.ClusterOperatorStatusCondition, error) { err := k8sClient.Get(ctx, clusterOperatorName, co) return co.Status.Conditions, err @@ -207,7 +206,7 @@ var _ = Describe("ClusterOperator controller", func() { { namespace: ns.GetName(), name: incompatible.GetName(), - maxOpenShiftVersion: fmt.Sprintf("%d.%d", parsedVersion.Major, parsedVersion.Minor), + maxOpenShiftVersion: clusterVersion, }, }.String(), LastTransitionTime: fixedNow(), @@ -266,7 +265,6 @@ var _ = Describe("ClusterOperator controller", func() { return k8sClient.Update(ctx, incompatible) }).Should(Succeed()) - _, parseErr := semver.ParseTolerant("garbage") Eventually(func() ([]configv1.ClusterOperatorStatusCondition, error) { err := k8sClient.Get(ctx, clusterOperatorName, co) return co.Status.Conditions, err @@ -276,9 +274,9 @@ var _ = Describe("ClusterOperator controller", func() { Reason: IncompatibleOperatorsInstalled, Message: skews{ { - namespace: ns.GetName(), - name: incompatible.GetName(), - err: fmt.Errorf(`failed to parse "garbage" as semver: %w`, parseErr), + namespace: ns.GetName(), + name: incompatible.GetName(), + maxOpenShiftVersion: "garbage", }, }.String(), LastTransitionTime: fixedNow(), diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go b/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go index 2e85462db2..66e1e14e8c 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "sort" "strings" "sync" @@ -74,39 +75,97 @@ func versionsMatch(a []configv1.OperandVersion, b []configv1.OperandVersion) boo return len(counts) < 1 } -type skews []skew +type skews []*skew func (s skews) String() string { - msg := make([]string, len(s)) - i, j := 0, len(s)-1 - for _, sk := range s { - m := sk.String() - // Partial order: error skews first - if sk.err != nil { - msg[i] = m - i++ - continue + // Sorting depends on a populated .err, to sort invalid skews + // first. incompatibleOperators sets .err through one call path, + // but check again here, to ensure reliability, regardless of call-chain. + var minVersion *semver.Version + for i := range s { + s[i].process() + if s[i].err == nil && (minVersion == nil || s[i].normalizedMaxOpenShiftVersion.LT(*minVersion)) { + minVersion = s[i].normalizedMaxOpenShiftVersion } - msg[j] = m - j-- } - // TODO Revisit this message after 4.23 release once release schedule is determined for future possible minor versions. - return fmt.Sprintf("ClusterServiceVersions blocking minor version upgrades to 4.23 or major version upgrades to 5.0:\n%s", strings.Join(msg, "\n")) + // Sort so the message does not depend on the (unstable) order in which the + // CSVs were listed: error skews first, then by namespace/name. A different + // order would be a different Message, and so a condition transition. + sorted := make(skews, len(s)) + copy(sorted, s) + sort.SliceStable(sorted, func(i, j int) bool { + a, b := sorted[i], sorted[j] + if (a.err != nil) != (b.err != nil) { + return a.err != nil + } + if a.namespace != b.namespace { + return a.namespace < b.namespace + } + return a.name < b.name + }) + + msg := make([]string, len(sorted)) + for i, sk := range sorted { + msg[i] = sk.String() + } + + var next string + if minVersion == nil { + next = "next major or minor version" + } else { + n := nextY(*minVersion) + next = fmt.Sprintf("%d.%d", n.Major, n.Minor) + } + return fmt.Sprintf("ClusterServiceVersions blocking minor version upgrades to %s or higher:\n%s", next, strings.Join(msg, "\n")) } type skew struct { namespace string name string maxOpenShiftVersion string - err error + + // the following properties are populated by skew.process() + err error + normalizedMaxOpenShiftVersion *semver.Version +} + +func (s *skew) process() { + if s == nil || s.normalizedMaxOpenShiftVersion != nil || s.err != nil { + return + } + + if s.maxOpenShiftVersion == "" { + // Handle "" separately, so parse doesn't treat it as a zero + s.err = fmt.Errorf(`%s value cannot be "" (an empty string)`, MaxOpenShiftVersionProperty) + s.normalizedMaxOpenShiftVersion = &semver.Version{} + return + } + + version, err := semver.ParseTolerant(s.maxOpenShiftVersion) + s.normalizedMaxOpenShiftVersion = &version + if err != nil { + s.err = fmt.Errorf(`failed to parse %s=%q as semver: %w`, MaxOpenShiftVersionProperty, s.maxOpenShiftVersion, err) + return + } + + truncatedVersion := semver.Version{Major: version.Major, Minor: version.Minor} + if !s.normalizedMaxOpenShiftVersion.EQ(truncatedVersion) { + s.err = fmt.Errorf("%s property must specify only . version, got invalid value %q", MaxOpenShiftVersionProperty, s.normalizedMaxOpenShiftVersion) + return + } + + // Normalize the operator's maxOpenShiftVersion: 4.23 and 5.0 are equivalent. + version = normalizeOCPVersion(*s.normalizedMaxOpenShiftVersion) + s.normalizedMaxOpenShiftVersion = &version } -func (s skew) String() string { +func (s *skew) String() string { + s.process() if s.err != nil { return fmt.Sprintf("- %s/%s has invalid %s properties: %s", s.namespace, s.name, MaxOpenShiftVersionProperty, s.err) } - return fmt.Sprintf("- maximum supported OCP version for %s/%s is %s", s.namespace, s.name, s.maxOpenShiftVersion) + return fmt.Sprintf("- maximum supported OCP version for %s/%s is %d.%d", s.namespace, s.name, s.normalizedMaxOpenShiftVersion.Major, s.normalizedMaxOpenShiftVersion.Minor) } type transientError struct { @@ -146,29 +205,18 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error continue } - s := skew{ - name: csv.GetName(), - namespace: csv.GetNamespace(), - } - max, err := maxOpenShiftVersion(&csv) - if err != nil { - s.err = err - incompatible = append(incompatible, s) + s := maxOpenShiftVersion(&csv) + if s == nil { continue - } - - if max == nil { + } else if s.err != nil { + incompatible = append(incompatible, s) continue } - // Normalize the operator's maxOpenShiftVersion: 4.23 and 5.0 are equivalent. - normalizedMax := normalizeOCPVersion(*max) - if normalizedMax.GTE(nextMinor) { + if s.normalizedMaxOpenShiftVersion.GTE(nextMinor) { continue } - s.maxOpenShiftVersion = fmt.Sprintf("%d.%d", max.Major, max.Minor) - incompatible = append(incompatible, s) } @@ -246,16 +294,22 @@ const ( MaxOpenShiftVersionProperty = "olm.maxOpenShiftVersion" ) -func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) (*semver.Version, error) { +func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) *skew { + s := &skew{ + name: csv.GetName(), + namespace: csv.GetNamespace(), + } + // Extract the property from the CSV's annotations if possible annotation, ok := csv.GetAnnotations()[projection.PropertiesAnnotationKey] if !ok { - return nil, nil + return nil } properties, err := projection.PropertyListFromPropertiesAnnotation(annotation) if err != nil { - return nil, err + s.err = err + return s } var max *string @@ -265,33 +319,23 @@ func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) (*semver. } if max != nil { - return nil, fmt.Errorf(`defining more than one "%s" property is not allowed`, MaxOpenShiftVersionProperty) + s.err = fmt.Errorf(`defining more than one %q property is not allowed`, MaxOpenShiftVersionProperty) + return s } max = &property.Value } if max == nil { - return nil, nil + return nil } // Account for any additional quoting value := strings.Trim(*max, "\"") - if value == "" { - // Handle "" separately, so parse doesn't treat it as a zero - return nil, fmt.Errorf(`value cannot be "" (an empty string)`) - } - version, err := semver.ParseTolerant(value) - if err != nil { - return nil, fmt.Errorf(`failed to parse "%s" as semver: %w`, value, err) - } - - truncatedVersion := semver.Version{Major: version.Major, Minor: version.Minor} - if !version.EQ(truncatedVersion) { - return nil, fmt.Errorf("property %s must specify only . version, got invalid value %s", MaxOpenShiftVersionProperty, version) - } - return &truncatedVersion, nil + s.maxOpenShiftVersion = value + s.process() + return s } func notCopiedSelector() (labels.Selector, error) { diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers_test.go index 43d8c5e63f..a7c6de2cfc 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers_test.go @@ -19,6 +19,14 @@ import ( "github.com/operator-framework/operator-registry/pkg/api" ) +func mustParse(s string) *semver.Version { + version, err := semver.ParseTolerant(s) + if err != nil { + panic(fmt.Sprintf("bad version given for test case: %s", err)) + } + return &version +} + func TestConditionsEqual(t *testing.T) { type args struct { a, b *configv1.ClusterOperatorStatusCondition @@ -284,29 +292,36 @@ func TestIncompatibleOperators(t *testing.T) { err: false, incompatible: skews{ { - name: "almond", - namespace: "default", - maxOpenShiftVersion: "1.0", + name: "almond", + namespace: "default", + maxOpenShiftVersion: "1.0.0", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0}, }, { - name: "beech", - namespace: "default", - maxOpenShiftVersion: "1.0", + name: "beech", + namespace: "default", + maxOpenShiftVersion: "1.0.0+build", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 1, Minor: 0}, }, { - name: "chestnut", - namespace: "default", - err: fmt.Errorf("property olm.maxOpenShiftVersion must specify only . version, got invalid value 1.1.0-pre"), + name: "chestnut", + namespace: "default", + maxOpenShiftVersion: "1.1.0-pre", + err: fmt.Errorf(`olm.maxOpenShiftVersion property must specify only . version, got invalid value "1.1.0-pre"`), + normalizedMaxOpenShiftVersion: mustParse("1.1.0-pre"), }, { - name: "drupe", - namespace: "default", - err: fmt.Errorf("property olm.maxOpenShiftVersion must specify only . version, got invalid value 1.1.0-pre+build"), + name: "drupe", + namespace: "default", + maxOpenShiftVersion: "1.1.0-pre+build", + err: fmt.Errorf(`olm.maxOpenShiftVersion property must specify only . version, got invalid value "1.1.0-pre+build"`), + normalizedMaxOpenShiftVersion: mustParse("1.1.0-pre+build"), }, { - name: "european-hazelnut", - namespace: "default", - maxOpenShiftVersion: "0.1", + name: "european-hazelnut", + namespace: "default", + maxOpenShiftVersion: "0.1.0", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 0, Minor: 1}, }, }, }, @@ -335,14 +350,16 @@ func TestIncompatibleOperators(t *testing.T) { err: false, incompatible: skews{ { - name: "beech", - namespace: "default", - maxOpenShiftVersion: "1.0", + name: "beech", + namespace: "default", + maxOpenShiftVersion: "1.0.0", + normalizedMaxOpenShiftVersion: mustParse("1.0.0"), }, { - name: "chestnut", - namespace: "default", - maxOpenShiftVersion: "1.0", + name: "chestnut", + namespace: "default", + maxOpenShiftVersion: "1.0", + normalizedMaxOpenShiftVersion: mustParse("1.0.0"), }, }, }, @@ -371,17 +388,20 @@ func TestIncompatibleOperators(t *testing.T) { err: false, incompatible: skews{ { - name: "beech", - namespace: "default", - maxOpenShiftVersion: "1.0", + name: "beech", + namespace: "default", + maxOpenShiftVersion: "1.0.0", + normalizedMaxOpenShiftVersion: mustParse("1.0.0"), }, { - name: "chestnut", - namespace: "default", - err: fmt.Errorf(`failed to parse "bad_version" as semver: %w`, func() error { + name: "chestnut", + namespace: "default", + maxOpenShiftVersion: "bad_version", + err: fmt.Errorf(`failed to parse olm.maxOpenShiftVersion="bad_version" as semver: %w`, func() error { _, err := semver.ParseTolerant("bad_version") return err }()), + normalizedMaxOpenShiftVersion: &semver.Version{}, }, }, }, @@ -435,9 +455,10 @@ func TestIncompatibleOperators(t *testing.T) { err: false, incompatible: skews{ { - name: "almond", - namespace: "default", - maxOpenShiftVersion: "1.1", + name: "almond", + namespace: "default", + maxOpenShiftVersion: "1.1.0", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 1, Minor: 1}, }, }, }, @@ -490,14 +511,16 @@ func TestIncompatibleOperators(t *testing.T) { expect: expect{ incompatible: skews{ { - name: "almond", - namespace: "default", - maxOpenShiftVersion: "4.23", + name: "almond", + namespace: "default", + maxOpenShiftVersion: "4.23", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 5, Minor: 0}, }, { - name: "beech", - namespace: "default", - maxOpenShiftVersion: "5.0", + name: "beech", + namespace: "default", + maxOpenShiftVersion: "5.0", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 5, Minor: 0}, }, }, }, @@ -525,14 +548,16 @@ func TestIncompatibleOperators(t *testing.T) { expect: expect{ incompatible: skews{ { - name: "almond", - namespace: "default", - maxOpenShiftVersion: "4.23", + name: "almond", + namespace: "default", + maxOpenShiftVersion: "4.23", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 5, Minor: 0}, }, { - name: "beech", - namespace: "default", - maxOpenShiftVersion: "5.0", + name: "beech", + namespace: "default", + maxOpenShiftVersion: "5.0", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 5, Minor: 0}, }, }, }, @@ -560,19 +585,22 @@ func TestIncompatibleOperators(t *testing.T) { expect: expect{ incompatible: skews{ { - name: "almond", - namespace: "default", - maxOpenShiftVersion: "4.23", + name: "almond", + namespace: "default", + maxOpenShiftVersion: "4.23", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 5, Minor: 0}, }, { - name: "beech", - namespace: "default", - maxOpenShiftVersion: "5.0", + name: "beech", + namespace: "default", + maxOpenShiftVersion: "5.0", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 5, Minor: 0}, }, { - name: "chestnut", - namespace: "default", - maxOpenShiftVersion: "5.1", + name: "chestnut", + namespace: "default", + maxOpenShiftVersion: "5.1", + normalizedMaxOpenShiftVersion: &semver.Version{Major: 5, Minor: 1}, }, }, }, @@ -619,14 +647,6 @@ func TestIncompatibleOperators(t *testing.T) { } func TestMaxOpenShiftVersion(t *testing.T) { - mustParse := func(s string) *semver.Version { - version, err := semver.ParseTolerant(s) - if err != nil { - panic(fmt.Sprintf("bad version given for test case: %s", err)) - } - return &version - } - type expect struct { err bool max *semver.Version @@ -648,7 +668,7 @@ func TestMaxOpenShiftVersion(t *testing.T) { in: []string{`""`}, expect: expect{ err: true, - max: nil, + max: &semver.Version{}, }, }, { @@ -656,7 +676,7 @@ func TestMaxOpenShiftVersion(t *testing.T) { in: []string{`"bad_version"`}, expect: expect{ err: true, - max: nil, + max: &semver.Version{}, }, }, { @@ -705,14 +725,18 @@ func TestMaxOpenShiftVersion(t *testing.T) { projection.PropertiesAnnotationKey: value, }) - max, err := maxOpenShiftVersion(csv) + skew := maxOpenShiftVersion(csv) if tt.expect.err { - require.Error(t, err) - } else { - require.NoError(t, err) + require.Error(t, skew.err) + } else if skew != nil { + require.NoError(t, skew.err) } - require.Equal(t, tt.expect.max, max) + if tt.in == nil { + require.Nil(t, skew) + } else { + require.Equal(t, tt.expect.max, skew.normalizedMaxOpenShiftVersion) + } }) } } @@ -815,3 +839,20 @@ func TestNormalizeOCPVersion(t *testing.T) { }) } } + +func TestSkewsStringIsDeterministic(t *testing.T) { + // The informer cache lists CSVs in map order. The condition message must not + // depend on that order, or every reorder counts as a condition transition. + a := &skew{namespace: "openshift-logging", name: "cluster-logging.v6.6.1", maxOpenShiftVersion: "4.23"} + b := &skew{namespace: "openshift-cluster-csi-drivers", name: "secrets-store-csi-driver-operator.v4.22.0", maxOpenShiftVersion: "5.0"} + invalid := &skew{namespace: "openshift-logging", name: "broken.v1.0.0", maxOpenShiftVersion: "invalid"} + + expected := "ClusterServiceVersions blocking minor version upgrades to 5.1 or higher:\n" + + invalid.String() + "\n" + // error skews first + b.String() + "\n" + // then by namespace/name + a.String() + + require.Equal(t, expected, skews{a, b, invalid}.String()) + require.Equal(t, expected, skews{invalid, b, a}.String()) + require.Equal(t, expected, skews{b, invalid, a}.String()) +} diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go index 2e85462db2..66e1e14e8c 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "sort" "strings" "sync" @@ -74,39 +75,97 @@ func versionsMatch(a []configv1.OperandVersion, b []configv1.OperandVersion) boo return len(counts) < 1 } -type skews []skew +type skews []*skew func (s skews) String() string { - msg := make([]string, len(s)) - i, j := 0, len(s)-1 - for _, sk := range s { - m := sk.String() - // Partial order: error skews first - if sk.err != nil { - msg[i] = m - i++ - continue + // Sorting depends on a populated .err, to sort invalid skews + // first. incompatibleOperators sets .err through one call path, + // but check again here, to ensure reliability, regardless of call-chain. + var minVersion *semver.Version + for i := range s { + s[i].process() + if s[i].err == nil && (minVersion == nil || s[i].normalizedMaxOpenShiftVersion.LT(*minVersion)) { + minVersion = s[i].normalizedMaxOpenShiftVersion } - msg[j] = m - j-- } - // TODO Revisit this message after 4.23 release once release schedule is determined for future possible minor versions. - return fmt.Sprintf("ClusterServiceVersions blocking minor version upgrades to 4.23 or major version upgrades to 5.0:\n%s", strings.Join(msg, "\n")) + // Sort so the message does not depend on the (unstable) order in which the + // CSVs were listed: error skews first, then by namespace/name. A different + // order would be a different Message, and so a condition transition. + sorted := make(skews, len(s)) + copy(sorted, s) + sort.SliceStable(sorted, func(i, j int) bool { + a, b := sorted[i], sorted[j] + if (a.err != nil) != (b.err != nil) { + return a.err != nil + } + if a.namespace != b.namespace { + return a.namespace < b.namespace + } + return a.name < b.name + }) + + msg := make([]string, len(sorted)) + for i, sk := range sorted { + msg[i] = sk.String() + } + + var next string + if minVersion == nil { + next = "next major or minor version" + } else { + n := nextY(*minVersion) + next = fmt.Sprintf("%d.%d", n.Major, n.Minor) + } + return fmt.Sprintf("ClusterServiceVersions blocking minor version upgrades to %s or higher:\n%s", next, strings.Join(msg, "\n")) } type skew struct { namespace string name string maxOpenShiftVersion string - err error + + // the following properties are populated by skew.process() + err error + normalizedMaxOpenShiftVersion *semver.Version +} + +func (s *skew) process() { + if s == nil || s.normalizedMaxOpenShiftVersion != nil || s.err != nil { + return + } + + if s.maxOpenShiftVersion == "" { + // Handle "" separately, so parse doesn't treat it as a zero + s.err = fmt.Errorf(`%s value cannot be "" (an empty string)`, MaxOpenShiftVersionProperty) + s.normalizedMaxOpenShiftVersion = &semver.Version{} + return + } + + version, err := semver.ParseTolerant(s.maxOpenShiftVersion) + s.normalizedMaxOpenShiftVersion = &version + if err != nil { + s.err = fmt.Errorf(`failed to parse %s=%q as semver: %w`, MaxOpenShiftVersionProperty, s.maxOpenShiftVersion, err) + return + } + + truncatedVersion := semver.Version{Major: version.Major, Minor: version.Minor} + if !s.normalizedMaxOpenShiftVersion.EQ(truncatedVersion) { + s.err = fmt.Errorf("%s property must specify only . version, got invalid value %q", MaxOpenShiftVersionProperty, s.normalizedMaxOpenShiftVersion) + return + } + + // Normalize the operator's maxOpenShiftVersion: 4.23 and 5.0 are equivalent. + version = normalizeOCPVersion(*s.normalizedMaxOpenShiftVersion) + s.normalizedMaxOpenShiftVersion = &version } -func (s skew) String() string { +func (s *skew) String() string { + s.process() if s.err != nil { return fmt.Sprintf("- %s/%s has invalid %s properties: %s", s.namespace, s.name, MaxOpenShiftVersionProperty, s.err) } - return fmt.Sprintf("- maximum supported OCP version for %s/%s is %s", s.namespace, s.name, s.maxOpenShiftVersion) + return fmt.Sprintf("- maximum supported OCP version for %s/%s is %d.%d", s.namespace, s.name, s.normalizedMaxOpenShiftVersion.Major, s.normalizedMaxOpenShiftVersion.Minor) } type transientError struct { @@ -146,29 +205,18 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error continue } - s := skew{ - name: csv.GetName(), - namespace: csv.GetNamespace(), - } - max, err := maxOpenShiftVersion(&csv) - if err != nil { - s.err = err - incompatible = append(incompatible, s) + s := maxOpenShiftVersion(&csv) + if s == nil { continue - } - - if max == nil { + } else if s.err != nil { + incompatible = append(incompatible, s) continue } - // Normalize the operator's maxOpenShiftVersion: 4.23 and 5.0 are equivalent. - normalizedMax := normalizeOCPVersion(*max) - if normalizedMax.GTE(nextMinor) { + if s.normalizedMaxOpenShiftVersion.GTE(nextMinor) { continue } - s.maxOpenShiftVersion = fmt.Sprintf("%d.%d", max.Major, max.Minor) - incompatible = append(incompatible, s) } @@ -246,16 +294,22 @@ const ( MaxOpenShiftVersionProperty = "olm.maxOpenShiftVersion" ) -func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) (*semver.Version, error) { +func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) *skew { + s := &skew{ + name: csv.GetName(), + namespace: csv.GetNamespace(), + } + // Extract the property from the CSV's annotations if possible annotation, ok := csv.GetAnnotations()[projection.PropertiesAnnotationKey] if !ok { - return nil, nil + return nil } properties, err := projection.PropertyListFromPropertiesAnnotation(annotation) if err != nil { - return nil, err + s.err = err + return s } var max *string @@ -265,33 +319,23 @@ func maxOpenShiftVersion(csv *operatorsv1alpha1.ClusterServiceVersion) (*semver. } if max != nil { - return nil, fmt.Errorf(`defining more than one "%s" property is not allowed`, MaxOpenShiftVersionProperty) + s.err = fmt.Errorf(`defining more than one %q property is not allowed`, MaxOpenShiftVersionProperty) + return s } max = &property.Value } if max == nil { - return nil, nil + return nil } // Account for any additional quoting value := strings.Trim(*max, "\"") - if value == "" { - // Handle "" separately, so parse doesn't treat it as a zero - return nil, fmt.Errorf(`value cannot be "" (an empty string)`) - } - version, err := semver.ParseTolerant(value) - if err != nil { - return nil, fmt.Errorf(`failed to parse "%s" as semver: %w`, value, err) - } - - truncatedVersion := semver.Version{Major: version.Major, Minor: version.Minor} - if !version.EQ(truncatedVersion) { - return nil, fmt.Errorf("property %s must specify only . version, got invalid value %s", MaxOpenShiftVersionProperty, version) - } - return &truncatedVersion, nil + s.maxOpenShiftVersion = value + s.process() + return s } func notCopiedSelector() (labels.Selector, error) {