internal/pkg/scaffold: move some utility functions to internal/util/k8sutil (#1635)

Author: Eric Stroczynski

internal/pkg/scaffold/crd.go

@@ -142,7 +142,7 @@ func (s *CRD) CustomRender() ([]byte, error) {
 	}
 
 	setCRDVersions(crd)
-	return k8sutil.GetObjectBytes(crd)
+	return k8sutil.GetObjectBytes(crd, yaml.Marshal)
 }
 
 func newCRDForResource(r *Resource) *apiextv1beta1.CustomResourceDefinition {

internal/pkg/scaffold/olm-catalog/csv.go

@@ -23,7 +23,6 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
-	"unicode"
 
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold"
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input"
@@ -138,7 +137,7 @@ func (s *CSV) CustomRender() ([]byte, error) {
 		}
 	}
 
-	return k8sutil.GetObjectBytes(csv)
+	return k8sutil.GetObjectBytes(csv, yaml.Marshal)
 }
 
 func (s *CSV) getBaseCSVIfExists() (*olmapiv1alpha1.ClusterServiceVersion, bool, error) {
@@ -190,41 +189,6 @@ func (s *CSV) getCSVPath(ver string) string {
 	return filepath.Join(s.pathPrefix, OLMCatalogDir, lowerProjName, ver, name)
 }
 
-// getDisplayName turns a project dir name in any of {snake, chain, camel}
-// cases, hierarchical dot structure, or space-delimited into a
-// space-delimited, title'd display name.
-// Ex. "another-_AppOperator_againTwiceThrice More"
-// ->  "Another App Operator Again Twice Thrice More"
-func getDisplayName(name string) string {
-	for _, sep := range ".-_ " {
-		splitName := strings.Split(name, string(sep))
-		for i := 0; i < len(splitName); i++ {
-			if splitName[i] == "" {
-				splitName = append(splitName[:i], splitName[i+1:]...)
-				i--
-			} else {
-				splitName[i] = strings.TrimSpace(splitName[i])
-			}
-		}
-		name = strings.Join(splitName, " ")
-	}
-	splitName := strings.Split(name, " ")
-	for i, word := range splitName {
-		temp := word
-		o := 0
-		for j, r := range word {
-			if unicode.IsUpper(r) {
-				if j > 0 && !unicode.IsUpper(rune(word[j-1])) {
-					temp = temp[0:j+o] + " " + temp[j+o:len(temp)]
-					o++
-				}
-			}
-		}
-		splitName[i] = temp
-	}
-	return strings.TrimSpace(strings.Title(strings.Join(splitName, " ")))
-}
-
 // initCSVFields initializes all csv fields that should be populated by a user
 // with sane defaults. initCSVFields should only be called for new csv's.
 func (s *CSV) initCSVFields(csv *olmapiv1alpha1.ClusterServiceVersion) {
@@ -237,7 +201,7 @@ func (s *CSV) initCSVFields(csv *olmapiv1alpha1.ClusterServiceVersion) {
 
 	// Spec fields
 	csv.Spec.Version = *semver.New(s.CSVVersion)
-	csv.Spec.DisplayName = getDisplayName(s.OperatorName)
+	csv.Spec.DisplayName = k8sutil.GetDisplayName(s.OperatorName)
 	csv.Spec.Description = "Placeholder description"
 	csv.Spec.Maturity = "alpha"
 	csv.Spec.Provider = olmapiv1alpha1.AppLink{}
@@ -371,7 +335,7 @@ func (s *CSV) updateCSVFromManifestFiles(cfg *CSVConfig, csv *olmapiv1alpha1.Clu
 		scanner := yamlutil.NewYAMLScanner(yamlData)
 		for scanner.Scan() {
 			yamlSpec := scanner.Bytes()
-			kind, err := getKindfromYAML(yamlSpec)
+			kind, err := k8sutil.GetKindfromYAML(yamlSpec)
 			if err != nil {
 				return fmt.Errorf("%s: %v", f, err)
 			}

internal/pkg/scaffold/olm-catalog/csv_test.go

@@ -167,33 +167,6 @@ func TestUpdateVersion(t *testing.T) {
 	}
 }
 
-func TestGetDisplayName(t *testing.T) {
-	cases := []struct {
-		input, wanted string
-	}{
-		{"Appoperator", "Appoperator"},
-		{"appoperator", "Appoperator"},
-		{"appoperatoR", "Appoperato R"},
-		{"AppOperator", "App Operator"},
-		{"appOperator", "App Operator"},
-		{"app-operator", "App Operator"},
-		{"app-_operator", "App Operator"},
-		{"App-operator", "App Operator"},
-		{"app-_Operator", "App Operator"},
-		{"app--Operator", "App Operator"},
-		{"app--_Operator", "App Operator"},
-		{"APP", "APP"},
-		{"another-AppOperator_againTwiceThrice More", "Another App Operator Again Twice Thrice More"},
-	}
-
-	for _, c := range cases {
-		dn := getDisplayName(c.input)
-		if dn != c.wanted {
-			t.Errorf("Wanted %s, got %s", c.wanted, dn)
-		}
-	}
-}
-
 func TestSetAndCheckOLMNamespaces(t *testing.T) {
 	depBytes, err := ioutil.ReadFile(filepath.Join(testDeployDir, "operator.yaml"))
 	if err != nil {

internal/pkg/scaffold/olm-catalog/csv_updaters.go

@@ -69,16 +69,6 @@ func (s *updaterStore) Apply(csv *olmapiv1alpha1.ClusterServiceVersion) error {
 	return nil
 }
 
-func getKindfromYAML(yamlData []byte) (string, error) {
-	var temp struct {
-		Kind string
-	}
-	if err := yaml.Unmarshal(yamlData, &temp); err != nil {
-		return "", err
-	}
-	return temp.Kind, nil
-}
-
 func (s *updaterStore) AddToUpdater(yamlSpec []byte, kind string) (found bool, err error) {
 	found = true
 	switch kind {

internal/pkg/scaffold/olm-catalog/package_manifest.go

@@ -15,7 +15,6 @@
 package catalog
 
 import (
-	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -24,6 +23,7 @@ import (
 
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold"
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input"
+	registryutil "github.com/operator-framework/operator-sdk/internal/util/operator-registry"
 
 	"github.com/ghodss/yaml"
 	olmregistry "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
@@ -95,7 +95,7 @@ func (s *PackageManifest) CustomRender() ([]byte, error) {
 		return nil, errors.Wrapf(err, "package manifest %s", path)
 	}
 
-	if err := validatePackageManifest(pm); err != nil {
+	if err := registryutil.ValidatePackageManifest(pm); err != nil {
 		return nil, errors.Wrapf(err, "failed to validate package manifest %s", pm.PackageName)
 	}
 
@@ -127,37 +127,6 @@ func (s *PackageManifest) newPackageManifest() *olmregistry.PackageManifest {
 	return pm
 }
 
-func validatePackageManifest(pm *olmregistry.PackageManifest) error {
-	if pm.PackageName == "" {
-		return fmt.Errorf("package name cannot be empty")
-	}
-	if len(pm.Channels) == 0 {
-		return fmt.Errorf("channels cannot be empty")
-	}
-	if pm.DefaultChannelName == "" {
-		return fmt.Errorf("default channel cannot be empty")
-	}
-
-	seen := map[string]struct{}{}
-	for i, c := range pm.Channels {
-		if c.Name == "" {
-			return fmt.Errorf("channel %d name cannot be empty", i)
-		}
-		if c.CurrentCSVName == "" {
-			return fmt.Errorf("channel %s currentCSV cannot be empty", c.Name)
-		}
-		if _, ok := seen[c.Name]; ok {
-			return fmt.Errorf("duplicate package manifest channel name %s; channel names must be unique", c.Name)
-		}
-		seen[c.Name] = struct{}{}
-	}
-	if _, ok := seen[pm.DefaultChannelName]; !ok {
-		return fmt.Errorf("default channel %s does not exist in channels", pm.DefaultChannelName)
-	}
-
-	return nil
-}
-
 // setChannels checks for duplicate channels in pm and sets the default
 // channel if possible.
 func (s *PackageManifest) setChannels(pm *olmregistry.PackageManifest) error {

internal/pkg/scaffold/olm-catalog/package_manifest_test.go

@@ -24,8 +24,6 @@ import (
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold"
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input"
 	"github.com/operator-framework/operator-sdk/internal/util/diffutil"
-
-	olmregistry "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
 )
 
 func TestPackageManifest(t *testing.T) {
@@ -70,74 +68,3 @@ const packageManifestExp = `channels:
 defaultChannel: stable
 packageName: app-operator
 `
-
-func TestValidatePackageManifest(t *testing.T) {
-	channels := []olmregistry.PackageChannel{
-		{Name: "foo", CurrentCSVName: "bar"},
-	}
-	pm := &olmregistry.PackageManifest{
-		Channels:           channels,
-		DefaultChannelName: "baz",
-		PackageName:        "test-package",
-	}
-
-	cases := []struct {
-		description string
-		wantErr     bool
-		errMsg      string
-		operation   func(*olmregistry.PackageManifest)
-	}{
-		{
-			"default channel does not exist",
-			true, "default channel baz does not exist in channels", nil,
-		},
-		{
-			"successful validation",
-			false, "",
-			func(pm *olmregistry.PackageManifest) {
-				pm.DefaultChannelName = pm.Channels[0].Name
-			},
-		},
-		{
-			"channels are empty",
-			true, "channels cannot be empty",
-			func(pm *olmregistry.PackageManifest) {
-				pm.Channels = nil
-			},
-		},
-		{
-			"one channel's CSVName is empty",
-			true, "channel foo currentCSV cannot be empty",
-			func(pm *olmregistry.PackageManifest) {
-				pm.Channels = make([]olmregistry.PackageChannel, 1)
-				copy(pm.Channels, channels)
-				pm.Channels[0].CurrentCSVName = ""
-			},
-		},
-		{
-			"duplicate channel name",
-			true, "duplicate package manifest channel name foo; channel names must be unique",
-			func(pm *olmregistry.PackageManifest) {
-				pm.Channels = append(channels, channels...)
-			},
-		},
-	}
-
-	for _, c := range cases {
-		if c.operation != nil {
-			c.operation(pm)
-		}
-		err := validatePackageManifest(pm)
-		if c.wantErr {
-			if err == nil {
-				t.Errorf(`%s: expected error "%s", got none`, c.description, c.errMsg)
-			} else if err.Error() != c.errMsg {
-				t.Errorf(`%s: expected error message "%s", got "%s"`, c.description, c.errMsg, err)
-			}
-		} else {
-			if err != nil {
-				t.Errorf(`%s: expected no error, got error "%s"`, c.description, err)
-			}
-		}
-	}
-}

internal/util/k8sutil/k8sutil.go

@@ -16,7 +16,10 @@ package k8sutil
 
 import (
 	"fmt"
+	"strings"
+	"unicode"
 
+	"github.com/ghodss/yaml"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
@@ -51,3 +54,48 @@ func GetKubeconfigAndNamespace(configPath string) (*rest.Config, string, error)
 	}
 	return kubeconfig, namespace, nil
 }
+
+func GetKindfromYAML(yamlData []byte) (string, error) {
+	var temp struct {
+		Kind string
+	}
+	if err := yaml.Unmarshal(yamlData, &temp); err != nil {
+		return "", err
+	}
+	return temp.Kind, nil
+}
+
+// GetDisplayName turns a project dir name in any of {snake, chain, camel}
+// cases, hierarchical dot structure, or space-delimited into a
+// space-delimited, title'd display name.
+// Ex. "another-_AppOperator_againTwiceThrice More"
+// ->  "Another App Operator Again Twice Thrice More"
+func GetDisplayName(name string) string {
+	for _, sep := range ".-_ " {
+		splitName := strings.Split(name, string(sep))
+		for i := 0; i < len(splitName); i++ {
+			if splitName[i] == "" {
+				splitName = append(splitName[:i], splitName[i+1:]...)
+				i--
+			} else {
+				splitName[i] = strings.TrimSpace(splitName[i])
+			}
+		}
+		name = strings.Join(splitName, " ")
+	}
+	splitName := strings.Split(name, " ")
+	for i, word := range splitName {
+		temp := word
+		o := 0
+		for j, r := range word {
+			if unicode.IsUpper(r) {
+				if j > 0 && !unicode.IsUpper(rune(word[j-1])) {
+					temp = temp[0:j+o] + " " + temp[j+o:len(temp)]
+					o++
+				}
+			}
+		}
+		splitName[i] = temp
+	}
+	return strings.TrimSpace(strings.Title(strings.Join(splitName, " ")))
+}

internal/util/k8sutil/k8sutil_test.go

@@ -0,0 +1,44 @@
+// 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 "testing"
+
+func TestGetDisplayName(t *testing.T) {
+	cases := []struct {
+		input, wanted string
+	}{
+		{"Appoperator", "Appoperator"},
+		{"appoperator", "Appoperator"},
+		{"appoperatoR", "Appoperato R"},
+		{"AppOperator", "App Operator"},
+		{"appOperator", "App Operator"},
+		{"app-operator", "App Operator"},
+		{"app-_operator", "App Operator"},
+		{"App-operator", "App Operator"},
+		{"app-_Operator", "App Operator"},
+		{"app--Operator", "App Operator"},
+		{"app--_Operator", "App Operator"},
+		{"APP", "APP"},
+		{"another-AppOperator_againTwiceThrice More", "Another App Operator Again Twice Thrice More"},
+	}
+
+	for _, c := range cases {
+		dn := GetDisplayName(c.input)
+		if dn != c.wanted {
+			t.Errorf("Wanted %s, got %s", c.wanted, dn)
+		}
+	}
+}