-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 custom templates to transform the value of custom annotations. | ||||||
|
||||||
| Function | Description | | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest adding two additional columns to better document the functions.
Suggested change
Additionally, see the following suggestions: | |
||||||
| -------- | ----------- | | ||||||
| `splitinput` | Splits the arguments at specified delimiter and returns an array of strings | | ||||||
| `triminput` | Trims the trailing and leading whitespaces from the arguments | | ||||||
|
||||||
For example, the following custom annotation custom.nginx.org/allowed-ips represents a comma separated list of IP addresses as a string. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
``` | ||||||
annotations: | ||||||
custom.nginx.org/allowed-ips: "192.168.1.3, 10.0.0.13" | ||||||
``` | ||||||
|
||||||
It is possible to use helper functions such as splitinput and triminput to transform the annotation into something meaningful for NGINX. The example below shows how to use these functions in a custom template for the given annotation: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
``` | ||||||
{{range $ip := splitinput (index $.Ingress.Annotations "custom.nginx.org/allowed-ips") ","}} | ||||||
allow {{triminput $ip}}; | ||||||
{{end}} | ||||||
deny all; | ||||||
``` | ||||||
|
||||||
The template will generate the following configuration: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
``` | ||||||
allow 192.168.1.3; | ||||||
allow 10.0.0.13; | ||||||
deny all; | ||||||
``` | ||||||
|
||||||
## Example | ||||||
|
||||||
See the [custom annotations example](../examples/custom-annotations). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package version1 | ||
|
||
import ( | ||
"strings" | ||
"text/template" | ||
) | ||
|
||
// SplitInput splits the input from "," and returns an array of strings | ||
func SplitInput(s string, delim string) []string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no need for these functions to be public - they are not used in other packages There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For simplicity, I suggest renaming functions to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have map the functions to split and trim. Is it ok |
||
return strings.Split(s, delim) | ||
} | ||
|
||
// TrimInput trims the leading and trailing spaces in the string | ||
func TrimInput(s string) string { | ||
return strings.TrimSpace(s) | ||
} | ||
|
||
// HelperFunctions to parse the annotations | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest removing this comment |
||
var helperFunctions = template.FuncMap{ | ||
"splitinput": SplitInput, //returns array of strings | ||
"triminput": TrimInput, //returns string with trimmed leading and trailing spaces | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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) | ||||||
} | ||||||
|
@@ -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) | ||||||
} | ||||||
|
@@ -169,3 +169,47 @@ func TestMainForNGINX(t *testing.T) { | |||||
t.Fatalf("Failed to write template %v", err) | ||||||
} | ||||||
} | ||||||
|
||||||
func TestSplitHelperFunctions(t *testing.T) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should use plural here and in the |
||||||
const tpl = `{{range $n := splitinput (index .) ","}}{{$n}} {{end}}` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
tmpl, err := template.New("testTemplate").Funcs(helperFunctions).Parse(tpl) | ||||||
if err != nil { | ||||||
t.Fatalf("Failed to parse template: %v", err) | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||
|
||||||
slice := "foo,bar" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||||||
expected := "foo bar " | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, slice) | ||||||
t.Log(buf.String()) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need to print the buffer? Same applies for TestTrimHelperFunctions There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider removing it here. |
||||||
if err != nil { | ||||||
t.Fatalf("Failed to write template %v", err) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. failed to execute the template? |
||||||
} | ||||||
|
||||||
if buf.String() != expected { | ||||||
t.Fatalf("Failed parsing the template, got %v but expected %v.", buf.String(), expected) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think here the template generated wrong config. The error message is misleading. Same applies for |
||||||
} | ||||||
} | ||||||
|
||||||
func TestTrimHelperFunctions(t *testing.T) { | ||||||
const tpl = `{{triminput (index .)}}` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
tmpl, err := template.New("testTemplate").Funcs(helperFunctions).Parse(tpl) | ||||||
if err != nil { | ||||||
t.Fatalf("Failed to parse template: %v", err) | ||||||
} | ||||||
var buf bytes.Buffer | ||||||
|
||||||
slice := " foobar " | ||||||
expected := "foobar" | ||||||
err = tmpl.Execute(&buf, slice) | ||||||
t.Log(buf.String()) | ||||||
if err != nil { | ||||||
t.Fatalf("Failed to write template %v", err) | ||||||
} | ||||||
|
||||||
if buf.String() != expected { | ||||||
t.Fatalf("Failed parsing the template, got %v but expected %v.", buf.String(), expected) | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.