refactor Package and CSV generators to use Inputs and OutputDir

Author: hasbro17

cmd/operator-sdk/generate/csv.go

@@ -38,7 +38,7 @@ type csvCmd struct {
 	fromVersion    string
 	operatorName   string
 	outputDir      string
-	includePaths   []string
+	inputs         map[string]string
 	updateCRDs     bool
 	defaultChannel bool
 }
@@ -80,10 +80,18 @@ version to --from-version. Otherwise the SDK will scaffold a new CSV manifest.`,
 	}
 	cmd.Flags().StringVar(&c.fromVersion, "from-version", "",
 		"Semantic version of an existing CSV to use as a base")
-	cmd.Flags().StringSliceVar(&c.includePaths, "include", []string{scaffold.DeployDir},
-		"Paths to include in CSV generation, ex. \"deploy/prod,deploy/test\". "+
-			"If this flag is set and you want to enable default behavior, "+
-			"you must include \"deploy/\" in the argument list")
+	cmd.Flags().StringToStringVar(&c.inputs, "inputs",
+		map[string]string{
+			gencatalog.DeployDirKey: "deploy",
+			gencatalog.APIsDirKey:   "pkg/apis",
+		},
+		`Key value input paths used in CSV generation.
+Use this to set custom paths for operator manifests and API type definitions
+E.g: --inputs deploy=config/production,apis=pkg/myapp/apis 
+Supported input keys:
+	- deploy=<path to root directory for operator manifests (Deployment, RBAC, CRDs)>
+	- apis=<path to root directory for API type defintions>
+`)
 	cmd.Flags().StringVar(&c.outputDir, "output-dir", scaffold.DeployDir,
 		"Base directory to output generated CSV. The resulting CSV bundle directory"+
 			"will be \"<output-dir>/olm-catalog/<operator-name>/<csv-version>\"")
@@ -109,8 +117,8 @@ func (c csvCmd) run() error {
 	}
 	cfg := gen.Config{
 		OperatorName: c.operatorName,
+		Inputs:       c.inputs,
 		OutputDir:    c.outputDir,
-		Filters:      gen.MakeFilters(c.includePaths...),
 	}
 
 	csv := gencatalog.NewCSV(cfg, c.csvVersion, c.fromVersion)
@@ -124,19 +132,19 @@ func (c csvCmd) run() error {
 
 	// Write CRD's to the new or updated CSV package dir.
 	if c.updateCRDs {
-		crdManifestSet, err := findCRDs(c.includePaths...)
+		// TODO: Any requirement to update CRD's from any place other
+		// than the input "deploy" directory?
+		// Can CRD manifests be present outside of the "deploy" directory?
+		crdManifestSet, err := findCRDFileSet(c.inputs[gencatalog.DeployDirKey])
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to update CRD's: %v", err)
 		}
-		baseDir := c.outputDir
-		if baseDir == "" {
-			baseDir = gencatalog.OLMCatalogDir
-		}
-		bundleDir := filepath.Join(baseDir, c.operatorName, c.csvVersion)
+		// TODO: This path should come from the CSV generator
+		bundleDir := filepath.Join(c.outputDir, gencatalog.OLMCatalogChildDir, c.operatorName, c.csvVersion)
 		for path, b := range crdManifestSet {
 			path = filepath.Join(bundleDir, path)
 			if err = ioutil.WriteFile(path, b, fileutil.DefaultFileMode); err != nil {
-				return err
+				return fmt.Errorf("failed to update CRD's: %v", err)
 			}
 		}
 	}
@@ -178,41 +186,28 @@ func validateVersion(version string) error {
 	return nil
 }
 
-// findCRDs searches directories and files in paths for CRD manifest paths,
+// findCRDFileSet searches all directories and files in path for CRD manifests,
 // returning a map of paths to file contents.
-func findCRDs(paths ...string) (map[string][]byte, error) {
+func findCRDFileSet(path string) (map[string][]byte, error) {
 	crdFileSet := map[string][]byte{}
-	for _, path := range paths {
-		info, err := os.Stat(path)
+	info, err := os.Stat(path)
+	if err != nil {
+		return nil, err
+	}
+	if !info.IsDir() {
+		return nil, fmt.Errorf("CRD's must be read from a directory. %s is a file", path)
+	}
+
+	crdPaths, err := k8sutil.GetCRDManifestPaths(path)
+	if err != nil {
+		return nil, err
+	}
+	for _, crdPath := range crdPaths {
+		b, err := ioutil.ReadFile(crdPath)
 		if err != nil {
 			return nil, err
 		}
-		if info.IsDir() {
-			subsetPaths, err := k8sutil.GetCRDManifestPaths(path)
-			if err != nil {
-				return nil, err
-			}
-			for _, crdPath := range subsetPaths {
-				b, err := ioutil.ReadFile(crdPath)
-				if err != nil {
-					return nil, err
-				}
-				crdFileSet[filepath.Base(crdPath)] = b
-			}
-		} else {
-			b, err := ioutil.ReadFile(path)
-			if err != nil {
-				return nil, err
-			}
-			typeMeta, err := k8sutil.GetTypeMetaFromBytes(b)
-			if err != nil {
-				log.Infof("Skipping non-manifest file %s", path)
-				continue
-			}
-			if typeMeta.Kind == "CustomResourceDefinition" {
-				crdFileSet[filepath.Base(path)] = b
-			}
-		}
+		crdFileSet[filepath.Base(crdPath)] = b
 	}
 	return crdFileSet, nil
 }

internal/generate/gen/config.go

@@ -31,14 +31,9 @@ type Config struct {
 	// on-disk inputs are required. If not set, a default is used on a
 	// per-generator basis.
 	Inputs map[string]string
-	// OutputDir is a dir in which to generate output files. If not set, a
-	// default is used on a per-generator basis.
+	// OutputDir is the root directory where the output files will be generated.
+	// If not set, a default is used on a per-generator basis.
 	OutputDir string
-	// Filters is a set of functional filters for paths that a generator may
-	// encounter while gathering data for generation. Filters provides
-	// fine-grained control over Inputs, since often those paths are often
-	// top-level directories.
-	Filters FilterFuncs
 }
 
 // FilterFuncs is a slice of filter funcs.

internal/generate/olm-catalog/csv.go

@@ -44,13 +44,20 @@ import (
 )
 
 const (
-	olmCatalogChildDir = "olm-catalog"
-	OLMCatalogDir      = scaffold.DeployDir + string(filepath.Separator) + olmCatalogChildDir
-	CSVYamlFileExt     = ".clusterserviceversion.yaml"
+	OLMCatalogChildDir = "olm-catalog"
+	// OLMCatalogDir is the default location for OLM catalog directory.
+	OLMCatalogDir  = scaffold.DeployDir + string(filepath.Separator) + OLMCatalogChildDir
+	csvYamlFileExt = ".clusterserviceversion.yaml"
 
-	BundleDirKey = "bundle"
+	// Input keys for CSV generator whose values are the filepaths for the respective input directories
+
+	// DEBUG: Why is this an input key? Users don't need to configure this.
+	// bundleDirKey is for the location of the bundle manifests directory
+	bundleDirKey = "bundle"
+	// DeployDirKey is for the location of the operator manifests directory e.g "deploy/production"
 	DeployDirKey = "deploy"
-	APIsDirKey   = "apis"
+	// APIsDirKey is for the location of the API types directory e.g "pkg/apis"
+	APIsDirKey = "apis"
 )
 
 type csvGenerator struct {
@@ -59,7 +66,7 @@ type csvGenerator struct {
 	csvVersion string
 	// fromVersion is the CSV version from which to build a new CSV. A CSV
 	// manifest with this version should exist at:
-	// deploy/olm-catalog/{from_version}/operator-name.v{from_version}.{CSVYamlFileExt}
+	// deploy/olm-catalog/{from_version}/operator-name.v{from_version}.{csvYamlFileExt}
 	fromVersion string
 }
 
@@ -79,23 +86,23 @@ func NewCSV(cfg gen.Config, csvVersion, fromVersion string) gen.Generator {
 		g.Inputs[DeployDirKey] = scaffold.DeployDir
 	} else {
 		// Set to non-standard location.
-		olmCatalogDir = filepath.Join(g.Inputs[DeployDirKey], olmCatalogChildDir)
+		olmCatalogDir = filepath.Join(g.Inputs[DeployDirKey], OLMCatalogChildDir)
 	}
-	if bundle, ok := g.Inputs[BundleDirKey]; !ok || bundle == "" {
+	if bundle, ok := g.Inputs[bundleDirKey]; !ok || bundle == "" {
 		// Initialize so we don't have to check for key existence elsewhere.
-		g.Inputs[BundleDirKey] = ""
+		g.Inputs[bundleDirKey] = ""
 		parentDir := filepath.Join(olmCatalogDir, g.OperatorName)
 		if isBundleDirExist(parentDir, g.fromVersion) {
-			g.Inputs[BundleDirKey] = filepath.Join(olmCatalogDir, g.OperatorName, g.fromVersion)
+			g.Inputs[bundleDirKey] = filepath.Join(olmCatalogDir, g.OperatorName, g.fromVersion)
 		} else if isBundleDirExist(parentDir, g.csvVersion) {
-			g.Inputs[BundleDirKey] = filepath.Join(olmCatalogDir, g.OperatorName, g.csvVersion)
+			g.Inputs[bundleDirKey] = filepath.Join(olmCatalogDir, g.OperatorName, g.csvVersion)
 		}
 	}
 	if apisDir, ok := g.Inputs[APIsDirKey]; !ok || apisDir == "" {
 		g.Inputs[APIsDirKey] = scaffold.ApisDir
 	}
 	if g.OutputDir == "" {
-		g.OutputDir = filepath.Join(olmCatalogDir, g.OperatorName, g.csvVersion)
+		g.OutputDir = scaffold.DeployDir
 	}
 	return g
 }
@@ -114,7 +121,7 @@ func getCSVName(name, version string) string {
 }
 
 func getCSVFileName(name, version string) string {
-	return getCSVName(strings.ToLower(name), version) + CSVYamlFileExt
+	return getCSVName(strings.ToLower(name), version) + csvYamlFileExt
 }
 
 // Generate allows a CSV to be written by marshalling
@@ -127,11 +134,14 @@ func (g csvGenerator) Generate() error {
 	if len(fileMap) == 0 {
 		return errors.New("error generating CSV manifest: no generated file found")
 	}
-	if err = os.MkdirAll(g.OutputDir, fileutil.DefaultDirFileMode); err != nil {
-		return errors.Wrapf(err, "error mkdir %s", g.OutputDir)
+
+	csvOutputDir := filepath.Join(g.OutputDir, OLMCatalogChildDir, g.OperatorName, g.csvVersion)
+	if err = os.MkdirAll(csvOutputDir, fileutil.DefaultDirFileMode); err != nil {
+		return errors.Wrapf(err, "error mkdir %s", csvOutputDir)
 	}
 	for fileName, b := range fileMap {
-		path := filepath.Join(g.OutputDir, fileName)
+		path := filepath.Join(csvOutputDir, fileName)
+		log.Debugf("CSV generator writing %s", path)
 		if err = ioutil.WriteFile(path, b, fileutil.DefaultFileMode); err != nil {
 			return err
 		}
@@ -140,7 +150,7 @@ func (g csvGenerator) Generate() error {
 }
 
 func (g csvGenerator) generate() (fileMap map[string][]byte, err error) {
-	bundle := g.Inputs[BundleDirKey]
+	bundle := g.Inputs[bundleDirKey]
 	// Get current CSV to update, otherwise start with a fresh CSV.
 	var csv *olmapiv1alpha1.ClusterServiceVersion
 	if bundle != "" {
@@ -176,6 +186,7 @@ func (g csvGenerator) generate() (fileMap map[string][]byte, err error) {
 	if err != nil {
 		return nil, err
 	}
+	// TODO: Why is this a file map when we only generate one CSV file?
 	fileMap = map[string][]byte{
 		path: b,
 	}
@@ -346,12 +357,11 @@ func (g csvGenerator) updateCSVFromManifests(csv *olmapiv1alpha1.ClusterServiceV
 	kindManifestMap := map[schema.GroupVersionKind][][]byte{}
 	crGVKSet := map[schema.GroupVersionKind]struct{}{}
 	err = filepath.Walk(g.Inputs[DeployDirKey], func(path string, info os.FileInfo, werr error) error {
+		// DEBUG: How is this walking to read deploy/crds/... manifests
+		// if it's not walking directories?
 		if werr != nil || info.IsDir() {
 			return werr
 		}
-		if !g.Filters.SatisfiesAny(path) {
-			return nil
-		}
 		b, err := ioutil.ReadFile(path)
 		if err != nil {
 			return err

internal/generate/olm-catalog/csv_go_test.go

@@ -75,7 +75,6 @@ func TestGoCSVGoNew(t *testing.T) {
 
 	cfg := gen.Config{
 		OperatorName: testProjectName,
-		Filters:      gen.MakeFilters(scaffold.DeployDir),
 	}
 	g := NewCSV(cfg, csvVersion, "")
 	fileMap, err := g.(csvGenerator).generate()
@@ -109,7 +108,6 @@ func TestGoCSVFromOld(t *testing.T) {
 
 	cfg := gen.Config{
 		OperatorName: testProjectName,
-		Filters:      gen.MakeFilters(scaffold.DeployDir),
 	}
 	g := NewCSV(cfg, csvVersion, fromVersion)
 	fileMap, err := g.(csvGenerator).generate()

internal/generate/olm-catalog/descriptor/descriptor.go

@@ -51,6 +51,9 @@ func GetCRDDescriptionForGVK(apisDir string, gvk schema.GroupVersionKind) (olmap
 	if strings.Contains(group, ".") {
 		group = strings.Split(group, ".")[0]
 	}
+	// TODO(hasbro17): Lookup for API directory should not be configured to
+	// <apisDir>/<gvk.group>/<gvk.Version>
+	// Kubebuilder's layout is different.
 	apiDir := filepath.Join(apisDir, group, gvk.Version)
 	universe, err := getTypesFromDir(apiDir)
 	if err != nil {

internal/generate/olm-catalog/package_manifest.go

@@ -26,16 +26,18 @@ import (
 	"github.com/operator-framework/api/pkg/validation"
 	"github.com/operator-framework/operator-registry/pkg/registry"
 	"github.com/operator-framework/operator-sdk/internal/generate/gen"
+	"github.com/operator-framework/operator-sdk/internal/scaffold"
 	"github.com/operator-framework/operator-sdk/internal/util/fileutil"
 
 	"github.com/ghodss/yaml"
 	log "github.com/sirupsen/logrus"
 )
 
 const (
-	PackageManifestFileExt = ".package.yaml"
+	packageManifestFileExt = ".package.yaml"
 
-	ManifestsDirKey = "manifests"
+	// TODO: Is this an input that needs to be set via the csv generator inputs flag?
+	manifestsDirKey = "manifests"
 )
 
 type pkgGenerator struct {
@@ -68,21 +70,22 @@ func NewPackageManifest(cfg gen.Config, csvVersion, channel string, isDefault bo
 	olmCatalogDir := OLMCatalogDir
 	if deployDir, ok := g.Inputs[DeployDirKey]; ok && deployDir != "" {
 		// Set to non-standard location.
-		olmCatalogDir = filepath.Join(g.Inputs[DeployDirKey], olmCatalogChildDir)
+		olmCatalogDir = filepath.Join(g.Inputs[DeployDirKey], OLMCatalogChildDir)
 	}
 
-	if manifests, ok := g.Inputs[ManifestsDirKey]; !ok || manifests == "" {
-		g.Inputs[ManifestsDirKey] = filepath.Join(olmCatalogDir, g.OperatorName)
+	if manifests, ok := g.Inputs[manifestsDirKey]; !ok || manifests == "" {
+		g.Inputs[manifestsDirKey] = filepath.Join(olmCatalogDir, g.OperatorName)
 	}
+
 	if g.OutputDir == "" {
-		g.OutputDir = filepath.Join(olmCatalogDir, g.OperatorName)
+		g.OutputDir = scaffold.DeployDir
 	}
 	return g
 }
 
 // getPkgFileName will return the name of the PackageManifestFile
 func getPkgFileName(operatorName string) string {
-	return strings.ToLower(operatorName) + PackageManifestFileExt
+	return strings.ToLower(operatorName) + packageManifestFileExt
 }
 
 func (g pkgGenerator) Generate() error {
@@ -93,11 +96,13 @@ func (g pkgGenerator) Generate() error {
 	if len(fileMap) == 0 {
 		return errors.New("error generating package manifest: no generated file found")
 	}
-	if err = os.MkdirAll(g.OutputDir, fileutil.DefaultDirFileMode); err != nil {
-		return fmt.Errorf("error mkdir %s: %v", g.OutputDir, err)
+	pkgManifestOutputDir := filepath.Join(g.OutputDir, OLMCatalogChildDir, g.OperatorName)
+	if err = os.MkdirAll(pkgManifestOutputDir, fileutil.DefaultDirFileMode); err != nil {
+		return fmt.Errorf("error mkdir %s: %v", pkgManifestOutputDir, err)
 	}
 	for fileName, b := range fileMap {
-		path := filepath.Join(g.OutputDir, fileName)
+		path := filepath.Join(pkgManifestOutputDir, fileName)
+		log.Debugf("Package manifest generator writing %s", path)
 		if err = ioutil.WriteFile(path, b, fileutil.DefaultFileMode); err != nil {
 			return err
 		}
@@ -126,6 +131,7 @@ func (g pkgGenerator) generate() (map[string][]byte, error) {
 		return nil, err
 	}
 
+	// TODO: Why is this a file map?
 	fileMap := map[string][]byte{
 		g.fileName: b,
 	}
@@ -136,7 +142,7 @@ func (g pkgGenerator) generate() (map[string][]byte, error) {
 // an existing one if found at the expected path.
 func (g pkgGenerator) buildPackageManifest() (registry.PackageManifest, error) {
 	pkg := registry.PackageManifest{}
-	path := filepath.Join(g.Inputs[ManifestsDirKey], g.fileName)
+	path := filepath.Join(g.Inputs[manifestsDirKey], g.fileName)
 	if _, err := os.Stat(path); err == nil {
 		b, err := ioutil.ReadFile(path)
 		if err != nil {

internal/generate/olm-catalog/package_manifest_test.go

@@ -30,7 +30,7 @@ func newTestPackageManifestGenerator() gen.Generator {
 	inputDir := filepath.Join(testGoDataDir, OLMCatalogDir, testProjectName)
 	cfg := gen.Config{
 		OperatorName: testProjectName,
-		Inputs:       map[string]string{ManifestsDirKey: inputDir},
+		Inputs:       map[string]string{manifestsDirKey: inputDir},
 	}
 	g := NewPackageManifest(cfg, csvVersion, "stable", true)
 	return g