plugin/log: allow various combinations of classes of responses (#1664)

This allows to log responses of different classes, for example, denial and error.

4 files changed, 61 insertions, 15 deletions

diff --git a/plugin/log/log.go b/plugin/log/log.go
index 26e825c8f..6b8d0c385 100644
--- a/plugin/log/log.go
+++ b/plugin/log/log.go
@@ -54,7 +54,9 @@ func (l Logger) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 
 		tpe, _ := response.Typify(rrw.Msg, time.Now().UTC())
 		class := response.Classify(tpe)
-		if rule.Class == response.All || rule.Class == class {
+		// If we don't set up a class in config, the default "all" will be added
+		// and we shouldn't have an empty rule.Class.
+		if rule.Class[response.All] || rule.Class[class] {
 			rep := replacer.New(r, rrw, CommonLogEmptyValue)
 			rule.Log.Println(rep.Replace(rule.Format))
 		}
@@ -71,7 +73,7 @@ func (l Logger) Name() string { return "log" }
 // Rule configures the logging plugin.
 type Rule struct {
 	NameScope string
-	Class     response.Class
+	Class     map[response.Class]bool
 	Format    string
 	Log       *log.Logger
 }
diff --git a/plugin/log/log_test.go b/plugin/log/log_test.go
index 05271478c..3a83e8d54 100644
--- a/plugin/log/log_test.go
+++ b/plugin/log/log_test.go
@@ -20,6 +20,7 @@ func TestLoggedStatus(t *testing.T) {
 		NameScope: ".",
 		Format:    DefaultLogFormat,
 		Log:       log.New(&f, "", 0),
+		Class:     map[response.Class]bool{response.All: true},
 	}
 
 	logger := Logger{
@@ -50,7 +51,7 @@ func TestLoggedClassDenial(t *testing.T) {
 		NameScope: ".",
 		Format:    DefaultLogFormat,
 		Log:       log.New(&f, "", 0),
-		Class:     response.Denial,
+		Class:     map[response.Class]bool{response.Denial: true},
 	}
 
 	logger := Logger{
@@ -78,7 +79,7 @@ func TestLoggedClassError(t *testing.T) {
 		NameScope: ".",
 		Format:    DefaultLogFormat,
 		Log:       log.New(&f, "", 0),
-		Class:     response.Error,
+		Class:     map[response.Class]bool{response.Error: true},
 	}
 
 	logger := Logger{
diff --git a/plugin/log/setup.go b/plugin/log/setup.go
index 8b1fc889a..1f9501d68 100644
--- a/plugin/log/setup.go
+++ b/plugin/log/setup.go
@@ -52,11 +52,13 @@ func logParse(c *caddy.Controller) ([]Rule, error) {
 			rules = append(rules, Rule{
 				NameScope: ".",
 				Format:    DefaultLogFormat,
+				Class:     make(map[response.Class]bool),
 			})
 		} else if len(args) == 1 {
 			rules = append(rules, Rule{
 				NameScope: dns.Fqdn(args[0]),
 				Format:    DefaultLogFormat,
+				Class:     make(map[response.Class]bool),
 			})
 		} else {
 			// Name scope, and maybe a format specified
@@ -74,28 +76,33 @@ func logParse(c *caddy.Controller) ([]Rule, error) {
 			rules = append(rules, Rule{
 				NameScope: dns.Fqdn(args[0]),
 				Format:    format,
+				Class:     make(map[response.Class]bool),
 			})
 		}
 
 		// Class refinements in an extra block.
 		for c.NextBlock() {
 			switch c.Val() {
-			// class followed by all, denial, error or success.
+			// class followed by combinations of all, denial, error and success.
 			case "class":
 				classes := c.RemainingArgs()
 				if len(classes) == 0 {
 					return nil, c.ArgErr()
 				}
-				cls, err := response.ClassFromString(classes[0])
-				if err != nil {
-					return nil, err
+				for _, c := range classes {
+					cls, err := response.ClassFromString(c)
+					if err != nil {
+						return nil, err
+					}
+					rules[len(rules)-1].Class[cls] = true
 				}
-				// update class and the last added Rule (bit icky)
-				rules[len(rules)-1].Class = cls
 			default:
 				return nil, c.ArgErr()
 			}
 		}
+		if len(rules[len(rules)-1].Class) == 0 {
+			rules[len(rules)-1].Class[response.All] = true
+		}
 	}
 
 	return rules, nil
diff --git a/plugin/log/setup_test.go b/plugin/log/setup_test.go
index d336dc668..c85e37a42 100644
--- a/plugin/log/setup_test.go
+++ b/plugin/log/setup_test.go
@@ -1,6 +1,7 @@
 package log
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/coredns/coredns/plugin/pkg/response"
@@ -17,34 +18,42 @@ func TestLogParse(t *testing.T) {
 		{`log`, false, []Rule{{
 			NameScope: ".",
 			Format:    DefaultLogFormat,
+			Class:     map[response.Class]bool{response.All: true},
 		}}},
 		{`log example.org`, false, []Rule{{
 			NameScope: "example.org.",
 			Format:    DefaultLogFormat,
+			Class:     map[response.Class]bool{response.All: true},
 		}}},
 		{`log example.org. {common}`, false, []Rule{{
 			NameScope: "example.org.",
 			Format:    CommonLogFormat,
+			Class:     map[response.Class]bool{response.All: true},
 		}}},
 		{`log example.org {combined}`, false, []Rule{{
 			NameScope: "example.org.",
 			Format:    CombinedLogFormat,
+			Class:     map[response.Class]bool{response.All: true},
 		}}},
 		{`log example.org.
 		log example.net {combined}`, false, []Rule{{
 			NameScope: "example.org.",
 			Format:    DefaultLogFormat,
+			Class:     map[response.Class]bool{response.All: true},
 		}, {
 			NameScope: "example.net.",
 			Format:    CombinedLogFormat,
+			Class:     map[response.Class]bool{response.All: true},
 		}}},
 		{`log example.org {host}
 			  log example.org {when}`, false, []Rule{{
 			NameScope: "example.org.",
 			Format:    "{host}",
+			Class:     map[response.Class]bool{response.All: true},
 		}, {
 			NameScope: "example.org.",
 			Format:    "{when}",
+			Class:     map[response.Class]bool{response.All: true},
 		}}},
 
 		{`log example.org {
@@ -52,22 +61,49 @@ func TestLogParse(t *testing.T) {
 			}`, false, []Rule{{
 			NameScope: "example.org.",
 			Format:    CommonLogFormat,
-			Class:     response.All,
+			Class:     map[response.Class]bool{response.All: true},
 		}}},
 		{`log example.org {
 			class denial
 		}`, false, []Rule{{
 			NameScope: "example.org.",
 			Format:    CommonLogFormat,
-			Class:     response.Denial,
+			Class:     map[response.Class]bool{response.Denial: true},
 		}}},
 		{`log {
 			class denial
 		}`, false, []Rule{{
 			NameScope: ".",
 			Format:    CommonLogFormat,
-			Class:     response.Denial,
+			Class:     map[response.Class]bool{response.Denial: true},
 		}}},
+		{`log {
+			class denial error
+		}`, false, []Rule{{
+			NameScope: ".",
+			Format:    CommonLogFormat,
+			Class:     map[response.Class]bool{response.Denial: true, response.Error: true},
+		}}},
+		{`log {
+			class denial
+			class error
+		}`, false, []Rule{{
+			NameScope: ".",
+			Format:    CommonLogFormat,
+			Class:     map[response.Class]bool{response.Denial: true, response.Error: true},
+		}}},
+		{`log {
+			class abracadabra
+		}`, true, []Rule{
+		}},
+		{`log {
+			class
+		}`, true, []Rule{
+		}},
+		{`log {
+			unknown
+		}`, true, []Rule{
+		}},
 	}
 	for i, test := range tests {
 		c := caddy.NewTestController("dns", test.inputLogRules)
@@ -95,8 +131,8 @@ func TestLogParse(t *testing.T) {
 					i, j, test.inputLogRules, test.expectedLogRules[j].Format, actualLogRule.Format)
 			}
 
-			if actualLogRule.Class != test.expectedLogRules[j].Class {
-				t.Errorf("Test %d expected %dth LogRule Class to be  %s  , but got %s",
+			if !reflect.DeepEqual(actualLogRule.Class, test.expectedLogRules[j].Class) {
+				t.Errorf("Test %d expected %dth LogRule Class to be  %v  , but got %v",
 					i, j, test.expectedLogRules[j].Class, actualLogRule.Class)
 			}
 		}