plugin/template: Add parseInt template function #5609
Conversation
Signed-off-by: Erik Johansson <ejohansson@spotify.com>
Codecov Report
@@ Coverage Diff @@
## master #5609 +/- ##
==========================================
+ Coverage 55.70% 56.87% +1.17%
==========================================
Files 224 243 +19
Lines 10016 15609 +5593
==========================================
+ Hits 5579 8878 +3299
- Misses 3978 6188 +2210
- Partials 459 543 +84
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Signed-off-by: Erik Johansson <ejohansson@spotify.com>
erikjoh
requested review from
bradbeam,
dilyevsky,
jameshartig,
greenpau,
Isolus,
johnbelamaric,
miekg,
pmoroney,
rajansandeep,
stp-ip,
SuperQ,
yongtang and
Tantalor93
as code owners
|
@chrisohaver Added test/template_test.go since I didn't find any existing file which seemed appropriate. |
|
I'm sorry I used confusing terminology - perhaps I should have said "functional test". IOW I meant a unit test that covers the functionality of the new operation added. e.g. the tests in
|
Signed-off-by: Erik Johansson <ejohansson@spotify.com>
Signed-off-by: Erik Johansson <ejohansson@spotify.com>
|
Sorry for that misunderstanding on my part as well and sorry to everyone who got pinged due to CODEOWNERS! 😓 |
|
OK - I don't see that as mocking, but I see what you mean - the template objects are being created manually in the test. So, either you'd need to replicate the function addition in the test, or better, break out the template creation into a function to be called from setup and the test e.g. for _, answer := range args {
tmpl, err := gotmpl.New("answer").Funcs(funcMap).Parse(answer)
if err != nil {
return handler, c.Errf("could not compile template: %s, %v", c.Val(), err)
}
t.answer = append(t.answer, tmpl)
}becomes for _, answer := range args {
err := t.addTemplate("answer", answer)
if err != nil {
return handler, c.Errf("could not compile template: %s, %v", c.Val(), err)
}
} |
|
Based on your suggestion, wouldn't func (t template) addTemplate(field, text string) (error) {
funcMap := gotmpl.FuncMap{
"parseInt": strconv.ParseUint,
}
tmpl, err := gotmpl.New(field).Funcs(funcMap).Parse(text)
if err != nil {
return err
}
switch field {
case "answer":
t.answer = append(t.answer, tmpl)
case "additional":
t.additional = append(t.additional, tmpl)
case "authority":
t.authority = append(t.authority, tmpl)
default:
return errors.New("not a valid field")
}
}Is that what you had in mind or would it be enough to just have a function to get a new template? func newTemplate(name, text string) (*gotmpl.Template, error) {
funcMap := gotmpl.FuncMap{
"parseInt": strconv.ParseUint,
}
return gotmpl.New(name).Funcs(funcMap).Parse(text)
}Considering how the objects are created in the test now I think the second option would be simpler. coredns/plugin/template/template_test.go Lines 20 to 27 in 4116a4e Also would you like me to remove the test from the last commit? |
|
Sorry. Fat fingered close on my phone. |
|
Yes, the 2nd is what I had in mind, to fit easily into the existing tests. It’s not how it came out when I gave the code example though. Thanks for catching that. |
Yes. I don’t think they are necessary. Sorry again about misleading you to write them. |
Signed-off-by: Erik Johansson <ejohansson@spotify.com>
* plugin/template: Add parseInt template function Signed-off-by: Erik Johansson <ejohansson@spotify.com>
Signed-off-by: Erik Johansson ejohansson@spotify.com
1. Why is this pull request needed and what does it do?
This change adds a template function
parseIntthat can be used in templates to for example parse an IPv4 addressin hexadecimal encoding.
This also allows replicating the PowerDNS feature of getting auto generated A records with the hexadecimal format (for example
ip7b7b7b7b.example.). Today, the template plugin only seems to allow for a dashed decimal IPv4 format (for exampleip-123-123-123-123.example.).I've tried to find a way to solve this with native template functions but it seems like adding
strconv.ParseUintlike this would be the best way to go.2. Which issues (if any) are related?
No issue found.
3. Which documentation changes (if any) need to be made?
Template plugin README.
4. Does this introduce a backward incompatible change or deprecation?
No.