pkg/k8sutil/crd*.go: add kubernetes-style version sorter (#1781)

* internal/util/k8sutil: Sort implementation for Kubernetes-style
version sorting, CRDVersions for sorting CRD spec.versions, and
unit tests for both

* internal/pkg/scaffold/olm-catalog: sort both owned and required CRDDescriptions

Author: Eric Stroczynski

internal/scaffold/crd.go

@@ -19,6 +19,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"sort"
 	"sync"
 
 	"github.com/operator-framework/operator-sdk/internal/scaffold/input"
@@ -153,6 +154,7 @@ func (s *CRD) CustomRender() ([]byte, error) {
 	if err := checkCRDVersions(crd); err != nil {
 		return nil, err
 	}
+	sort.Sort(k8sutil.CRDVersions(crd.Spec.Versions))
 	return k8sutil.GetObjectBytes(crd, yaml.Marshal)
 }
 

internal/scaffold/olm-catalog/csv_updaters.go

@@ -17,6 +17,7 @@ package catalog
 import (
 	"bytes"
 	"encoding/json"
+	"sort"
 	"strings"
 
 	"github.com/operator-framework/operator-sdk/pkg/k8sutil"
@@ -31,6 +32,7 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/version"
 )
 
 // CSVUpdater is an interface for any data that can be in a CSV, which will be
@@ -244,6 +246,20 @@ type CustomResourceDefinitionsUpdate struct {
 	crIDs map[string]struct{}
 }
 
+type descSorter []olmapiv1alpha1.CRDDescription
+
+func (descs descSorter) Len() int { return len(descs) }
+func (descs descSorter) Less(i, j int) bool {
+	if descs[i].Name == descs[j].Name {
+		if descs[i].Kind == descs[j].Kind {
+			return version.CompareKubeAwareVersionStrings(descs[i].Version, descs[j].Version) > 0
+		}
+		return descs[i].Kind < descs[j].Kind
+	}
+	return descs[i].Name < descs[j].Name
+}
+func (descs descSorter) Swap(i, j int) { descs[i], descs[j] = descs[j], descs[i] }
+
 func (store *updaterStore) AddOwnedCRD(yamlDoc []byte) error {
 	crd := &apiextv1beta1.CustomResourceDefinition{}
 	if err := yaml.Unmarshal(yamlDoc, crd); err != nil {
@@ -317,6 +333,8 @@ func (u *CustomResourceDefinitionsUpdate) Apply(csv *olmapiv1alpha1.ClusterServi
 		}
 	}
 	csv.Spec.CustomResourceDefinitions.Owned = newDescs
+	sort.Sort(descSorter(csv.Spec.CustomResourceDefinitions.Owned))
+	sort.Sort(descSorter(csv.Spec.CustomResourceDefinitions.Required))
 	return nil
 }
 

internal/scaffold/olm-catalog/testdata/deploy/olm-catalog/app-operator/0.1.0/app-operator.v0.1.0.clusterserviceversion.yaml

@@ -10,14 +10,14 @@ spec:
   apiservicedefinitions: {}
   customresourcedefinitions:
     owned:
-    - displayName: some display name
-      kind: AppService2
-      name: appservice2.example.com
-      version: v1alpha2
     - description: some description
       kind: AppService
       name: appservice.example.com
       version: v1alpha1
+    - displayName: some display name
+      kind: AppService2
+      name: appservice2.example.com
+      version: v1alpha2
     required:
     - description: Represents a cluster of etcd nodes.
       displayName: etcd Cluster

internal/util/k8sutil/crd.go

@@ -25,6 +25,7 @@ import (
 	yaml "github.com/ghodss/yaml"
 	"github.com/pkg/errors"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	"k8s.io/apimachinery/pkg/version"
 )
 
 // GetCRDs parses all CRD manifests in the directory crdsDir and all of its subdirectories.
@@ -152,3 +153,11 @@ func CreateFQAPIs(pkg string, gvs map[string][]string) (apis []string) {
 	}
 	return apis
 }
+
+type CRDVersions []apiextv1beta1.CustomResourceDefinitionVersion
+
+func (vs CRDVersions) Len() int { return len(vs) }
+func (vs CRDVersions) Less(i, j int) bool {
+	return version.CompareKubeAwareVersionStrings(vs[i].Name, vs[j].Name) > 0
+}
+func (vs CRDVersions) Swap(i, j int) { vs[i], vs[j] = vs[j], vs[i] }

internal/util/k8sutil/crd_test.go

@@ -0,0 +1,72 @@
+// Copyright 2019 The Operator-SDK Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package k8sutil
+
+import (
+	"reflect"
+	"sort"
+	"testing"
+
+	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+)
+
+func TestSortVersions(t *testing.T) {
+	cases := []struct {
+		inputVersions []string
+		expected      []string
+	}{
+		{[]string{""}, []string{""}},
+		{[]string{"v1"}, []string{"v1"}},
+		{[]string{"v1alpha1"}, []string{"v1alpha1"}},
+		{[]string{"v1alpha1", "v1"}, []string{"v1", "v1alpha1"}},
+		{
+			[]string{"foo1", "foo10", "foo2", "foo13", "foo52", "foo23", "foo32", "foo33", "foo100"},
+			[]string{"foo1", "foo10", "foo100", "foo13", "foo2", "foo23", "foo32", "foo33", "foo52"},
+		},
+		{
+			[]string{"v1alpha10", "v1alpha1", "v1alpha2000", "v1alpha3", "v1alpha2", "v1alpha300"},
+			[]string{"v1alpha2000", "v1alpha300", "v1alpha10", "v1alpha3", "v1alpha2", "v1alpha1"},
+		},
+		{
+			[]string{"v3beta1", "v12alpha1", "v12alpha2", "v10beta3", "v1", "v11alpha2", "foo1", "v10", "v2", "foo10", "v11beta2"},
+			[]string{"v10", "v2", "v1", "v11beta2", "v10beta3", "v3beta1", "v12alpha2", "v12alpha1", "v11alpha2", "foo1", "foo10"},
+		},
+	}
+
+	for _, c := range cases {
+		cvs := stringsToCRDVersions(c.inputVersions)
+		sort.Sort(cvs)
+		vs := crdVersionsToStrings(cvs)
+		if !reflect.DeepEqual(vs, c.expected) {
+			t.Errorf("Output not sorted as expected:\noutput:   %+q\nexpected: %+q", vs, c.expected)
+		}
+	}
+}
+
+func stringsToCRDVersions(vs []string) (cvs CRDVersions) {
+	for _, v := range vs {
+		cvs = append(cvs, apiextv1beta1.CustomResourceDefinitionVersion{
+			Name: v,
+		})
+	}
+	return cvs
+}
+
+func crdVersionsToStrings(cvs []apiextv1beta1.CustomResourceDefinitionVersion) (vs []string) {
+	for _, v := range cvs {
+		vs = append(vs, v.Name)
+	}
+	return vs
+}