Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper Functions for custom annotations #691

Merged
merged 3 commits into from
Sep 17, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions docs/custom-annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,37 @@ If you'd like to use custom annotations with Mergeable Ingress resources, please

* Minions do not inherent custom annotations of the master.

### Helper Functions

Helper functions can be used in the Ingress template to parse the values of custom annotations.

| Function | Input Arguments| Return Arguments | Description |
| -------- | -------------- | ---------------- | ----------- |
| `split` | s, sep string | []string | Splits the string s into a slice of strings separated by the sep. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `split` | s, sep string | []string | Splits the string s into a slice of strings separated by the sep. |
| `split` | `s, sep string` | `[]string` | Splits the string `s` into a slice of strings separated by the `sep`. |

| `trim` | s string | string | Trims the trailing and leading whitespace from the string s. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `trim` | s string | string | Trims the trailing and leading whitespace from the string s. |
| `trim` | `s string` | `string` | Trims the trailing and leading whitespace from the string `s`. |


Consider the following custom annotation `custom.nginx.org/allowed-ips`, which expects a comma-separated list of IP addresses:
```
annotations:
custom.nginx.org/allowed-ips: "192.168.1.3, 10.0.0.13"
```

The helper functions can parse the value of the `custom.nginx.org/allowed-ips` annotation, so that in the template you can use each IP address separately. Consider the following template excerpt:

```
{{range $ip := split (index $.Ingress.Annotations "custom.nginx.org/allowed-ips") ","}}
allow {{trim $ip}};
{{end}}
deny all;
```

The template excerpt will generate the following configuration:
```
allow 192.168.1.3;
allow 10.0.0.13;
deny all;
```

## Example

See the [custom annotations example](../examples/custom-annotations).
4 changes: 2 additions & 2 deletions internal/configs/version1/template_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func NewTemplateExecutor(mainTemplatePath string, ingressTemplatePath string) (*
return nil, err
}

ingressTemplate, err := template.New(path.Base(ingressTemplatePath)).ParseFiles(ingressTemplatePath)
ingressTemplate, err := template.New(path.Base(ingressTemplatePath)).Funcs(helperFunctions).ParseFiles(ingressTemplatePath)
if err != nil {
return nil, err
}
Expand All @@ -45,7 +45,7 @@ func (te *TemplateExecutor) UpdateMainTemplate(templateString *string) error {

// UpdateIngressTemplate updates the ingress template.
func (te *TemplateExecutor) UpdateIngressTemplate(templateString *string) error {
newTemplate, err := template.New("ingressTemplate").Parse(*templateString)
newTemplate, err := template.New("ingressTemplate").Funcs(helperFunctions).Parse(*templateString)
if err != nil {
return err
}
Expand Down
21 changes: 21 additions & 0 deletions internal/configs/version1/template_helper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package version1

import (
"strings"
"text/template"
)

// splitinput splits the input from "," and returns an array of strings
func splitinput(s string, delim string) []string {
Copy link
Contributor

@pleshakov pleshakov Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For multiword names, the convention is to use mixed caps. Like "splitInput". See https://golang.org/doc/effective_go.html#mixed-caps
At the same time, as mentioned here #691 (comment), I suggest renaming to split. This will make the name simpler, as the suffix input seems unnecessary.
Same applies for triminput.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation for the function is not correct. I suggest removing it complety - the function is private and also strait-forward.
Same applies for triminput.

return strings.Split(s, delim)
}

// triminput trims the leading and trailing spaces in the string
func triminput(s string) string {
return strings.TrimSpace(s)
}

var helperFunctions = template.FuncMap{
"split": splitinput, //returns array of strings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest removing these comments completely. The comment for split s already outdated. Having the docs in the doc is enough.

"trim": triminput, //returns string with trimmed leading and trailing spaces
}
52 changes: 50 additions & 2 deletions internal/configs/version1/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ var mainCfg = MainConfig{
}

func TestIngressForNGINXPlus(t *testing.T) {
tmpl, err := template.New(nginxPlusIngressTmpl).ParseFiles(nginxPlusIngressTmpl)
tmpl, err := template.New(nginxPlusIngressTmpl).Funcs(helperFunctions).ParseFiles(nginxPlusIngressTmpl)
if err != nil {
t.Fatalf("Failed to parse template file: %v", err)
}
Expand All @@ -126,7 +126,7 @@ func TestIngressForNGINXPlus(t *testing.T) {
}

func TestIngressForNGINX(t *testing.T) {
tmpl, err := template.New(nginxIngressTmpl).ParseFiles(nginxIngressTmpl)
tmpl, err := template.New(nginxIngressTmpl).Funcs(helperFunctions).ParseFiles(nginxIngressTmpl)
if err != nil {
t.Fatalf("Failed to parse template file: %v", err)
}
Expand Down Expand Up @@ -169,3 +169,51 @@ func TestMainForNGINX(t *testing.T) {
t.Fatalf("Failed to write template %v", err)
}
}

func TestSplitHelperFunction(t *testing.T) {
const tpl = `{{range $n := split . ","}}{{$n}} {{end}}`

tmpl, err := template.New("testTemplate").Funcs(helperFunctions).Parse(tpl)
if err != nil {
t.Fatalf("Failed to parse template: %v", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider adding a blank line here to improve readability. Same applies for TestTrimHelperFunctions


var buf bytes.Buffer

input := "foo,bar"
expected := "foo bar "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider adding a blank line here to improve readability. Same applies for TestTrimHelperFunctions


err = tmpl.Execute(&buf, input)
t.Log(buf.String())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to print the buffer? Same applies for TestTrimHelperFunctions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other tests are following same pattern to print buf if it fails. So did same here. Let me know if it shoud be removed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider removing it here.
The other tests do not check the generated config against the expected output -- they only check that there is no error returned. That's mostly why t.Log(buf.String()), so that the output could be checked visually. In contrast, this test checks the output against the expected output, so no printing is necessary.

if err != nil {
t.Fatalf("Failed to execute the template %v", err)
}

if buf.String() != expected {
t.Fatalf("Template generated wrong config, got %v but expected %v.", buf.String(), expected)
}
}

func TestTrimHelperFunction(t *testing.T) {
const tpl = `{{trim .}}`

tmpl, err := template.New("testTemplate").Funcs(helperFunctions).Parse(tpl)
if err != nil {
t.Fatalf("Failed to parse template: %v", err)
}

var buf bytes.Buffer

input := " foobar "
expected := "foobar"

err = tmpl.Execute(&buf, input)
t.Log(buf.String())
if err != nil {
t.Fatalf("Failed to execute the template %v", err)
}

if buf.String() != expected {
t.Fatalf("Template generated wrong config, got %v but expected %v.", buf.String(), expected)
}
}