Plumbs the service name back and uses agent-specific TTL settings as a fallback.

command/agent/dns.go

@@ -587,13 +587,10 @@ RPC:
 		goto RPC
 	}
 
-	// TODO (slackpad) Do we want to apply the DNS server's per-service TTL
-	// configs if the query's TTL is not set? That seems like it adds a lot
-	// of complexity (we'd have to plumb the service name back with the query
-	// results to do this), but is it what people would expect?
-
 	// Determine the TTL. The parse should never fail since we vet it when
-	// the query is created, but we check anyway.
+	// the query is created, but we check anyway. If the query didn't
+	// specify a TTL then we will try to use the agent's service-specific
+	// TTL configs.
 	var ttl time.Duration
 	if out.DNS.TTL != "" {
 		var err error
@@ -601,6 +598,12 @@ RPC:
 		if err != nil {
 			d.logger.Printf("[WARN] dns: Failed to parse TTL '%s' for prepared query '%s', ignoring", out.DNS.TTL, query)
 		}
+	} else if d.config.ServiceTTL != nil {
+		var ok bool
+		ttl, ok = d.config.ServiceTTL[out.Service]
+		if !ok {
+			ttl = d.config.ServiceTTL["*"]
+		}
 	}
 
 	// If we have no nodes, return not found!

command/agent/dns_test.go

@@ -2087,7 +2087,6 @@ func TestDNS_ServiceLookup_TTL(t *testing.T) {
 		}
 		c.AllowStale = true
 		c.MaxStale = time.Second
-
 	}
 	dir, srv := makeDNSServerConfig(t, nil, confFn)
 	defer os.RemoveAll(dir)
@@ -2184,7 +2183,15 @@ func TestDNS_ServiceLookup_TTL(t *testing.T) {
 }
 
 func TestDNS_PreparedQuery_TTL(t *testing.T) {
-	dir, srv := makeDNSServer(t)
+	confFn := func(c *DNSConfig) {
+		c.ServiceTTL = map[string]time.Duration{
+			"db": 10 * time.Second,
+			"*":  5 * time.Second,
+		}
+		c.AllowStale = true
+		c.MaxStale = time.Second
+	}
+	dir, srv := makeDNSServerConfig(t, nil, confFn)
 	defer os.RemoveAll(dir)
 	defer srv.agent.Shutdown()
 
@@ -2207,20 +2214,34 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) {
 		if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
 			t.Fatalf("err: %v", err)
 		}
+
+		args = &structs.RegisterRequest{
+			Datacenter: "dc1",
+			Node:       "foo",
+			Address:    "127.0.0.1",
+			Service: &structs.NodeService{
+				Service: "api",
+				Port:    2222,
+			},
+		}
+		if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil {
+			t.Fatalf("err: %v", err)
+		}
 	}
 
-	// Register prepared queries with and without a TTL set.
+	// Register prepared queries with and without a TTL set for "db", as
+	// well as one for "api".
 	{
 		args := &structs.PreparedQueryRequest{
 			Datacenter: "dc1",
 			Op:         structs.PreparedQueryCreate,
 			Query: &structs.PreparedQuery{
-				Name: "ttl",
+				Name: "db-ttl",
 				Service: structs.ServiceQuery{
 					Service: "db",
 				},
 				DNS: structs.QueryDNSOptions{
-					TTL: "10s",
+					TTL: "18s",
 				},
 			},
 		}
@@ -2234,7 +2255,7 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) {
 			Datacenter: "dc1",
 			Op:         structs.PreparedQueryCreate,
 			Query: &structs.PreparedQuery{
-				Name: "nottl",
+				Name: "db-nottl",
 				Service: structs.ServiceQuery{
 					Service: "db",
 				},
@@ -2244,11 +2265,27 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) {
 		if err := srv.agent.RPC("PreparedQuery.Apply", args, &id); err != nil {
 			t.Fatalf("err: %v", err)
 		}
+
+		args = &structs.PreparedQueryRequest{
+			Datacenter: "dc1",
+			Op:         structs.PreparedQueryCreate,
+			Query: &structs.PreparedQuery{
+				Name: "api-nottl",
+				Service: structs.ServiceQuery{
+					Service: "api",
+				},
+			},
 		}
 
-	// Make sure the TTL is set when requested.
+		if err := srv.agent.RPC("PreparedQuery.Apply", args, &id); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	}
+
+	// Make sure the TTL is set when requested, and overrides the agent-
+	// specific config since the query takes precedence.
 	m := new(dns.Msg)
-	m.SetQuestion("ttl.query.consul.", dns.TypeSRV)
+	m.SetQuestion("db-ttl.query.consul.", dns.TypeSRV)
 
 	c := new(dns.Client)
 	addr, _ := srv.agent.config.ClientListener("", srv.agent.config.Ports.DNS)
@@ -2265,7 +2302,7 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) {
 	if !ok {
 		t.Fatalf("Bad: %#v", in.Answer[0])
 	}
-	if srvRec.Hdr.Ttl != 10 {
+	if srvRec.Hdr.Ttl != 18 {
 		t.Fatalf("Bad: %#v", in.Answer[0])
 	}
 
@@ -2273,13 +2310,14 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) {
 	if !ok {
 		t.Fatalf("Bad: %#v", in.Extra[0])
 	}
-	if aRec.Hdr.Ttl != 10 {
+	if aRec.Hdr.Ttl != 18 {
 		t.Fatalf("Bad: %#v", in.Extra[0])
 	}
 
-	// And the TTL should default to 0 otherwise.
+	// And the TTL should take the service-specific value from the agent's
+	// config otherwise.
 	m = new(dns.Msg)
-	m.SetQuestion("nottl.query.consul.", dns.TypeSRV)
+	m.SetQuestion("db-nottl.query.consul.", dns.TypeSRV)
 	in, _, err = c.Exchange(m, addr.String())
 	if err != nil {
 		t.Fatalf("err: %v", err)
@@ -2293,7 +2331,7 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) {
 	if !ok {
 		t.Fatalf("Bad: %#v", in.Answer[0])
 	}
-	if srvRec.Hdr.Ttl != 0 {
+	if srvRec.Hdr.Ttl != 10 {
 		t.Fatalf("Bad: %#v", in.Answer[0])
 	}
 
@@ -2301,7 +2339,36 @@ func TestDNS_PreparedQuery_TTL(t *testing.T) {
 	if !ok {
 		t.Fatalf("Bad: %#v", in.Extra[0])
 	}
-	if aRec.Hdr.Ttl != 0 {
+	if aRec.Hdr.Ttl != 10 {
+		t.Fatalf("Bad: %#v", in.Extra[0])
+	}
+
+	// If there's no query TTL and no service-specific value then the wild
+	// card value should be used.
+	m = new(dns.Msg)
+	m.SetQuestion("api-nottl.query.consul.", dns.TypeSRV)
+	in, _, err = c.Exchange(m, addr.String())
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if len(in.Answer) != 1 {
+		t.Fatalf("Bad: %#v", in)
+	}
+
+	srvRec, ok = in.Answer[0].(*dns.SRV)
+	if !ok {
+		t.Fatalf("Bad: %#v", in.Answer[0])
+	}
+	if srvRec.Hdr.Ttl != 5 {
+		t.Fatalf("Bad: %#v", in.Answer[0])
+	}
+
+	aRec, ok = in.Extra[0].(*dns.A)
+	if !ok {
+		t.Fatalf("Bad: %#v", in.Extra[0])
+	}
+	if aRec.Hdr.Ttl != 5 {
 		t.Fatalf("Bad: %#v", in.Extra[0])
 	}
 }

consul/prepared_query_endpoint.go

@@ -386,6 +386,7 @@ func (p *PreparedQuery) execute(query *structs.PreparedQuery,
 	}
 
 	// Capture the nodes and pass the DNS information through to the reply.
+	reply.Service = query.Service.Service
 	reply.Nodes = nodes
 	reply.DNS = query.DNS
 

consul/prepared_query_endpoint_test.go

@@ -1100,6 +1100,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 10 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1121,6 +1122,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 3 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1162,6 +1164,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 10 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1186,6 +1189,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 10 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1244,6 +1248,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 9 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1270,6 +1275,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 10 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1297,6 +1303,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 9 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1331,6 +1338,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 8 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1359,6 +1367,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 0 ||
 			reply.Datacenter != "dc1" || reply.Failovers != 0 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1385,6 +1394,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 9 ||
 			reply.Datacenter != "dc2" || reply.Failovers != 1 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1412,6 +1422,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 3 ||
 			reply.Datacenter != "dc2" || reply.Failovers != 1 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)
@@ -1438,6 +1449,7 @@ func TestPreparedQuery_Execute(t *testing.T) {
 
 		if len(reply.Nodes) != 9 ||
 			reply.Datacenter != "dc2" || reply.Failovers != 1 ||
+			reply.Service != query.Query.Service.Service ||
 			!reflect.DeepEqual(reply.DNS, query.Query.DNS) ||
 			!reply.QueryMeta.KnownLeader {
 			t.Fatalf("bad: %v", reply)

consul/structs/prepared_query.go

@@ -186,6 +186,9 @@ func (q *PreparedQueryExecuteRemoteRequest) RequestDatacenter() string {
 
 // PreparedQueryExecuteResponse has the results of executing a query.
 type PreparedQueryExecuteResponse struct {
+	// Service is the service that was queried.
+	Service string
+
 	// Nodes has the nodes that were output by the query.
 	Nodes CheckServiceNodes