Peer CLI communicate with orderers with expired TLS certs #1863
Conversation
wlahti
force-pushed
the
fab-18205
branch
4 times, most recently
from
f556ce0 to
6f852a0
Compare
| if c.OutputFile != "" { | ||
| args = append(args, c.OutputFile) | ||
| "--channelID", c.ChannelID, | ||
| "--orderer", c.Orderer, |
There was a problem hiding this comment.
Do we never use this command to pull blocks from a peer? Certainly channelID should not be empty, but it seems that the others could be?
There was a problem hiding this comment.
I made this change because the code here was unnecessarily complex and inconsistent with (at least the majority) of the command helpers. For these args, we can just set them regardless of whether they're empty or not. The CLI logic will fetch from the peer if the specified orderer arg is empty.
internal/peer/channel/channel.go
Outdated
| @@ -86,6 +89,7 @@ func resetFlags() { | |||
| flags.StringVarP(&outputBlock, "outputBlock", "", common.UndefinedParamValue, `The path to write the genesis block for the channel. (default ./<channelID>.block)`) | |||
| flags.DurationVarP(&timeout, "timeout", "t", 10*time.Second, "Channel creation timeout") | |||
| flags.BoolVarP(&bestEffort, "bestEffort", "", false, "Whether fetch requests should ignore errors and return blocks on a best effort basis") | |||
| flags.DurationVarP(&tlsHandshakeTimeShift, "tlsHandshakeTimeShift", "", 0, "The amount of time to shift backwards during TLS handshakes") | |||
There was a problem hiding this comment.
Perhaps expand a bit in the descriptive text, something like "... of time to shift backwards for certificate expiration checks during ..."
| @@ -22,11 +23,12 @@ type OrdererClient struct { | |||
|
|
|||
| // NewOrdererClientFromEnv creates an instance of an OrdererClient from the | |||
| // global Viper instance | |||
| func NewOrdererClientFromEnv() (*OrdererClient, error) { | |||
| func NewOrdererClientFromEnv(tlsHandshakeTimeShift time.Duration) (*OrdererClient, error) { | |||
There was a problem hiding this comment.
Not saying that the viper approach is a great one, but it's a little odd that all of the parameters for setting up the ordering connection are wired in via configFromEnv, but not the timeshift. And, it's also pretty ugly that everyone calling for this function has to explicitly disable the timeshift by passing in a zero value. I don't know that it gets any less ugly following the existing pattern, but thought I'd ask?
There was a problem hiding this comment.
Yeah, I definitely don't love this change. I did it this way because I hadn't realized the viper config that configFromEnv was using actually derives from flags that are then viper.Set in peer/common/ordererenv.go's SetOrdererEnv(). I'll update things to be consistent with the others now that I realize that.
Implement a TLS handshake timeshift for the "peer channel fetch" and "peer channel update" comands to allow fetching config blocks and updating the config for orderers with expired TLS certificates. FAB-18205 #done Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
| if c.OutputFile != "" { | ||
| args = append(args, c.OutputFile) | ||
| "--channelID", c.ChannelID, | ||
| "--orderer", c.Orderer, |
…r#1863) Implement a TLS handshake timeshift for the "peer channel fetch" and "peer channel update" comands to allow fetching config blocks and updating the config for orderers with expired TLS certificates. FAB-18205 #done Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
Implement a TLS handshake timeshift for the "peer channel fetch" and "peer channel update" comands to allow fetching config blocks and updating the config for orderers with expired TLS certificates. FAB-18205 #done Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
Type of change
Description
Implement a TLS handshake timeshift for the "peer channel fetch" and "peer channel update" comands to allow fetching config blocks and updating the config for orderers with expired TLS certificates.
Related issues
FAB-18205