[FAB-2238] Move Policies Handler to PolicyHander

https://jira.hyperledger.org/browse/FAB-2238

Policy handling is done different from config value handling.  This CR
addresses TODOs in the code to migrate Policies management onto its own
Handler type.

Change-Id: I7605b418cf1f498a0503f2a91f87924c6aab8f30
Signed-off-by: Jason Yellick <[email protected]>

Author: Jason Yellick

common/cauthdsl/policy_test.go

@@ -26,8 +26,8 @@ import (
 	"github.com/golang/protobuf/proto"
 )
 
-var acceptAllPolicy []byte
-var rejectAllPolicy []byte
+var acceptAllPolicy *cb.Policy
+var rejectAllPolicy *cb.Policy
 
 func init() {
 	acceptAllPolicy = makePolicySource(true)
@@ -43,24 +43,23 @@ func marshalOrPanic(msg proto.Message) []byte {
 	return data
 }
 
-func makePolicySource(policyResult bool) []byte {
+func makePolicySource(policyResult bool) *cb.Policy {
 	var policyData *cb.SignaturePolicyEnvelope
 	if policyResult {
 		policyData = AcceptAllPolicy
 	} else {
 		policyData = RejectAllPolicy
 	}
-	marshaledPolicy := marshalOrPanic(&cb.Policy{
+	return &cb.Policy{
 		Type:   int32(cb.Policy_SIGNATURE),
 		Policy: marshalOrPanic(policyData),
-	})
-	return marshaledPolicy
+	}
 }
 
-func addPolicy(manager *policies.ManagerImpl, id string, policy []byte) {
+func addPolicy(manager *policies.ManagerImpl, id string, policy *cb.Policy) {
 	manager.BeginConfig()
-	err := manager.ProposeConfig(id, &cb.ConfigValue{
-		Value: policy,
+	err := manager.ProposePolicy(id, []string{}, &cb.ConfigPolicy{
+		Policy: policy,
 	})
 	if err != nil {
 		panic(err)

common/configtx/api/api.go

@@ -80,6 +80,7 @@ type OrdererConfig interface {
 }
 
 // Handler provides a hook which allows other pieces of code to participate in config proposals
+// TODO, this should probably be renamed to ValueHandler
 type Handler interface {
 	// ProposeConfig called when config is added to a proposal
 	ProposeConfig(key string, configValue *cb.ConfigValue) error
@@ -125,8 +126,8 @@ type Resources interface {
 	MSPManager() msp.MSPManager
 }
 
-// SubInitializer is used downstream from initializer
-type SubInitializer interface {
+// Transactional is an interface which allows for an update to be proposed and rolled back
+type Transactional interface {
 	// BeginConfig called when a config proposal is begun
 	BeginConfig()
 
@@ -135,18 +136,30 @@ type SubInitializer interface {
 
 	// CommitConfig called when a config proposal is committed
 	CommitConfig()
+}
+
+// SubInitializer is used downstream from initializer
+type SubInitializer interface {
+	Transactional
 
 	// Handler returns the associated value handler for a given config path
 	Handler(path []string) (Handler, error)
 }
 
+// PolicyHandler is used for config updates to policy
+type PolicyHandler interface {
+	Transactional
+
+	ProposePolicy(key string, path []string, policy *cb.ConfigPolicy) error
+}
+
 // Initializer is used as indirection between Manager and Handler to allow
 // for single Handlers to handle multiple paths
 type Initializer interface {
 	SubInitializer
 
 	Resources
 
-	// PolicyProposer as a Handler is a temporary hack
-	PolicyProposer() Handler
+	// PolicyProposer returns the PolicyHandler to handle updates to policy
+	PolicyHandler() PolicyHandler
 }

common/configtx/initializer.go

@@ -131,7 +131,7 @@ func (i *initializer) CommitConfig() {
 	i.mspConfigHandler.CommitConfig()
 }
 
-func (i *initializer) PolicyProposer() api.Handler {
+func (i *initializer) PolicyHandler() api.PolicyHandler {
 	return i.policyManager
 }
 

common/configtx/manager.go

@@ -178,10 +178,7 @@ func (cm *configManager) proposeConfig(config map[string]comparable) error {
 				return err
 			}
 		case c.ConfigPolicy != nil:
-			if err := cm.initializer.PolicyProposer().ProposeConfig(c.key, &cb.ConfigValue{
-				// TODO, fix policy interface to take the policy directly
-				Value: utils.MarshalOrPanic(c.ConfigPolicy.Policy),
-			}); err != nil {
+			if err := cm.initializer.PolicyHandler().ProposePolicy(c.key, c.path, c.ConfigPolicy); err != nil {
 				return err
 			}
 		}

common/mocks/configtx/configtx.go

@@ -66,31 +66,55 @@ func (r *Resources) MSPManager() msp.MSPManager {
 	return r.MSPManagerVal
 }
 
+type Transactional struct{}
+
+// BeginConfig calls through to the HandlerVal
+func (t *Transactional) BeginConfig() {}
+
+// CommitConfig calls through to the HandlerVal
+func (t *Transactional) CommitConfig() {}
+
+// RollbackConfig calls through to the HandlerVal
+func (t *Transactional) RollbackConfig() {}
+
 // Initializer mocks the configtxapi.Initializer interface
 type Initializer struct {
+	Transactional
 	Resources
 
 	// HandlersVal is returned as the result of Handlers()
 	HandlerVal configtxapi.Handler
+
+	// PolicyHandlerVal is reutrned at the result of PolicyHandler()
+	PolicyHandlerVal *PolicyHandler
 }
 
 // Returns the HandlersVal
 func (i *Initializer) Handler(path []string) (configtxapi.Handler, error) {
 	return i.HandlerVal, nil
 }
 
-func (i *Initializer) PolicyProposer() configtxapi.Handler {
-	panic("Unimplemented")
+// Returns the PolicyHandlerVal
+func (i *Initializer) PolicyHandler() configtxapi.PolicyHandler {
+	return i.PolicyHandlerVal
 }
 
-// BeginConfig calls through to the HandlerVal
-func (i *Initializer) BeginConfig() {}
-
-// CommitConfig calls through to the HandlerVal
-func (i *Initializer) CommitConfig() {}
+// PolicyHandler mocks the configtxapi.PolicyHandler interface
+type PolicyHandler struct {
+	Transactional
+	LastKey               string
+	LastPath              []string
+	LastValue             *cb.ConfigPolicy
+	ErrorForProposePolicy error
+}
 
-// RollbackConfig calls through to the HandlerVal
-func (i *Initializer) RollbackConfig() {}
+// ProposeConfig sets LastKey to key, LastPath to path, and LastPolicy to configPolicy, returning ErrorForProposedConfig
+func (ph *PolicyHandler) ProposePolicy(key string, path []string, configPolicy *cb.ConfigPolicy) error {
+	ph.LastKey = key
+	ph.LastValue = configPolicy
+	ph.LastPath = path
+	return ph.ErrorForProposePolicy
+}
 
 // Handler mocks the configtxapi.Handler interface
 type Handler struct {

common/mocks/configtx/configtx_test.go

@@ -22,6 +22,14 @@ import (
 	configtxapi "github.com/hyperledger/fabric/common/configtx/api"
 )
 
+func TestConfigtxTransactionalInterface(t *testing.T) {
+	_ = configtxapi.Transactional(&Transactional{})
+}
+
+func TestConfigtxPolicyHandlerInterface(t *testing.T) {
+	_ = configtxapi.PolicyHandler(&PolicyHandler{})
+}
+
 func TestConfigtxInitializerInterface(t *testing.T) {
 	_ = configtxapi.Initializer(&Initializer{})
 }

common/policies/policy.go

@@ -18,10 +18,10 @@ package policies
 
 import (
 	"fmt"
+	"strings"
 
 	cb "github.com/hyperledger/fabric/protos/common"
 
-	"github.com/golang/protobuf/proto"
 	logging "github.com/op/go-logging"
 )
 
@@ -106,12 +106,11 @@ func (pm *ManagerImpl) CommitConfig() {
 	pm.pendingPolicies = nil
 }
 
-// TODO, move this off of *cb.ConfigValue and onto *cb.ConfigPolicy
-func (pm *ManagerImpl) ProposeConfig(key string, configValue *cb.ConfigValue) error {
-	policy := &cb.Policy{}
-	err := proto.Unmarshal(configValue.Value, policy)
-	if err != nil {
-		return err
+// ProposePolicy takes key, path, and ConfigPolicy and registers it in the proposed PolicyManager, or errors
+func (pm *ManagerImpl) ProposePolicy(key string, path []string, configPolicy *cb.ConfigPolicy) error {
+	policy := configPolicy.Policy
+	if policy == nil {
+		return fmt.Errorf("Policy cannot be nil")
 	}
 
 	provider, ok := pm.providers[int32(policy.Type)]
@@ -124,7 +123,18 @@ func (pm *ManagerImpl) ProposeConfig(key string, configValue *cb.ConfigValue) er
 		return err
 	}
 
-	pm.pendingPolicies[key] = cPolicy
+	prefix := strings.Join(path, "/")
+	if len(path) == 0 {
+		prefix = "/"
+	}
+
+	// TODO, once the other components are ready for it, use '_' below as fqKey
+	_ = prefix + "/" + key
+	fqKey := key
+
+	logger.Debugf("Writing policy with fqKey: %s", fqKey)
+
+	pm.pendingPolicies[fqKey] = cPolicy
 
 	logger.Debugf("Proposed new policy %s", key)
 	return nil