From 3a61f6b61be404eb778d5d1be5179795eca218d0 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Thu, 16 Feb 2017 01:43:08 -0500 Subject: [PATCH] [FAB-2261] Make Handler creation transactional https://jira.hyperledger.org/browse/FAB-2261 At the organization level, handlers were being instaniated dynamically, but, the handlers were only being instructed to begin transactions statically. This caused a crash when a dynamically created org handler would be told to process a config item before begin the config transaction. This CR reworks the handler instantiation to be done at the beginning of a transaction. Change-Id: Ifbd2f9adf5809cb9fca3108573f105db4b0d790a Signed-off-by: Jason Yellick --- common/cauthdsl/policy_test.go | 2 +- common/configtx/api/api.go | 14 +- .../handlers/application/organization.go | 9 +- .../handlers/application/organization_test.go | 8 +- .../handlers/application/sharedconfig.go | 32 ++-- .../handlers/application/sharedconfig_test.go | 4 +- .../configtx/handlers/channel/sharedconfig.go | 39 ++-- .../handlers/channel/sharedconfig_test.go | 10 +- common/configtx/handlers/msp/config.go | 10 - .../configtx/handlers/orderer/sharedconfig.go | 34 ++-- .../handlers/orderer/sharedconfig_test.go | 28 +-- common/configtx/handlers/organization.go | 16 +- common/configtx/handlers/organization_test.go | 6 +- common/configtx/initializer.go | 50 ++--- common/configtx/manager.go | 172 ++++++++++++------ common/configtx/manager_test.go | 13 +- common/mocks/configtx/configtx.go | 27 +-- common/policies/policy.go | 6 +- 18 files changed, 264 insertions(+), 216 deletions(-) diff --git a/common/cauthdsl/policy_test.go b/common/cauthdsl/policy_test.go index ffb75d23a8f..e1acf92e0d4 100644 --- a/common/cauthdsl/policy_test.go +++ b/common/cauthdsl/policy_test.go @@ -57,7 +57,7 @@ func makePolicySource(policyResult bool) *cb.Policy { } func addPolicy(manager *policies.ManagerImpl, id string, policy *cb.Policy) { - manager.BeginConfig() + manager.BeginConfig(nil) err := manager.ProposePolicy(id, []string{}, &cb.ConfigPolicy{ Policy: policy, }) diff --git a/common/configtx/api/api.go b/common/configtx/api/api.go index 040cd1b6d2d..aa16397a9c2 100644 --- a/common/configtx/api/api.go +++ b/common/configtx/api/api.go @@ -82,6 +82,8 @@ 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 { + Transactional + // ProposeConfig called when config is added to a proposal ProposeConfig(key string, configValue *cb.ConfigValue) error } @@ -129,7 +131,7 @@ type Resources 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() + BeginConfig(groups []string) ([]Handler, error) // RollbackConfig called when a config proposal is abandoned RollbackConfig() @@ -138,14 +140,6 @@ type Transactional interface { 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 @@ -156,7 +150,7 @@ type PolicyHandler interface { // Initializer is used as indirection between Manager and Handler to allow // for single Handlers to handle multiple paths type Initializer interface { - SubInitializer + Handler Resources diff --git a/common/configtx/handlers/application/organization.go b/common/configtx/handlers/application/organization.go index 0ee97291287..2c066f012b8 100644 --- a/common/configtx/handlers/application/organization.go +++ b/common/configtx/handlers/application/organization.go @@ -19,6 +19,7 @@ package application import ( "fmt" + "github.com/hyperledger/fabric/common/configtx/api" "github.com/hyperledger/fabric/common/configtx/handlers" mspconfig "github.com/hyperledger/fabric/common/configtx/handlers/msp" cb "github.com/hyperledger/fabric/protos/common" @@ -62,18 +63,23 @@ func (oc *ApplicationOrgConfig) AnchorPeers() []*pb.AnchorPeer { } // BeginConfig is used to start a new config proposal -func (oc *ApplicationOrgConfig) BeginConfig() { +func (oc *ApplicationOrgConfig) BeginConfig(groups []string) ([]api.Handler, error) { logger.Debugf("Beginning a possible new org config") + if len(groups) != 0 { + return nil, fmt.Errorf("ApplicationGroup does not support subgroups") + } if oc.pendingConfig != nil { logger.Panicf("Programming error, cannot call begin in the middle of a proposal") } oc.pendingConfig = &applicationOrgConfig{} + return oc.OrgConfig.BeginConfig(groups) } // RollbackConfig is used to abandon a new config proposal func (oc *ApplicationOrgConfig) RollbackConfig() { logger.Debugf("Rolling back proposed org config") oc.pendingConfig = nil + oc.OrgConfig.RollbackConfig() } // CommitConfig is used to commit a new config proposal @@ -84,6 +90,7 @@ func (oc *ApplicationOrgConfig) CommitConfig() { } oc.config = oc.pendingConfig oc.pendingConfig = nil + oc.OrgConfig.CommitConfig() } // ProposeConfig is used to add new config to the config proposal diff --git a/common/configtx/handlers/application/organization_test.go b/common/configtx/handlers/application/organization_test.go index 51429916bd1..9406004e7e4 100644 --- a/common/configtx/handlers/application/organization_test.go +++ b/common/configtx/handlers/application/organization_test.go @@ -47,13 +47,13 @@ func groupToKeyValue(configGroup *cb.ConfigGroup) (string, *cb.ConfigValue) { } func TestApplicationOrgInterface(t *testing.T) { - _ = configtxapi.SubInitializer(NewApplicationOrgConfig("id", nil)) + _ = configtxapi.Handler(NewApplicationOrgConfig("id", nil)) } func TestApplicationOrgDoubleBegin(t *testing.T) { m := NewApplicationOrgConfig("id", nil) - m.BeginConfig() - assert.Panics(t, m.BeginConfig, "Two begins back to back should have caused a panic") + m.BeginConfig(nil) + assert.Panics(t, func() { m.BeginConfig(nil) }, "Two begins back to back should have caused a panic") } func TestApplicationOrgCommitWithoutBegin(t *testing.T) { @@ -76,7 +76,7 @@ func TestApplicationOrgAnchorPeers(t *testing.T) { invalidMessage := makeInvalidConfigValue() validMessage := TemplateAnchorPeers("id", endVal) m := NewApplicationOrgConfig("id", nil) - m.BeginConfig() + m.BeginConfig(nil) assert.Error(t, m.ProposeConfig(AnchorPeersKey, invalidMessage), "Should have failed on invalid message") assert.NoError(t, m.ProposeConfig(groupToKeyValue(validMessage)), "Should not have failed on invalid message") diff --git a/common/configtx/handlers/application/sharedconfig.go b/common/configtx/handlers/application/sharedconfig.go index c8dda514df9..095163d0d93 100644 --- a/common/configtx/handlers/application/sharedconfig.go +++ b/common/configtx/handlers/application/sharedconfig.go @@ -17,8 +17,6 @@ limitations under the License. package application import ( - "fmt" - "github.com/hyperledger/fabric/common/configtx/api" "github.com/hyperledger/fabric/common/configtx/handlers" "github.com/hyperledger/fabric/common/configtx/handlers/msp" @@ -77,7 +75,7 @@ func NewSharedConfigImpl(mspConfig *msp.MSPConfigHandler) *SharedConfigImpl { } // BeginConfig is used to start a new config proposal -func (di *SharedConfigImpl) BeginConfig() { +func (di *SharedConfigImpl) BeginConfig(groups []string) ([]api.Handler, error) { logger.Debugf("Beginning a possible new peer shared config") if di.pendingConfig != nil { logger.Panicf("Programming error, cannot call begin in the middle of a proposal") @@ -85,6 +83,16 @@ func (di *SharedConfigImpl) BeginConfig() { di.pendingConfig = &sharedConfig{ orgs: make(map[string]api.ApplicationOrgConfig), } + orgHandlers := make([]api.Handler, len(groups)) + for i, group := range groups { + org, ok := di.pendingConfig.orgs[group] + if !ok { + org = NewApplicationOrgConfig(group, di.mspConfig) + di.pendingConfig.orgs[group] = org + } + orgHandlers[i] = org.(*ApplicationOrgConfig) + } + return orgHandlers, nil } // RollbackConfig is used to abandon a new config proposal @@ -113,21 +121,3 @@ func (di *SharedConfigImpl) ProposeConfig(key string, configValue *cb.ConfigValu func (di *SharedConfigImpl) Organizations() map[string]api.ApplicationOrgConfig { return di.config.orgs } - -// Handler returns the associated api.Handler for the given path -func (pm *SharedConfigImpl) Handler(path []string) (api.Handler, error) { - if len(path) == 0 { - return pm, nil - } - - if len(path) > 1 { - return nil, fmt.Errorf("Application group allows only one further level of nesting") - } - - org, ok := pm.pendingConfig.orgs[path[0]] - if !ok { - org = NewApplicationOrgConfig(path[0], pm.mspConfig) - pm.pendingConfig.orgs[path[0]] = org - } - return org.(*ApplicationOrgConfig), nil -} diff --git a/common/configtx/handlers/application/sharedconfig_test.go b/common/configtx/handlers/application/sharedconfig_test.go index 8938812110e..50239ed75f5 100644 --- a/common/configtx/handlers/application/sharedconfig_test.go +++ b/common/configtx/handlers/application/sharedconfig_test.go @@ -40,8 +40,8 @@ func TestApplicationDoubleBegin(t *testing.T) { }() m := NewSharedConfigImpl(nil) - m.BeginConfig() - m.BeginConfig() + m.BeginConfig(nil) + m.BeginConfig(nil) } func TestApplicationCommitWithoutBegin(t *testing.T) { diff --git a/common/configtx/handlers/channel/sharedconfig.go b/common/configtx/handlers/channel/sharedconfig.go index 4c6c17ca390..69bfdd2eded 100644 --- a/common/configtx/handlers/channel/sharedconfig.go +++ b/common/configtx/handlers/channel/sharedconfig.go @@ -106,11 +106,26 @@ func (pm *SharedConfigImpl) OrdererAddresses() []string { } // BeginConfig is used to start a new config proposal -func (pm *SharedConfigImpl) BeginConfig() { +func (pm *SharedConfigImpl) BeginConfig(groups []string) ([]api.Handler, error) { + handlers := make([]api.Handler, len(groups)) + + for i, group := range groups { + switch group { + case application.GroupKey: + handlers[i] = pm.applicationConfig + case orderer.GroupKey: + handlers[i] = pm.ordererConfig + default: + return nil, fmt.Errorf("Disallowed channel group: %s", group) + } + } + if pm.pendingConfig != nil { logger.Panicf("Programming error, cannot call begin in the middle of a proposal") } + pm.pendingConfig = &chainConfig{} + return handlers, nil } // RollbackConfig is used to abandon a new config proposal @@ -163,25 +178,3 @@ func (pm *SharedConfigImpl) ProposeConfig(key string, configValue *cb.ConfigValu } return nil } - -// Handler returns the associated api.Handler for the given path -func (pm *SharedConfigImpl) Handler(path []string) (api.Handler, error) { - if len(path) == 0 { - return pm, nil - } - - var initializer api.SubInitializer - - switch path[0] { - case application.GroupKey: - initializer = pm.applicationConfig - case orderer.GroupKey: - initializer = pm.ordererConfig - default: - return nil, fmt.Errorf("Disallowed channel group: %s", path[0]) - } - - return initializer.Handler(path[1:]) - - return nil, fmt.Errorf("Unallowed group") -} diff --git a/common/configtx/handlers/channel/sharedconfig_test.go b/common/configtx/handlers/channel/sharedconfig_test.go index 615f9a4aa0e..691d28bc543 100644 --- a/common/configtx/handlers/channel/sharedconfig_test.go +++ b/common/configtx/handlers/channel/sharedconfig_test.go @@ -55,8 +55,8 @@ func TestDoubleBegin(t *testing.T) { }() m := NewSharedConfigImpl(nil, nil) - m.BeginConfig() - m.BeginConfig() + m.BeginConfig(nil) + m.BeginConfig(nil) } func TestCommitWithoutBegin(t *testing.T) { @@ -85,7 +85,7 @@ func TestHashingAlgorithm(t *testing.T) { validAlgorithm := DefaultHashingAlgorithm() m := NewSharedConfigImpl(nil, nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(HashingAlgorithmKey, invalidMessage) if err == nil { @@ -115,7 +115,7 @@ func TestBlockDataHashingStructure(t *testing.T) { validWidth := DefaultBlockDataHashingStructure() m := NewSharedConfigImpl(nil, nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(BlockDataHashingStructureKey, invalidMessage) if err == nil { @@ -143,7 +143,7 @@ func TestOrdererAddresses(t *testing.T) { invalidMessage := makeInvalidConfigValue() validMessage := DefaultOrdererAddresses() m := NewSharedConfigImpl(nil, nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(OrdererAddressesKey, invalidMessage) if err == nil { diff --git a/common/configtx/handlers/msp/config.go b/common/configtx/handlers/msp/config.go index 4258a2a284c..091c9de48da 100644 --- a/common/configtx/handlers/msp/config.go +++ b/common/configtx/handlers/msp/config.go @@ -21,7 +21,6 @@ import ( "reflect" "github.com/golang/protobuf/proto" - "github.com/hyperledger/fabric/common/configtx/api" "github.com/hyperledger/fabric/msp" "github.com/hyperledger/fabric/protos/common" mspprotos "github.com/hyperledger/fabric/protos/msp" @@ -83,12 +82,3 @@ func (bh *MSPConfigHandler) ProposeConfig(key string, configValue *common.Config bh.proposedMgr = msp.NewMSPManager() return bh.proposedMgr.Setup(bh.config) } - -// Handler returns the associated api.Handler for the given path -func (bh *MSPConfigHandler) Handler(path []string) (api.Handler, error) { - if len(path) > 0 { - return nil, fmt.Errorf("MSP handler does not support nested groups") - } - - return bh, nil -} diff --git a/common/configtx/handlers/orderer/sharedconfig.go b/common/configtx/handlers/orderer/sharedconfig.go index 4e7f17261c5..8e66cbe3776 100644 --- a/common/configtx/handlers/orderer/sharedconfig.go +++ b/common/configtx/handlers/orderer/sharedconfig.go @@ -157,14 +157,24 @@ func (pm *ManagerImpl) EgressPolicyNames() []string { } // BeginConfig is used to start a new config proposal -func (pm *ManagerImpl) BeginConfig() { - logger.Debugf("Beginning possible new orderer config") +func (pm *ManagerImpl) BeginConfig(groups []string) ([]api.Handler, error) { + logger.Debugf("Beginning a possible new orderer shared config") if pm.pendingConfig != nil { - logger.Fatalf("Programming error, cannot call begin in the middle of a proposal") + logger.Panicf("Programming error, cannot call begin in the middle of a proposal") } pm.pendingConfig = &ordererConfig{ orgs: make(map[string]*handlers.OrgConfig), } + orgHandlers := make([]api.Handler, len(groups)) + for i, group := range groups { + org, ok := pm.pendingConfig.orgs[group] + if !ok { + org = handlers.NewOrgConfig(group, pm.mspConfig) + pm.pendingConfig.orgs[group] = org + } + orgHandlers[i] = org + } + return orgHandlers, nil } // RollbackConfig is used to abandon a new config proposal @@ -275,24 +285,6 @@ func (pm *ManagerImpl) ProposeConfig(key string, configValue *cb.ConfigValue) er return nil } -// Handler returns the associated api.Handler for the given path -func (pm *ManagerImpl) Handler(path []string) (api.Handler, error) { - if len(path) == 0 { - return pm, nil - } - - if len(path) > 1 { - return nil, fmt.Errorf("Orderer group allows only one further level of nesting") - } - - org, ok := pm.pendingConfig.orgs[path[0]] - if !ok { - org = handlers.NewOrgConfig(path[0], pm.mspConfig) - pm.pendingConfig.orgs[path[0]] = org - } - return org, nil -} - // This does just a barebones sanity check. func brokerEntrySeemsValid(broker string) bool { if !strings.Contains(broker, ":") { diff --git a/common/configtx/handlers/orderer/sharedconfig_test.go b/common/configtx/handlers/orderer/sharedconfig_test.go index 0cb2ba60291..7e7d50e777b 100644 --- a/common/configtx/handlers/orderer/sharedconfig_test.go +++ b/common/configtx/handlers/orderer/sharedconfig_test.go @@ -66,8 +66,8 @@ func doesFuncCrash(crasher func(), test string) bool { func TestDoubleBegin(t *testing.T) { crashes := doesFuncCrash(func() { m := NewManagerImpl(nil) - m.BeginConfig() - m.BeginConfig() + m.BeginConfig(nil) + m.BeginConfig(nil) }, "TestDoubleBegin") if !crashes { @@ -102,7 +102,7 @@ func TestConsensusType(t *testing.T) { otherValidMessage := TemplateConsensusType("bar") m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(groupToKeyValue(validMessage)) if err != nil { @@ -110,7 +110,7 @@ func TestConsensusType(t *testing.T) { } m.CommitConfig() - m.BeginConfig() + m.BeginConfig(nil) err = m.ProposeConfig(ConsensusTypeKey, invalidMessage) if err == nil { @@ -142,7 +142,7 @@ func TestBatchSize(t *testing.T) { t.Run("ValidConfig", func(t *testing.T) { m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig( groupToKeyValue(TemplateBatchSize(&ab.BatchSize{MaxMessageCount: validMaxMessageCount, AbsoluteMaxBytes: validAbsoluteMaxBytes, PreferredMaxBytes: validPreferredMaxBytes})), ) @@ -161,7 +161,7 @@ func TestBatchSize(t *testing.T) { t.Run("UnserializableConfig", func(t *testing.T) { m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(BatchSizeKey, invalidMessage()) assert.NotNil(t, err, "Should have failed on invalid message") m.CommitConfig() @@ -169,7 +169,7 @@ func TestBatchSize(t *testing.T) { t.Run("ZeroMaxMessageCount", func(t *testing.T) { m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(groupToKeyValue(TemplateBatchSize(&ab.BatchSize{MaxMessageCount: 0, AbsoluteMaxBytes: validAbsoluteMaxBytes, PreferredMaxBytes: validPreferredMaxBytes}))) assert.NotNil(t, err, "Should have rejected batch size max message count of 0") m.CommitConfig() @@ -177,7 +177,7 @@ func TestBatchSize(t *testing.T) { t.Run("ZeroAbsoluteMaxBytes", func(t *testing.T) { m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(groupToKeyValue(TemplateBatchSize(&ab.BatchSize{MaxMessageCount: validMaxMessageCount, AbsoluteMaxBytes: 0, PreferredMaxBytes: validPreferredMaxBytes}))) assert.NotNil(t, err, "Should have rejected batch size absolute max message bytes of 0") m.CommitConfig() @@ -185,7 +185,7 @@ func TestBatchSize(t *testing.T) { t.Run("TooLargePreferredMaxBytes", func(t *testing.T) { m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(groupToKeyValue(TemplateBatchSize(&ab.BatchSize{MaxMessageCount: validMaxMessageCount, AbsoluteMaxBytes: validAbsoluteMaxBytes, PreferredMaxBytes: validAbsoluteMaxBytes + 1}))) assert.NotNil(t, err, "Should have rejected batch size preferred max message bytes greater than absolute max message bytes") m.CommitConfig() @@ -200,7 +200,7 @@ func TestBatchTimeout(t *testing.T) { validMessage := TemplateBatchTimeout(endBatchTimeout.String()) m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(groupToKeyValue(validMessage)) if err != nil { @@ -243,7 +243,7 @@ func TestKafkaBrokers(t *testing.T) { validMessage := TemplateKafkaBrokers(endList) m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(groupToKeyValue(validMessage)) if err != nil { @@ -283,7 +283,7 @@ func testPolicyNames(m *ManagerImpl, key string, initializer func(val []string) invalidMessage := invalidMessage() validMessage := initializer(endPolicy) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(groupToKeyValue(validMessage)) if err != nil { @@ -291,7 +291,7 @@ func testPolicyNames(m *ManagerImpl, key string, initializer func(val []string) } m.CommitConfig() - m.BeginConfig() + m.BeginConfig(nil) err = m.ProposeConfig(key, invalidMessage) if err == nil { @@ -328,7 +328,7 @@ func TestChainCreationPolicyNames(t *testing.T) { func TestEmptyChainCreationPolicyNames(t *testing.T) { m := NewManagerImpl(nil) - m.BeginConfig() + m.BeginConfig(nil) err := m.ProposeConfig(groupToKeyValue(TemplateChainCreationPolicyNames(nil))) if err != nil { diff --git a/common/configtx/handlers/organization.go b/common/configtx/handlers/organization.go index 93b113ecbfe..c4a2290ee63 100644 --- a/common/configtx/handlers/organization.go +++ b/common/configtx/handlers/organization.go @@ -59,12 +59,17 @@ func NewOrgConfig(id string, mspConfig *mspconfig.MSPConfigHandler) *OrgConfig { } // BeginConfig is used to start a new config proposal -func (oc *OrgConfig) BeginConfig() { +func (oc *OrgConfig) BeginConfig(groups []string) ([]api.Handler, error) { logger.Debugf("Beginning a possible new org config") + if len(groups) != 0 { + return nil, fmt.Errorf("Orgs do not support sub-groups") + } + if oc.pendingConfig != nil { logger.Panicf("Programming error, cannot call begin in the middle of a proposal") } oc.pendingConfig = &orgConfig{} + return nil, nil } // RollbackConfig is used to abandon a new config proposal @@ -94,12 +99,3 @@ func (oc *OrgConfig) ProposeConfig(key string, configValue *cb.ConfigValue) erro } return nil } - -// Handler returns the associated api.Handler for the given path -func (oc *OrgConfig) Handler(path []string) (api.Handler, error) { - if len(path) == 0 { - return oc, nil - } - - return nil, fmt.Errorf("Organizations do not further nesting") -} diff --git a/common/configtx/handlers/organization_test.go b/common/configtx/handlers/organization_test.go index eabe174a6e2..b204ab5f553 100644 --- a/common/configtx/handlers/organization_test.go +++ b/common/configtx/handlers/organization_test.go @@ -29,7 +29,7 @@ func init() { } func TestInterface(t *testing.T) { - _ = configtxapi.SubInitializer(NewOrgConfig("id", nil)) + _ = configtxapi.Handler(NewOrgConfig("id", nil)) } func TestDoubleBegin(t *testing.T) { @@ -40,8 +40,8 @@ func TestDoubleBegin(t *testing.T) { }() m := NewOrgConfig("id", nil) - m.BeginConfig() - m.BeginConfig() + m.BeginConfig(nil) + m.BeginConfig(nil) } func TestCommitWithoutBegin(t *testing.T) { diff --git a/common/configtx/initializer.go b/common/configtx/initializer.go index a5e093b2ad3..0e6e7533d71 100644 --- a/common/configtx/initializer.go +++ b/common/configtx/initializer.go @@ -105,46 +105,48 @@ func NewInitializer() api.Initializer { } // BeginConfig is used to start a new config proposal -func (i *initializer) BeginConfig() { - i.policyManager.BeginConfig() - i.channelConfig.BeginConfig() - i.ordererConfig.BeginConfig() - i.applicationConfig.BeginConfig() +func (i *initializer) BeginConfig(groups []string) ([]api.Handler, error) { + if len(groups) != 1 { + logger.Panicf("Initializer only supports having one root group") + } i.mspConfigHandler.BeginConfig() + return []api.Handler{i.channelConfig}, nil } // RollbackConfig is used to abandon a new config proposal func (i *initializer) RollbackConfig() { - i.policyManager.RollbackConfig() - i.channelConfig.RollbackConfig() - i.ordererConfig.RollbackConfig() - i.applicationConfig.RollbackConfig() i.mspConfigHandler.RollbackConfig() } // CommitConfig is used to commit a new config proposal func (i *initializer) CommitConfig() { - i.policyManager.CommitConfig() - i.channelConfig.CommitConfig() - i.ordererConfig.CommitConfig() - i.applicationConfig.CommitConfig() i.mspConfigHandler.CommitConfig() } -func (i *initializer) PolicyHandler() api.PolicyHandler { - return i.policyManager +type importHack struct { + *policies.ManagerImpl } -func (i *initializer) Handler(path []string) (api.Handler, error) { - if len(path) == 0 { - return nil, fmt.Errorf("Empty path") +func (ih importHack) BeginConfig(groups []string) ([]api.Handler, error) { + policyManagers, err := ih.ManagerImpl.BeginConfig(groups) + if err != nil { + return nil, err } - - switch path[0] { - case RootGroupKey: - return i.channelConfig.Handler(path[1:]) - default: - return nil, fmt.Errorf("Unknown root group: %s", path[0]) + handlers := make([]api.Handler, len(policyManagers)) + for i, policyManager := range policyManagers { + handlers[i] = &importHack{ManagerImpl: policyManager} } + return handlers, err +} + +func (ih importHack) ProposeConfig(key string, value *cb.ConfigValue) error { + return fmt.Errorf("Temporary hack") +} + +func (i *initializer) PolicyHandler() api.PolicyHandler { + return importHack{ManagerImpl: i.policyManager} +} +func (i *initializer) ProposeConfig(key string, value *cb.ConfigValue) error { + return fmt.Errorf("Programming error, this should never be invoked") } diff --git a/common/configtx/manager.go b/common/configtx/manager.go index 8f0d127348f..6f70baf70b4 100644 --- a/common/configtx/manager.go +++ b/common/configtx/manager.go @@ -43,10 +43,23 @@ var ( // NewConfigItemPolicyKey is the ID of the policy used when no other policy can be resolved, for instance when attempting to create a new config item const NewConfigItemPolicyKey = "NewConfigItemPolicy" -type acceptAllPolicy struct{} +type configResult struct { + handler api.Transactional + subResults []*configResult +} -func (ap *acceptAllPolicy) Evaluate(signedData []*cb.SignedData) error { - return nil +func (cr *configResult) commit() { + for _, subResult := range cr.subResults { + subResult.commit() + } + cr.handler.CommitConfig() +} + +func (cr *configResult) rollback() { + for _, subResult := range cr.subResults { + subResult.rollback() + } + cr.handler.RollbackConfig() } type configManager struct { @@ -137,54 +150,80 @@ func NewManagerImpl(configEnv *cb.ConfigEnvelope, initializer api.Initializer, c callOnUpdate: callOnUpdate, } - cm.beginHandlers() - if err := cm.proposeConfig(configMap); err != nil { - cm.rollbackHandlers() + result, err := cm.processConfig(configEnv.Config.Channel) + if err != nil { return nil, err } - cm.commitHandlers() + result.commit() + cm.commitCallbacks() return cm, nil } -func (cm *configManager) beginHandlers() { - logger.Debugf("Beginning new config for chain %s", cm.chainID) - cm.initializer.BeginConfig() +func (cm *configManager) commitCallbacks() { + for _, callback := range cm.callOnUpdate { + callback(cm) + } } -func (cm *configManager) rollbackHandlers() { - logger.Debugf("Rolling back config for chain %s", cm.chainID) - cm.initializer.RollbackConfig() -} +// proposeGroup proposes a group configuration with a given handler +// it will in turn recursively call itself until all groups have been exhausted +// at each call, it returns the handler that was passed in, plus any handlers returned +// by recursive calls into proposeGroup +func (cm *configManager) proposeGroup(name string, group *cb.ConfigGroup, handler api.Handler) (*configResult, error) { + subGroups := make([]string, len(group.Groups)) + i := 0 + for subGroup := range group.Groups { + subGroups[i] = subGroup + i++ + } -func (cm *configManager) commitHandlers() { - logger.Debugf("Committing config for chain %s", cm.chainID) - cm.initializer.CommitConfig() - for _, callback := range cm.callOnUpdate { - callback(cm) + logger.Debugf("Beginning new config for channel %s and group %s", cm.chainID, name) + subHandlers, err := handler.BeginConfig(subGroups) + if err != nil { + return nil, err } + + if len(subHandlers) != len(subGroups) { + return nil, fmt.Errorf("Programming error, did not return as many handlers as groups %d vs %d", len(subHandlers), len(subGroups)) + } + + result := &configResult{ + handler: handler, + subResults: make([]*configResult, 0, len(subGroups)), + } + + for i, subGroup := range subGroups { + subResult, err := cm.proposeGroup(name+"/"+subGroup, group.Groups[subGroup], subHandlers[i]) + if err != nil { + result.rollback() + return nil, err + } + result.subResults = append(result.subResults, subResult) + } + + for key, value := range group.Values { + if err := handler.ProposeConfig(key, value); err != nil { + result.rollback() + return nil, err + } + } + + return result, nil } -func (cm *configManager) proposeConfig(config map[string]comparable) error { - for fqPath, c := range config { - logger.Debugf("Proposing: %s", fqPath) - switch { - case c.ConfigValue != nil: - valueHandler, err := cm.initializer.Handler(c.path) - if err != nil { - return err - } - if err := valueHandler.ProposeConfig(c.key, c.ConfigValue); err != nil { - return err - } - case c.ConfigPolicy != nil: - if err := cm.initializer.PolicyHandler().ProposePolicy(c.key, c.path, c.ConfigPolicy); err != nil { - return err - } +func (cm *configManager) proposePolicies(rootGroup *cb.ConfigGroup) (*configResult, error) { + cm.initializer.PolicyHandler().BeginConfig(nil) // XXX temporary workaround until policy manager is adapted with sub-policies + + for key, policy := range rootGroup.Policies { + logger.Debugf("Proposing policy: %s", key) + if err := cm.initializer.PolicyHandler().ProposePolicy(key, []string{RootGroupKey}, policy); err != nil { + cm.initializer.PolicyHandler().RollbackConfig() + return nil, err } } - return nil + return &configResult{handler: cm.initializer.PolicyHandler()}, nil } // authorizeUpdate validates that all modified config has the corresponding modification policies satisfied by the signature set @@ -278,7 +317,7 @@ func (cm *configManager) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop } - return configMap, nil + return cm.computeUpdateResult(configMap), nil } // computeUpdateResult takes a configMap generated by an update and produces a new configMap overlaying it onto the old config @@ -294,17 +333,22 @@ func (cm *configManager) computeUpdateResult(updatedConfig map[string]comparable return newConfigMap } -func (cm *configManager) processConfig(configtx *cb.ConfigUpdateEnvelope) (map[string]comparable, error) { - cm.beginHandlers() - configMap, err := cm.authorizeUpdate(configtx) +func (cm *configManager) processConfig(channelGroup *cb.ConfigGroup) (*configResult, error) { + helperGroup := cb.NewConfigGroup() + helperGroup.Groups[RootGroupKey] = channelGroup + groupResult, err := cm.proposeGroup("", helperGroup, cm.initializer) if err != nil { return nil, err } - computedResult := cm.computeUpdateResult(configMap) - if err := cm.proposeConfig(computedResult); err != nil { + + policyResult, err := cm.proposePolicies(channelGroup) + if err != nil { + groupResult.rollback() return nil, err } - return computedResult, nil + policyResult.subResults = []*configResult{groupResult} + + return policyResult, nil } func envelopeToConfigUpdate(configtx *cb.Envelope) (*cb.ConfigUpdateEnvelope, error) { @@ -335,9 +379,24 @@ func (cm *configManager) Validate(configtx *cb.Envelope) error { if err != nil { return err } - _, err = cm.processConfig(configUpdateEnv) - cm.rollbackHandlers() - return err + + configMap, err := cm.authorizeUpdate(configUpdateEnv) + if err != nil { + return err + } + + channelGroup, err := configMapToConfig(configMap) + if err != nil { + return fmt.Errorf("Could not turn configMap back to channelGroup: %s", err) + } + + result, err := cm.processConfig(channelGroup) + if err != nil { + return err + } + + result.rollback() + return nil } // Apply attempts to apply a configtx to become the new config @@ -346,19 +405,28 @@ func (cm *configManager) Apply(configtx *cb.Envelope) error { if err != nil { return err } - configMap, err := cm.processConfig(configUpdateEnv) + + configMap, err := cm.authorizeUpdate(configUpdateEnv) if err != nil { - cm.rollbackHandlers() return err } - cm.config = configMap - cm.sequence++ - cm.commitHandlers() + channelGroup, err := configMapToConfig(configMap) if err != nil { - logger.Panicf("Config was validated and applied, but could not be transformed back into proto form: %s", err) + return fmt.Errorf("Could not turn configMap back to channelGroup: %s", err) } + result, err := cm.processConfig(channelGroup) + if err != nil { + return err + } + + result.commit() + cm.commitCallbacks() + + cm.config = configMap + cm.sequence++ + cm.configEnv = &cb.ConfigEnvelope{ Config: &cb.Config{ // XXX add header diff --git a/common/configtx/manager_test.go b/common/configtx/manager_test.go index b961601e019..32688ae9b9c 100644 --- a/common/configtx/manager_test.go +++ b/common/configtx/manager_test.go @@ -36,7 +36,18 @@ func defaultInitializer() *mockconfigtx.Initializer { Policy: &mockpolicies.Policy{}, }, }, - HandlerVal: &mockconfigtx.Handler{}, + PolicyHandlerVal: &mockconfigtx.PolicyHandler{ + Handler: mockconfigtx.Handler{ + Transactional: mockconfigtx.Transactional{ + HandlerVal: &mockconfigtx.Handler{}, + }, + }, + }, + Handler: mockconfigtx.Handler{ + Transactional: mockconfigtx.Transactional{ + HandlerVal: &mockconfigtx.Handler{}, + }, + }, } } diff --git a/common/mocks/configtx/configtx.go b/common/mocks/configtx/configtx.go index 9ada48c3e21..ea6bff97efc 100644 --- a/common/mocks/configtx/configtx.go +++ b/common/mocks/configtx/configtx.go @@ -66,10 +66,18 @@ func (r *Resources) MSPManager() msp.MSPManager { return r.MSPManagerVal } -type Transactional struct{} +type Transactional struct { + HandlerVal *Handler +} -// BeginConfig calls through to the HandlerVal -func (t *Transactional) BeginConfig() {} +// BeginConfig returns slices populated by HandlerVal +func (t *Transactional) BeginConfig(groups []string) ([]configtxapi.Handler, error) { + handlers := make([]configtxapi.Handler, len(groups)) + for i := range handlers { + handlers[i] = t.HandlerVal + } + return handlers, nil +} // CommitConfig calls through to the HandlerVal func (t *Transactional) CommitConfig() {} @@ -79,21 +87,13 @@ func (t *Transactional) RollbackConfig() {} // Initializer mocks the configtxapi.Initializer interface type Initializer struct { - Transactional + Handler 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 -} - // Returns the PolicyHandlerVal func (i *Initializer) PolicyHandler() configtxapi.PolicyHandler { return i.PolicyHandlerVal @@ -101,7 +101,7 @@ func (i *Initializer) PolicyHandler() configtxapi.PolicyHandler { // PolicyHandler mocks the configtxapi.PolicyHandler interface type PolicyHandler struct { - Transactional + Handler LastKey string LastPath []string LastValue *cb.ConfigPolicy @@ -118,6 +118,7 @@ func (ph *PolicyHandler) ProposePolicy(key string, path []string, configPolicy * // Handler mocks the configtxapi.Handler interface type Handler struct { + Transactional LastKey string LastValue *cb.ConfigValue ErrorForProposeConfig error diff --git a/common/policies/policy.go b/common/policies/policy.go index 1cf8576d1b0..34d1a589a55 100644 --- a/common/policies/policy.go +++ b/common/policies/policy.go @@ -85,11 +85,15 @@ func (pm *ManagerImpl) GetPolicy(id string) (Policy, bool) { } // BeginConfig is used to start a new config proposal -func (pm *ManagerImpl) BeginConfig() { +func (pm *ManagerImpl) BeginConfig(groups []string) ([]*ManagerImpl, error) { + if len(groups) != 0 { + return nil, fmt.Errorf("Unimplemented") // XXX Temporary workaround until policy manager is enhanced to support hierarchy + } if pm.pendingPolicies != nil { logger.Panicf("Programming error, cannot call begin in the middle of a proposal") } pm.pendingPolicies = make(map[string]Policy) + return nil, nil } // RollbackConfig is used to abandon a new config proposal