internal/pkg/scaffold/crd.go: overwrite CRD manifests for Go operators

Gopkg.toml

@@ -30,6 +30,12 @@
   name = "sigs.k8s.io/controller-runtime"
   version = "=v0.1.10"
 
+# This override revision has a fix that allows CRD unit tests to run correctly.
+# Remove once v0.1.11 is released.
+[[override]]
+  name = "sigs.k8s.io/controller-tools"
+  revision = "9d55346c2bde73fb3326ac22eac2e5210a730207"
+
 [[constraint]]
   name = "github.com/sergi/go-diff"
   version = "1.0.0"

doc/user/generating-a-crd.md

@@ -0,0 +1 @@
+# Generating a Custom Resource Definition Manifest

internal/pkg/scaffold/crd.go

@@ -16,8 +16,6 @@ package scaffold
 
 import (
 	"fmt"
-	"io/ioutil"
-	"os"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -41,6 +39,20 @@ type CRD struct {
 
 	// IsOperatorGo is true when the operator is written in Go.
 	IsOperatorGo bool
+
+	once sync.Once
+	fs   afero.Fs // For testing, ex. afero.NewMemMapFs()
+}
+
+func (s *CRD) initFS(fs afero.Fs) {
+	s.once.Do(func() {
+		s.fs = fs
+	})
+}
+
+func (s *CRD) getFS() afero.Fs {
+	s.initFS(afero.NewOsFs())
+	return s.fs
 }
 
 func (s *CRD) GetInput() (input.Input, error) {
@@ -76,77 +88,78 @@ func initCache() {
 	})
 }
 
-func (s *CRD) SetFS(_ afero.Fs) {}
+var _ CustomRenderer = &CRD{}
+
+func (s *CRD) SetFS(fs afero.Fs) { s.initFS(fs) }
 
 func (s *CRD) CustomRender() ([]byte, error) {
-	i, _ := s.GetInput()
-	// controller-tools generates crd file names with no _crd.yaml suffix:
-	// <group>_<version>_<kind>.yaml.
-	path := strings.Replace(filepath.Base(i.Path), "_crd.yaml", ".yaml", 1)
+	i, err := s.GetInput()
+	if err != nil {
+		return nil, err
+	}
 
 	// controller-tools' generators read and make crds for all apis in pkg/apis,
 	// so generate crds in a cached, in-memory fs to extract the data we need.
-	if s.IsOperatorGo && !cache.fileExists(path) {
-		g := &crdgenerator.Generator{
-			RootPath:          s.AbsProjectPath,
-			Domain:            strings.SplitN(s.Resource.FullGroup, ".", 2)[1],
-			OutputDir:         ".",
-			SkipMapValidation: false,
-			OutFs:             cache,
-		}
-		if err := g.ValidateAndInitFields(); err != nil {
-			return nil, err
-		}
-		if err := g.Do(); err != nil {
-			return nil, err
+	var crd *apiextv1beta1.CustomResourceDefinition
+	if s.IsOperatorGo {
+		// controller-tools generates crd file names with no _crd.yaml suffix:
+		// <group>_<version>_<kind>.yaml.
+		path := strings.Replace(filepath.Base(i.Path), "_crd.yaml", ".yaml", 1)
+
+		if !cache.fileExists(path) {
+			g := &crdgenerator.Generator{
+				RootPath:          s.AbsProjectPath,
+				Domain:            strings.SplitN(s.Resource.FullGroup, ".", 2)[1],
+				Repo:              s.Repo,
+				OutputDir:         ".",
+				SkipMapValidation: false,
+				OutFs:             cache,
+			}
+			if err := g.ValidateAndInitFields(); err != nil {
+				return nil, err
+			}
+			if err := g.Do(); err != nil {
+				return nil, err
+			}
 		}
-	}
 
-	dstCRD := newCRDForResource(s.Resource)
-	// Get our generated crd's from the in-memory fs. If it doesn't exist in the
-	// fs, the corresponding API does not exist yet, so scaffold a fresh crd
-	// without a validation spec.
-	// If the crd exists in the fs, and a local crd exists, append the validation
-	// spec. If a local crd does not exist, use the generated crd.
-	if _, err := cache.Stat(path); err != nil && !os.IsNotExist(err) {
-		return nil, err
-	} else if err == nil {
 		b, err := afero.ReadFile(cache, path)
 		if err != nil {
 			return nil, err
 		}
-		dstCRD = &apiextv1beta1.CustomResourceDefinition{}
-		if err = yaml.Unmarshal(b, dstCRD); err != nil {
+		crd = &apiextv1beta1.CustomResourceDefinition{}
+		if err = yaml.Unmarshal(b, crd); err != nil {
 			return nil, err
 		}
-		val := dstCRD.Spec.Validation.DeepCopy()
-
-		// If the crd exists at i.Path, append the validation spec to its crd spec.
-		if _, err := os.Stat(i.Path); err == nil {
-			cb, err := ioutil.ReadFile(i.Path)
+		// controller-tools does not set ListKind or Singular names.
+		setCRDNamesForResource(crd, s.Resource)
+		// Remove controller-tools default label.
+		delete(crd.Labels, "controller-tools.k8s.io")
+	} else {
+		// There are currently no commands to update CRD manifests for non-Go
+		// operators, so if a CRD manifests already exists for this gvk, this
+		// scaffold is a no-op.
+		path := filepath.Join(s.AbsProjectPath, i.Path)
+		if _, err = s.getFS().Stat(path); err == nil {
+			b, err := afero.ReadFile(s.getFS(), path)
 			if err != nil {
 				return nil, err
 			}
-			if len(cb) > 0 {
-				dstCRD = &apiextv1beta1.CustomResourceDefinition{}
-				if err = yaml.Unmarshal(cb, dstCRD); err != nil {
-					return nil, err
-				}
-				dstCRD.Spec.Validation = val
+			crd = &apiextv1beta1.CustomResourceDefinition{}
+			if err = yaml.Unmarshal(b, crd); err != nil {
+				return nil, err
 			}
+		} else {
+			crd = newCRDForResource(s.Resource)
 		}
-		// controller-tools does not set ListKind or Singular names.
-		dstCRD.Spec.Names = getCRDNamesForResource(s.Resource)
-		// Remove controller-tools default label.
-		delete(dstCRD.Labels, "controller-tools.k8s.io")
 	}
-	addCRDSubresource(dstCRD)
-	addCRDVersions(dstCRD)
-	return k8sutil.GetObjectBytes(dstCRD)
+
+	setCRDVersions(crd)
+	return k8sutil.GetObjectBytes(crd)
 }
 
 func newCRDForResource(r *Resource) *apiextv1beta1.CustomResourceDefinition {
-	return &apiextv1beta1.CustomResourceDefinition{
+	crd := &apiextv1beta1.CustomResourceDefinition{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "apiextensions.k8s.io/v1beta1",
 			Kind:       "CustomResourceDefinition",
@@ -156,35 +169,33 @@ func newCRDForResource(r *Resource) *apiextv1beta1.CustomResourceDefinition {
 		},
 		Spec: apiextv1beta1.CustomResourceDefinitionSpec{
 			Group:   r.FullGroup,
-			Names:   getCRDNamesForResource(r),
 			Scope:   apiextv1beta1.NamespaceScoped,
 			Version: r.Version,
 			Subresources: &apiextv1beta1.CustomResourceSubresources{
 				Status: &apiextv1beta1.CustomResourceSubresourceStatus{},
 			},
 		},
 	}
+	setCRDNamesForResource(crd, r)
+	return crd
 }
 
-func getCRDNamesForResource(r *Resource) apiextv1beta1.CustomResourceDefinitionNames {
-	return apiextv1beta1.CustomResourceDefinitionNames{
-		Kind:     r.Kind,
-		ListKind: r.Kind + "List",
-		Plural:   r.Resource,
-		Singular: r.LowerKind,
+func setCRDNamesForResource(crd *apiextv1beta1.CustomResourceDefinition, r *Resource) {
+	if crd.Spec.Names.Kind == "" {
+		crd.Spec.Names.Kind = r.Kind
 	}
-}
-
-func addCRDSubresource(crd *apiextv1beta1.CustomResourceDefinition) {
-	if crd.Spec.Subresources == nil {
-		crd.Spec.Subresources = &apiextv1beta1.CustomResourceSubresources{}
+	if crd.Spec.Names.ListKind == "" {
+		crd.Spec.Names.ListKind = r.Kind + "List"
+	}
+	if crd.Spec.Names.Plural == "" {
+		crd.Spec.Names.Plural = r.Resource
 	}
-	if crd.Spec.Subresources.Status == nil {
-		crd.Spec.Subresources.Status = &apiextv1beta1.CustomResourceSubresourceStatus{}
+	if crd.Spec.Names.Singular == "" {
+		crd.Spec.Names.Singular = r.LowerKind
 	}
 }
 
-func addCRDVersions(crd *apiextv1beta1.CustomResourceDefinition) {
+func setCRDVersions(crd *apiextv1beta1.CustomResourceDefinition) {
 	// crd.Version is deprecated, use crd.Versions instead.
 	var crdVersions []apiextv1beta1.CustomResourceDefinitionVersion
 	if crd.Spec.Version != "" {

internal/pkg/scaffold/crd_test.go

@@ -21,37 +21,45 @@ import (
 	"testing"
 
 	"github.com/operator-framework/operator-sdk/internal/pkg/scaffold/input"
+	testutil "github.com/operator-framework/operator-sdk/internal/pkg/scaffold/internal/testutil"
 	"github.com/operator-framework/operator-sdk/internal/util/diffutil"
+	"github.com/operator-framework/operator-sdk/internal/util/fileutil"
+
+	"github.com/spf13/afero"
 )
 
-func TestCRDGoProject(t *testing.T) {
-	r, err := NewResource("cache.example.com/v1alpha1", "Memcached")
-	if err != nil {
-		t.Fatal(err)
-	}
-	s, buf := setupScaffoldAndWriter()
+func setupCRDConfig(t *testing.T, s *Scaffold) *input.Config {
 	absPath, err := os.Getwd()
 	if err != nil {
 		t.Fatal(err)
 	}
+	absPath = absPath[:strings.Index(absPath, "internal/pkg")]
+	tfDir := filepath.Join(absPath, "test", "test-framework")
+
 	// Set the project and repo paths to {abs}/test/test-framework, which
 	// contains pkg/apis for the memcached-operator.
-	tfDir := filepath.Join("test", "test-framework")
-	pkgIdx := strings.Index(absPath, "internal/pkg")
-	cfg := &input.Config{
-		Repo:           filepath.Join(absPath[strings.Index(absPath, "github.com"):pkgIdx], tfDir),
-		AbsProjectPath: filepath.Join(absPath[:pkgIdx], tfDir),
-		ProjectName:    tfDir,
+	return &input.Config{
+		Repo:           tfDir[strings.Index(absPath, "github.com"):],
+		AbsProjectPath: tfDir,
+		ProjectName:    filepath.Base(tfDir),
+	}
+}
+
+func TestCRDGoProject(t *testing.T) {
+	r, err := NewResource("cache.example.com/v1alpha1", "Memcached")
+	if err != nil {
+		t.Fatal(err)
 	}
-	if err := os.Chdir(cfg.AbsProjectPath); err != nil {
+	s, buf := setupScaffoldAndWriter()
+	s.Fs = afero.NewMemMapFs()
+	cfg := setupCRDConfig(t, s)
+
+	err = testutil.WriteOSPathToFS(afero.NewOsFs(), s.Fs, cfg.AbsProjectPath)
+	if err != nil {
 		t.Fatal(err)
 	}
-	defer func() { os.Chdir(absPath) }()
-	err = s.Execute(cfg, &CRD{
-		Input:        input.Input{Path: filepath.Join(tfDir, "cache_v1alpha1_memcached.yaml")},
-		Resource:     r,
-		IsOperatorGo: true,
-	})
+
+	err = s.Execute(cfg, &CRD{Resource: r, IsOperatorGo: true})
 	if err != nil {
 		t.Fatalf("Failed to execute the scaffold: (%v)", err)
 	}
@@ -74,8 +82,6 @@ spec:
     plural: memcacheds
     singular: memcached
   scope: Namespaced
-  subresources:
-    status: {}
   validation:
     openAPIV3Schema:
       properties:
@@ -118,13 +124,27 @@ spec:
 `
 
 func TestCRDNonGoProject(t *testing.T) {
+	s, buf := setupScaffoldAndWriter()
+	s.Fs = afero.NewMemMapFs()
+
 	r, err := NewResource(appApiVersion, appKind)
 	if err != nil {
 		t.Fatal(err)
 	}
-	s, buf := setupScaffoldAndWriter()
-	err = s.Execute(appConfig, &CRD{Resource: r})
+
+	crd := &CRD{Resource: r}
+	i, err := crd.GetInput()
 	if err != nil {
+		t.Fatal(err)
+	}
+	cfg := setupCRDConfig(t, s)
+	path := filepath.Join(cfg.AbsProjectPath, i.Path)
+	err = afero.WriteFile(s.Fs, path, []byte(crdNonGoExp), fileutil.DefaultFileMode)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err = s.Execute(cfg, crd); err != nil {
 		t.Fatalf("Failed to execute the scaffold: (%v)", err)
 	}
 
@@ -134,6 +154,9 @@ func TestCRDNonGoProject(t *testing.T) {
 	}
 }
 
+// crdNonGoExp contains a simple validation block to make sure manually-added
+// validation is not overwritten. Non-go projects don't have the luxury of
+// kubebuilder annotations.
 const crdNonGoExp = `apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
@@ -148,6 +171,26 @@ spec:
   scope: Namespaced
   subresources:
     status: {}
+  validation:
+    openAPIV3Schema:
+      properties:
+        spec:
+          properties:
+            size:
+              format: int32
+              type: integer
+          required:
+          - size
+          type: object
+        status:
+          properties:
+            nodes:
+              items:
+                type: string
+              type: array
+          required:
+          - nodes
+          type: object
   version: v1alpha1
   versions:
   - name: v1alpha1