Merge pull request #744 from droot/bugfix/main-updater-refinements

🐛 fix missing add-to-scheme calls on new API version

pkg/scaffold/api.go

@@ -212,17 +212,23 @@ func (api *API) scaffoldV2() error {
 			return fmt.Errorf("error scaffolding controller: %v", err)
 		}
 
-		err = ctrlScaffolder.UpdateMain("main.go")
-		if err != nil {
-			return fmt.Errorf("error updating main.go with reconciler code: %v", err)
-		}
-
-		err = testsuiteScaffolder.UpdateTestSuite()
+		err = testsuiteScaffolder.Update()
 		if err != nil {
 			return fmt.Errorf("error updating suite_test.go under controllers pkg: %v", err)
 		}
 	}
 
+	err := (&resourcev2.Main{}).Update(
+		&resourcev2.MainUpdateOptions{
+			Project:        api.project,
+			WireResource:   api.DoResource,
+			WireController: api.DoController,
+			Resource:       r,
+		})
+	if err != nil {
+		return fmt.Errorf("error updating main.go: %v", err)
+	}
+
 	return nil
 }
 

pkg/scaffold/v2/controller.go

@@ -27,7 +27,6 @@ import (
 
 	"sigs.k8s.io/kubebuilder/pkg/scaffold/input"
 	"sigs.k8s.io/kubebuilder/pkg/scaffold/v1/resource"
-	"sigs.k8s.io/kubebuilder/pkg/scaffold/v2/internal"
 )
 
 // Controller scaffolds a Controller for a Resource
@@ -66,46 +65,6 @@ func (a *Controller) GetInput() (input.Input, error) {
 	return a.Input, nil
 }
 
-// UpdateMain updates given file (main.go) with code fragments required for
-// setting up a new reconciler.
-func (a *Controller) UpdateMain(path string) error {
-
-	a.ResourcePackage, a.GroupDomain = getResourceInfo(a.Resource, a.Input)
-	if a.Plural == "" {
-		rs := inflect.NewDefaultRuleset()
-		a.Plural = rs.Pluralize(strings.ToLower(a.Resource.Kind))
-	}
-
-	ctrlImportCodeFragment := fmt.Sprintf(`"%s/controllers"
-`, a.Repo)
-	apiImportCodeFragment := fmt.Sprintf(`%s%s "%s/%s"
-`, a.Resource.Group, a.Resource.Version, a.ResourcePackage, a.Resource.Version)
-
-	addschemeCodeFragment := fmt.Sprintf(`%s%s.AddToScheme(scheme)
-`, a.Resource.Group, a.Resource.Version)
-
-	reconcilerSetupCodeFragment := fmt.Sprintf(`err = (&controllers.%sReconciler{
-	 	Client: mgr.GetClient(),
-        Log: ctrl.Log.WithName("controllers").WithName("%s"),
-	 }).SetupWithManager(mgr)
-	 if err != nil {
-	 	setupLog.Error(err, "unable to create controller", "controller", "%s")
-	 	os.Exit(1)
-	 }
-`, a.Resource.Kind, a.Resource.Kind, a.Resource.Kind)
-
-	err := internal.InsertStringsInFile(path,
-		apiPkgImportScaffoldMarker, ctrlImportCodeFragment,
-		apiPkgImportScaffoldMarker, apiImportCodeFragment,
-		apiSchemeScaffoldMarker, addschemeCodeFragment,
-		reconcilerSetupScaffoldMarker, reconcilerSetupCodeFragment)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func getResourceInfo(r *resource.Resource, in input.Input) (resourcePackage, groupDomain string) {
 	// Use the k8s.io/api package for core resources
 	coreGroups := map[string]string{

pkg/scaffold/v2/controller_suitetest.go

@@ -125,9 +125,9 @@ var _ = AfterSuite(func() {
 })
 `
 
-// UpdateTestSuite updates given file (suite_test.go) with code fragments required for
+// Update updates given file (suite_test.go) with code fragments required for
 // adding import paths and code setup for new types.
-func (a *ControllerSuiteTest) UpdateTestSuite() error {
+func (a *ControllerSuiteTest) Update() error {
 
 	a.ResourcePackage, a.GroupDomain = getResourceInfo(a.Resource, a.Input)
 	if a.Plural == "" {
@@ -146,9 +146,10 @@ Expect(err).NotTo(HaveOccurred())
 `, a.Resource.Group, a.Resource.Version)
 
 	err := internal.InsertStringsInFile(a.Path,
-		apiPkgImportScaffoldMarker, ctrlImportCodeFragment,
-		apiPkgImportScaffoldMarker, apiImportCodeFragment,
-		apiSchemeScaffoldMarker, addschemeCodeFragment)
+		map[string][]string{
+			apiPkgImportScaffoldMarker: []string{ctrlImportCodeFragment, apiImportCodeFragment},
+			apiSchemeScaffoldMarker:    []string{addschemeCodeFragment},
+		})
 	if err != nil {
 		return err
 	}

pkg/scaffold/v2/crd/kustomization.go

@@ -29,8 +29,8 @@ import (
 )
 
 const (
-	kustomizeResourceScaffoldMarker = "+kubebuilder:scaffold:kustomizeresource"
-	kustomizePatchScaffoldMarker    = "+kubebuilder:scaffold:kustomizepatch"
+	kustomizeResourceScaffoldMarker = "# +kubebuilder:scaffold:kustomizeresource"
+	kustomizePatchScaffoldMarker    = "# +kubebuilder:scaffold:kustomizepatch"
 )
 
 var _ input.File = &Kustomization{}
@@ -67,19 +67,21 @@ func (c *Kustomization) Update() error {
 	kustomizePatchCodeFragment := fmt.Sprintf("#- patches/webhook_in_%s.yaml\n", plural)
 
 	return internal.InsertStringsInFile(c.Path,
-		kustomizeResourceScaffoldMarker, kustomizeResourceCodeFragment,
-		kustomizePatchScaffoldMarker, kustomizePatchCodeFragment)
+		map[string][]string{
+			kustomizeResourceScaffoldMarker: []string{kustomizeResourceCodeFragment},
+			kustomizePatchScaffoldMarker:    []string{kustomizePatchCodeFragment},
+		})
 }
 
 var kustomizationTemplate = fmt.Sprintf(`# This kustomization.yaml is not intended to be run by itself,
 # since it depends on service name and namespace that are out of this kustomize package.
 # It should be run by config/default
 resources:
-# %s
+%s
 
 patches:
 # patches here are for enabling the conversion webhook for each CRD
-# %s
+%s
 
 # the following config is for teaching kustomize how to do kustomization for CRDs.
 configurations:

pkg/scaffold/v2/internal/string_utils.go

@@ -18,7 +18,6 @@ package internal
 import (
 	"bufio"
 	"bytes"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -29,51 +28,48 @@ import (
 )
 
 // insertStrings reads content from given reader and insert string below the
-// line line containing marker string. So for ex. in insertStrings(r, m1, v1, m2, v2)
+// line containing marker string. So for ex. in insertStrings(r, {'m1':
+// [v1], 'm2': [v2]})
 // v1 will be inserted below the lines containing m1 string and v2 will be inserted
 // below line containing m2 string.
-func insertStrings(r io.Reader, markerAndValues ...string) (io.Reader, error) {
-	if len(markerAndValues)%2 != 0 {
-		return nil, fmt.Errorf("invalid marker and value pairs")
-	}
+func insertStrings(r io.Reader, markerAndValues map[string][]string) (io.Reader, error) {
+	// reader clone is needed since we will be reading twice from the given reader
+	buf := new(bytes.Buffer)
+	rClone := io.TeeReader(r, buf)
 
-	mvPairs := map[string]string{}
-	for i, s := range markerAndValues {
-		if i%2 == 0 {
-			mvPairs[s] = markerAndValues[i+1]
-		}
+	err := filterExistingValues(rClone, markerAndValues)
+	if err != nil {
+		return nil, err
 	}
 
-	buf := new(bytes.Buffer)
+	out := new(bytes.Buffer)
 
-	scanner := bufio.NewScanner(r)
+	scanner := bufio.NewScanner(buf)
 	for scanner.Scan() {
 		line := scanner.Text()
 
-		for m, v := range mvPairs {
-			if strings.TrimSpace(line) == strings.TrimSpace(v) {
-				// since value already exist, so avoid duplication
-				delete(mvPairs, m)
-			}
-			if strings.Contains(line, m) {
-				_, err := buf.WriteString(v)
-				if err != nil {
-					return nil, err
+		for marker, vals := range markerAndValues {
+			if strings.TrimSpace(line) == strings.TrimSpace(marker) {
+				for _, val := range vals {
+					_, err := out.WriteString(val)
+					if err != nil {
+						return nil, err
+					}
 				}
 			}
 		}
-		_, err := buf.WriteString(line + "\n")
+		_, err := out.WriteString(line + "\n")
 		if err != nil {
 			return nil, err
 		}
 	}
 	if err := scanner.Err(); err != nil {
 		return nil, err
 	}
-	return buf, nil
+	return out, nil
 }
 
-func InsertStringsInFile(path string, markerAndValues ...string) error {
+func InsertStringsInFile(path string, markerAndValues map[string][]string) error {
 	isGoFile := false
 	if ext := filepath.Ext(path); ext == ".go" {
 		isGoFile = true
@@ -84,7 +80,7 @@ func InsertStringsInFile(path string, markerAndValues ...string) error {
 		return err
 	}
 
-	r, err := insertStrings(f, markerAndValues...)
+	r, err := insertStrings(f, markerAndValues)
 	if err != nil {
 		return err
 	}
@@ -115,3 +111,24 @@ func InsertStringsInFile(path string, markerAndValues ...string) error {
 
 	return err
 }
+
+// filterExistingValues removes the single-line values that already exists in
+// the given reader. Multi-line values are ignore currently simply because we
+// don't have a use-case for it.
+func filterExistingValues(r io.Reader, markerAndValues map[string][]string) error {
+	scanner := bufio.NewScanner(r)
+	for scanner.Scan() {
+		line := scanner.Text()
+		for marker, vals := range markerAndValues {
+			for i, val := range vals {
+				if strings.TrimSpace(line) == strings.TrimSpace(val) {
+					markerAndValues[marker] = append(vals[:i], vals[i+1:]...)
+				}
+			}
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		return err
+	}
+	return nil
+}

pkg/scaffold/v2/internal/string_utils_test.go

@@ -22,11 +22,10 @@ import (
 )
 
 type insertStrTest struct {
-	input    string
-	marker   string
-	str      string
-	expected string
-	got      string
+	input         string
+	markerNValues map[string][]string
+	expected      string
+	got           string
 }
 
 func TestInsertStrBelowMarker(t *testing.T) {
@@ -37,11 +36,14 @@ func TestInsertStrBelowMarker(t *testing.T) {
 v1beta1.AddToScheme(scheme)
 // +kubebuilder:scaffold:apis-add-scheme
 `,
-			marker: "+kubebuilder:scaffold:apis-add-scheme",
-			str:    "v1.AddToScheme(scheme)\n",
+			markerNValues: map[string][]string{
+				"// +kubebuilder:scaffold:apis-add-scheme": []string{
+					"v1.AddToScheme(scheme)\n", "somefunc()\n"},
+			},
 			expected: `
 v1beta1.AddToScheme(scheme)
 v1.AddToScheme(scheme)
+somefunc()
 // +kubebuilder:scaffold:apis-add-scheme
 `,
 		},
@@ -50,10 +52,13 @@ v1.AddToScheme(scheme)
 v1beta1.AddToScheme(scheme)
 // +kubebuilder:scaffold:apis-add-scheme
 `,
-			marker: "+kubebuilder:scaffold:apis-add-scheme",
-			str:    "v1beta1.AddToScheme(scheme)\n",
+			markerNValues: map[string][]string{
+				"// +kubebuilder:scaffold:apis-add-scheme": []string{
+					"v1beta1.AddToScheme(scheme)\n", "v1.AddToScheme(scheme)\n"},
+			},
 			expected: `
 v1beta1.AddToScheme(scheme)
+v1.AddToScheme(scheme)
 // +kubebuilder:scaffold:apis-add-scheme
 `,
 		},
@@ -63,9 +68,9 @@ v1beta1.AddToScheme(scheme)
 v1beta1.AddToScheme(scheme)
 // +kubebuilder:scaffold:apis-add-scheme
 `,
-			marker: "+kubebuilder:scaffold:apis-add-scheme",
-			str: `v1.AddToScheme(scheme)
-`,
+			markerNValues: map[string][]string{
+				"// +kubebuilder:scaffold:apis-add-scheme": []string{`v1.AddToScheme(scheme)
+`}},
 			expected: `
 v1beta1.AddToScheme(scheme)
 v1.AddToScheme(scheme)
@@ -75,7 +80,7 @@ v1.AddToScheme(scheme)
 	}
 
 	for _, test := range tests {
-		result, err := insertStrings(bytes.NewBufferString(test.input), test.marker, test.str)
+		result, err := insertStrings(bytes.NewBufferString(test.input), test.markerNValues)
 		if err != nil {
 			t.Errorf("error %v", err)
 		}
@@ -89,5 +94,4 @@ v1.AddToScheme(scheme)
 			t.Errorf("got: %s and wanted: %s", string(b), test.expected)
 		}
 	}
-
 }