Skip to content

Commit

Permalink
Remove spaces from machine-readable comments (v3 only)
Browse files Browse the repository at this point in the history
Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>
  • Loading branch information
Adirio committed Dec 11, 2020
1 parent c52c367 commit 9f8fedd
Show file tree
Hide file tree
Showing 99 changed files with 294 additions and 277 deletions.
23 changes: 20 additions & 3 deletions pkg/model/file/marker.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,16 @@ import (
const prefix = "+kubebuilder:scaffold:"

var commentsByExt = map[string]string{
// TODO(v3): machine-readable comments should not have spaces by Go convention. However,
// this is a backwards incompatible change, and thus should be done for next project version.
".go": "//",
".yaml": "#",
// When adding additional file extensions, update also the NewMarkerFor documentation and error
}

// TODO: remove when go.kubebuilder.io/v2 is removed
var commentsByExtV2 = map[string]string{
".go": "// ",
".yaml": "# ",
// When adding additional file extensions, update also the NewMarkerFor documentation and error
// When adding additional file extensions, update also the NewMarkerForV2 documentation and error
}

// Marker represents a machine-readable comment that will be used for scaffolding purposes
Expand All @@ -48,6 +53,18 @@ func NewMarkerFor(path string, value string) Marker {
panic(fmt.Errorf("unknown file extension: '%s', expected '.go' or '.yaml'", ext))
}

// NewMarkerForV2 creates a new marker customized for the specific v2 file
// Supported file extensions: .go, .ext
// TODO: remove when go.kubebuilder.io/v2 is removed
func NewMarkerForV2(path string, value string) Marker {
ext := filepath.Ext(path)
if comment, found := commentsByExtV2[ext]; found {
return Marker{comment, value}
}

panic(fmt.Errorf("unknown file extension: '%s', expected '.go' or '.yaml'", ext))
}

// String implements Stringer
func (m Marker) String() string {
return m.comment + prefix + m.value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ func (f *Kustomization) SetTemplateDefaults() error {
f.Path = f.Resource.Replacer().Replace(f.Path)

f.TemplateBody = fmt.Sprintf(kustomizationTemplate,
file.NewMarkerFor(f.Path, resourceMarker),
file.NewMarkerFor(f.Path, webhookPatchMarker),
file.NewMarkerFor(f.Path, caInjectionPatchMarker),
file.NewMarkerForV2(f.Path, resourceMarker),
file.NewMarkerForV2(f.Path, webhookPatchMarker),
file.NewMarkerForV2(f.Path, caInjectionPatchMarker),
)

return nil
Expand All @@ -57,9 +57,9 @@ const (
// GetMarkers implements file.Inserter
func (f *Kustomization) GetMarkers() []file.Marker {
return []file.Marker{
file.NewMarkerFor(f.Path, resourceMarker),
file.NewMarkerFor(f.Path, webhookPatchMarker),
file.NewMarkerFor(f.Path, caInjectionPatchMarker),
file.NewMarkerForV2(f.Path, resourceMarker),
file.NewMarkerForV2(f.Path, webhookPatchMarker),
file.NewMarkerForV2(f.Path, caInjectionPatchMarker),
}
}

Expand Down Expand Up @@ -90,13 +90,13 @@ func (f *Kustomization) GetCodeFragments() file.CodeFragmentsMap {

// Only store code fragments in the map if the slices are non-empty
if len(res) != 0 {
fragments[file.NewMarkerFor(f.Path, resourceMarker)] = res
fragments[file.NewMarkerForV2(f.Path, resourceMarker)] = res
}
if len(webhookPatch) != 0 {
fragments[file.NewMarkerFor(f.Path, webhookPatchMarker)] = webhookPatch
fragments[file.NewMarkerForV2(f.Path, webhookPatchMarker)] = webhookPatch
}
if len(caInjectionPatch) != 0 {
fragments[file.NewMarkerFor(f.Path, caInjectionPatchMarker)] = caInjectionPatch
fragments[file.NewMarkerForV2(f.Path, caInjectionPatchMarker)] = caInjectionPatch
}

return fragments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ func (f *SuiteTest) SetTemplateDefaults() error {
f.Path = f.Resource.Replacer().Replace(f.Path)

f.TemplateBody = fmt.Sprintf(controllerSuiteTestTemplate,
file.NewMarkerFor(f.Path, importMarker),
file.NewMarkerFor(f.Path, addSchemeMarker),
file.NewMarkerForV2(f.Path, importMarker),
file.NewMarkerForV2(f.Path, addSchemeMarker),
)

// If is multigroup the path needs to be ../../ since it has
Expand All @@ -75,8 +75,8 @@ const (
// GetMarkers implements file.Inserter
func (f *SuiteTest) GetMarkers() []file.Marker {
return []file.Marker{
file.NewMarkerFor(f.Path, importMarker),
file.NewMarkerFor(f.Path, addSchemeMarker),
file.NewMarkerForV2(f.Path, importMarker),
file.NewMarkerForV2(f.Path, addSchemeMarker),
}
}

Expand Down Expand Up @@ -107,10 +107,10 @@ func (f *SuiteTest) GetCodeFragments() file.CodeFragmentsMap {

// Only store code fragments in the map if the slices are non-empty
if len(imports) != 0 {
fragments[file.NewMarkerFor(f.Path, importMarker)] = imports
fragments[file.NewMarkerForV2(f.Path, importMarker)] = imports
}
if len(addScheme) != 0 {
fragments[file.NewMarkerFor(f.Path, addSchemeMarker)] = addScheme
fragments[file.NewMarkerForV2(f.Path, addSchemeMarker)] = addScheme
}

return fragments
Expand Down
18 changes: 9 additions & 9 deletions pkg/plugins/golang/v2/scaffolds/internal/templates/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ func (f *Main) SetTemplateDefaults() error {
}

f.TemplateBody = fmt.Sprintf(mainTemplate,
file.NewMarkerFor(f.Path, importMarker),
file.NewMarkerFor(f.Path, addSchemeMarker),
file.NewMarkerFor(f.Path, setupMarker),
file.NewMarkerForV2(f.Path, importMarker),
file.NewMarkerForV2(f.Path, addSchemeMarker),
file.NewMarkerForV2(f.Path, setupMarker),
)

return nil
Expand Down Expand Up @@ -81,9 +81,9 @@ const (
// GetMarkers implements file.Inserter
func (f *MainUpdater) GetMarkers() []file.Marker {
return []file.Marker{
file.NewMarkerFor(defaultMainPath, importMarker),
file.NewMarkerFor(defaultMainPath, addSchemeMarker),
file.NewMarkerFor(defaultMainPath, setupMarker),
file.NewMarkerForV2(defaultMainPath, importMarker),
file.NewMarkerForV2(defaultMainPath, addSchemeMarker),
file.NewMarkerForV2(defaultMainPath, setupMarker),
}
}

Expand Down Expand Up @@ -175,13 +175,13 @@ func (f *MainUpdater) GetCodeFragments() file.CodeFragmentsMap {

// Only store code fragments in the map if the slices are non-empty
if len(imports) != 0 {
fragments[file.NewMarkerFor(defaultMainPath, importMarker)] = imports
fragments[file.NewMarkerForV2(defaultMainPath, importMarker)] = imports
}
if len(addScheme) != 0 {
fragments[file.NewMarkerFor(defaultMainPath, addSchemeMarker)] = addScheme
fragments[file.NewMarkerForV2(defaultMainPath, addSchemeMarker)] = addScheme
}
if len(setup) != 0 {
fragments[file.NewMarkerFor(defaultMainPath, setupMarker)] = setup
fragments[file.NewMarkerForV2(defaultMainPath, setupMarker)] = setup
}

return fragments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ func (f *Group) SetTemplateDefaults() error {
const groupTemplate = `{{ .Boilerplate }}
// Package {{ .Resource.Version }} contains API Schema definitions for the {{ .Resource.Group }} {{ .Resource.Version }} API group
// +kubebuilder:object:generate=true
// +groupName={{ .Resource.Domain }}
//+kubebuilder:object:generate=true
//+groupName={{ .Resource.Domain }}
package {{ .Resource.Version }}
import (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ type {{ .Resource.Kind }}Status struct {
// Important: Run "make" to regenerate code after modifying this file
}
// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
{{ if not .Resource.Namespaced }} // +kubebuilder:resource:scope=Cluster {{ end }}
//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
{{ if not .Resource.Namespaced }} //+kubebuilder:resource:scope=Cluster {{ end }}
// {{ .Resource.Kind }} is the Schema for the {{ .Resource.Plural }} API
type {{ .Resource.Kind }} struct {
Expand All @@ -95,7 +95,7 @@ type {{ .Resource.Kind }} struct {
Status {{ .Resource.Kind }}Status ` + "`" + `json:"status,omitempty"` + "`" + `
}
// +kubebuilder:object:root=true
//+kubebuilder:object:root=true
// {{ .Resource.Kind }}List contains a list of {{ .Resource.Kind }}
type {{ .Resource.Kind }}List struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (r *{{ .Resource.Kind }}) SetupWebhookWithManager(mgr ctrl.Manager) error {
// TODO(estroz): update admissionReviewVersions to include v1 when controller-runtime supports that version.
//nolint:lll
defaultingWebhookTemplate = `
// +kubebuilder:webhook:{{ if ne .WebhookVersion "v1" }}webhookVersions={{"{"}}{{ .WebhookVersion }}{{"}"}},{{ end }}path=/mutate-{{ .GroupDomainWithDash }}-{{ .Resource.Version }}-{{ lower .Resource.Kind }},mutating=true,failurePolicy=fail,sideEffects=None,groups={{ .Resource.Domain }},resources={{ .Resource.Plural }},verbs=create;update,versions={{ .Resource.Version }},name=m{{ lower .Resource.Kind }}.kb.io,admissionReviewVersions={v1beta1}
//+kubebuilder:webhook:{{ if ne .WebhookVersion "v1" }}webhookVersions={{"{"}}{{ .WebhookVersion }}{{"}"}},{{ end }}path=/mutate-{{ .GroupDomainWithDash }}-{{ .Resource.Version }}-{{ lower .Resource.Kind }},mutating=true,failurePolicy=fail,sideEffects=None,groups={{ .Resource.Domain }},resources={{ .Resource.Plural }},verbs=create;update,versions={{ .Resource.Version }},name=m{{ lower .Resource.Kind }}.kb.io,admissionReviewVersions={v1beta1}
var _ webhook.Defaulter = &{{ .Resource.Kind }}{}
Expand All @@ -127,7 +127,7 @@ func (r *{{ .Resource.Kind }}) Default() {
//nolint:lll
validatingWebhookTemplate = `
// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
// +kubebuilder:webhook:{{ if ne .WebhookVersion "v1" }}webhookVersions={{"{"}}{{ .WebhookVersion }}{{"}"}},{{ end }}path=/validate-{{ .GroupDomainWithDash }}-{{ .Resource.Version }}-{{ lower .Resource.Kind }},mutating=false,failurePolicy=fail,sideEffects=None,groups={{ .Resource.Domain }},resources={{ .Resource.Plural }},verbs=create;update,versions={{ .Resource.Version }},name=v{{ lower .Resource.Kind }}.kb.io,admissionReviewVersions={v1beta1}
//+kubebuilder:webhook:{{ if ne .WebhookVersion "v1" }}webhookVersions={{"{"}}{{ .WebhookVersion }}{{"}"}},{{ end }}path=/validate-{{ .GroupDomainWithDash }}-{{ .Resource.Version }}-{{ lower .Resource.Kind }},mutating=false,failurePolicy=fail,sideEffects=None,groups={{ .Resource.Domain }},resources={{ .Resource.Plural }},verbs=create;update,versions={{ .Resource.Version }},name=v{{ lower .Resource.Kind }}.kb.io,admissionReviewVersions={v1beta1}
var _ webhook.Validator = &{{ .Resource.Kind }}{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ type {{ .Resource.Kind }}Reconciler struct {
Scheme *runtime.Scheme
}
// +kubebuilder:rbac:groups={{ .Resource.Domain }},resources={{ .Resource.Plural }},verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups={{ .Resource.Domain }},resources={{ .Resource.Plural }}/status,verbs=get;update;patch
// +kubebuilder:rbac:groups={{ .Resource.Domain }},resources={{ .Resource.Plural }}/finalizers,verbs=update
//+kubebuilder:rbac:groups={{ .Resource.Domain }},resources={{ .Resource.Plural }},verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups={{ .Resource.Domain }},resources={{ .Resource.Plural }}/status,verbs=get;update;patch
//+kubebuilder:rbac:groups={{ .Resource.Domain }},resources={{ .Resource.Plural }}/finalizers,verbs=update
// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
Expand Down
40 changes: 20 additions & 20 deletions pkg/plugins/internal/machinery/scaffold_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,25 +206,25 @@ var _ = Describe("Scaffold", func() {
},
Entry("should insert lines for go files",
`
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`,
`
1
2
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`,
fakeInserter{codeFragments: file.CodeFragmentsMap{
file.NewMarkerFor("file.go", "-"): {"1\n", "2\n"}},
},
),
Entry("should insert lines for yaml files",
`
# +kubebuilder:scaffold:-
#+kubebuilder:scaffold:-
`,
`
1
2
# +kubebuilder:scaffold:-
#+kubebuilder:scaffold:-
`,
fakeInserter{codeFragments: file.CodeFragmentsMap{
file.NewMarkerFor("file.yaml", "-"): {"1\n", "2\n"}},
Expand All @@ -235,10 +235,10 @@ var _ = Describe("Scaffold", func() {
`
1
2
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`,
fakeTemplate{fakeBuilder: fakeBuilder{ifExistsAction: file.Overwrite}, body: `
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`},
fakeInserter{codeFragments: file.CodeFragmentsMap{
file.NewMarkerFor("file.go", "-"): {"1\n", "2\n"}},
Expand All @@ -249,23 +249,23 @@ var _ = Describe("Scaffold", func() {
`
1
2
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`,
fakeTemplate{fakeBuilder: fakeBuilder{ifExistsAction: file.Overwrite}, body: `
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`},
fakeInserter{codeFragments: file.CodeFragmentsMap{
file.NewMarkerFor("file.go", "-"): {"1\n", "2\n"}},
},
),
Entry("should use files over optional models",
`
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`,
`
1
2
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`,
fakeTemplate{body: fileContent},
fakeInserter{
Expand All @@ -276,14 +276,14 @@ var _ = Describe("Scaffold", func() {
),
Entry("should filter invalid markers",
`
// +kubebuilder:scaffold:-
// +kubebuilder:scaffold:*
//+kubebuilder:scaffold:-
//+kubebuilder:scaffold:*
`,
`
1
2
// +kubebuilder:scaffold:-
// +kubebuilder:scaffold:*
//+kubebuilder:scaffold:-
//+kubebuilder:scaffold:*
`,
fakeInserter{
markers: []file.Marker{file.NewMarkerFor("file.go", "-")},
Expand All @@ -296,18 +296,18 @@ var _ = Describe("Scaffold", func() {
Entry("should filter already existing one-line code fragments",
`
1
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
3
4
// +kubebuilder:scaffold:*
//+kubebuilder:scaffold:*
`,
`
1
2
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
3
4
// +kubebuilder:scaffold:*
//+kubebuilder:scaffold:*
`,
fakeInserter{
codeFragments: file.CodeFragmentsMap{
Expand All @@ -319,10 +319,10 @@ var _ = Describe("Scaffold", func() {
Entry("should not insert anything if no code fragment",
"", // input is provided through a template as mock fs doesn't copy it to the output buffer if no-op
`
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`,
fakeTemplate{body: `
// +kubebuilder:scaffold:-
//+kubebuilder:scaffold:-
`},
fakeInserter{
codeFragments: file.CodeFragmentsMap{
Expand Down
10 changes: 5 additions & 5 deletions testdata/project-v3-addon/api/v1/admiral_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ type AdmiralStatus struct {
// Important: Run "make" to regenerate code after modifying this file
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:resource:scope=Cluster
//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
//+kubebuilder:resource:scope=Cluster

// Admiral is the Schema for the admirals API
type Admiral struct {
Expand Down Expand Up @@ -76,8 +76,8 @@ func (o *Admiral) SetCommonStatus(s addonv1alpha1.CommonStatus) {
o.Status.CommonStatus = s
}

// +kubebuilder:object:root=true
// +kubebuilder:resource:scope=Cluster
//+kubebuilder:object:root=true
//+kubebuilder:resource:scope=Cluster

// AdmiralList contains a list of Admiral
type AdmiralList struct {
Expand Down
6 changes: 3 additions & 3 deletions testdata/project-v3-addon/api/v1/captain_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ type CaptainStatus struct {
// Important: Run "make" to regenerate code after modifying this file
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
//+kubebuilder:object:root=true
//+kubebuilder:subresource:status

// Captain is the Schema for the captains API
type Captain struct {
Expand Down Expand Up @@ -75,7 +75,7 @@ func (o *Captain) SetCommonStatus(s addonv1alpha1.CommonStatus) {
o.Status.CommonStatus = s
}

// +kubebuilder:object:root=true
//+kubebuilder:object:root=true

// CaptainList contains a list of Captain
type CaptainList struct {
Expand Down
Loading

0 comments on commit 9f8fedd

Please sign in to comment.