Introduce adapter composition model.

The type 'Adapter' is a singleton per adapter binary linked into the mixer. The type 'Instance' captures individual instantiations of a single adapter. Updated the fact mapper code to follow the new model.
gnirodi · Nov 27, 2016 · df6a80c · df6a80c
1 parent b613811
commit df6a80c
Showing 11 changed files with 228 additions and 72 deletions.
diff --git a/Makefile b/Makefile
@@ -13,8 +13,7 @@
 ## limitations under the License.
 
 # Primary build targets
-all: inst build
-build: build_api build_server
+build: inst build_api build_server
 clean: clean_api clean_server
 test: test_server
 
@@ -41,18 +40,18 @@ GO_SRC = server/*.go adapters/*.go adapters/factMapper/*.go
 
 mixer.bin: $(GO_SRC) $(PROTO_SRC)
 	@echo "Building server"
+	@go build -o mixer.bin server/*.go
+
+build_server: mixer.bin
 	@go tool vet -shadowstrict server adapters
 	@golint -set_exit_status server/...
 	@golint -set_exit_status adapters/...
 	@gofmt -w -s $(GO_SRC)
-	@go build -o mixer.bin server/*.go
-
-build_server: mixer.bin
 
 clean_server:
 	@rm -f mixer.bin
 
-test_server: build
+test_server: build_server
 	@echo "Running tests"
 	@go test -race -cpu 1,4 ./server/... ./adapters/...
 

diff --git a/README.md b/README.md
@@ -1,7 +1,7 @@
-<center>
+<div style="text-align: center">
 ![Mixer](docs/logo.png)
 <h1>The Istio Mixer</h1>
-</center>
+</div>
 
 The Istio mixer provides three distinct features:
 

diff --git a/adapters/adapter.go b/adapters/adapter.go
@@ -0,0 +1,50 @@
+// Copyright 2016 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package adapters
+
+// AdapterConfig is used to configure an adapter.
+type AdapterConfig interface{}
+
+// Adapter represents the handle that the mixer has on an individual adapter. The mixer holds on
+// to one of these per logical adapter, which the mixer uses to control the lifecycle of both
+// adapters and adapter instances.
+type Adapter interface {
+	// Name returns the official name of this adapter for use in diagnostics and in config
+	Name() string
+
+	// Description returns a user-friendly description of this adapter.
+	Description() string
+
+	// DefaultConfig returns a default configuration struct for this adapter.
+	// This will be used by the configuration system to establish the shape of a block
+	// of configuration state passed to the Activate function.
+	DefaultConfig() AdapterConfig
+
+	// Activate the adapter with the given configuration. Once an adapter is active,
+	// the mixer can start calling the CreateInstance function to instantiate the adapter
+	Activate(config AdapterConfig) error
+
+	// Deactivate the adapter, allowing it to clean up any resources it might be holding.
+	// Once this function is called, the mixer may no longer call the CreateInstance function.
+	Deactivate()
+
+	// DefaultInstanceConfig returns a default configuration struct for instances
+	// of this adapter. This will be used by the configuration system to establish
+	// the shape of a block of configuration state passed to the Activate function
+	DefaultInstanceConfig() InstanceConfig
+
+	// CreateInstance creates a single instance of the adapter based on the supplied configuration.
+	CreateInstance(config InstanceConfig) (Instance, error)
+}
diff --git a/adapters/FactConversionAdapter.go → adapters/factConversion.go b/adapters/FactConversionAdapter.go → adapters/factConversion.go
@@ -14,10 +14,12 @@
 
 package adapters
 
-// FactConversionAdapter is a factory of fact converters, which
+// FactConversionInstance is a factory of fact converters, which
 // are responsible for converting from a set of facts into a set
 // of labels.
-type FactConversionAdapter interface {
+type FactConversionInstance interface {
+	Instance
+
 	// NewConverter returns a fresh converter
 	NewConverter() FactConverter
 }

diff --git a/adapters/factMapper/adapter.go b/adapters/factMapper/adapter.go
@@ -0,0 +1,58 @@
+// Copyright 2016 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package factMapper
+
+import (
+	"github.com/istio/mixer/adapters"
+)
+
+// AdapterConfig is used to configure a fact mapper adapter.
+type AdapterConfig struct{}
+
+type adapter struct{}
+
+// NewFactMapperAdapter returns an Adapter
+func NewFactMapperAdapter() adapters.Adapter {
+	return adapter{}
+}
+
+func (a adapter) Name() string {
+	return "FactMapper"
+}
+
+func (a adapter) Description() string {
+	return "Performs config-driven mapping of facts to labels"
+}
+
+func (a adapter) DefaultConfig() adapters.AdapterConfig {
+	return AdapterConfig{}
+}
+
+func (a adapter) Activate(config adapters.AdapterConfig) error {
+	_ = config.(AdapterConfig)
+	return nil
+}
+
+func (a adapter) Deactivate() {
+}
+
+func (a adapter) DefaultInstanceConfig() adapters.InstanceConfig {
+	return InstanceConfig{}
+}
+
+func (a adapter) CreateInstance(config adapters.InstanceConfig) (adapters.Instance, error) {
+	c := config.(InstanceConfig)
+	return newInstance(&c)
+}
diff --git a/adapters/factMapper/factMapper.go → adapters/factMapper/instance.go b/adapters/factMapper/factMapper.go → adapters/factMapper/instance.go
@@ -21,28 +21,32 @@ import (
 	"github.com/istio/mixer/adapters"
 )
 
-type factMapperAdapter struct {
+// InstanceConfig is used to configure fact mapper instances
+type InstanceConfig struct {
+	adapters.InstanceConfig
+
+	// Rules specifies the set of label mapping rules. The keys of the map represent the
+	// name of labels, while the value specifies the mapping rules to
+	// turn individual fact values into label values.
+	//
+	// Mapping rules consist of a set of fact names separated by |. The
+	// label's value is derived by iterating through all the stated facts
+	// and picking the first one that is defined.
+	Rules map[string]string
+}
+
+type instance struct {
 	// for each label, has an ordered slice of facts that can contribute to the label
 	labelFacts map[string][]string
 
 	// for each fact that matters, has a list of the labels to update if the fact changes
 	factLabels map[string][]string
 }
 
-// NewFactMapperAdapter returns a new instances of a FactMapper adapter.
-//
-// The single argument specifies
-// the set of label mapping rules. The keys of the map represent the
-// name of labels, while the value specifies the mapping rules to
-// turn individual fact values into label values.
-//
-// Mapping rules consist of a set of fact names separated by |. The
-// label's value is derived by iterating through all the stated facts
-// and picking the first one that is defined.
-//
-// TODO: need to ingest configuration state that defines the mapping rules
-func NewFactMapperAdapter(labelRules map[string]string) (adapters.FactConversionAdapter, error) {
+// newInstance returns a new instance of a FactMapper adapter.
+func newInstance(config *InstanceConfig) (*instance, error) {
 	// build our lookup tables
+	labelRules := config.Rules
 	labelFacts := make(map[string][]string)
 	factLabels := make(map[string][]string)
 	for label, rule := range labelRules {
@@ -63,11 +67,16 @@ func NewFactMapperAdapter(labelRules map[string]string) (adapters.FactConversion
 		}
 	}
 
-	return &factMapperAdapter{
+	return &instance{
 		labelFacts: labelFacts,
 		factLabels: factLabels}, nil
 }
 
-func (f *factMapperAdapter) NewConverter() adapters.FactConverter {
-	return newConverter(f.labelFacts, f.factLabels)
+func (inst *instance) Delete() {
+	inst.labelFacts = nil
+	inst.factLabels = nil
+}
+
+func (inst *instance) NewConverter() adapters.FactConverter {
+	return newConverter(inst.labelFacts, inst.factLabels)
 }
diff --git a/adapters/factMapper/factMapper_test.go → adapters/factMapper/instance_test.go b/adapters/factMapper/factMapper_test.go → adapters/factMapper/instance_test.go
@@ -15,19 +15,18 @@
 package factMapper
 
 import (
-	"github.com/istio/mixer/adapters"
 	"testing"
 )
 
 func TestNoRules(t *testing.T) {
 	rules := make(map[string]string)
-	var fm adapters.FactConversionAdapter
+	var inst *instance
 	var err error
-	if fm, err = NewFactMapperAdapter(rules); err != nil {
+	if inst, err = newInstance(&InstanceConfig{Rules: rules}); err != nil {
 		t.Error("Expected to successfully create a mapper")
 	}
 
-	conv := fm.NewConverter()
+	conv := inst.NewConverter()
 
 	labels := conv.GetLabels()
 	if len(labels) != 0 {
@@ -59,27 +58,27 @@ func TestNoRules(t *testing.T) {
 func TestOddballRules(t *testing.T) {
 	rules := make(map[string]string)
 	rules["Lab1"] = "|A|B|C"
-	if _, err := NewFactMapperAdapter(rules); err == nil {
+	if _, err := newInstance(&InstanceConfig{Rules: rules}); err == nil {
 		t.Error("Expecting to not be able to create a mapper")
 	}
 
 	rules["Lab1"] = "A|B|"
-	if _, err := NewFactMapperAdapter(rules); err == nil {
+	if _, err := newInstance(&InstanceConfig{Rules: rules}); err == nil {
 		t.Error("Expecting to not be able to create a mapper")
 	}
 
 	rules["Lab1"] = "A| |C"
-	if _, err := NewFactMapperAdapter(rules); err == nil {
+	if _, err := newInstance(&InstanceConfig{Rules: rules}); err == nil {
 		t.Error("Expecting to not be able to create a mapper")
 	}
 
 	rules["Lab1"] = "A||C"
-	if _, err := NewFactMapperAdapter(rules); err == nil {
+	if _, err := newInstance(&InstanceConfig{Rules: rules}); err == nil {
 		t.Error("Expecting to not be able to create a mapper")
 	}
 
 	rules["Lab1"] = "A | B | C"
-	if _, err := NewFactMapperAdapter(rules); err != nil {
+	if _, err := newInstance(&InstanceConfig{Rules: rules}); err != nil {
 		t.Error("Expecting to be able to create a mapper")
 	}
 }
@@ -88,13 +87,13 @@ func TestNoFacts(t *testing.T) {
 	rules := make(map[string]string)
 	rules["Lab1"] = "Fact1|Fact2|Fact3"
 	rules["Lab2"] = "Fact3|Fact2|Fact1"
-	var fm adapters.FactConversionAdapter
+	var inst *instance
 	var err error
-	if fm, err = NewFactMapperAdapter(rules); err != nil {
+	if inst, err = newInstance(&InstanceConfig{Rules: rules}); err != nil {
 		t.Error("Expected to be able to create a mapper")
 	}
 
-	conv := fm.NewConverter()
+	conv := inst.NewConverter()
 
 	labels := conv.GetLabels()
 	if len(labels) != 0 {
@@ -130,13 +129,13 @@ func TestAddRemoveFacts(t *testing.T) {
 	rules := make(map[string]string)
 	rules["Lab1"] = "Fact1|Fact2|Fact3"
 	rules["Lab2"] = "Fact3|Fact2|Fact1"
-	var fm adapters.FactConversionAdapter
+	var inst *instance
 	var err error
-	if fm, err = NewFactMapperAdapter(rules); err != nil {
+	if inst, err = newInstance(&InstanceConfig{Rules: rules}); err != nil {
 		t.Error("Expected to be able to create a mapper")
 	}
 
-	conv := fm.NewConverter()
+	conv := inst.NewConverter()
 
 	// add some facts and try again
 	facts := make(map[string]string)

diff --git a/adapters/instance.go b/adapters/instance.go
@@ -0,0 +1,24 @@
+// Copyright 2016 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package adapters
+
+// InstanceConfig is used to configure an individual instance of an adapter.
+type InstanceConfig interface{}
+
+// Instance defines lifecycle functions for individual adapter instances.
+type Instance interface {
+	// Delete is called by the mixer to indicate it is done with a particular adapter instance.
+	Delete()
+}
diff --git a/server/apiServer.go b/server/apiServer.go
@@ -67,21 +67,21 @@ type APIServerOptions struct {
 	// for all API methods
 	Handlers APIHandlers
 
-	// FactConversionAdapter is a pointer to the global fact
-	// adapter to use.
+	// FactConversionInstance is a pointer to the global fact conversion
+	// adapter instance to use.
 	//
 	// TODO: This will be replaced with a more general pointer such
 	// as an AdapterManager or something of the sort
-	FactConversionAdapter adapters.FactConversionAdapter
+	FactConversionInstance adapters.FactConversionInstance
 }
 
 // APIServer holds the state for the gRPC API server.
 // Use NewAPIServer to get one of these.
 type APIServer struct {
-	server                *grpc.Server
-	listener              net.Listener
-	handler               APIHandlers
-	factConversionAdapter adapters.FactConversionAdapter
+	server                 *grpc.Server
+	listener               net.Listener
+	handler                APIHandlers
+	factConversionInstance adapters.FactConversionInstance
 }
 
 // NewAPIServer creates the gRPC serving stack.
@@ -120,7 +120,7 @@ func NewAPIServer(options *APIServerOptions) (*APIServer, error) {
 
 	// get everything wired up
 	grpcServer := grpc.NewServer(grpcOptions...)
-	apiServer := &APIServer{grpcServer, listener, options.Handlers, options.FactConversionAdapter}
+	apiServer := &APIServer{grpcServer, listener, options.Handlers, options.FactConversionInstance}
 	mixpb.RegisterMixerServer(grpcServer, apiServer)
 	return apiServer, nil
 }
@@ -140,7 +140,7 @@ func (s *APIServer) Stop() {
 type handlerFunc func(conv adapters.FactConverter, request proto.Message, response proto.Message)
 
 func (s *APIServer) streamLoop(stream grpc.ServerStream, request proto.Message, response proto.Message, handler handlerFunc) error {
-	conv := s.factConversionAdapter.NewConverter()
+	conv := s.factConversionInstance.NewConverter()
 	for {
 		// get a single message
 		if err := stream.RecvMsg(request); err == io.EOF {