support ssl-hostname-overwrite #243
Conversation
|
resolve a point on #222 |
| conn, connError = gRPCClient.NewConnection(node.Addr, func(tlsConfig *tls.Config) { tlsConfig.InsecureSkipVerify = true }) | ||
| conn, connError = gRPCClient.NewConnection(node.Addr, func(tlsConfig *tls.Config) { | ||
| tlsConfig.InsecureSkipVerify = true | ||
| tlsConfig.ServerName = node.SslTargetNameOverride |
There was a problem hiding this comment.
what if SslTargetNameOverride leave to empty?
There was a problem hiding this comment.
and I am not sure if we should add any kind of test as coverage?
There was a problem hiding this comment.
option.servername is a string so should be default to empty string
Good idea to have test coverage.
I will migrate my test back.
| TLSCAKeyByte []byte | ||
| TLSCARootByte []byte | ||
| Addr string `yaml:"addr"` | ||
| SslTargetNameOverride string `yaml:"ssl_target_name_override"` |
There was a problem hiding this comment.
need a sample in readme document for usage
There was a problem hiding this comment.
Where is the original document for struct Node?
There was a problem hiding this comment.
There was a problem hiding this comment.
有ssl hostname overwrite 之后就不需要在环境里面搞这段了
您可能还需要在 hosts 文件中增加节点域名和 IP 的映射关系
There was a problem hiding this comment.
at my point of view, ssl hostname overwrite still need you to change /etc/host if needed....
There was a problem hiding this comment.
/etc/host/ can be another layer if user want, but not mandatory for cheating grpc
davidkhala
force-pushed
the
ssl-name-overwrite
branch
3 times, most recently
from
feea25c to
05e02fc
Compare
|
@SamYuan1990 在做这个的途中发现logging的策略比较模糊，可能需要清理下，可以另外开issue么？ |
好的，欢迎 |
Dockerfile
Outdated
| @@ -8,6 +8,7 @@ ENV GOPROXY=https://goproxy.cn,direct | |||
| ENV export GOSUMDB=off | |||
|
|
|||
| COPY . . | |||
| RUN go mod vendor | |||
There was a problem hiding this comment.
? why we need go mod vendor here?
There was a problem hiding this comment.
解决依赖冲突的时候加入的代码，刚测试过，由于go build的时候也会做类似的事情，所以多余
将此回滚
| TLSCAKeyByte []byte | ||
| TLSCARootByte []byte | ||
| Addr string `yaml:"addr"` | ||
| SslTargetNameOverride string `yaml:"ssl_target_name_override"` |
There was a problem hiding this comment.
at my point of view, ssl hostname overwrite still need you to change /etc/host if needed....
| # docker pull hyperledger/fabric-orderer:amd64-1.4 | ||
| # Error response from daemon: manifest for hyperledger/fabric-orderer:amd64-1.4 not found: manifest unknown: manifest unknown | ||
|
|
||
| echo y | ./byfn.sh up -i 1.4.12 |
There was a problem hiding this comment.
I suggest change back to 1.4.10
is 1.4 still LTS?
are we going to upgrade CI changes with this pr?
There was a problem hiding this comment.
1.4 is no more LTS, would you consider a removal for this?
There was a problem hiding this comment.
So far not, as it is previous TLS, and I make a PR for supporting at #250
but honestly speaking, for my point of view, as 1.4 is no previous LTS, we don't need to bump up version in CI in high frequency.
There was a problem hiding this comment.
1.4.12 should be last version of release-1.4, I think no further bump up in future.
Signed-off-by: DavidLiu <[email protected]>
davidkhala
force-pushed
the
ssl-name-overwrite
branch
from
05e02fc to
3095fb4
Compare
|
@SamYuan1990 这个CI出的error让我想请教下 更为惊奇的是虽然 msp/keystore/priv_sk不存在，但是master上的1_4测试竟然也可以通过。 |
davidkhala
force-pushed
the
ssl-name-overwrite
branch
from
e019a49 to
1975035
Compare
了解到priv_sk是tape一直以来用的格式，也是2.x以来cryptogen生成的格式 |
davidkhala
force-pushed
the
ssl-name-overwrite
branch
2 times, most recently
from
1e021ad to
9a22448
Compare
Signed-off-by: DavidLiu <[email protected]>
davidkhala
force-pushed
the
ssl-name-overwrite
branch
from
9a22448 to
4f59030
Compare
Signed-off-by: DavidLiu [email protected]