add endpoint and consenter consistency validations to OrdererRules#657
Conversation
moradna
force-pushed
the
add-orderer-endpoints-validate
branch
2 times, most recently
from
2388d3e to
1c699b0
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends DefaultOrdererRules.ValidateNewConfig to validate orderer-organization endpoints and to enforce consistency between the orderer consenter mapping and the shared (consensus-metadata) parties config, aligning with the “Validate params” work from Issue #94.
Changes:
- Add validation that each orderer org has non-empty endpoints and includes both
broadcastanddeliverroles. - Add validation that
Orderers.ConsenterMappingmatchesSharedConfig.PartiesConfig(host/port/identity/TLS certs). - Update config generation + config-update test utilities and add unit tests to exercise the new validations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
config/verify/orderer_rules.go |
Adds endpoint-role and consenter-mapping consistency validations in ValidateNewConfig. |
config/verify/orderer_rules_test.go |
Adds tests for invalid endpoints and consenter-mapping inconsistency. |
config/generate/config_block_gen.go |
Fixes generated consenter identity to use the signing cert and generates endpoints in the new format. |
testutil/configutil/config_update_utils.go |
Updates test config update builder to keep PartiesConfig, consenter_mapping, and org endpoints in sync. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
moradna
force-pushed
the
add-orderer-endpoints-validate
branch
from
99f88fc to
3e54507
Compare
config/generate/config_block_gen.go
Outdated
| Policies: make(map[string]*configtxgen.Policy), | ||
| AnchorPeers: templateAppOrg.AnchorPeers, | ||
| OrdererEndpoints: templateAppOrg.OrdererEndpoints, // TODO: org.OrdererEndpoints in the new format | ||
| OrdererEndpoints: buildOrdererEndpoints(uint32(p.PartyID), p.RouterConfig.Host, int(p.RouterConfig.Port), p.AssemblerConfig.Host, int(p.AssemblerConfig.Port)), |
There was a problem hiding this comment.
profile.Application.Organizations do not have orderer endpoints
config/verify/orderer_rules.go
Outdated
| return errors.Errorf("party ID %d missing from shared config", consenter.Id) | ||
| } | ||
| if party.ConsenterConfig == nil { | ||
| return errors.Errorf("consenter config missing for party %d", consenter.Id) |
There was a problem hiding this comment.
return errors.Errorf("consenter config missing in shared config for party %d", consenter.Id)
config/verify/orderer_rules.go
Outdated
| if !slices.Equal(consenter.ServerTlsCert, nodeCfg.TlsCert) || !slices.Equal(consenter.ClientTlsCert, nodeCfg.TlsCert) { | ||
| return errors.Errorf("TLS certificate mismatch for party %d", consenter.Id) | ||
| } |
There was a problem hiding this comment.
We don't need to enforce this rule, as clients should not be using these TLS certs to connect to the consenters directly. In fact, they can remain empty in the consenters mapping.
I say that we can check like so:
If consenter.ServerTlsCert (or consenter.ClientTlsCert) is empty, it is ok (this is permitted for Arma).
However, if they do contain something, it must be equal to nodeCfg.TlsCert.
Signed-off-by: Natalie Morad <natalie.morad@ibm.com>
Signed-off-by: Natalie Morad <natalie.morad@ibm.com>
moradna
force-pushed
the
add-orderer-endpoints-validate
branch
from
3e54507 to
53b4a52
Compare
|
By the way, it is perfectly fine to add unit tests in the |
#94