Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove global-level endpoints from channel config - WIP #4778

Conversation

MayRosenbaum
Copy link
Contributor

Type of change

  • config update

Description

This PR aims to block the option to specify global-level endpoints and enforce that endpoints per org are defined.

Related issues

issue #4763

@MayRosenbaum MayRosenbaum requested a review from a team as a code owner April 2, 2024 13:45
@MayRosenbaum MayRosenbaum force-pushed the Remove_global_level_endpoints_from_the_channel_config branch from ccc955b to 2bc8ea1 Compare April 2, 2024 13:47
@@ -15,6 +15,8 @@ import (
"syscall"
"time"

"github.com/hyperledger/fabric/common/channelconfig"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please delete the empty line and run the command 'goimports -l -w'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.
I'm still working on this PR, the title is updated to be W.I.P.

@MayRosenbaum MayRosenbaum changed the title Remove global-level endpoints from channel config Remove global-level endpoints from channel config - WIP Apr 2, 2024
@@ -329,6 +329,8 @@ func NewOrdererOrgGroup(conf *genesisconfig.Organization) (*cb.ConfigGroup, erro

if len(conf.OrdererEndpoints) > 0 {
addValue(ordererOrgGroup, channelconfig.EndpointsValue(conf.OrdererEndpoints), channelconfig.AdminsPolicyKey)
} else {
return nil, errors.Errorf("orderer endpoints for organization %s are missing and must be cofigured", conf.Name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to make OrdererEndpoints required? For example, could an OrdererOrg be added to the channel initially without OrdererEndpoints (without any orderer nodes), with the intent for that organization to later add their own nodes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dave, this is encoder.go. It's only used when creating genesis blocks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we add to a new channel we don't use that, but we download the existing config.

@MayRosenbaum MayRosenbaum force-pushed the Remove_global_level_endpoints_from_the_channel_config branch from 5b40832 to 51bd13c Compare April 3, 2024 11:24
Signed-off-by: May Rosenbaum <mayro1595@gmail.com>
Signed-off-by: May Rosenbaum <mayro1595@gmail.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
@@ -142,7 +142,7 @@ func NewChannelGroup(conf *genesisconfig.Profile) (*cb.ConfigGroup, error) {
addValue(channelGroup, channelconfig.HashingAlgorithmValue(), channelconfig.AdminsPolicyKey)
addValue(channelGroup, channelconfig.BlockDataHashingStructureValue(), channelconfig.AdminsPolicyKey)
if conf.Orderer != nil && len(conf.Orderer.Addresses) > 0 {
addValue(channelGroup, channelconfig.OrdererAddressesValue(conf.Orderer.Addresses), ordererAdminsPolicyName)
return nil, errors.New("error adding global level endpoints to channel group, in V3.0 global level endpoints are not supported")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not be here but outside the if

@@ -161,6 +161,9 @@ func (cs *ChainSupport) ProposeConfigUpdate(configtx *cb.Envelope) (*cb.ConfigEn
}

if err = cs.ValidateConsensusMetadata(oldOrdererConfig, newOrdererConfig, false); err != nil {
if env.Config.ChannelGroup.Values["OrdererAddressesKey"] != nil && env.Config.ChannelGroup.Values["OrdererAddressesKey"].Value == nil {
return nil, errors.WithMessage(err, "consensus metadata update for channel config update is invalid since it includes global level endpoints which are not supported in V3.0")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors.WithMessage returns nil if the err is nil. It wraps an error if it's not nil.
You need to use errors.Errof or something similar... otherwise we return here nil, nil

Signed-off-by: May Rosenbaum <mayro1595@gmail.com>
@MayRosenbaum MayRosenbaum force-pushed the Remove_global_level_endpoints_from_the_channel_config branch from 51bd13c to d5dcb47 Compare April 4, 2024 11:54
@MayRosenbaum MayRosenbaum marked this pull request as draft April 17, 2024 11:46
@yacovm
Copy link
Contributor

yacovm commented May 3, 2024

@MayRosenbaum which one is the relevant one? This one or #4800 ? Please close the other one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants