Skip to content

Commit e096922

Browse files
authored
Merge pull request #744 from droot/bugfix/main-updater-refinements
🐛 fix missing add-to-scheme calls on new API version
2 parents 024627e + ca5af80 commit e096922

File tree

8 files changed

+160
-105
lines changed

8 files changed

+160
-105
lines changed

pkg/scaffold/api.go

+12-6
Original file line numberDiff line numberDiff line change
@@ -212,17 +212,23 @@ func (api *API) scaffoldV2() error {
212212
return fmt.Errorf("error scaffolding controller: %v", err)
213213
}
214214

215-
err = ctrlScaffolder.UpdateMain("main.go")
216-
if err != nil {
217-
return fmt.Errorf("error updating main.go with reconciler code: %v", err)
218-
}
219-
220-
err = testsuiteScaffolder.UpdateTestSuite()
215+
err = testsuiteScaffolder.Update()
221216
if err != nil {
222217
return fmt.Errorf("error updating suite_test.go under controllers pkg: %v", err)
223218
}
224219
}
225220

221+
err := (&resourcev2.Main{}).Update(
222+
&resourcev2.MainUpdateOptions{
223+
Project: api.project,
224+
WireResource: api.DoResource,
225+
WireController: api.DoController,
226+
Resource: r,
227+
})
228+
if err != nil {
229+
return fmt.Errorf("error updating main.go: %v", err)
230+
}
231+
226232
return nil
227233
}
228234

pkg/scaffold/v2/controller.go

-41
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727

2828
"sigs.k8s.io/kubebuilder/pkg/scaffold/input"
2929
"sigs.k8s.io/kubebuilder/pkg/scaffold/v1/resource"
30-
"sigs.k8s.io/kubebuilder/pkg/scaffold/v2/internal"
3130
)
3231

3332
// Controller scaffolds a Controller for a Resource
@@ -66,46 +65,6 @@ func (a *Controller) GetInput() (input.Input, error) {
6665
return a.Input, nil
6766
}
6867

69-
// UpdateMain updates given file (main.go) with code fragments required for
70-
// setting up a new reconciler.
71-
func (a *Controller) UpdateMain(path string) error {
72-
73-
a.ResourcePackage, a.GroupDomain = getResourceInfo(a.Resource, a.Input)
74-
if a.Plural == "" {
75-
rs := inflect.NewDefaultRuleset()
76-
a.Plural = rs.Pluralize(strings.ToLower(a.Resource.Kind))
77-
}
78-
79-
ctrlImportCodeFragment := fmt.Sprintf(`"%s/controllers"
80-
`, a.Repo)
81-
apiImportCodeFragment := fmt.Sprintf(`%s%s "%s/%s"
82-
`, a.Resource.Group, a.Resource.Version, a.ResourcePackage, a.Resource.Version)
83-
84-
addschemeCodeFragment := fmt.Sprintf(`%s%s.AddToScheme(scheme)
85-
`, a.Resource.Group, a.Resource.Version)
86-
87-
reconcilerSetupCodeFragment := fmt.Sprintf(`err = (&controllers.%sReconciler{
88-
Client: mgr.GetClient(),
89-
Log: ctrl.Log.WithName("controllers").WithName("%s"),
90-
}).SetupWithManager(mgr)
91-
if err != nil {
92-
setupLog.Error(err, "unable to create controller", "controller", "%s")
93-
os.Exit(1)
94-
}
95-
`, a.Resource.Kind, a.Resource.Kind, a.Resource.Kind)
96-
97-
err := internal.InsertStringsInFile(path,
98-
apiPkgImportScaffoldMarker, ctrlImportCodeFragment,
99-
apiPkgImportScaffoldMarker, apiImportCodeFragment,
100-
apiSchemeScaffoldMarker, addschemeCodeFragment,
101-
reconcilerSetupScaffoldMarker, reconcilerSetupCodeFragment)
102-
if err != nil {
103-
return err
104-
}
105-
106-
return nil
107-
}
108-
10968
func getResourceInfo(r *resource.Resource, in input.Input) (resourcePackage, groupDomain string) {
11069
// Use the k8s.io/api package for core resources
11170
coreGroups := map[string]string{

pkg/scaffold/v2/controller_suitetest.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,9 @@ var _ = AfterSuite(func() {
125125
})
126126
`
127127

128-
// UpdateTestSuite updates given file (suite_test.go) with code fragments required for
128+
// Update updates given file (suite_test.go) with code fragments required for
129129
// adding import paths and code setup for new types.
130-
func (a *ControllerSuiteTest) UpdateTestSuite() error {
130+
func (a *ControllerSuiteTest) Update() error {
131131

132132
a.ResourcePackage, a.GroupDomain = getResourceInfo(a.Resource, a.Input)
133133
if a.Plural == "" {
@@ -146,9 +146,10 @@ Expect(err).NotTo(HaveOccurred())
146146
`, a.Resource.Group, a.Resource.Version)
147147

148148
err := internal.InsertStringsInFile(a.Path,
149-
apiPkgImportScaffoldMarker, ctrlImportCodeFragment,
150-
apiPkgImportScaffoldMarker, apiImportCodeFragment,
151-
apiSchemeScaffoldMarker, addschemeCodeFragment)
149+
map[string][]string{
150+
apiPkgImportScaffoldMarker: []string{ctrlImportCodeFragment, apiImportCodeFragment},
151+
apiSchemeScaffoldMarker: []string{addschemeCodeFragment},
152+
})
152153
if err != nil {
153154
return err
154155
}

pkg/scaffold/v2/crd/kustomization.go

+8-6
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ import (
2929
)
3030

3131
const (
32-
kustomizeResourceScaffoldMarker = "+kubebuilder:scaffold:kustomizeresource"
33-
kustomizePatchScaffoldMarker = "+kubebuilder:scaffold:kustomizepatch"
32+
kustomizeResourceScaffoldMarker = "# +kubebuilder:scaffold:kustomizeresource"
33+
kustomizePatchScaffoldMarker = "# +kubebuilder:scaffold:kustomizepatch"
3434
)
3535

3636
var _ input.File = &Kustomization{}
@@ -67,19 +67,21 @@ func (c *Kustomization) Update() error {
6767
kustomizePatchCodeFragment := fmt.Sprintf("#- patches/webhook_in_%s.yaml\n", plural)
6868

6969
return internal.InsertStringsInFile(c.Path,
70-
kustomizeResourceScaffoldMarker, kustomizeResourceCodeFragment,
71-
kustomizePatchScaffoldMarker, kustomizePatchCodeFragment)
70+
map[string][]string{
71+
kustomizeResourceScaffoldMarker: []string{kustomizeResourceCodeFragment},
72+
kustomizePatchScaffoldMarker: []string{kustomizePatchCodeFragment},
73+
})
7274
}
7375

7476
var kustomizationTemplate = fmt.Sprintf(`# This kustomization.yaml is not intended to be run by itself,
7577
# since it depends on service name and namespace that are out of this kustomize package.
7678
# It should be run by config/default
7779
resources:
78-
# %s
80+
%s
7981
8082
patches:
8183
# patches here are for enabling the conversion webhook for each CRD
82-
# %s
84+
%s
8385
8486
# the following config is for teaching kustomize how to do kustomization for CRDs.
8587
configurations:

pkg/scaffold/v2/internal/string_utils.go

+43-26
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package internal
1818
import (
1919
"bufio"
2020
"bytes"
21-
"fmt"
2221
"io"
2322
"io/ioutil"
2423
"os"
@@ -29,51 +28,48 @@ import (
2928
)
3029

3130
// insertStrings reads content from given reader and insert string below the
32-
// line line containing marker string. So for ex. in insertStrings(r, m1, v1, m2, v2)
31+
// line containing marker string. So for ex. in insertStrings(r, {'m1':
32+
// [v1], 'm2': [v2]})
3333
// v1 will be inserted below the lines containing m1 string and v2 will be inserted
3434
// below line containing m2 string.
35-
func insertStrings(r io.Reader, markerAndValues ...string) (io.Reader, error) {
36-
if len(markerAndValues)%2 != 0 {
37-
return nil, fmt.Errorf("invalid marker and value pairs")
38-
}
35+
func insertStrings(r io.Reader, markerAndValues map[string][]string) (io.Reader, error) {
36+
// reader clone is needed since we will be reading twice from the given reader
37+
buf := new(bytes.Buffer)
38+
rClone := io.TeeReader(r, buf)
3939

40-
mvPairs := map[string]string{}
41-
for i, s := range markerAndValues {
42-
if i%2 == 0 {
43-
mvPairs[s] = markerAndValues[i+1]
44-
}
40+
err := filterExistingValues(rClone, markerAndValues)
41+
if err != nil {
42+
return nil, err
4543
}
4644

47-
buf := new(bytes.Buffer)
45+
out := new(bytes.Buffer)
4846

49-
scanner := bufio.NewScanner(r)
47+
scanner := bufio.NewScanner(buf)
5048
for scanner.Scan() {
5149
line := scanner.Text()
5250

53-
for m, v := range mvPairs {
54-
if strings.TrimSpace(line) == strings.TrimSpace(v) {
55-
// since value already exist, so avoid duplication
56-
delete(mvPairs, m)
57-
}
58-
if strings.Contains(line, m) {
59-
_, err := buf.WriteString(v)
60-
if err != nil {
61-
return nil, err
51+
for marker, vals := range markerAndValues {
52+
if strings.TrimSpace(line) == strings.TrimSpace(marker) {
53+
for _, val := range vals {
54+
_, err := out.WriteString(val)
55+
if err != nil {
56+
return nil, err
57+
}
6258
}
6359
}
6460
}
65-
_, err := buf.WriteString(line + "\n")
61+
_, err := out.WriteString(line + "\n")
6662
if err != nil {
6763
return nil, err
6864
}
6965
}
7066
if err := scanner.Err(); err != nil {
7167
return nil, err
7268
}
73-
return buf, nil
69+
return out, nil
7470
}
7571

76-
func InsertStringsInFile(path string, markerAndValues ...string) error {
72+
func InsertStringsInFile(path string, markerAndValues map[string][]string) error {
7773
isGoFile := false
7874
if ext := filepath.Ext(path); ext == ".go" {
7975
isGoFile = true
@@ -84,7 +80,7 @@ func InsertStringsInFile(path string, markerAndValues ...string) error {
8480
return err
8581
}
8682

87-
r, err := insertStrings(f, markerAndValues...)
83+
r, err := insertStrings(f, markerAndValues)
8884
if err != nil {
8985
return err
9086
}
@@ -115,3 +111,24 @@ func InsertStringsInFile(path string, markerAndValues ...string) error {
115111

116112
return err
117113
}
114+
115+
// filterExistingValues removes the single-line values that already exists in
116+
// the given reader. Multi-line values are ignore currently simply because we
117+
// don't have a use-case for it.
118+
func filterExistingValues(r io.Reader, markerAndValues map[string][]string) error {
119+
scanner := bufio.NewScanner(r)
120+
for scanner.Scan() {
121+
line := scanner.Text()
122+
for marker, vals := range markerAndValues {
123+
for i, val := range vals {
124+
if strings.TrimSpace(line) == strings.TrimSpace(val) {
125+
markerAndValues[marker] = append(vals[:i], vals[i+1:]...)
126+
}
127+
}
128+
}
129+
}
130+
if err := scanner.Err(); err != nil {
131+
return err
132+
}
133+
return nil
134+
}

pkg/scaffold/v2/internal/string_utils_test.go

+18-14
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@ import (
2222
)
2323

2424
type insertStrTest struct {
25-
input string
26-
marker string
27-
str string
28-
expected string
29-
got string
25+
input string
26+
markerNValues map[string][]string
27+
expected string
28+
got string
3029
}
3130

3231
func TestInsertStrBelowMarker(t *testing.T) {
@@ -37,11 +36,14 @@ func TestInsertStrBelowMarker(t *testing.T) {
3736
v1beta1.AddToScheme(scheme)
3837
// +kubebuilder:scaffold:apis-add-scheme
3938
`,
40-
marker: "+kubebuilder:scaffold:apis-add-scheme",
41-
str: "v1.AddToScheme(scheme)\n",
39+
markerNValues: map[string][]string{
40+
"// +kubebuilder:scaffold:apis-add-scheme": []string{
41+
"v1.AddToScheme(scheme)\n", "somefunc()\n"},
42+
},
4243
expected: `
4344
v1beta1.AddToScheme(scheme)
4445
v1.AddToScheme(scheme)
46+
somefunc()
4547
// +kubebuilder:scaffold:apis-add-scheme
4648
`,
4749
},
@@ -50,10 +52,13 @@ v1.AddToScheme(scheme)
5052
v1beta1.AddToScheme(scheme)
5153
// +kubebuilder:scaffold:apis-add-scheme
5254
`,
53-
marker: "+kubebuilder:scaffold:apis-add-scheme",
54-
str: "v1beta1.AddToScheme(scheme)\n",
55+
markerNValues: map[string][]string{
56+
"// +kubebuilder:scaffold:apis-add-scheme": []string{
57+
"v1beta1.AddToScheme(scheme)\n", "v1.AddToScheme(scheme)\n"},
58+
},
5559
expected: `
5660
v1beta1.AddToScheme(scheme)
61+
v1.AddToScheme(scheme)
5762
// +kubebuilder:scaffold:apis-add-scheme
5863
`,
5964
},
@@ -63,9 +68,9 @@ v1beta1.AddToScheme(scheme)
6368
v1beta1.AddToScheme(scheme)
6469
// +kubebuilder:scaffold:apis-add-scheme
6570
`,
66-
marker: "+kubebuilder:scaffold:apis-add-scheme",
67-
str: `v1.AddToScheme(scheme)
68-
`,
71+
markerNValues: map[string][]string{
72+
"// +kubebuilder:scaffold:apis-add-scheme": []string{`v1.AddToScheme(scheme)
73+
`}},
6974
expected: `
7075
v1beta1.AddToScheme(scheme)
7176
v1.AddToScheme(scheme)
@@ -75,7 +80,7 @@ v1.AddToScheme(scheme)
7580
}
7681

7782
for _, test := range tests {
78-
result, err := insertStrings(bytes.NewBufferString(test.input), test.marker, test.str)
83+
result, err := insertStrings(bytes.NewBufferString(test.input), test.markerNValues)
7984
if err != nil {
8085
t.Errorf("error %v", err)
8186
}
@@ -89,5 +94,4 @@ v1.AddToScheme(scheme)
8994
t.Errorf("got: %s and wanted: %s", string(b), test.expected)
9095
}
9196
}
92-
9397
}

0 commit comments

Comments
 (0)