plugin/metadata: metadata is just label=value (#1914)

This revert 17d807f0 and re-adds the metadata plugin as a plugin that
just sets a label to a value function.

Add package documentation on how to use the metadata package. Make it
clear that any caching is up to the Func implemented.

There are now - no in tree users. We could add the request metadata by
default under names that copy request.Request, i.e

request/ip - remote IP
request/port - remote port

Variables.go has been deleted.

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

plugin/metadata/metadata.go

@@ -4,7 +4,6 @@ import (
 	"context"
 
 	"github.com/coredns/coredns/plugin"
-	"github.com/coredns/coredns/plugin/pkg/variables"
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
@@ -24,18 +23,13 @@ func (m *Metadata) Name() string { return "metadata" }
 // ServeDNS implements the plugin.Handler interface.
 func (m *Metadata) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
 
-	ctx = context.WithValue(ctx, metadataKey{}, M{})
-	md, _ := FromContext(ctx)
+	ctx = context.WithValue(ctx, key{}, md{})
 
 	state := request.Request{W: w, Req: r}
 	if plugin.Zones(m.Zones).Matches(state.Name()) != "" {
 		// Go through all Providers and collect metadata.
-		for _, provider := range m.Providers {
-			for _, varName := range provider.MetadataVarNames() {
-				if val, ok := provider.Metadata(ctx, state, varName); ok {
-					md.SetValue(varName, val)
-				}
-			}
+		for _, p := range m.Providers {
+			ctx = p.Metadata(ctx, state)
 		}
 	}
 
@@ -43,14 +37,3 @@ func (m *Metadata) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Ms
 
 	return rcode, err
 }
-
-// MetadataVarNames implements the plugin.Provider interface.
-func (m *Metadata) MetadataVarNames() []string { return variables.All }
-
-// Metadata implements the plugin.Provider interface.
-func (m *Metadata) Metadata(ctx context.Context, state request.Request, varName string) (interface{}, bool) {
-	if val, err := variables.GetValue(state, varName); err == nil {
-		return val, true
-	}
-	return nil, false
-}

plugin/metadata/metadata_test.go

@@ -10,26 +10,18 @@ import (
 	"github.com/miekg/dns"
 )
 
-// testProvider implements fake Providers. Plugins which inmplement Provider interface
-type testProvider map[string]interface{}
+type testProvider map[string]Func
 
-func (m testProvider) MetadataVarNames() []string {
-	keys := []string{}
-	for k := range m {
-		keys = append(keys, k)
+func (tp testProvider) Metadata(ctx context.Context, state request.Request) context.Context {
+	for k, v := range tp {
+		SetValueFunc(ctx, k, v)
 	}
-	return keys
+	return ctx
 }
 
-func (m testProvider) Metadata(ctx context.Context, state request.Request, key string) (val interface{}, ok bool) {
-	value, ok := m[key]
-	return value, ok
-}
-
-// testHandler implements plugin.Handler.
 type testHandler struct{ ctx context.Context }
 
-func (m *testHandler) Name() string { return "testHandler" }
+func (m *testHandler) Name() string { return "test" }
 
 func (m *testHandler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
 	m.ctx = ctx
@@ -38,8 +30,8 @@ func (m *testHandler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns
 
 func TestMetadataServeDNS(t *testing.T) {
 	expectedMetadata := []testProvider{
-		testProvider{"testkey1": "testvalue1"},
-		testProvider{"testkey2": 2, "testkey3": "testvalue3"},
+		testProvider{"test/key1": func() string { return "testvalue1" }},
+		testProvider{"test/key2": func() string { return "two" }, "test/key3": func() string { return "testvalue3" }},
 	}
 	// Create fake Providers based on expectedMetadata
 	providers := []Provider{}
@@ -48,32 +40,22 @@ func TestMetadataServeDNS(t *testing.T) {
 	}
 
 	next := &testHandler{} // fake handler which stores the resulting context
-	metadata := Metadata{
+	m := Metadata{
 		Zones:     []string{"."},
 		Providers: providers,
 		Next:      next,
 	}
-	metadata.ServeDNS(context.TODO(), &test.ResponseWriter{}, new(dns.Msg))
 
-	// Verify that next plugin can find metadata in context from all Providers
+	ctx := context.TODO()
+	m.ServeDNS(ctx, &test.ResponseWriter{}, new(dns.Msg))
+	nctx := next.ctx
+
 	for _, expected := range expectedMetadata {
-		md, ok := FromContext(next.ctx)
-		if !ok {
-			t.Fatalf("Metadata is expected but not present inside the context")
-		}
-		for expKey, expVal := range expected {
-			metadataVal, valOk := md.Value(expKey)
-			if !valOk {
-				t.Fatalf("Value by key %v can't be retrieved", expKey)
+		for label, expVal := range expected {
+			val := ValueFunc(nctx, label)
+			if val() != expVal() {
+				t.Errorf("Expected value %s for %s, but got %s", expVal(), label, val())
 			}
-			if metadataVal != expVal {
-				t.Errorf("Expected value %v, but got %v", expVal, metadataVal)
-			}
-		}
-		wrongKey := "wrong_key"
-		metadataVal, ok := md.Value(wrongKey)
-		if ok {
-			t.Fatalf("Value by key %v is not expected to be recieved, but got: %v", wrongKey, metadataVal)
 		}
 	}
 }

plugin/metadata/provider.go

@@ -1,3 +1,33 @@
+// Package metadata provides an API that allows plugins to add metadata to the context.
+// Each metadata is stored under a label that has the form <plugin>/<name>. Each metadata
+// is returned as a Func. When Func is called the metadata is returned. If Func is expensive to
+// execute it is its responsibility to provide some form of caching. During the handling of a
+// query it is expected the metadata stays constant.
+//
+// Basic example:
+//
+// Implement the Provder interface for a plugin:
+//
+//    func (p P) Metadata(ctx context.Context, state request.Request) context.Context {
+//       cached := ""
+//       f := func() string {
+//		if cached != "" {
+//                 return cached
+//             }
+//             cached = expensiveFunc()
+//             return cached
+//       }
+//       metadata.SetValueFunc(ctx, "test/something", f)
+//	 return ctx
+//    }
+//
+// Check the metadata from another plugin:
+//
+//    // ...
+//    valueFunc := metadata.ValueFunc(ctx, "test/something")
+//    value := valueFunc()
+//    // use 'value'
+//
 package metadata
 
 import (
@@ -8,40 +38,62 @@ import (
 
 // Provider interface needs to be implemented by each plugin willing to provide
 // metadata information for other plugins.
-// Note: this method should work quickly, because it is called for every request
-// from the metadata plugin.
 type Provider interface {
-	// List of variables which are provided by current Provider. Must remain constant.
-	MetadataVarNames() []string
-	// Metadata is expected to return a value with metadata information by the key
-	// from 4th argument. Value can be later retrieved from context by any other plugin.
-	// If value is not available by some reason returned boolean value should be false.
-	Metadata(ctx context.Context, state request.Request, variable string) (interface{}, bool)
+	// Metadata adds metadata to the context and returns a (potentially) new context.
+	// Note: this method should work quickly, because it is called for every request
+	// from the metadata plugin.
+	Metadata(ctx context.Context, state request.Request) context.Context
 }
 
-// M is metadata information storage.
-type M map[string]interface{}
+// Func is the type of function in the metadata, when called they return the value of the label.
+type Func func() string
 
-// FromContext retrieves the metadata from the context.
-func FromContext(ctx context.Context) (M, bool) {
-	if metadata := ctx.Value(metadataKey{}); metadata != nil {
-		if m, ok := metadata.(M); ok {
-			return m, true
+// Labels returns all metadata keys stored in the context. These label names should be named
+// as: plugin/NAME, where NAME is something descriptive.
+func Labels(ctx context.Context) []string {
+	if metadata := ctx.Value(key{}); metadata != nil {
+		if m, ok := metadata.(md); ok {
+			return keys(m)
 		}
 	}
-	return M{}, false
+	return nil
 }
 
-// Value returns metadata value by key.
-func (m M) Value(key string) (value interface{}, ok bool) {
-	value, ok = m[key]
-	return value, ok
+// ValueFunc returns the value function of label. If none can be found nil is returned. Calling the
+// function returns the value of the label.
+func ValueFunc(ctx context.Context, label string) Func {
+	if metadata := ctx.Value(key{}); metadata != nil {
+		if m, ok := metadata.(md); ok {
+			return m[label]
+		}
+	}
+	return nil
 }
 
-// SetValue sets the metadata value under key.
-func (m M) SetValue(key string, val interface{}) {
-	m[key] = val
+// SetValueFunc set the metadata label to the value function. If no metadata can be found this is a noop and
+// false is returned. Any existing value is overwritten.
+func SetValueFunc(ctx context.Context, label string, f Func) bool {
+	if metadata := ctx.Value(key{}); metadata != nil {
+		if m, ok := metadata.(md); ok {
+			m[label] = f
+			return true
+		}
+	}
+	return false
 }
 
-// metadataKey defines the type of key that is used to save metadata into the context.
-type metadataKey struct{}
+// md is metadata information storage.
+type md map[string]Func
+
+// key defines the type of key that is used to save metadata into the context.
+type key struct{}
+
+func keys(m map[string]Func) []string {
+	s := make([]string, len(m))
+	i := 0
+	for k := range m {
+		s[i] = k
+		i++
+	}
+	return s
+}

plugin/metadata/provider_test.go

@@ -1,48 +0,0 @@
-package metadata
-
-import (
-	"context"
-	"reflect"
-	"testing"
-)
-
-func TestMD(t *testing.T) {
-	tests := []struct {
-		addValues      map[string]interface{}
-		expectedValues map[string]interface{}
-	}{
-		{
-			// Add initial metadata key/vals
-			map[string]interface{}{"key1": "val1", "key2": 2},
-			map[string]interface{}{"key1": "val1", "key2": 2},
-		},
-		{
-			// Add additional key/vals.
-			map[string]interface{}{"key3": 3, "key4": 4.5},
-			map[string]interface{}{"key1": "val1", "key2": 2, "key3": 3, "key4": 4.5},
-		},
-	}
-
-	// Using one same md and ctx for all test cases
-	ctx := context.TODO()
-	ctx = context.WithValue(ctx, metadataKey{}, M{})
-	m, _ := FromContext(ctx)
-
-	for i, tc := range tests {
-		for k, v := range tc.addValues {
-			m.SetValue(k, v)
-		}
-		if !reflect.DeepEqual(tc.expectedValues, map[string]interface{}(m)) {
-			t.Errorf("Test %d: Expected %v but got %v", i, tc.expectedValues, m)
-		}
-
-		// Make sure that md is recieved from context successfullly
-		mFromContext, ok := FromContext(ctx)
-		if !ok {
-			t.Errorf("Test %d: md is not recieved from the context", i)
-		}
-		if !reflect.DeepEqual(m, mFromContext) {
-			t.Errorf("Test %d: md recieved from context differs from initial. Initial: %v, from context: %v", i, m, mFromContext)
-		}
-	}
-}

plugin/metadata/setup.go

@@ -1,8 +1,6 @@
 package metadata
 
 import (
-	"fmt"
-
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/plugin"
 
@@ -28,16 +26,8 @@ func setup(c *caddy.Controller) error {
 
 	c.OnStartup(func() error {
 		plugins := dnsserver.GetConfig(c).Handlers()
-		// Collect all plugins which implement Provider interface
-		metadataVariables := map[string]bool{}
 		for _, p := range plugins {
 			if met, ok := p.(Provider); ok {
-				for _, varName := range met.MetadataVarNames() {
-					if _, ok := metadataVariables[varName]; ok {
-						return fmt.Errorf("Metadata variable '%v' has duplicates", varName)
-					}
-					metadataVariables[varName] = true
-				}
 				m.Providers = append(m.Providers, met)
 			}
 		}