Skip to content

Commit

Permalink
Remove context.Context from request.Request (coredns#2726)
Browse files Browse the repository at this point in the history 
* Remove context.Context from request.Request

This removes the context from request.Request and makes all the changes
in the code to make it compile again. It's all mechanical. It did
unearth some weirdness in that the context was kept in handler structs
which may cause havoc with concurrently handling of requests.

Fixes coredns#2721

Signed-off-by: Miek Gieben <miek@miek.nl>

* Make test compile

Signed-off-by: Miek Gieben <miek@miek.nl>
  • Loading branch information
@miekg
miekg authored Mar 26, 2019
1 parent 6492f77 commit 53f3f0b
Show file tree
Hide file tree
Showing 20 changed files with 117 additions and 118 deletions.
4 changes: 2 additions & 2 deletions plugin/auto/auto.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type (

// ServeDNS implements the plugin.Handler interface.
func (a Auto) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
state := request.Request{W: w, Req: r, Context: ctx}
state := request.Request{W: w, Req: r}
qname := state.Name()

// Precheck with the origins, i.e. are we allowed to look here?
Expand All @@ -66,7 +66,7 @@ func (a Auto) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
return xfr.ServeDNS(ctx, w, r)
}

answer, ns, extra, result := z.Lookup(state, qname)
answer, ns, extra, result := z.Lookup(ctx, state, qname)

m := new(dns.Msg)
m.SetReply(r)
Expand Down
8 changes: 4 additions & 4 deletions plugin/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@ import (
type ServiceBackend interface {
// Services communicates with the backend to retrieve the service definitions. Exact indicates
// on exact match should be returned.
Services(state request.Request, exact bool, opt Options) ([]msg.Service, error)
Services(ctx context.Context, state request.Request, exact bool, opt Options) ([]msg.Service, error)

// Reverse communicates with the backend to retrieve service definition based on a IP address
// instead of a name. I.e. a reverse DNS lookup.
Reverse(state request.Request, exact bool, opt Options) ([]msg.Service, error)
Reverse(ctx context.Context, state request.Request, exact bool, opt Options) ([]msg.Service, error)

// Lookup is used to find records else where.
Lookup(state request.Request, name string, typ uint16) (*dns.Msg, error)
Lookup(ctx context.Context, state request.Request, name string, typ uint16) (*dns.Msg, error)

// Returns _all_ services that matches a certain name.
// Note: it does not implement a specific service.
Records(state request.Request, exact bool) ([]msg.Service, error)
Records(ctx context.Context, state request.Request, exact bool) ([]msg.Service, error)

// IsNameError return true if err indicated a record not found condition
IsNameError(err error) bool
Expand Down
67 changes: 34 additions & 33 deletions plugin/backend_lookup.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package plugin

import (
"context"
"fmt"
"math"
"net"
Expand All @@ -13,8 +14,8 @@ import (
)

// A returns A records from Backend or an error.
func A(b ServiceBackend, zone string, state request.Request, previousRecords []dns.RR, opt Options) (records []dns.RR, err error) {
services, err := checkForApex(b, zone, state, opt)
func A(ctx context.Context, b ServiceBackend, zone string, state request.Request, previousRecords []dns.RR, opt Options) (records []dns.RR, err error) {
services, err := checkForApex(ctx, b, zone, state, opt)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -43,7 +44,7 @@ func A(b ServiceBackend, zone string, state request.Request, previousRecords []d
if dns.IsSubDomain(zone, dns.Fqdn(serv.Host)) {
state1 := state.NewWithQuestion(serv.Host, state.QType())
state1.Zone = zone
nextRecords, err := A(b, zone, state1, append(previousRecords, newRecord), opt)
nextRecords, err := A(ctx, b, zone, state1, append(previousRecords, newRecord), opt)

if err == nil {
// Not only have we found something we should add the CNAME and the IP addresses.
Expand All @@ -57,7 +58,7 @@ func A(b ServiceBackend, zone string, state request.Request, previousRecords []d
// This means we can not complete the CNAME, try to look else where.
target := newRecord.Target
// Lookup
m1, e1 := b.Lookup(state, target, state.QType())
m1, e1 := b.Lookup(ctx, state, target, state.QType())
if e1 != nil {
continue
}
Expand All @@ -80,8 +81,8 @@ func A(b ServiceBackend, zone string, state request.Request, previousRecords []d
}

// AAAA returns AAAA records from Backend or an error.
func AAAA(b ServiceBackend, zone string, state request.Request, previousRecords []dns.RR, opt Options) (records []dns.RR, err error) {
services, err := checkForApex(b, zone, state, opt)
func AAAA(ctx context.Context, b ServiceBackend, zone string, state request.Request, previousRecords []dns.RR, opt Options) (records []dns.RR, err error) {
services, err := checkForApex(ctx, b, zone, state, opt)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -111,7 +112,7 @@ func AAAA(b ServiceBackend, zone string, state request.Request, previousRecords
if dns.IsSubDomain(zone, dns.Fqdn(serv.Host)) {
state1 := state.NewWithQuestion(serv.Host, state.QType())
state1.Zone = zone
nextRecords, err := AAAA(b, zone, state1, append(previousRecords, newRecord), opt)
nextRecords, err := AAAA(ctx, b, zone, state1, append(previousRecords, newRecord), opt)

if err == nil {
// Not only have we found something we should add the CNAME and the IP addresses.
Expand All @@ -124,7 +125,7 @@ func AAAA(b ServiceBackend, zone string, state request.Request, previousRecords
}
// This means we can not complete the CNAME, try to look else where.
target := newRecord.Target
m1, e1 := b.Lookup(state, target, state.QType())
m1, e1 := b.Lookup(ctx, state, target, state.QType())
if e1 != nil {
continue
}
Expand All @@ -149,8 +150,8 @@ func AAAA(b ServiceBackend, zone string, state request.Request, previousRecords

// SRV returns SRV records from the Backend.
// If the Target is not a name but an IP address, a name is created on the fly.
func SRV(b ServiceBackend, zone string, state request.Request, opt Options) (records, extra []dns.RR, err error) {
services, err := b.Services(state, false, opt)
func SRV(ctx context.Context, b ServiceBackend, zone string, state request.Request, opt Options) (records, extra []dns.RR, err error) {
services, err := b.Services(ctx, state, false, opt)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -199,12 +200,12 @@ func SRV(b ServiceBackend, zone string, state request.Request, opt Options) (rec
lookup[srv.Target] = struct{}{}

if !dns.IsSubDomain(zone, srv.Target) {
m1, e1 := b.Lookup(state, srv.Target, dns.TypeA)
m1, e1 := b.Lookup(ctx, state, srv.Target, dns.TypeA)
if e1 == nil {
extra = append(extra, m1.Answer...)
}

m1, e1 = b.Lookup(state, srv.Target, dns.TypeAAAA)
m1, e1 = b.Lookup(ctx, state, srv.Target, dns.TypeAAAA)
if e1 == nil {
// If we have seen CNAME's we *assume* that they are already added.
for _, a := range m1.Answer {
Expand All @@ -218,7 +219,7 @@ func SRV(b ServiceBackend, zone string, state request.Request, opt Options) (rec
// Internal name, we should have some info on them, either v4 or v6
// Clients expect a complete answer, because we are a recursor in their view.
state1 := state.NewWithQuestion(srv.Target, dns.TypeA)
addr, e1 := A(b, zone, state1, nil, opt)
addr, e1 := A(ctx, b, zone, state1, nil, opt)
if e1 == nil {
extra = append(extra, addr...)
}
Expand All @@ -242,8 +243,8 @@ func SRV(b ServiceBackend, zone string, state request.Request, opt Options) (rec
}

// MX returns MX records from the Backend. If the Target is not a name but an IP address, a name is created on the fly.
func MX(b ServiceBackend, zone string, state request.Request, opt Options) (records, extra []dns.RR, err error) {
services, err := b.Services(state, false, opt)
func MX(ctx context.Context, b ServiceBackend, zone string, state request.Request, opt Options) (records, extra []dns.RR, err error) {
services, err := b.Services(ctx, state, false, opt)
if err != nil {
return nil, nil, err
}
Expand All @@ -266,12 +267,12 @@ func MX(b ServiceBackend, zone string, state request.Request, opt Options) (reco
lookup[mx.Mx] = struct{}{}

if !dns.IsSubDomain(zone, mx.Mx) {
m1, e1 := b.Lookup(state, mx.Mx, dns.TypeA)
m1, e1 := b.Lookup(ctx, state, mx.Mx, dns.TypeA)
if e1 == nil {
extra = append(extra, m1.Answer...)
}

m1, e1 = b.Lookup(state, mx.Mx, dns.TypeAAAA)
m1, e1 = b.Lookup(ctx, state, mx.Mx, dns.TypeAAAA)
if e1 == nil {
// If we have seen CNAME's we *assume* that they are already added.
for _, a := range m1.Answer {
Expand All @@ -284,7 +285,7 @@ func MX(b ServiceBackend, zone string, state request.Request, opt Options) (reco
}
// Internal name
state1 := state.NewWithQuestion(mx.Mx, dns.TypeA)
addr, e1 := A(b, zone, state1, nil, opt)
addr, e1 := A(ctx, b, zone, state1, nil, opt)
if e1 == nil {
extra = append(extra, addr...)
}
Expand All @@ -308,8 +309,8 @@ func MX(b ServiceBackend, zone string, state request.Request, opt Options) (reco
}

// CNAME returns CNAME records from the backend or an error.
func CNAME(b ServiceBackend, zone string, state request.Request, opt Options) (records []dns.RR, err error) {
services, err := b.Services(state, true, opt)
func CNAME(ctx context.Context, b ServiceBackend, zone string, state request.Request, opt Options) (records []dns.RR, err error) {
services, err := b.Services(ctx, state, true, opt)
if err != nil {
return nil, err
}
Expand All @@ -324,8 +325,8 @@ func CNAME(b ServiceBackend, zone string, state request.Request, opt Options) (r
}

// TXT returns TXT records from Backend or an error.
func TXT(b ServiceBackend, zone string, state request.Request, opt Options) (records []dns.RR, err error) {
services, err := b.Services(state, false, opt)
func TXT(ctx context.Context, b ServiceBackend, zone string, state request.Request, opt Options) (records []dns.RR, err error) {
services, err := b.Services(ctx, state, false, opt)
if err != nil {
return nil, err
}
Expand All @@ -337,8 +338,8 @@ func TXT(b ServiceBackend, zone string, state request.Request, opt Options) (rec
}

// PTR returns the PTR records from the backend, only services that have a domain name as host are included.
func PTR(b ServiceBackend, zone string, state request.Request, opt Options) (records []dns.RR, err error) {
services, err := b.Reverse(state, true, opt)
func PTR(ctx context.Context, b ServiceBackend, zone string, state request.Request, opt Options) (records []dns.RR, err error) {
services, err := b.Reverse(ctx, state, true, opt)
if err != nil {
return nil, err
}
Expand All @@ -357,14 +358,14 @@ func PTR(b ServiceBackend, zone string, state request.Request, opt Options) (rec
}

// NS returns NS records from the backend
func NS(b ServiceBackend, zone string, state request.Request, opt Options) (records, extra []dns.RR, err error) {
func NS(ctx context.Context, b ServiceBackend, zone string, state request.Request, opt Options) (records, extra []dns.RR, err error) {
// NS record for this zone live in a special place, ns.dns.<zone>. Fake our lookup.
// only a tad bit fishy...
old := state.QName()

state.Clear()
state.Req.Question[0].Name = "ns.dns." + zone
services, err := b.Services(state, false, opt)
services, err := b.Services(ctx, state, false, opt)
if err != nil {
return nil, nil, err
}
Expand All @@ -387,7 +388,7 @@ func NS(b ServiceBackend, zone string, state request.Request, opt Options) (reco
}

// SOA returns a SOA record from the backend.
func SOA(b ServiceBackend, zone string, state request.Request, opt Options) ([]dns.RR, error) {
func SOA(ctx context.Context, b ServiceBackend, zone string, state request.Request, opt Options) ([]dns.RR, error) {
minTTL := b.MinTTL(state)
ttl := uint32(300)
if minTTL < ttl {
Expand Down Expand Up @@ -416,11 +417,11 @@ func SOA(b ServiceBackend, zone string, state request.Request, opt Options) ([]d
}

// BackendError writes an error response to the client.
func BackendError(b ServiceBackend, zone string, rcode int, state request.Request, err error, opt Options) (int, error) {
func BackendError(ctx context.Context, b ServiceBackend, zone string, rcode int, state request.Request, err error, opt Options) (int, error) {
m := new(dns.Msg)
m.SetRcode(state.Req, rcode)
m.Authoritative = true
m.Ns, _ = SOA(b, zone, state, opt)
m.Ns, _ = SOA(ctx, b, zone, state, opt)

state.W.WriteMsg(m)
// Return success as the rcode to signal we have written to the client.
Expand All @@ -439,9 +440,9 @@ func newAddress(s msg.Service, name string, ip net.IP, what uint16) dns.RR {
}

// checkForApex checks the special apex.dns directory for records that will be returned as A or AAAA.
func checkForApex(b ServiceBackend, zone string, state request.Request, opt Options) ([]msg.Service, error) {
func checkForApex(ctx context.Context, b ServiceBackend, zone string, state request.Request, opt Options) ([]msg.Service, error) {
if state.Name() != zone {
return b.Services(state, false, opt)
return b.Services(ctx, state, false, opt)
}

// If the zone name itself is queried we fake the query to search for a special entry
Expand All @@ -450,14 +451,14 @@ func checkForApex(b ServiceBackend, zone string, state request.Request, opt Opti
state.Clear()
state.Req.Question[0].Name = dnsutil.Join("apex.dns", zone)

services, err := b.Services(state, false, opt)
services, err := b.Services(ctx, state, false, opt)
if err == nil {
state.Req.Question[0].Name = old
return services, err
}

state.Req.Question[0].Name = old
return b.Services(state, false, opt)
return b.Services(ctx, state, false, opt)
}

// item holds records.
Expand Down
21 changes: 10 additions & 11 deletions plugin/etcd/etcd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,13 @@ type Etcd struct {
PathPrefix string
Upstream *upstream.Upstream
Client *etcdcv3.Client
Ctx context.Context

endpoints []string // Stored here as well, to aid in testing.
}

// Services implements the ServiceBackend interface.
func (e *Etcd) Services(state request.Request, exact bool, opt plugin.Options) (services []msg.Service, err error) {
services, err = e.Records(state, exact)
func (e *Etcd) Services(ctx context.Context, state request.Request, exact bool, opt plugin.Options) (services []msg.Service, err error) {
services, err = e.Records(ctx, state, exact)
if err != nil {
return
}
Expand All @@ -53,13 +52,13 @@ func (e *Etcd) Services(state request.Request, exact bool, opt plugin.Options) (
}

// Reverse implements the ServiceBackend interface.
func (e *Etcd) Reverse(state request.Request, exact bool, opt plugin.Options) (services []msg.Service, err error) {
return e.Services(state, exact, opt)
func (e *Etcd) Reverse(ctx context.Context, state request.Request, exact bool, opt plugin.Options) (services []msg.Service, err error) {
return e.Services(ctx, state, exact, opt)
}

// Lookup implements the ServiceBackend interface.
func (e *Etcd) Lookup(state request.Request, name string, typ uint16) (*dns.Msg, error) {
return e.Upstream.Lookup(state, name, typ)
func (e *Etcd) Lookup(ctx context.Context, state request.Request, name string, typ uint16) (*dns.Msg, error) {
return e.Upstream.Lookup(ctx, state, name, typ)
}

// IsNameError implements the ServiceBackend interface.
Expand All @@ -69,20 +68,20 @@ func (e *Etcd) IsNameError(err error) bool {

// Records looks up records in etcd. If exact is true, it will lookup just this
// name. This is used when find matches when completing SRV lookups for instance.
func (e *Etcd) Records(state request.Request, exact bool) ([]msg.Service, error) {
func (e *Etcd) Records(ctx context.Context, state request.Request, exact bool) ([]msg.Service, error) {
name := state.Name()

path, star := msg.PathWithWildcard(name, e.PathPrefix)
r, err := e.get(path, !exact)
r, err := e.get(ctx, path, !exact)
if err != nil {
return nil, err
}
segments := strings.Split(msg.Path(name, e.PathPrefix), "/")
return e.loopNodes(r.Kvs, segments, star, state.QType())
}

func (e *Etcd) get(path string, recursive bool) (*etcdcv3.GetResponse, error) {
ctx, cancel := context.WithTimeout(e.Ctx, etcdTimeout)
func (e *Etcd) get(ctx context.Context, path string, recursive bool) (*etcdcv3.GetResponse, error) {
ctx, cancel := context.WithTimeout(ctx, etcdTimeout)
defer cancel()
if recursive == true {
if !strings.HasSuffix(path, "/") {
Expand Down
Loading

0 comments on commit 53f3f0b

Please sign in to comment.