Merge pull request #13 from ziollek/handle-timeout-if-none-response-i…

…s-present

Return SRVFAIL if no sub-query is ready on timeout + caveats section in README

README.md

@@ -164,3 +164,18 @@ distributed.local. {
 | request_count_total    | server, qualified (that will be proxied further), type | Count of requests handled by plugin |
 | sub_request_count_total    | server, prefix, type, code                             | Count of sub-requests generated by plugin |
 
+
+## Caveats
+
+### Merging responses with different response codes
+
+During gathering data from several sub-queries some discrepancy in `RCODE` results may occur.
+This plugin assumes that even one successful sub-query means that the merged response should be successful.
+So during merging any set of responses that contains `NOERROR` the merged response will be `NOERROR`.
+In other cases the result is unspecified, but it will be consistent with one of the codes returned by sub-queries.
+
+### Cooperation with `cancel` plugin
+
+If the plugin is put after `cancel` plugin (in compilation time) then the timeouts defined there will be respected.
+It is worth adding that if the timeout occurs client could receive a successful `NOERROR` response for a similar reason as mentioned above.
+If the response for any sub-requests is not ready on timeout then `SERVFAIL` with extended `Error Code 23 - Network Error` will be returned.

gathersrv.go

@@ -129,7 +129,7 @@ func (gatherSrv GatherSrv) ServeDNS(ctx context.Context, w dns.ResponseWriter, r
 			waitCnt = 0
 		}
 	}
-	pw.Flush()
+	pw.Flush(r)
 	return mergedResponse.Code, mergedResponse.Err
 }
 
@@ -263,17 +263,27 @@ func (w *GatherResponsePrinter) Masquerade(rr dns.RR) {
 	}
 }
 
-func (w *GatherResponsePrinter) Flush() {
+func (w *GatherResponsePrinter) Flush(r *dns.Msg) {
 	w.lockCh <- true
 	defer func() {
 		<-w.lockCh
 	}()
-	if w.state != nil {
-		if err := w.ResponseWriter.WriteMsg(w.state); err != nil {
-			log.Errorf(
-				"error occurred while writing response: question=%v, error=%s", w.originalQuestion, err,
-			)
-		}
+	response := w.state
+	if w.state == nil {
+		// prepare SRVFAIL, Error Code 23 - Network Error response if no sub-queries are not completed
+		response = new(dns.Msg)
+		response.SetReply(r)
+		response.Rcode = dns.RcodeServerFailure
+		response = response.SetEdns0(4096, true)
+		response.IsEdns0().Option = append(
+			response.IsEdns0().Option,
+			&dns.EDNS0_EDE{InfoCode: dns.ExtendedErrorCodeNetworkError, ExtraText: "Sub-queries canceled due to timeout"},
+		)
+	}
+	if err := w.ResponseWriter.WriteMsg(response); err != nil {
+		log.Errorf(
+			"error occurred while writing response: question=%v, error=%s", w.originalQuestion, err,
+		)
 	}
 	w.shortMessage()
 }
@@ -294,7 +304,7 @@ func (w *GatherResponsePrinter) shortMessage() {
 		)
 	} else {
 		log.Errorf(
-			"response printer has an empty state, original question was: %v", w.originalQuestion,
+			"response printer has an empty state - SERVFAIL returned, original question was: %v", w.originalQuestion,
 		)
 	}
 }

gathersrv_test.go

@@ -239,6 +239,28 @@ func TestShouldProvideNoErrorResponseWhenNoErrorAndErrorResponsesOccurredTogethe
 	require.Equal(t, expectedExtras, msg.Extra)
 }
 
+func TestShouldReturnServerFailCodeWithExtendedContextIfNoSubQueryIsReadyOnFlush(t *testing.T) {
+	req := new(dns.Msg)
+	req.SetQuestion(dns.Fqdn("_http._tcp.demo.svc.distro.local."), dns.TypeSRV)
+	rec := dnstest.NewRecorder(&test.ResponseWriter{})
+	printer := NewResponsePrinter(
+		rec,
+		req,
+		"distro.local.",
+		[]Cluster{{Suffix: "cluster-a.local.", Prefix: "a-"}},
+		1,
+	)
+
+	printer.Flush(req)
+
+	require.Equal(t, dns.RcodeServerFailure, rec.Msg.Rcode)
+	require.Len(t, rec.Msg.IsEdns0().Option, 1)
+	require.Equal(t, uint16(dns.EDNS0EDE), rec.Msg.IsEdns0().Option[0].Option())
+	extendedError, ok := rec.Msg.IsEdns0().Option[0].(*dns.EDNS0_EDE)
+	require.True(t, ok)
+	require.Equal(t, dns.ExtendedErrorCodeNetworkError, extendedError.InfoCode)
+}
+
 func PrepareOnlyCodeNextHandler(expectedQuestions map[string]Assertion) test.Handler {
 	return test.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
 		m := new(dns.Msg)