-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
e2e grpcproxy tests #8277
e2e grpcproxy tests #8277
Conversation
@gyuho no need to backport the transport one-- there's special cases everywhere to set it in the tls.Config derived from the tlsinfo. The snapshot EOF patch seems worth backporting though. |
f3bbd81
to
338160d
Compare
What is |
338160d
to
cd00945
Compare
@gyuho EndpointsJSON wasn't needed; removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. thanks!
cd00945
to
8f4f722
Compare
8f4f722
to
ae601f6
Compare
for i := 0; i < len(cfg.tlsArgs); i++ { | ||
switch cfg.tlsArgs[i] { | ||
case "--cert-file": | ||
tlsArgs = append(tlsArgs, "--cert", cfg.tlsArgs[i+1], "--cert-file", cfg.tlsArgs[i+1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have duplicate flags "--cert-file", cfg.tlsArgs[i+1]
(for etcd) in etcd grpc-proxy
command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--cert is proxy <-> etcd (in grpcproxy already), --cert-file is client <-> proxy (new)
cf8c515
to
95a1bff
Compare
switch os.Args[1] { | ||
cmd := os.Args[1] | ||
if covArgs := os.Getenv("ETCDCOV_ARGS"); len(covArgs) > 0 { | ||
args := strings.Split(os.Getenv("ETCDCOV_ARGS"), "\xe7\xcd")[1:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is \xe7\xcd
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string separator, in case there's a space in the arguments; etcdctl has something similar since the arguments can't be passed via command line when enabling coverage instrumentation
b7805f7
to
954ec4d
Compare
Some functions take a TLSInfo to generate a tls.Config and there was no way to force the InsecureSkipVerify flag.
Was defaulting to PeerTLSInfo for client connections to the etcd cluster. Since proxy users may rely on this behavior, only use the client tls info if given, and fall back to peer tls otherwise.
Enables TLS termination in grpcproxy.
Gets "code = OutOfRange desc = EOF" errors otherwise.
Was dropping the last argument in the slice.
Enables -tags cluster_proxy for e2e.
Turns out a lot of stuff was broken! Skips v3 auth tests for now.