Fix http2 authority header in single endpoint scenario

[serathius]

Fixes #13192

This doesn't fully fix authority in multiple endpoint scenario, but
should at least fixes single endpoint scenario.

cc @ptabor @tatsuhiro-t @menghanl

[codecov-commenter]

Codecov Report

Merging #13359 (97756e3) into main (c2937d7) will increase coverage by 31.81%.
The diff coverage is n/a.

❗ Current head 97756e3 differs from pull request most recent head 171cca3. Consider uploading reports for the commit 171cca3 to get more accurate results

@@             Coverage Diff             @@
##             main   #13359       +/-   ##
===========================================
+ Coverage   39.49%   71.31%   +31.81%     
===========================================
  Files         460      453        -7     
  Lines       39141    38856      -285     
===========================================
+ Hits        15459    27709    +12250     
+ Misses      22009     9114    -12895     
- Partials     1673     2033      +360     

Flag	Coverage Δ
all	71.31% <ø> (+31.81%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
raft/quorum/quorum.go	0.00% <0.00%> (-100.00%)	⬇️
raft/tracker/state.go	0.00% <0.00%> (-100.00%)	⬇️
client/pkg/v3/tlsutil/cipher_suites.go	0.00% <0.00%> (-100.00%)	⬇️
client/pkg/v3/transport/sockopt_unix.go	0.00% <0.00%> (-100.00%)	⬇️
client/pkg/v3/transport/timeout_dialer.go	0.00% <0.00%> (-100.00%)	⬇️
client/pkg/v3/srv/srv.go	0.00% <0.00%> (-89.42%)	⬇️
client/pkg/v3/types/set.go	19.00% <0.00%> (-74.00%)	⬇️
client/pkg/v3/transport/sockopt.go	11.76% <0.00%> (-70.59%)	⬇️
raft/util.go	25.58% <0.00%> (-62.02%)	⬇️
raft/quorum/voteresult_string.go	0.00% <0.00%> (-60.00%)	⬇️
... and 352 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2937d7...171cca3. Read the comment docs.

[serathius]

Pushed original fix that uses first endpoint but with huge integration tests rewrite. Needed to rewrite tests to remove hacks that caused problems with testing authority. Those hacks are:

• Using bridge in integration tests. Integration tests use unix bridge to allow connection manipulation in some tests. I made bridge optional, so only it's only enabled in tests that require it.
• Only using unix socket in integration tests. Integration tests used only unix socket to remove the need to garbage collect TCP sockets. I added option to enable TCP connections. This is required to integration tests as Unix socket are not officially supported.

Please review new test suites that should cover all possible client endpoint configurations. We still cant cover all cases in https://github.com/grpc/grpc/blob/master/doc/naming.md but as we add more supported names adding them should be much easier.

I decided to use the old fix to confirm that new tests work. I expect that before merging this PR I will try to fix case with multiple endpoints.

[serathius]

As discussed in #13192 (comment) etcd never supported correct authority in multiple endpoint scenario. This PR restores behavior of etcd v3.4 where first endpoint is used to determine authority header.

This PR should be good to review when tests pass.
PTAL @ptabor

[ptabor]

As this was previous default and majority of tests got to be changed to get True supplied here explicitly,
have you considered making it WithoutBridge or NoBridge setting to let opt-in for the new behaviour ?

[serathius]

Using WithoutBridge would require less changes/work, but I don't think it would be better. I think that enabling bridge is very bad default, it is not as transparent as it might seem. It changes how broken connections are handled hiding some issues which we would want to fix. It also forces integration tests to go through unix socket, which is not officially supported (you cannot bind etcd server to unix socket). This makes maintaining integration and e2e tests much harder, as it increases differences between two environments. When something breaks you don't know if this is specific to integration test environment or etcd logic itself.

I think we should strive to have integration and e2e tests be almost identical in setup, differing only in what is executed underneath (etcdctl binary in e2e and client library in integration). This way it is easy to attribute failures to etcd or environment. Using bridges as a common denominator creates unnecessary complexity, causing us to waste time on fixing integration test code instead of focusing on etcd itself.

[ptabor]

SG.

Does it mean, there is a follow-up TODO to check which of the existing tests would pass with 'False' ?

[serathius]

I have only passed "true" only in tests that were failing, so I think this should be done. Only think it that there might be difference between what test was intended to test and it's implementation. Possibly if we changed implementation of some tests we could further reduce usage of bridge, but I don't think profit from this would outweigh amount of work that this would require us to invest.

I think it would make sense to propose rewrite of e2e/integration tests to unified framework and slowly migrate existing tests there, case by case. Removing bridge could be done at the same time and results would be more beneficial (halved amount of easier to debug tests).

[ptabor]

grpcproxy: TestV3KVInflightRangeRequests seems to be continuesly failing (tried 2 retries).

[ptabor]

Please edit the changelog with the new URL schema:
https://github.com/etcd-io/etcd/blob/main/CHANGELOG-3.5.md

[serathius]

Done