From a50bd081f48548a51b45e1dee1b7c87a61d9ff69 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Tue, 3 Oct 2017 16:29:37 -0400 Subject: [PATCH] [FAB-6426] Remove Capability msg 'required' field The Capability msg currently only has a single field: 'required'. This field doesn't make a ton of sense, because if a capability were not required, then it could simply not be added to the capabilities map. In the interest of not allowing the expression of the same idea two ways (and the complexity which comes with it), this CR removes the 'required' field from the Capability message and instead makes it an empty message which may be extended in the future, should the need ever arise. Change-Id: I4c48d2a3d0c4fedb4bcf86f8476f34470ad494c5 Signed-off-by: Jason Yellick --- common/capabilities/capabilities.go | 7 +- common/capabilities/capabilities_test.go | 40 +++------- common/channelconfig/channel_util.go | 5 +- protos/common/configuration.go | 2 + protos/common/configuration.pb.go | 98 +++++++++++++----------- protos/common/configuration.proto | 51 ++++++++---- protos/orderer/configuration.go | 2 + 7 files changed, 108 insertions(+), 97 deletions(-) diff --git a/common/capabilities/capabilities.go b/common/capabilities/capabilities.go index 817ac6fbbee..9d98b01284b 100644 --- a/common/capabilities/capabilities.go +++ b/common/capabilities/capabilities.go @@ -42,15 +42,12 @@ func newRegistry(p provider, capabilities map[string]*cb.Capability) *registry { // Supported checks that all of the required capabilities are supported by this binary. func (r *registry) Supported() error { - for capabilityName, capability := range r.capabilities { + for capabilityName := range r.capabilities { if r.provider.HasCapability(capabilityName) { continue } - if capability.Required { - return errors.Errorf("%s capability %s is required but not supported", r.provider.Type(), capabilityName) - } - logger.Debugf("Found unknown %s capability %s but it is not required", r.provider.Type(), capabilityName) + return errors.Errorf("%s capability %s is required but not supported", r.provider.Type(), capabilityName) } return nil } diff --git a/common/capabilities/capabilities_test.go b/common/capabilities/capabilities_test.go index 766011b71f3..ff328e90a19 100644 --- a/common/capabilities/capabilities_test.go +++ b/common/capabilities/capabilities_test.go @@ -20,41 +20,19 @@ func init() { } func TestSatisfied(t *testing.T) { - t.Run("NilCapabilities", func(t *testing.T) { - var capsMap map[string]*cb.Capability - for _, provider := range []*registry{ - NewChannelProvider(capsMap).registry, - NewOrdererProvider(capsMap).registry, - NewApplicationProvider(capsMap).registry, - } { - assert.Nil(t, provider.Supported()) - } - }) - - t.Run("NotRequiredCapabilities", func(t *testing.T) { - capsMap := map[string]*cb.Capability{ - "FakeCapability1": &cb.Capability{ - Required: false, - }, - "FakeCapability2": &cb.Capability{ - Required: false, - }, - } - for _, provider := range []*registry{ - NewChannelProvider(capsMap).registry, - NewOrdererProvider(capsMap).registry, - NewApplicationProvider(capsMap).registry, - } { - assert.Nil(t, provider.Supported()) - } - }) + var capsMap map[string]*cb.Capability + for _, provider := range []*registry{ + NewChannelProvider(capsMap).registry, + NewOrdererProvider(capsMap).registry, + NewApplicationProvider(capsMap).registry, + } { + assert.Nil(t, provider.Supported()) + } } func TestNotSatisfied(t *testing.T) { capsMap := map[string]*cb.Capability{ - "FakeCapability": &cb.Capability{ - Required: true, - }, + "FakeCapability": &cb.Capability{}, } for _, provider := range []*registry{ NewChannelProvider(capsMap).registry, diff --git a/common/channelconfig/channel_util.go b/common/channelconfig/channel_util.go index 097fb9f59c5..b6c34d66ec1 100644 --- a/common/channelconfig/channel_util.go +++ b/common/channelconfig/channel_util.go @@ -68,9 +68,10 @@ func capabilitiesFromBoolMap(capabilities map[string]bool) *cb.Capabilities { Capabilities: make(map[string]*cb.Capability), } for capability, required := range capabilities { - value.Capabilities[capability] = &cb.Capability{ - Required: required, + if !required { + continue } + value.Capabilities[capability] = &cb.Capability{} } return value } diff --git a/protos/common/configuration.go b/protos/common/configuration.go index 0a9ed6d4d52..cdfe2d560a9 100644 --- a/protos/common/configuration.go +++ b/protos/common/configuration.go @@ -85,6 +85,8 @@ func (dccv *DynamicChannelConfigValue) VariablyOpaqueFieldProto(name string) (pr return &OrdererAddresses{}, nil case "Consortium": return &Consortium{}, nil + case "Capabilities": + return &Capabilities{}, nil default: return nil, fmt.Errorf("unknown Channel ConfigValue name: %s", dccv.name) } diff --git a/protos/common/configuration.pb.go b/protos/common/configuration.pb.go index e1d17955df4..0cff37b2e0d 100644 --- a/protos/common/configuration.pb.go +++ b/protos/common/configuration.pb.go @@ -86,14 +86,36 @@ func (m *Consortium) GetName() string { return "" } -// Capabilities message contains all the capabilities that a channel requires -// participant entities to comply with. The entity should drop off the channel -// if it can't fulfill any of the required capabilities. -// Capabilities is encoded into the configuaration as Values of each type -// Orderer, Channel, or Application. -// The key string should represent the capability name, and it must be unique -// within each type. For readability, it may be advisable to prefix the key with -// its type (eg app-acl) +// Capabilities message defines the capabilities a particular binary must implement +// for that binary to be able to safely participate in the channel. The capabilities +// message is defined at the /Channel level, the /Channel/Application level, and the +// /Channel/Orderer level. +// +// The /Channel level capabilties define capabilities which both the orderer and peer +// binaries must satisfy. These capabilties might be things like a new MSP type, +// or a new policy type. +// +// The /Channel/Orderer level capabilties define capabilities which must be supported +// by the orderer, but which have no bearing on the behavior of the peer. For instance +// if the orderer changes the logic for how it constructs new channels, only all orderers +// must agree on the new logic. The peers do not need to be aware of this change as +// they only interact with the channel after it has been constructed. +// +// Finally, the /Channel/Application level capabilities define capabilities which the peer +// binary must satisfy, but which have no bearing on the orderer. For instance, if the +// peer adds a new UTXO transaction type, or changes the chaincode lifecycle requirements, +// all peers must agree on the new logic. However, orderers never inspect transactions +// this deeply, and therefore have no need to be aware of the change. +// +// The capabilities strings defined in these messages typically correspond to release +// binary versions (e.g. "V1.1"), and are used primarilly as a mechanism for a fully +// upgraded network to switch from one set of logic to a new one. +// +// Although for V1.1, the orderers must be upgraded to V1.1 prior to the rest of the +// network, going forward, because of the split between the /Channel, /Channel/Orderer +// and /Channel/Application capabilities. It should be possible for the orderer and +// application networks to upgrade themselves independently (with the exception of any +// new capabilities defined at the /Channel level). type Capabilities struct { Capabilities map[string]*Capability `protobuf:"bytes,1,rep,name=capabilities" json:"capabilities,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` } @@ -110,13 +132,11 @@ func (m *Capabilities) GetCapabilities() map[string]*Capability { return nil } -// Capability holds a set of options. We can add more as needed in the -// future. For now, whether it is required or not. If a configured capability -// is not required, it must be completely compatible with previous releases. -// Compatible features are not required to be encoded as capabilities; they -// only provide flexibility for the admins to control the features. +// Capability is an empty message for the time being. It is defined as a protobuf +// message rather than a constant, so that we may extend capabilities with other fields +// if the need arises in the future. For the time being, a capability being in the +// capabilities map requires that that capability be supported. type Capability struct { - Required bool `protobuf:"varint,1,opt,name=required" json:"required,omitempty"` } func (m *Capability) Reset() { *m = Capability{} } @@ -124,13 +144,6 @@ func (m *Capability) String() string { return proto.CompactTextString func (*Capability) ProtoMessage() {} func (*Capability) Descriptor() ([]byte, []int) { return fileDescriptor3, []int{5} } -func (m *Capability) GetRequired() bool { - if m != nil { - return m.Required - } - return false -} - func init() { proto.RegisterType((*HashingAlgorithm)(nil), "common.HashingAlgorithm") proto.RegisterType((*BlockDataHashingStructure)(nil), "common.BlockDataHashingStructure") @@ -143,26 +156,25 @@ func init() { func init() { proto.RegisterFile("common/configuration.proto", fileDescriptor3) } var fileDescriptor3 = []byte{ - // 329 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x91, 0x41, 0x6b, 0xea, 0x40, - 0x10, 0xc7, 0x89, 0x3e, 0x45, 0x47, 0x1f, 0xf8, 0x96, 0x77, 0xf0, 0xc9, 0x3b, 0x84, 0x50, 0x24, - 0x50, 0x48, 0x5a, 0x7b, 0x29, 0xbd, 0xa9, 0x2d, 0x94, 0x5e, 0x0a, 0xf1, 0xd6, 0xdb, 0x26, 0x19, - 0x93, 0xc5, 0x24, 0x6b, 0x67, 0x77, 0x5b, 0xf2, 0xa9, 0xfa, 0x15, 0x8b, 0x59, 0x5b, 0x15, 0x7b, - 0x9b, 0x5f, 0xe6, 0xf7, 0x9f, 0x99, 0xb0, 0x30, 0x49, 0x64, 0x59, 0xca, 0x2a, 0x4c, 0x64, 0xb5, - 0x16, 0x99, 0x21, 0xae, 0x85, 0xac, 0x82, 0x2d, 0x49, 0x2d, 0x59, 0xd7, 0xf6, 0xbc, 0x29, 0x8c, - 0x1e, 0xb9, 0xca, 0x45, 0x95, 0xcd, 0x8b, 0x4c, 0x92, 0xd0, 0x79, 0xc9, 0x18, 0xfc, 0xaa, 0x78, - 0x89, 0x63, 0xc7, 0x75, 0xfc, 0x7e, 0xd4, 0xd4, 0xde, 0x35, 0xfc, 0x5b, 0x14, 0x32, 0xd9, 0xdc, - 0x73, 0xcd, 0xf7, 0x81, 0x95, 0x26, 0x93, 0x68, 0x43, 0xc8, 0xfe, 0x42, 0xe7, 0x5d, 0xa4, 0x3a, - 0x6f, 0x12, 0xbf, 0x23, 0x0b, 0xde, 0x15, 0x8c, 0x9e, 0x29, 0x45, 0x42, 0x9a, 0xa7, 0x29, 0xa1, - 0x52, 0xa8, 0xd8, 0x7f, 0xe8, 0xf3, 0x2f, 0x18, 0x3b, 0x6e, 0xdb, 0xef, 0x47, 0x87, 0x0f, 0x9e, - 0x0b, 0xb0, 0x94, 0x95, 0x92, 0xa4, 0x85, 0xf9, 0xf9, 0x8c, 0x0f, 0x07, 0x86, 0x4b, 0xbe, 0xe5, - 0xb1, 0x28, 0x84, 0x16, 0xa8, 0xd8, 0x13, 0x0c, 0x93, 0x23, 0x6e, 0x66, 0x0e, 0x66, 0xd3, 0xc0, - 0xfe, 0x5e, 0x70, 0xec, 0x9e, 0xc0, 0x43, 0xa5, 0xa9, 0x8e, 0x4e, 0xb2, 0x93, 0x15, 0xfc, 0x39, - 0x53, 0xd8, 0x08, 0xda, 0x1b, 0xac, 0xf7, 0x47, 0xec, 0x4a, 0xe6, 0x43, 0xe7, 0x8d, 0x17, 0x06, - 0xc7, 0x2d, 0xd7, 0xf1, 0x07, 0x33, 0x76, 0xb6, 0xab, 0x8e, 0xac, 0x70, 0xd7, 0xba, 0x75, 0x3c, - 0x1f, 0xe0, 0xd0, 0x60, 0x13, 0xe8, 0x11, 0xbe, 0x1a, 0x41, 0x98, 0x36, 0x23, 0x7b, 0xd1, 0x37, - 0x2f, 0x56, 0x70, 0x21, 0x29, 0x0b, 0xf2, 0x7a, 0x8b, 0x54, 0x60, 0x9a, 0x21, 0x05, 0x6b, 0x1e, - 0x93, 0x48, 0xec, 0x93, 0xa9, 0xfd, 0x9e, 0x97, 0xcb, 0x4c, 0xe8, 0xdc, 0xc4, 0x3b, 0x0c, 0x8f, - 0xe4, 0xd0, 0xca, 0xa1, 0x95, 0x43, 0x2b, 0xc7, 0xdd, 0x06, 0x6f, 0x3e, 0x03, 0x00, 0x00, 0xff, - 0xff, 0x62, 0x6f, 0x69, 0xeb, 0x0c, 0x02, 0x00, 0x00, + // 314 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x91, 0x41, 0x6b, 0xf2, 0x40, + 0x10, 0x86, 0x89, 0x7e, 0x0a, 0x8e, 0x7e, 0x60, 0x97, 0x1e, 0xac, 0xf4, 0x10, 0x42, 0x91, 0x40, + 0x21, 0x69, 0xed, 0xa5, 0xf4, 0xa6, 0xb6, 0x50, 0x7a, 0x29, 0xc4, 0x5b, 0x6f, 0x9b, 0x64, 0x4c, + 0x16, 0x93, 0x5d, 0x99, 0xdd, 0xb4, 0xe4, 0x57, 0xf5, 0x2f, 0x16, 0xb3, 0x16, 0x23, 0xf6, 0x36, + 0xcf, 0xce, 0xf3, 0xce, 0xce, 0xb2, 0x30, 0x4d, 0x54, 0x59, 0x2a, 0x19, 0x26, 0x4a, 0x6e, 0x44, + 0x56, 0x11, 0x37, 0x42, 0xc9, 0x60, 0x47, 0xca, 0x28, 0xd6, 0xb7, 0x3d, 0x6f, 0x06, 0xe3, 0x57, + 0xae, 0x73, 0x21, 0xb3, 0x45, 0x91, 0x29, 0x12, 0x26, 0x2f, 0x19, 0x83, 0x7f, 0x92, 0x97, 0x38, + 0x71, 0x5c, 0xc7, 0x1f, 0x44, 0x4d, 0xed, 0xdd, 0xc3, 0xd5, 0xb2, 0x50, 0xc9, 0xf6, 0x99, 0x1b, + 0x7e, 0x08, 0xac, 0x0d, 0x55, 0x89, 0xa9, 0x08, 0xd9, 0x25, 0xf4, 0xbe, 0x44, 0x6a, 0xf2, 0x26, + 0xf1, 0x3f, 0xb2, 0xe0, 0xdd, 0xc1, 0xf8, 0x9d, 0x52, 0x24, 0xa4, 0x45, 0x9a, 0x12, 0x6a, 0x8d, + 0x9a, 0x5d, 0xc3, 0x80, 0xff, 0xc2, 0xc4, 0x71, 0xbb, 0xfe, 0x20, 0x3a, 0x1e, 0x78, 0x2e, 0xc0, + 0x4a, 0x49, 0xad, 0xc8, 0x88, 0xea, 0xef, 0x35, 0xbe, 0x1d, 0x18, 0xad, 0xf8, 0x8e, 0xc7, 0xa2, + 0x10, 0x46, 0xa0, 0x66, 0x6f, 0x30, 0x4a, 0x5a, 0xdc, 0xcc, 0x1c, 0xce, 0x67, 0x81, 0x7d, 0x5e, + 0xd0, 0x76, 0x4f, 0xe0, 0x45, 0x1a, 0xaa, 0xa3, 0x93, 0xec, 0x74, 0x0d, 0x17, 0x67, 0x0a, 0x1b, + 0x43, 0x77, 0x8b, 0xf5, 0x61, 0x89, 0x7d, 0xc9, 0x7c, 0xe8, 0x7d, 0xf2, 0xa2, 0xc2, 0x49, 0xc7, + 0x75, 0xfc, 0xe1, 0x9c, 0x9d, 0xdd, 0x55, 0x47, 0x56, 0x78, 0xea, 0x3c, 0x3a, 0xde, 0x08, 0xe0, + 0xd8, 0x58, 0xae, 0xe1, 0x46, 0x51, 0x16, 0xe4, 0xf5, 0x0e, 0xa9, 0xc0, 0x34, 0x43, 0x0a, 0x36, + 0x3c, 0x26, 0x91, 0xd8, 0x6f, 0xd1, 0x87, 0x59, 0x1f, 0xb7, 0x99, 0x30, 0x79, 0x15, 0xef, 0x31, + 0x6c, 0xc9, 0xa1, 0x95, 0x43, 0x2b, 0x87, 0x56, 0x8e, 0xfb, 0x0d, 0x3e, 0xfc, 0x04, 0x00, 0x00, + 0xff, 0xff, 0xd6, 0x7e, 0xb4, 0x89, 0xf0, 0x01, 0x00, 0x00, } diff --git a/protos/common/configuration.proto b/protos/common/configuration.proto index ef9e230ebd6..6c7bbbf29e5 100644 --- a/protos/common/configuration.proto +++ b/protos/common/configuration.proto @@ -48,23 +48,42 @@ message Consortium { } -// Capabilities message contains all the capabilities that a channel requires -// participant entities to comply with. The entity should drop off the channel -// if it can't fulfill any of the required capabilities. -// Capabilities is encoded into the configuaration as Values of each type -// Orderer, Channel, or Application. -// The key string should represent the capability name, and it must be unique -// within each type. For readability, it may be advisable to prefix the key with -// its type (eg app-acl) +// Capabilities message defines the capabilities a particular binary must implement +// for that binary to be able to safely participate in the channel. The capabilities +// message is defined at the /Channel level, the /Channel/Application level, and the +// /Channel/Orderer level. +// +// The /Channel level capabilties define capabilities which both the orderer and peer +// binaries must satisfy. These capabilties might be things like a new MSP type, +// or a new policy type. +// +// The /Channel/Orderer level capabilties define capabilities which must be supported +// by the orderer, but which have no bearing on the behavior of the peer. For instance +// if the orderer changes the logic for how it constructs new channels, only all orderers +// must agree on the new logic. The peers do not need to be aware of this change as +// they only interact with the channel after it has been constructed. +// +// Finally, the /Channel/Application level capabilities define capabilities which the peer +// binary must satisfy, but which have no bearing on the orderer. For instance, if the +// peer adds a new UTXO transaction type, or changes the chaincode lifecycle requirements, +// all peers must agree on the new logic. However, orderers never inspect transactions +// this deeply, and therefore have no need to be aware of the change. +// +// The capabilities strings defined in these messages typically correspond to release +// binary versions (e.g. "V1.1"), and are used primarilly as a mechanism for a fully +// upgraded network to switch from one set of logic to a new one. +// +// Although for V1.1, the orderers must be upgraded to V1.1 prior to the rest of the +// network, going forward, because of the split between the /Channel, /Channel/Orderer +// and /Channel/Application capabilities. It should be possible for the orderer and +// application networks to upgrade themselves independently (with the exception of any +// new capabilities defined at the /Channel level). message Capabilities { map capabilities = 1; } -// Capability holds a set of options. We can add more as needed in the -// future. For now, whether it is required or not. If a configured capability -// is not required, it must be completely compatible with previous releases. -// Compatible features are not required to be encoded as capabilities; they -// only provide flexibility for the admins to control the features. -message Capability { - bool required = 1; -} +// Capability is an empty message for the time being. It is defined as a protobuf +// message rather than a constant, so that we may extend capabilities with other fields +// if the need arises in the future. For the time being, a capability being in the +// capabilities map requires that that capability be supported. +message Capability { } diff --git a/protos/orderer/configuration.go b/protos/orderer/configuration.go index bebc1526840..fb71b899279 100644 --- a/protos/orderer/configuration.go +++ b/protos/orderer/configuration.go @@ -109,6 +109,8 @@ func (docv *DynamicOrdererConfigValue) VariablyOpaqueFieldProto(name string) (pr return &KafkaBrokers{}, nil case "ChannelRestrictions": return &ChannelRestrictions{}, nil + case "Capabilities": + return &common.Capabilities{}, nil default: return nil, fmt.Errorf("unknown Orderer ConfigValue name: %s", docv.name) }