Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug 1992677: validate maxocpversion to have major.minor format #169

Merged
merged 3 commits into from Aug 20, 2021
Merged

Bug 1992677: validate maxocpversion to have major.minor format #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc481df
clarify maxOpenShiftVersion upgrade error message (#2326)
ankitathomas Aug 17, 2021
9caf73f
warn on maxOpenShiftVersion specifying more than major.minor (#146)
ankitathomas Aug 17, 2021
a5bcfe0
add maxOpenShiftVersion test for parsing full version string (#147)
ankitathomas Aug 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 25 additions & 17 deletions staging/api/pkg/validation/internal/community.go
View file
Open in desktop
Expand Up @@ -149,12 +149,20 @@ func checkMaxOpenShiftVersion(checks CommunityOperatorChecks, v1beta1MsgForResou

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

truncatedMaxOcp := semver.Version{Major: semVerVersionMaxOcp.Major, Minor: semVerVersionMaxOcp.Minor}
if !semVerVersionMaxOcp.EQ(truncatedMaxOcp) {
checks.warns = append(checks.warns, fmt.Errorf("csv.Annotations.%s has an invalid value. "+
"%s must specify only major.minor versions, %s will be truncated to %s",
olmproperties, olmmaxOpenShiftVersion, semVerVersionMaxOcp, truncatedMaxOcp))
return checks
}

if semVerVersionMaxOcp.GE(semVerOCPV1beta1Unsupported) {
checks.errs = append(checks.errs, fmt.Errorf("csv.Annotations.%s with the "+
"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. "+
Expand Down Expand Up @@ -248,12 +256,12 @@ func validateImageFile(checks CommunityOperatorChecks, deprecatedAPImsg string)
}

if verParsed.GE(semVerOCPV1beta1Unsupported) {
checks.errs = append(checks.errs, fmt.Errorf("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. " +
checks.errs = append(checks.errs, fmt.Errorf("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. "+
"Migrate the API(s) for "+
"%s or provide compatible version(s) by using the %s annotation in " +
"`metadata/annotations.yaml` to ensure that the index image will be geneared " +
"%s or provide compatible version(s) by using the %s annotation in "+
"`metadata/annotations.yaml` to ensure that the index image will be geneared "+
"with its label. (e.g. LABEL %s='4.6-4.8')",
deprecatedAPImsg,
ocpLabelindex,
Expand All @@ -263,12 +271,12 @@ func validateImageFile(checks CommunityOperatorChecks, deprecatedAPImsg string)
} else {
// if not has not the = then the value needs contains - value less < 4.9
if !strings.Contains(indexRange, "-") {
checks.errs = append(checks.errs, fmt.Errorf("this bundle is using APIs which were " +
"deprecated and removed in v1.22. " +
checks.errs = append(checks.errs, fmt.Errorf("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 "+
"The %s allows to distribute it on >= %s. Migrate the API(s) for "+
"%s or provide compatible version(s) by using the %s annotation in " +
"`metadata/annotations.yaml` to ensure that the index image will be generated " +
"%s or provide compatible version(s) by using the %s annotation in "+
"`metadata/annotations.yaml` to ensure that the index image will be generated "+
"with its label. (e.g. LABEL %s='4.6-4.8')",
indexRange,
ocpVerV1beta1Unsupported,
Expand All @@ -287,12 +295,12 @@ func validateImageFile(checks CommunityOperatorChecks, deprecatedAPImsg string)
}

if verParsed.GE(semVerOCPV1beta1Unsupported) {
checks.errs = append(checks.errs, fmt.Errorf("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. " +
checks.errs = append(checks.errs, fmt.Errorf("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. "+
"Upgrade the APIs from "+
"for %s or provide compatible distribution version(s) by using the %s " +
"annotation in `metadata/annotations.yaml` to ensure that the index image will " +
"for %s or provide compatible distribution version(s) by using the %s "+
"annotation in `metadata/annotations.yaml` to ensure that the index image will "+
"be generated with its label. (e.g. LABEL %s='4.6-4.8')",
deprecatedAPImsg,
ocpLabelindex,
Expand All @@ -310,8 +318,8 @@ func validateImageFile(checks CommunityOperatorChecks, deprecatedAPImsg string)
}
}
} else {
checks.errs = append(checks.errs, fmt.Errorf("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. " +
checks.errs = append(checks.errs, fmt.Errorf("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. "+
"Migrate the APIs "+
"for %s or provide compatible version(s) via the labels. (e.g. LABEL %s='4.6-4.8')",
deprecatedAPImsg,
Expand Down
25 changes: 25 additions & 0 deletions staging/api/pkg/validation/internal/community_test.go
View file
Open in desktop
Expand Up @@ -104,6 +104,31 @@ func Test_communityValidator(t *testing.T) {
"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 "},
},
{
name: "should warn on patch version in maxOpenShiftVersion",
wantWarning: true,
args: args{
bundleDir: "./testdata/valid_bundle_v1beta1",
imageIndexPath: "./testdata/dockerfile/valid_bundle.Dockerfile",
annotations: map[string]string{
"olm.properties": fmt.Sprintf(`[{"type": "olm.maxOpenShiftVersion", "value": "4.8.1"}]`),
},
},
warnStrings: []string{
"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",
},
},
{
name: "should pass when the maxOpenShiftVersion is semantically equivalent to <major>.<minor>.0",
wantError: false,
args: args{
bundleDir: "./testdata/valid_bundle_v1beta1",
imageIndexPath: "./testdata/dockerfile/valid_bundle.Dockerfile",
annotations: map[string]string{
"olm.properties": fmt.Sprintf(`[{"type": "olm.maxOpenShiftVersion", "value": "4.8.0+build"}]`),
},
},
},
}

for _, tt := range tests {
Expand Down
6 changes: 3 additions & 3 deletions ...r-lifecycle-manager/pkg/controller/operators/openshift/clusteroperator_controller_test.go
View file
Open in desktop
Expand Up @@ -2,7 +2,6 @@ package openshift

import (
"fmt"

semver "github.com/blang/semver/v4"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -213,6 +212,7 @@ 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
Expand All @@ -224,7 +224,7 @@ var _ = Describe("ClusterOperator controller", func() {
{
namespace: ns.GetName(),
name: incompatible.GetName(),
maxOpenShiftVersion: clusterVersion,
maxOpenShiftVersion: fmt.Sprintf("%d.%d", parsedVersion.Major, parsedVersion.Minor),
},
}.String(),
LastTransitionTime: fixedNow(),
Expand Down Expand Up @@ -270,7 +270,7 @@ var _ = Describe("ClusterOperator controller", func() {
{
namespace: ns.GetName(),
name: incompatible.GetName(),
maxOpenShiftVersion: short + ".0",
maxOpenShiftVersion: short,
},
}.String(),
LastTransitionTime: fixedNow(),
Expand Down
11 changes: 8 additions & 3 deletions staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers.go
View file
Open in desktop
Expand Up @@ -110,7 +110,7 @@ func (s skew) String() string {
return fmt.Sprintf("%s/%s has invalid %s properties: %s", s.namespace, s.name, MaxOpenShiftVersionProperty, s.err)
}

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

type transientError struct {
Expand Down Expand Up @@ -165,7 +165,8 @@ func incompatibleOperators(ctx context.Context, cli client.Client) (skews, error
if max == nil || max.GTE(next) {
continue
}
s.maxOpenShiftVersion = max.String()

s.maxOpenShiftVersion = fmt.Sprintf("%d.%d", max.Major, max.Minor)

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

return &version, nil
truncatedVersion := semver.Version{Major: version.Major, Minor: version.Minor}
if !version.EQ(truncatedVersion) {
return nil, fmt.Errorf("property %s must specify only <major>.<minor> version, got invalid value %s", MaxOpenShiftVersionProperty, version)
}
return &truncatedVersion, nil
}

func notCopiedSelector() (labels.Selector, error) {
Expand Down
49 changes: 14 additions & 35 deletions staging/operator-lifecycle-manager/pkg/controller/operators/openshift/helpers_test.go
View file
Open in desktop
Expand Up @@ -251,11 +251,6 @@ func TestIncompatibleOperators(t *testing.T) {
{
name: "chestnut",
namespace: "default",
maxOpenShiftVersion: "1.2.0-pre+build",
},
{
name: "drupe",
namespace: "default",
maxOpenShiftVersion: "2.0.0",
},
},
Expand Down Expand Up @@ -309,27 +304,27 @@ func TestIncompatibleOperators(t *testing.T) {
{
name: "almond",
namespace: "default",
maxOpenShiftVersion: "1.0.0",
maxOpenShiftVersion: "1.0",
},
{
name: "beech",
namespace: "default",
maxOpenShiftVersion: "1.0.0+build",
maxOpenShiftVersion: "1.0",
},
{
name: "chestnut",
namespace: "default",
maxOpenShiftVersion: "1.1.0-pre",
name: "chestnut",
namespace: "default",
err: fmt.Errorf("property olm.maxOpenShiftVersion must specify only <major>.<minor> version, got invalid value 1.1.0-pre"),
},
{
name: "drupe",
namespace: "default",
maxOpenShiftVersion: "1.1.0-pre+build",
name: "drupe",
namespace: "default",
err: fmt.Errorf("property olm.maxOpenShiftVersion must specify only <major>.<minor> version, got invalid value 1.1.0-pre+build"),
},
{
name: "european-hazelnut",
namespace: "default",
maxOpenShiftVersion: "0.1.0",
maxOpenShiftVersion: "0.1",
},
},
},
Expand Down Expand Up @@ -369,12 +364,12 @@ func TestIncompatibleOperators(t *testing.T) {
{
name: "beech",
namespace: "default",
maxOpenShiftVersion: "1.0.0",
maxOpenShiftVersion: "1.0",
},
{
name: "chestnut",
namespace: "default",
maxOpenShiftVersion: "1.0.0",
maxOpenShiftVersion: "1.0",
},
},
},
Expand Down Expand Up @@ -414,7 +409,7 @@ func TestIncompatibleOperators(t *testing.T) {
{
name: "beech",
namespace: "default",
maxOpenShiftVersion: "1.0.0",
maxOpenShiftVersion: "1.0",
},
{
name: "chestnut",
Expand Down Expand Up @@ -469,11 +464,6 @@ func TestIncompatibleOperators(t *testing.T) {
},
},
in: skews{
{
name: "almond",
namespace: "default",
maxOpenShiftVersion: "1.1.2",
},
{
name: "beech",
namespace: "default",
Expand Down Expand Up @@ -503,21 +493,10 @@ func TestIncompatibleOperators(t *testing.T) {
namespace: "default",
maxOpenShiftVersion: "1.1.0",
},
{
name: "beech",
namespace: "default",
maxOpenShiftVersion: "1.1.0-pre",
},
},
expect: expect{
err: false,
incompatible: skews{
{
name: "beech",
namespace: "default",
maxOpenShiftVersion: "1.1.0-pre",
},
},
err: false,
incompatible: nil,
},
},
} {
Expand Down
42 changes: 25 additions & 17 deletions vendor/github.com/operator-framework/api/pkg/validation/internal/community.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.