Skip to content

Commit 520df43

Browse files
camilamacedo86camilamacedo86
authored
Merge pull request #2891 from ruanxin/bugfix_grafana_plugin_json_format
 ✨ Allow template to change parser delimiters
2 parents ed5d217 + e451cb3 commit 520df43

File tree

8 files changed

+94
-45
lines changed

8 files changed

+94
-45
lines changed

‎pkg/machinery/interfaces.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ type Template interface {
4444
GetBody() string
4545
// SetTemplateDefaults sets the default values for templates
4646
SetTemplateDefaults() error
47+
// SetDelim sets an action delimiters to replace default delimiters: {{ }}
48+
SetDelim(left, right string)
49+
// GetDelim returns the alternative delimiters
50+
GetDelim() (string, string)
4751
}
4852

4953
// Inserter is a file builder that inserts code fragments in marked positions

‎pkg/machinery/mixins.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,27 @@ type TemplateMixin struct {
4848
IfExistsActionMixin
4949

5050
// TemplateBody is the template body to execute
51-
TemplateBody string
51+
TemplateBody string
52+
parseDelimLeft string
53+
parseDelimRight string
5254
}
5355

5456
// GetBody implements Template
5557
func (t *TemplateMixin) GetBody() string {
5658
return t.TemplateBody
5759
}
5860

61+
// SetDelim implements Template
62+
func (t *TemplateMixin) SetDelim(left, right string) {
63+
t.parseDelimLeft = left
64+
t.parseDelimRight = right
65+
}
66+
67+
// GetDelim implements Template
68+
func (t *TemplateMixin) GetDelim() (string, string) {
69+
return t.parseDelimLeft, t.parseDelimRight
70+
}
71+
5972
// InserterMixin is the mixin that should be embedded in Inserter builders
6073
type InserterMixin struct {
6174
PathMixin

‎pkg/machinery/scaffold.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,10 @@ func (Scaffold) buildFileModel(t Template, models map[string]*File) error {
196196
func doTemplate(t Template) ([]byte, error) {
197197
// Create a new template.Template using the type of the Template as the name
198198
temp := template.New(fmt.Sprintf("%T", t))
199+
leftDelim, rightDelim := t.GetDelim()
200+
if leftDelim != "" && rightDelim != "" {
201+
temp.Delims(leftDelim, rightDelim)
202+
}
199203

200204
// Set the function map to be used
201205
fm := DefaultFuncMap()

‎pkg/machinery/scaffold_test.go

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -134,21 +134,32 @@ var _ = Describe("Scaffold", func() {
134134
},
135135
Entry("should write the file",
136136
path, content,
137-
fakeTemplate{fakeBuilder: fakeBuilder{path: path}, body: content},
137+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path}, body: content},
138138
),
139139
Entry("should skip optional models if already have one",
140140
path, content,
141-
fakeTemplate{fakeBuilder: fakeBuilder{path: path}, body: content},
142-
fakeTemplate{fakeBuilder: fakeBuilder{path: path}},
141+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path}, body: content},
142+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path}},
143143
),
144144
Entry("should overwrite required models if already have one",
145145
path, content,
146-
fakeTemplate{fakeBuilder: fakeBuilder{path: path}},
147-
fakeTemplate{fakeBuilder: fakeBuilder{path: path, ifExistsAction: OverwriteFile}, body: content},
146+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path}},
147+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path, ifExistsAction: OverwriteFile}, body: content},
148148
),
149149
Entry("should format a go file",
150150
pathGo, "package file\n",
151-
fakeTemplate{fakeBuilder: fakeBuilder{path: pathGo}, body: "package file"},
151+
&fakeTemplate{fakeBuilder: fakeBuilder{path: pathGo}, body: "package file"},
152+
),
153+
154+
Entry("should render actions correctly",
155+
path, "package testValue",
156+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path, TestField: "testValue"}, body: "package {{.TestField}}"},
157+
),
158+
159+
Entry("should render actions with alternative delimiters correctly",
160+
path, "package testValue",
161+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path, TestField: "testValue"},
162+
body: "package [[.TestField]]", parseDelimLeft: "[[", parseDelimRight: "]]"},
152163
),
153164
)
154165

@@ -164,17 +175,17 @@ var _ = Describe("Scaffold", func() {
164175
),
165176
Entry("should fail if unable to set default values for a template",
166177
&SetTemplateDefaultsError{},
167-
fakeTemplate{err: testErr},
178+
&fakeTemplate{err: testErr},
168179
),
169180
Entry("should fail if an unexpected previous model is found",
170181
&ModelAlreadyExistsError{},
171-
fakeTemplate{fakeBuilder: fakeBuilder{path: path}},
172-
fakeTemplate{fakeBuilder: fakeBuilder{path: path, ifExistsAction: Error}},
182+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path}},
183+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path, ifExistsAction: Error}},
173184
),
174185
Entry("should fail if behavior if-exists-action is not defined",
175186
&UnknownIfExistsActionError{},
176-
fakeTemplate{fakeBuilder: fakeBuilder{path: path}},
177-
fakeTemplate{fakeBuilder: fakeBuilder{path: path, ifExistsAction: -1}},
187+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path}},
188+
&fakeTemplate{fakeBuilder: fakeBuilder{path: path, ifExistsAction: -1}},
178189
),
179190
)
180191

@@ -187,15 +198,15 @@ var _ = Describe("Scaffold", func() {
187198
},
188199
Entry("should fail if a template is broken",
189200
"template: ",
190-
fakeTemplate{body: "{{ .Field }"},
201+
&fakeTemplate{body: "{{ .Field }"},
191202
),
192203
Entry("should fail if a template params aren't provided",
193204
"template: ",
194-
fakeTemplate{body: "{{ .Field }}"},
205+
&fakeTemplate{body: "{{ .Field }}"},
195206
),
196207
Entry("should fail if unable to format a go file",
197208
"expected 'package', found ",
198-
fakeTemplate{fakeBuilder: fakeBuilder{path: pathGo}, body: content},
209+
&fakeTemplate{fakeBuilder: fakeBuilder{path: pathGo}, body: content},
199210
),
200211
)
201212

@@ -254,7 +265,7 @@ var b int
254265
2
255266
#+kubebuilder:scaffold:-
256267
`,
257-
fakeTemplate{fakeBuilder: fakeBuilder{path: pathYaml, ifExistsAction: OverwriteFile}, body: `
268+
&fakeTemplate{fakeBuilder: fakeBuilder{path: pathYaml, ifExistsAction: OverwriteFile}, body: `
258269
#+kubebuilder:scaffold:-
259270
`},
260271
fakeInserter{
@@ -272,7 +283,7 @@ var b int
272283
2
273284
#+kubebuilder:scaffold:-
274285
`,
275-
fakeTemplate{fakeBuilder: fakeBuilder{path: pathYaml, ifExistsAction: OverwriteFile}, body: `
286+
&fakeTemplate{fakeBuilder: fakeBuilder{path: pathYaml, ifExistsAction: OverwriteFile}, body: `
276287
#+kubebuilder:scaffold:-
277288
`},
278289
fakeInserter{
@@ -292,7 +303,7 @@ var b int
292303
2
293304
#+kubebuilder:scaffold:-
294305
`,
295-
fakeTemplate{fakeBuilder: fakeBuilder{path: pathYaml}, body: content},
306+
&fakeTemplate{fakeBuilder: fakeBuilder{path: pathYaml}, body: content},
296307
fakeInserter{
297308
fakeBuilder: fakeBuilder{path: pathYaml},
298309
codeFragments: CodeFragmentsMap{
@@ -402,12 +413,12 @@ func init() {
402413
},
403414
Entry("should fail if inserting into a model that fails when a file exists and it does exist",
404415
&FileAlreadyExistsError{},
405-
fakeTemplate{fakeBuilder: fakeBuilder{path: "filename", ifExistsAction: Error}},
416+
&fakeTemplate{fakeBuilder: fakeBuilder{path: "filename", ifExistsAction: Error}},
406417
fakeInserter{fakeBuilder: fakeBuilder{path: "filename"}},
407418
),
408419
Entry("should fail if inserting into a model with unknown behavior if the file exists and it does exist",
409420
&UnknownIfExistsActionError{},
410-
fakeTemplate{fakeBuilder: fakeBuilder{path: "filename", ifExistsAction: -1}},
421+
&fakeTemplate{fakeBuilder: fakeBuilder{path: "filename", ifExistsAction: -1}},
411422
fakeInserter{fakeBuilder: fakeBuilder{path: "filename"}},
412423
),
413424
)
@@ -418,7 +429,7 @@ func init() {
418429
})
419430

420431
It("should skip the file by default", func() {
421-
Expect(s.Execute(fakeTemplate{
432+
Expect(s.Execute(&fakeTemplate{
422433
fakeBuilder: fakeBuilder{path: path},
423434
body: content,
424435
})).To(Succeed())
@@ -429,7 +440,7 @@ func init() {
429440
})
430441

431442
It("should write the file if configured to do so", func() {
432-
Expect(s.Execute(fakeTemplate{
443+
Expect(s.Execute(&fakeTemplate{
433444
fakeBuilder: fakeBuilder{path: path, ifExistsAction: OverwriteFile},
434445
body: content,
435446
})).To(Succeed())
@@ -440,7 +451,7 @@ func init() {
440451
})
441452

442453
It("should error if configured to do so", func() {
443-
err := s.Execute(fakeTemplate{
454+
err := s.Execute(&fakeTemplate{
444455
fakeBuilder: fakeBuilder{path: path, ifExistsAction: Error},
445456
body: content,
446457
})
@@ -457,6 +468,7 @@ var _ Builder = fakeBuilder{}
457468
type fakeBuilder struct {
458469
path string
459470
ifExistsAction IfExistsAction
471+
TestField string // test go template actions
460472
}
461473

462474
// GetPath implements Builder
@@ -483,23 +495,34 @@ func (f fakeRequiresValidation) Validate() error {
483495
return f.validateErr
484496
}
485497

486-
var _ Template = fakeTemplate{}
498+
var _ Template = &fakeTemplate{}
487499

488500
// fakeTemplate is used to mock a File in order to test Scaffold
489501
type fakeTemplate struct {
490502
fakeBuilder
491503

492-
body string
493-
err error
504+
body string
505+
err error
506+
parseDelimLeft string
507+
parseDelimRight string
508+
}
509+
510+
func (f *fakeTemplate) SetDelim(left, right string) {
511+
f.parseDelimLeft = left
512+
f.parseDelimRight = right
513+
}
514+
515+
func (f *fakeTemplate) GetDelim() (string, string) {
516+
return f.parseDelimLeft, f.parseDelimRight
494517
}
495518

496519
// GetBody implements Template
497-
func (f fakeTemplate) GetBody() string {
520+
func (f *fakeTemplate) GetBody() string {
498521
return f.body
499522
}
500523

501524
// SetTemplateDefaults implements Template
502-
func (f fakeTemplate) SetTemplateDefaults() error {
525+
func (f *fakeTemplate) SetTemplateDefaults() error {
503526
if f.err != nil {
504527
return f.err
505528
}

‎pkg/plugins/optional/grafana/v1alpha/scaffolds/internal/templates/resources.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ func (f *ResourcesManifest) SetTemplateDefaults() error {
3535
f.Path = filepath.Join("grafana", "controller-resources-metrics.json")
3636
}
3737

38+
// Grafana syntax use {{ }} quite often, which is collided with default delimiter for go template parsing.
39+
// Provide an alternative delimiter here to avoid overlaps.
40+
f.SetDelim("[[", "]]")
3841
f.TemplateBody = controllerResourcesTemplate
3942

4043
f.IfExistsAction = machinery.OverwriteFile
@@ -169,7 +172,7 @@ const controllerResourcesTemplate = `{
169172
"format": "time_series",
170173
"interval": "",
171174
"intervalFactor": 2,
172-
"legendFormat": "Pod: {{"{{pod}}"}} | Container: {{"{{container}}"}}",
175+
"legendFormat": "Pod: {{pod}} | Container: {{container}}",
173176
"refId": "A",
174177
"step": 10
175178
}
@@ -259,7 +262,7 @@ const controllerResourcesTemplate = `{
259262
"format": "time_series",
260263
"interval": "",
261264
"intervalFactor": 2,
262-
"legendFormat": "Pod: {{"{{pod}}"}} | Container: {{"{{container}}"}}",
265+
"legendFormat": "Pod: {{pod}} | Container: {{container}}",
263266
"refId": "A",
264267
"step": 10
265268
}

‎pkg/plugins/optional/grafana/v1alpha/scaffolds/internal/templates/runtime.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,10 @@ func (f *RuntimeManifest) SetTemplateDefaults() error {
3535
f.Path = filepath.Join("grafana", "controller-runtime-metrics.json")
3636
}
3737

38+
// Grafana syntax use {{ }} quite often, which is collided with default delimiter for go template parsing.
39+
// Provide an alternative delimiter here to avoid overlaps.
40+
f.SetDelim("[[", "]]")
3841
f.TemplateBody = controllerRuntimeTemplate
39-
4042
f.IfExistsAction = machinery.OverwriteFile
4143

4244
return nil
@@ -182,7 +184,7 @@ const controllerRuntimeTemplate = `{
182184
"exemplar": true,
183185
"expr": "sum(rate(controller_runtime_reconcile_total{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, pod)",
184186
"interval": "",
185-
"legendFormat": "{{"{{instance}}"}} {{"{{pod}}"}}",
187+
"legendFormat": "{{instance}} {{pod}}",
186188
"range": true,
187189
"refId": "A"
188190
}
@@ -269,7 +271,7 @@ const controllerRuntimeTemplate = `{
269271
"exemplar": true,
270272
"expr": "sum(rate(controller_runtime_reconcile_errors_total{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, pod)",
271273
"interval": "",
272-
"legendFormat": "{{"{{instance}}"}} {{"{{pod}}"}}",
274+
"legendFormat": "{{instance}} {{pod}}",
273275
"range": true,
274276
"refId": "A"
275277
}
@@ -371,7 +373,7 @@ const controllerRuntimeTemplate = `{
371373
"exemplar": true,
372374
"expr": "histogram_quantile(0.50, sum(rate(workqueue_queue_duration_seconds_bucket{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, name, le))",
373375
"interval": "",
374-
"legendFormat": "P50 {{"{{name}}"}} {{"{{instance}}"}} ",
376+
"legendFormat": "P50 {{name}} {{instance}} ",
375377
"refId": "A"
376378
},
377379
{
@@ -380,7 +382,7 @@ const controllerRuntimeTemplate = `{
380382
"expr": "histogram_quantile(0.90, sum(rate(workqueue_queue_duration_seconds_bucket{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, name, le))",
381383
"hide": false,
382384
"interval": "",
383-
"legendFormat": "P90 {{"{{name}}"}} {{"{{instance}}"}} ",
385+
"legendFormat": "P90 {{name}} {{instance}} ",
384386
"refId": "B"
385387
},
386388
{
@@ -389,7 +391,7 @@ const controllerRuntimeTemplate = `{
389391
"expr": "histogram_quantile(0.99, sum(rate(workqueue_queue_duration_seconds_bucket{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, name, le))",
390392
"hide": false,
391393
"interval": "",
392-
"legendFormat": "P99 {{"{{name}}"}} {{"{{instance}}"}} ",
394+
"legendFormat": "P99 {{name}} {{instance}} ",
393395
"refId": "C"
394396
}
395397
],
@@ -474,7 +476,7 @@ const controllerRuntimeTemplate = `{
474476
"exemplar": true,
475477
"expr": "sum(rate(workqueue_adds_total{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, name)",
476478
"interval": "",
477-
"legendFormat": "{{"{{name}}"}} {{"{{instance}}"}}",
479+
"legendFormat": "{{name}} {{instance}}",
478480
"refId": "A"
479481
}
480482
],
@@ -562,7 +564,7 @@ const controllerRuntimeTemplate = `{
562564
"exemplar": true,
563565
"expr": "histogram_quantile(0.50, sum(rate(workqueue_work_duration_seconds_bucket{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, name, le))",
564566
"interval": "",
565-
"legendFormat": "P50 {{"{{name}}"}} {{"{{instance}}"}} ",
567+
"legendFormat": "P50 {{name}} {{instance}} ",
566568
"refId": "A"
567569
},
568570
{
@@ -571,7 +573,7 @@ const controllerRuntimeTemplate = `{
571573
"expr": "histogram_quantile(0.90, sum(rate(workqueue_work_duration_seconds_bucket{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, name, le))",
572574
"hide": false,
573575
"interval": "",
574-
"legendFormat": "P90 {{"{{name}}"}} {{"{{instance}}"}} ",
576+
"legendFormat": "P90 {{name}} {{instance}} ",
575577
"refId": "B"
576578
},
577579
{
@@ -580,7 +582,7 @@ const controllerRuntimeTemplate = `{
580582
"expr": "histogram_quantile(0.99, sum(rate(workqueue_work_duration_seconds_bucket{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, name, le))",
581583
"hide": false,
582584
"interval": "",
583-
"legendFormat": "P99 {{"{{name}}"}} {{"{{instance}}"}} ",
585+
"legendFormat": "P99 {{name}} {{instance}} ",
584586
"refId": "C"
585587
}
586588
],
@@ -665,7 +667,7 @@ const controllerRuntimeTemplate = `{
665667
"exemplar": true,
666668
"expr": "sum(rate(workqueue_retries_total{job=\"$job\", namespace=\"$namespace\"}[5m])) by (instance, name)",
667669
"interval": "",
668-
"legendFormat": "{{"{{name}}"}} {{"{{instance}}"}} ",
670+
"legendFormat": "{{name}} {{instance}} ",
669671
"refId": "A"
670672
}
671673
],

‎testdata/project-v3-addon-and-grafana/grafana/controller-resources-metrics.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@
124124
"format": "time_series",
125125
"interval": "",
126126
"intervalFactor": 2,
127-
"legendFormat": "Pod: {{pod}} | Container: {{container}}",
127+
"legendFormat": "Pod: {{pod}} | Container: {{container}}",
128128
"refId": "A",
129129
"step": 10
130130
}
@@ -214,7 +214,7 @@
214214
"format": "time_series",
215215
"interval": "",
216216
"intervalFactor": 2,
217-
"legendFormat": "Pod: {{pod}} | Container: {{container}}",
217+
"legendFormat": "Pod: {{pod}} | Container: {{container}}",
218218
"refId": "A",
219219
"step": 10
220220
}

0 commit comments

Comments
 (0)