Merge commit from fork

Instead of casting lease ID to uint32, fix the TTL() function
to use etcd time-to-live API for determining TTL. Add configurable
min-lease-ttl and max-lease-ttl options to prevent extreme TTL
values. By default, lease records now go through bounds checking
with 30s to 1d as the min/max.

Added unit tests for validation and docs.

Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
This commit is contained in:
Ville Vesilehto
2025-09-05 03:14:27 +03:00
committed by GitHub
parent 066e51675c
commit e1768a5d27
6 changed files with 333 additions and 15 deletions

View File

@@ -1,5 +1,5 @@
.\" Generated by Mmark Markdown Processer - mmark.miek.nl .\" Generated by Mmark Markdown Processer - mmark.miek.nl
.TH "COREDNS-ETCD" 7 "March 2021" "CoreDNS" "CoreDNS Plugins" .TH "COREDNS-ETCD" 7 "August 2025" "CoreDNS" "CoreDNS Plugins"
.SH "NAME" .SH "NAME"
.PP .PP
@@ -85,6 +85,10 @@ file - if the server certificate is not signed by a system-installed CA and clie
is needed. is needed.
.RE .RE
.IP \(bu 4
\fB\fCmin-lease-ttl\fR the minimum TTL for DNS records based on etcd lease duration. Accepts flexible time formats like '30', '30s', '5m', '1h', '2h30m'. Default: 30 seconds.
.IP \(bu 4
\fB\fCmax-lease-ttl\fR the maximum TTL for DNS records based on etcd lease duration. Accepts flexible time formats like '30', '30s', '5m', '1h', '2h30m'. Default: 24 hours.
.SH "SPECIAL BEHAVIOUR" .SH "SPECIAL BEHAVIOUR"
@@ -93,7 +97,7 @@ The \fIetcd\fP plugin leverages directory structure to look for related entries.
an entry \fB\fC/skydns/test/skydns/mx\fR would have entries like \fB\fC/skydns/test/skydns/mx/a\fR, an entry \fB\fC/skydns/test/skydns/mx\fR would have entries like \fB\fC/skydns/test/skydns/mx/a\fR,
\fB\fC/skydns/test/skydns/mx/b\fR and so on. Similarly a directory \fB\fC/skydns/test/skydns/mx1\fR will have all \fB\fC/skydns/test/skydns/mx/b\fR and so on. Similarly a directory \fB\fC/skydns/test/skydns/mx1\fR will have all
\fB\fCmx1\fR entries. Note this plugin will search through the entire (sub)tree for records. In case of the \fB\fCmx1\fR entries. Note this plugin will search through the entire (sub)tree for records. In case of the
first example, a query for \fB\fCmx.skydns.text\fR will return both the contents of the \fB\fCa\fR and \fB\fCb\fR records. first example, a query for \fB\fCmx.skydns.test\fR will return both the contents of the \fB\fCa\fR and \fB\fCb\fR records.
If the directory extends deeper those records are returned as well. If the directory extends deeper those records are returned as well.
.PP .PP
@@ -120,6 +124,8 @@ skydns.local {
etcd { etcd {
path /skydns path /skydns
endpoint http://localhost:2379 endpoint http://localhost:2379
min\-lease\-ttl 60 # minimum 1 minute for lease\-based records
max\-lease\-ttl 1h # maximum 1 hour for lease\-based records
} }
prometheus prometheus
cache cache
@@ -349,6 +355,7 @@ If you would like to use \fB\fCTXT\fR records, you can set the following:
.nf .nf
% etcdctl put /skydns/local/skydns/x6 '{"ttl":60,"text":"this is a random text message."}' % etcdctl put /skydns/local/skydns/x6 '{"ttl":60,"text":"this is a random text message."}'
% etcdctl put /skydns/local/skydns/x7 '{"ttl":60,"text":"this is a another random text message."}'
.fi .fi
.RE .RE
@@ -362,6 +369,7 @@ If you query the zone name for \fB\fCTXT\fR now, you will get the following resp
.nf .nf
% dig +short skydns.local TXT @localhost % dig +short skydns.local TXT @localhost
"this is a random text message." "this is a random text message."
"this is a another random text message."
.fi .fi
.RE .RE

View File

@@ -55,6 +55,8 @@ etcd [ZONES...] {
* three arguments - path to cert PEM file, path to client private key PEM file, path to CA PEM * three arguments - path to cert PEM file, path to client private key PEM file, path to CA PEM
file - if the server certificate is not signed by a system-installed CA and client certificate file - if the server certificate is not signed by a system-installed CA and client certificate
is needed. is needed.
* `min-lease-ttl` the minimum TTL for DNS records based on etcd lease duration. Accepts flexible time formats like '30', '30s', '5m', '1h', '2h30m'. Default: 30 seconds.
* `max-lease-ttl` the maximum TTL for DNS records based on etcd lease duration. Accepts flexible time formats like '30', '30s', '5m', '1h', '2h30m'. Default: 24 hours.
## Special Behaviour ## Special Behaviour
@@ -83,6 +85,8 @@ skydns.local {
etcd { etcd {
path /skydns path /skydns
endpoint http://localhost:2379 endpoint http://localhost:2379
min-lease-ttl 60 # minimum 1 minute for lease-based records
max-lease-ttl 1h # maximum 1 hour for lease-based records
} }
prometheus prometheus
cache cache

View File

@@ -21,21 +21,25 @@ import (
) )
const ( const (
priority = 10 // default priority when nothing is set defaultPriority = 10 // default priority when nothing is set
ttl = 300 // default ttl when nothing is set defaultTTL = 300 // default ttl when nothing is set
etcdTimeout = 5 * time.Second defaultLeaseMinTTL = 30 // default minimum TTL for lease-based records
defaultLeaseMaxTTL = 86400 // default maximum TTL for lease-based records
etcdTimeout = 5 * time.Second
) )
var errKeyNotFound = errors.New("key not found") var errKeyNotFound = errors.New("key not found")
// Etcd is a plugin talks to an etcd cluster. // Etcd is a plugin talks to an etcd cluster.
type Etcd struct { type Etcd struct {
Next plugin.Handler Next plugin.Handler
Fall fall.F Fall fall.F
Zones []string Zones []string
PathPrefix string PathPrefix string
Upstream *upstream.Upstream Upstream *upstream.Upstream
Client *etcdcv3.Client Client *etcdcv3.Client
MinLeaseTTL uint32 // minimum TTL for lease-based records
MaxLeaseTTL uint32 // maximum TTL for lease-based records
endpoints []string // Stored here as well, to aid in testing. endpoints []string // Stored here as well, to aid in testing.
} }
@@ -146,7 +150,7 @@ Nodes:
serv.TTL = e.TTL(n, serv) serv.TTL = e.TTL(n, serv)
if serv.Priority == 0 { if serv.Priority == 0 {
serv.Priority = priority serv.Priority = defaultPriority
} }
if shouldInclude(serv, qType) { if shouldInclude(serv, qType) {
@@ -159,10 +163,39 @@ Nodes:
// TTL returns the smaller of the etcd TTL and the service's // TTL returns the smaller of the etcd TTL and the service's
// TTL. If neither of these are set (have a zero value), a default is used. // TTL. If neither of these are set (have a zero value), a default is used.
func (e *Etcd) TTL(kv *mvccpb.KeyValue, serv *msg.Service) uint32 { func (e *Etcd) TTL(kv *mvccpb.KeyValue, serv *msg.Service) uint32 {
etcdTTL := uint32(kv.Lease) var etcdTTL uint32
// Get actual lease TTL from etcd if lease exists and client is available
if kv.Lease != 0 && e.Client != nil {
if resp, err := e.Client.TimeToLive(context.Background(), etcdcv3.LeaseID(kv.Lease)); err == nil && resp.TTL > 0 {
leaseTTL := resp.TTL
// Get bounds with defaults
minTTL := e.MinLeaseTTL
if minTTL == 0 {
minTTL = defaultLeaseMinTTL
}
maxTTL := e.MaxLeaseTTL
if maxTTL == 0 {
maxTTL = defaultLeaseMaxTTL
}
// Clamp lease TTL to configured bounds
minTTL64 := int64(minTTL)
maxTTL64 := int64(maxTTL)
if leaseTTL < minTTL64 {
leaseTTL = minTTL64
} else if leaseTTL > maxTTL64 {
leaseTTL = maxTTL64
}
etcdTTL = uint32(leaseTTL)
}
}
if etcdTTL == 0 && serv.TTL == 0 { if etcdTTL == 0 && serv.TTL == 0 {
return ttl return defaultTTL
} }
if etcdTTL == 0 { if etcdTTL == 0 {
return serv.TTL return serv.TTL

View File

@@ -2,7 +2,11 @@ package etcd
import ( import (
"crypto/tls" "crypto/tls"
"errors"
"path/filepath" "path/filepath"
"strconv"
"strings"
"time"
"github.com/coredns/caddy" "github.com/coredns/caddy"
"github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/core/dnsserver"
@@ -33,7 +37,11 @@ func setup(c *caddy.Controller) error {
func etcdParse(c *caddy.Controller) (*Etcd, error) { func etcdParse(c *caddy.Controller) (*Etcd, error) {
config := dnsserver.GetConfig(c) config := dnsserver.GetConfig(c)
etc := Etcd{PathPrefix: "skydns"} etc := Etcd{
PathPrefix: "skydns",
MinLeaseTTL: defaultLeaseMinTTL,
MaxLeaseTTL: defaultLeaseMaxTTL,
}
var ( var (
tlsConfig *tls.Config tlsConfig *tls.Config
err error err error
@@ -88,6 +96,24 @@ func etcdParse(c *caddy.Controller) (*Etcd, error) {
return &Etcd{}, c.Errf("credentials requires 2 arguments, username and password") return &Etcd{}, c.Errf("credentials requires 2 arguments, username and password")
} }
username, password = args[0], args[1] username, password = args[0], args[1]
case "min-lease-ttl":
if !c.NextArg() {
return &Etcd{}, c.ArgErr()
}
minLeaseTTL, err := parseTTL(c.Val())
if err != nil {
return &Etcd{}, c.Errf("invalid min-lease-ttl value: %v", err)
}
etc.MinLeaseTTL = minLeaseTTL
case "max-lease-ttl":
if !c.NextArg() {
return &Etcd{}, c.ArgErr()
}
maxLeaseTTL, err := parseTTL(c.Val())
if err != nil {
return &Etcd{}, c.Errf("invalid max-lease-ttl value: %v", err)
}
etc.MaxLeaseTTL = maxLeaseTTL
default: default:
if c.Val() != "}" { if c.Val() != "}" {
return &Etcd{}, c.Errf("unknown property '%s'", c.Val()) return &Etcd{}, c.Errf("unknown property '%s'", c.Val())
@@ -124,3 +150,35 @@ func newEtcdClient(endpoints []string, cc *tls.Config, username, password string
} }
const defaultEndpoint = "http://localhost:2379" const defaultEndpoint = "http://localhost:2379"
// parseTTL parses a TTL value with flexible time units using Go's standard duration parsing.
// Supports formats like: "30", "30s", "5m", "1h", "90s", "2h30m", etc.
func parseTTL(s string) (uint32, error) {
s = strings.TrimSpace(s)
if s == "" {
return 0, nil
}
// Handle plain numbers (assume seconds)
if _, err := strconv.ParseUint(s, 10, 64); err == nil {
// If it's just a number, append "s" for seconds
s += "s"
}
// Use Go's standard time.ParseDuration for robust parsing
duration, err := time.ParseDuration(s)
if err != nil {
return 0, errors.New("invalid TTL format, use format like '30', '30s', '5m', '1h', or '2h30m'")
}
// Convert to seconds and check bounds
seconds := duration.Seconds()
if seconds < 0 {
return 0, errors.New("TTL must be non-negative")
}
if seconds > 4294967295 { // uint32 max value
return 0, errors.New("TTL too large, maximum is 4294967295 seconds")
}
return uint32(seconds), nil
}

View File

@@ -66,6 +66,31 @@ func TestSetupEtcd(t *testing.T) {
} }
`, true, "skydns", []string{"http://localhost:2379"}, "Wrong argument count", "", "", `, true, "skydns", []string{"http://localhost:2379"}, "Wrong argument count", "", "",
}, },
// with custom min-lease-ttl
{
`etcd {
endpoint http://localhost:2379
min-lease-ttl 60
}
`, false, "skydns", []string{"http://localhost:2379"}, "", "", "",
},
// with custom max-lease-ttl
{
`etcd {
endpoint http://localhost:2379
max-lease-ttl 1h
}
`, false, "skydns", []string{"http://localhost:2379"}, "", "", "",
},
// with both custom min-lease-ttl and max-lease-ttl
{
`etcd {
endpoint http://localhost:2379
min-lease-ttl 120
max-lease-ttl 7200
}
`, false, "skydns", []string{"http://localhost:2379"}, "", "", "",
},
} }
for i, test := range tests { for i, test := range tests {
@@ -113,6 +138,84 @@ func TestSetupEtcd(t *testing.T) {
t.Errorf("Etcd password not correctly set for input %s. Expected: '%+v', actual: '%+v'", test.input, test.password, etcd.Client.Password) t.Errorf("Etcd password not correctly set for input %s. Expected: '%+v', actual: '%+v'", test.input, test.password, etcd.Client.Password)
} }
} }
// Check TTL configuration for specific test cases
if strings.Contains(test.input, "min-lease-ttl 60") {
if etcd.MinLeaseTTL != 60 {
t.Errorf("MinLeaseTTL not set correctly for input %s. Expected: 60, actual: %d", test.input, etcd.MinLeaseTTL)
}
}
if strings.Contains(test.input, "max-lease-ttl 1h") {
if etcd.MaxLeaseTTL != 3600 {
t.Errorf("MaxLeaseTTL not set correctly for input %s. Expected: 3600, actual: %d", test.input, etcd.MaxLeaseTTL)
}
}
if strings.Contains(test.input, "min-lease-ttl 120") && strings.Contains(test.input, "max-lease-ttl 7200") {
if etcd.MinLeaseTTL != 120 {
t.Errorf("MinLeaseTTL not set correctly for input %s. Expected: 120, actual: %d", test.input, etcd.MinLeaseTTL)
}
if etcd.MaxLeaseTTL != 7200 {
t.Errorf("MaxLeaseTTL not set correctly for input %s. Expected: 7200, actual: %d", test.input, etcd.MaxLeaseTTL)
}
}
} }
} }
} }
func TestParseTTL(t *testing.T) {
tests := []struct {
input string
expected uint32
hasError bool
desc string
}{
// Plain numbers (assumed to be seconds)
{"30", 30, false, "plain number should be treated as seconds"},
{"300", 300, false, "plain number should be treated as seconds"},
// Explicit seconds
{"30s", 30, false, "explicit seconds"},
{"90s", 90, false, "explicit seconds"},
// Minutes
{"5m", 300, false, "5 minutes"},
{"1m", 60, false, "1 minute"},
// Hours
{"1h", 3600, false, "1 hour"},
{"2h", 7200, false, "2 hours"},
// Complex durations (Go's ParseDuration supports this)
{"2h30m", 9000, false, "2 hours 30 minutes"},
{"1h30m45s", 5445, false, "1 hour 30 minutes 45 seconds"},
// Edge cases
{"0", 0, false, "zero should be allowed"},
{"0s", 0, false, "zero seconds should be allowed"},
{"", 0, false, "empty string should return 0"},
// Error cases
{"-30s", 0, true, "negative duration should error"},
{"abc", 0, true, "invalid format should error"},
{"1y", 0, true, "unsupported unit should error"},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
result, err := parseTTL(tt.input)
if tt.hasError {
if err == nil {
t.Errorf("parseTTL(%q) expected error but got none", tt.input)
}
} else {
if err != nil {
t.Errorf("parseTTL(%q) unexpected error: %v", tt.input, err)
}
if result != tt.expected {
t.Errorf("parseTTL(%q) = %d, expected %d", tt.input, result, tt.expected)
}
}
})
}
}

112
plugin/etcd/ttl_test.go Normal file
View File

@@ -0,0 +1,112 @@
package etcd
import (
"testing"
"github.com/coredns/coredns/plugin/etcd/msg"
"go.etcd.io/etcd/api/v3/mvccpb"
)
func TestTTL(t *testing.T) {
tests := []struct {
name string
leaseID int64
serviceTTL uint32
minLeaseTTL uint32
maxLeaseTTL uint32
hasClient bool
expectedTTL uint32
}{
{
name: "no client, large lease ID falls back to default",
leaseID: 0x12345678FFFFFFFF, // Large lease ID that would cause issues
serviceTTL: 0,
minLeaseTTL: 0,
maxLeaseTTL: 0,
hasClient: false,
expectedTTL: defaultTTL,
},
{
name: "no client, zero lease ID falls back to default",
leaseID: 0,
serviceTTL: 0,
minLeaseTTL: 0,
maxLeaseTTL: 0,
hasClient: false,
expectedTTL: defaultTTL,
},
{
name: "no client, service TTL takes precedence",
leaseID: 120,
serviceTTL: 300,
minLeaseTTL: 0,
maxLeaseTTL: 0,
hasClient: false,
expectedTTL: 300,
},
{
name: "no client, smaller service TTL wins",
leaseID: 600,
serviceTTL: 120,
minLeaseTTL: 0,
maxLeaseTTL: 0,
hasClient: false,
expectedTTL: 120,
},
{
name: "custom bounds, no client",
leaseID: 0x12345678FFFFFFFF,
serviceTTL: 0,
minLeaseTTL: 60, // 1 minute
maxLeaseTTL: 3600, // 1 hour
hasClient: false,
expectedTTL: defaultTTL,
},
{
name: "zero service TTL with lease ID",
leaseID: 600,
serviceTTL: 0,
minLeaseTTL: 0,
maxLeaseTTL: 0,
hasClient: false,
expectedTTL: defaultTTL,
},
{
name: "both zero, falls back to default",
leaseID: 0,
serviceTTL: 0,
minLeaseTTL: 0,
maxLeaseTTL: 0,
hasClient: false,
expectedTTL: defaultTTL,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Create Etcd instance with test configuration
e := &Etcd{
MinLeaseTTL: tt.minLeaseTTL,
MaxLeaseTTL: tt.maxLeaseTTL,
}
// Create test data
kv := &mvccpb.KeyValue{
Key: []byte("/test/service"),
Value: []byte(`{"host": "test.example.com"}`),
Lease: tt.leaseID,
}
serv := &msg.Service{
Host: "test.example.com",
TTL: tt.serviceTTL,
}
resultingTTL := e.TTL(kv, serv)
if resultingTTL != tt.expectedTTL {
t.Errorf("TTL() = %d, expected %d", resultingTTL, tt.expectedTTL)
}
})
}
}