[FAB-2615] Remove Ingress/EgressPolicyNames refs

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

Now that the hierarchical config and policies are in place, the old
style of specifying ingress and egress policies is wrong.

This CR switches the orderer to use the ChannelReaders and
ChannelWriters policies instead of the Ingress/Egress policy names and
removes the references to those now removed config policies.

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

Author: Jason Yellick

common/configtx/tool/provisional/provisional.go

@@ -107,8 +107,6 @@ func New(conf *genesisconfig.Profile) Generator {
 				PreferredMaxBytes: conf.Orderer.BatchSize.PreferredMaxBytes,
 			}),
 			configtxorderer.TemplateBatchTimeout(conf.Orderer.BatchTimeout.String()),
-			configtxorderer.TemplateIngressPolicyNames([]string{AcceptAllPolicyKey}),
-			configtxorderer.TemplateEgressPolicyNames([]string{AcceptAllPolicyKey}),
 
 			// Initialize the default Reader/Writer/Admins orderer policies, as well as block validation policy
 			policies.TemplateImplicitMetaPolicyWithSubPolicy([]string{configtxorderer.GroupKey}, BlockValidationPolicyKey, configvaluesmsp.WritersPolicyKey, cb.ImplicitMetaPolicy_ANY),

common/configvalues/api.go

@@ -68,12 +68,6 @@ type Orderer interface {
 	// Kafka brokers, i.e. this is not necessarily the entire set of Kafka brokers
 	// used for ordering
 	KafkaBrokers() []string
-
-	// IngressPolicyNames returns the name of the policy to validate incoming broadcast messages against
-	IngressPolicyNames() []string
-
-	// EgressPolicyNames returns the name of the policy to validate incoming broadcast messages against
-	EgressPolicyNames() []string
 }
 
 type ValueProposer interface {

common/configvalues/channel/orderer/sharedconfig.go

@@ -58,8 +58,6 @@ var Schema = &cb.ConfigGroupSchema{
 		BatchTimeoutKey:             nil,
 		ChainCreationPolicyNamesKey: nil,
 		KafkaBrokersKey:             nil,
-		IngressPolicyNamesKey:       nil,
-		EgressPolicyNamesKey:        nil,
 	},
 	Policies: map[string]*cb.ConfigPolicySchema{
 	// TODO, set appropriately once hierarchical policies are implemented
@@ -81,12 +79,6 @@ const (
 
 	// KafkaBrokersKey is the cb.ConfigItem type key name for the KafkaBrokers message
 	KafkaBrokersKey = "KafkaBrokers"
-
-	// IngressPolicyNamesKey is the cb.ConfigItem type key name for the IngressPolicyNames message
-	IngressPolicyNamesKey = "IngressPolicyNames"
-
-	// EgressPolicyNamesKey is the cb.ConfigItem type key name for the EgressPolicyNames message
-	EgressPolicyNamesKey = "EgressPolicyNames"
 )
 
 var logger = logging.MustGetLogger("configtx/handlers/orderer")
@@ -97,8 +89,6 @@ type ordererConfig struct {
 	batchTimeout             time.Duration
 	chainCreationPolicyNames []string
 	kafkaBrokers             []string
-	ingressPolicyNames       []string
-	egressPolicyNames        []string
 	orgs                     map[string]*organization.OrgConfig
 }
 
@@ -146,16 +136,6 @@ func (pm *ManagerImpl) KafkaBrokers() []string {
 	return pm.config.kafkaBrokers
 }
 
-// IngressPolicyNames returns the name of the policy to validate incoming broadcast messages against
-func (pm *ManagerImpl) IngressPolicyNames() []string {
-	return pm.config.ingressPolicyNames
-}
-
-// EgressPolicyNames returns the name of the policy to validate incoming deliver seeks against
-func (pm *ManagerImpl) EgressPolicyNames() []string {
-	return pm.config.egressPolicyNames
-}
-
 // BeginValueProposals is used to start a new config proposal
 func (pm *ManagerImpl) BeginValueProposals(groups []string) ([]api.ValueProposer, error) {
 	logger.Debugf("Beginning a possible new orderer shared config")
@@ -255,18 +235,6 @@ func (pm *ManagerImpl) ProposeValue(key string, configValue *cb.ConfigValue) err
 		} else {
 			pm.pendingConfig.chainCreationPolicyNames = chainCreationPolicyNames.Names
 		}
-	case IngressPolicyNamesKey:
-		ingressPolicyNames := &ab.IngressPolicyNames{}
-		if err := proto.Unmarshal(configValue.Value, ingressPolicyNames); err != nil {
-			return fmt.Errorf("Unmarshaling error for IngressPolicyNames: %s", err)
-		}
-		pm.pendingConfig.ingressPolicyNames = ingressPolicyNames.Names
-	case EgressPolicyNamesKey:
-		egressPolicyNames := &ab.EgressPolicyNames{}
-		if err := proto.Unmarshal(configValue.Value, egressPolicyNames); err != nil {
-			return fmt.Errorf("Unmarshaling error for EgressPolicyNames: %s", err)
-		}
-		pm.pendingConfig.egressPolicyNames = egressPolicyNames.Names
 	case KafkaBrokersKey:
 		kafkaBrokers := &ab.KafkaBrokers{}
 		if err := proto.Unmarshal(configValue.Value, kafkaBrokers); err != nil {

common/configvalues/channel/orderer/sharedconfig_test.go

@@ -310,16 +310,6 @@ func testPolicyNames(m *ManagerImpl, key string, initializer func(val []string)
 	}
 }
 
-func TestIngressPolicyNames(t *testing.T) {
-	m := NewManagerImpl(nil)
-	testPolicyNames(m, IngressPolicyNamesKey, TemplateIngressPolicyNames, m.IngressPolicyNames, t)
-}
-
-func TestEgressPolicyNames(t *testing.T) {
-	m := NewManagerImpl(nil)
-	testPolicyNames(m, EgressPolicyNamesKey, TemplateEgressPolicyNames, m.EgressPolicyNames, t)
-}
-
 func TestChainCreationPolicyNames(t *testing.T) {
 	m := NewManagerImpl(nil)
 	testPolicyNames(m, ChainCreationPolicyNamesKey, TemplateChainCreationPolicyNames, m.ChainCreationPolicyNames, t)

common/configvalues/channel/orderer/sharedconfig_util.go

@@ -51,16 +51,6 @@ func TemplateChainCreationPolicyNames(names []string) *cb.ConfigGroup {
 	return configGroup(ChainCreationPolicyNamesKey, utils.MarshalOrPanic(&ab.ChainCreationPolicyNames{Names: names}))
 }
 
-// TemplateIngressPolicyNames creates a headerless config item representing the ingress policy names
-func TemplateIngressPolicyNames(names []string) *cb.ConfigGroup {
-	return configGroup(IngressPolicyNamesKey, utils.MarshalOrPanic(&ab.IngressPolicyNames{Names: names}))
-}
-
-// TemplateEgressPolicyNames creates a headerless config item representing the egress policy names
-func TemplateEgressPolicyNames(names []string) *cb.ConfigGroup {
-	return configGroup(EgressPolicyNamesKey, utils.MarshalOrPanic(&ab.EgressPolicyNames{Names: names}))
-}
-
 // TemplateKafkaBrokers creates a headerless config item representing the kafka brokers
 func TemplateKafkaBrokers(brokers []string) *cb.ConfigGroup {
 	return configGroup(KafkaBrokersKey, utils.MarshalOrPanic(&ab.KafkaBrokers{Brokers: brokers}))

common/policies/policy.go

@@ -38,6 +38,12 @@ const (
 	// OrdererPrefix is used in the path of standard orderer policy paths
 	OrdererPrefix = "Orderer"
 
+	// ChannelReaders is the label for the channel's readers policy (encompassing both orderer and application readers)
+	ChannelReaders = PathSeparator + ChannelPrefix + PathSeparator + "Readers"
+
+	// ChannelWriters is the label for the channel's writers policy (encompassing both orderer and application writers)
+	ChannelWriters = PathSeparator + ChannelPrefix + PathSeparator + "Writers"
+
 	// ChannelApplicationReaders is the label for the channel's application readers policy
 	ChannelApplicationReaders = PathSeparator + ChannelPrefix + PathSeparator + ApplicationPrefix + PathSeparator + "Readers"
 
@@ -266,6 +272,14 @@ func (pm *ManagerImpl) CommitProposals() {
 	pm.pendingConfig = nil
 
 	if pm.parent == nil && pm.basePath == ChannelPrefix {
+		for _, policyName := range []string{ChannelReaders, ChannelWriters} {
+			_, ok := pm.GetPolicy(policyName)
+			if !ok {
+				logger.Warningf("Current configuration has no policy '%s', this will likely cause problems in production systems", policyName)
+			} else {
+				logger.Debugf("As expected, current configuration has policy '%s'", policyName)
+			}
+		}
 		if _, ok := pm.config.managers[ApplicationPrefix]; ok {
 			// Check for default application policies if the application component is defined
 			for _, policyName := range []string{

orderer/common/deliver/deliver.go

@@ -19,7 +19,6 @@ package deliver
 import (
 	"fmt"
 
-	configvaluesapi "github.com/hyperledger/fabric/common/configvalues"
 	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/orderer/common/filter"
 	"github.com/hyperledger/fabric/orderer/common/sigfilter"
@@ -51,9 +50,6 @@ type Support interface {
 
 	// Reader returns the chain Reader for the chain
 	Reader() ledger.Reader
-
-	// SharedConfig returns the shared config manager for this chain
-	SharedConfig() configvaluesapi.Orderer
 }
 
 type deliverServer struct {
@@ -99,7 +95,7 @@ func (ds *deliverServer) Handle(srv ab.AtomicBroadcast_DeliverServer) error {
 			return sendStatusReply(srv, cb.Status_NOT_FOUND)
 		}
 
-		sf := sigfilter.New(chain.SharedConfig().EgressPolicyNames, chain.PolicyManager())
+		sf := sigfilter.New(policies.ChannelReaders, chain.PolicyManager())
 		result, _ := sf.Apply(envelope)
 		if result != filter.Forward {
 			return sendStatusReply(srv, cb.Status_FORBIDDEN)

orderer/common/deliver/deliver_test.go

@@ -22,8 +22,6 @@ import (
 	"time"
 
 	"github.com/hyperledger/fabric/common/configtx/tool/provisional"
-	configvaluesapi "github.com/hyperledger/fabric/common/configvalues"
-	mockconfigvaluesorderer "github.com/hyperledger/fabric/common/mocks/configvalues/channel/orderer"
 	mockpolicies "github.com/hyperledger/fabric/common/mocks/policies"
 	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/orderer/ledger"
@@ -82,7 +80,6 @@ func (mm *mockSupportManager) GetChain(chainID string) (Support, bool) {
 
 type mockSupport struct {
 	ledger        ledger.ReadWriter
-	sharedConfig  *mockconfigvaluesorderer.SharedConfig
 	policyManager *mockpolicies.Manager
 }
 
@@ -101,18 +98,13 @@ func NewRAMLedger() ledger.ReadWriter {
 	return rl
 }
 
-func (mcs *mockSupport) SharedConfig() configvaluesapi.Orderer {
-	return mcs.sharedConfig
-}
-
 func newMockMultichainManager() *mockSupportManager {
 	rl := NewRAMLedger()
 	mm := &mockSupportManager{
 		chains: make(map[string]*mockSupport),
 	}
 	mm.chains[systemChainID] = &mockSupport{
 		ledger:        rl,
-		sharedConfig:  &mockconfigvaluesorderer.SharedConfig{EgressPolicyNamesVal: []string{"somePolicy"}},
 		policyManager: &mockpolicies.Manager{Policy: &mockpolicies.Policy{}},
 	}
 	return mm

orderer/common/sigfilter/sigfilter.go

@@ -27,7 +27,7 @@ import (
 var logger = logging.MustGetLogger("orderer/common/sigfilter")
 
 type sigFilter struct {
-	policySource  func() []string
+	policySource  string
 	policyManager policies.Manager
 }
 
@@ -36,7 +36,7 @@ type sigFilter struct {
 // In general, both the policy name and the policy itself are mutable, this is why
 // not only the policy is retrieved at each invocation, but also the name of which
 // policy to retrieve
-func New(policySource func() []string, policyManager policies.Manager) filter.Rule {
+func New(policySource string, policyManager policies.Manager) filter.Rule {
 	return &sigFilter{
 		policySource:  policySource,
 		policyManager: policyManager,
@@ -54,24 +54,22 @@ func (sf *sigFilter) Apply(message *cb.Envelope) (filter.Action, filter.Committe
 		return filter.Reject, nil
 	}
 
-	for _, policy := range sf.policySource() {
-		policy, ok := sf.policyManager.GetPolicy(policy)
-		if !ok {
-			logger.Debugf("Could not find policy %s", policy)
-			continue
+	policy, ok := sf.policyManager.GetPolicy(sf.policySource)
+	if !ok {
+		if logger.IsEnabledFor(logging.DEBUG) {
+			logger.Debugf("Could not find policy %s", sf.policySource)
 		}
+		return filter.Reject, nil
+	}
 
-		err = policy.Evaluate(signedData)
+	err = policy.Evaluate(signedData)
 
-		if err == nil {
-			logger.Debugf("Accepting validly signed message for policy %s", policy)
-			return filter.Forward, nil
+	if err == nil {
+		if logger.IsEnabledFor(logging.DEBUG) {
+			logger.Debugf("Forwarding validly signed message for policy %s", policy)
 		}
-
+		return filter.Forward, nil
 	}
 
-	if logger.IsEnabledFor(logging.DEBUG) {
-		logger.Debugf("Rejecting message because it was not appropriately signed for any allowed policy among %s", sf.policySource())
-	}
 	return filter.Reject, nil
 }

orderer/common/sigfilter/sigfilter_test.go

@@ -42,13 +42,9 @@ func makeEnvelope() *cb.Envelope {
 	}
 }
 
-func fooSource() []string {
-	return []string{"foo"}
-}
-
 func TestAccept(t *testing.T) {
 	mpm := &mockpolicies.Manager{Policy: &mockpolicies.Policy{}}
-	sf := New(fooSource, mpm)
+	sf := New("foo", mpm)
 	result, _ := sf.Apply(makeEnvelope())
 	if result != filter.Forward {
 		t.Fatalf("Should have accepted envelope")
@@ -57,7 +53,7 @@ func TestAccept(t *testing.T) {
 
 func TestMissingPolicy(t *testing.T) {
 	mpm := &mockpolicies.Manager{}
-	sf := New(fooSource, mpm)
+	sf := New("foo", mpm)
 	result, _ := sf.Apply(makeEnvelope())
 	if result != filter.Reject {
 		t.Fatalf("Should have rejected when missing policy")
@@ -66,7 +62,7 @@ func TestMissingPolicy(t *testing.T) {
 
 func TestEmptyPayload(t *testing.T) {
 	mpm := &mockpolicies.Manager{Policy: &mockpolicies.Policy{}}
-	sf := New(fooSource, mpm)
+	sf := New("foo", mpm)
 	result, _ := sf.Apply(&cb.Envelope{})
 	if result != filter.Reject {
 		t.Fatalf("Should have rejected when payload empty")
@@ -75,7 +71,7 @@ func TestEmptyPayload(t *testing.T) {
 
 func TestErrorOnPolicy(t *testing.T) {
 	mpm := &mockpolicies.Manager{Policy: &mockpolicies.Policy{Err: fmt.Errorf("Error")}}
-	sf := New(fooSource, mpm)
+	sf := New("foo", mpm)
 	result, _ := sf.Apply(makeEnvelope())
 	if result != filter.Reject {
 		t.Fatalf("Should have rejected when policy evaluated to err")

orderer/multichain/chainsupport.go

@@ -145,7 +145,7 @@ func createStandardFilters(ledgerResources *ledgerResources) *filter.RuleSet {
 	return filter.NewRuleSet([]filter.Rule{
 		filter.EmptyRejectRule,
 		sizefilter.MaxBytesRule(ledgerResources.SharedConfig().BatchSize().AbsoluteMaxBytes),
-		sigfilter.New(ledgerResources.SharedConfig().IngressPolicyNames, ledgerResources.PolicyManager()),
+		sigfilter.New(policies.ChannelWriters, ledgerResources.PolicyManager()),
 		configtxfilter.NewFilter(ledgerResources),
 		filter.AcceptRule,
 	})
@@ -157,7 +157,7 @@ func createSystemChainFilters(ml *multiLedger, ledgerResources *ledgerResources)
 	return filter.NewRuleSet([]filter.Rule{
 		filter.EmptyRejectRule,
 		sizefilter.MaxBytesRule(ledgerResources.SharedConfig().BatchSize().AbsoluteMaxBytes),
-		sigfilter.New(ledgerResources.SharedConfig().IngressPolicyNames, ledgerResources.PolicyManager()),
+		sigfilter.New(policies.ChannelWriters, ledgerResources.PolicyManager()),
 		newSystemChainFilter(ledgerResources, ml),
 		configtxfilter.NewFilter(ledgerResources),
 		filter.AcceptRule,

orderer/multichain/manager_test.go

@@ -22,7 +22,6 @@ import (
 	"time"
 
 	"github.com/hyperledger/fabric/common/configtx"
-	configtxtest "github.com/hyperledger/fabric/common/configtx/test"
 	genesisconfig "github.com/hyperledger/fabric/common/configtx/tool/localconfig"
 	"github.com/hyperledger/fabric/common/configtx/tool/provisional"
 	mockcrypto "github.com/hyperledger/fabric/common/mocks/crypto"
@@ -241,7 +240,7 @@ func TestNewChain(t *testing.T) {
 
 	newChainID := "TestNewChain"
 
-	configEnv, err := configtx.NewChainCreationTemplate(provisional.AcceptAllPolicyKey, configtxtest.CompositeTemplate()).Envelope(newChainID)
+	configEnv, err := configtx.NewChainCreationTemplate(provisional.AcceptAllPolicyKey, provisional.New(conf).ChannelTemplate()).Envelope(newChainID)
 	if err != nil {
 		t.Fatalf("Error constructing configtx")
 	}