Bump go-connections for TLS 1.3 support by thaJeztah · Pull Request #41084 · moby/moby

thaJeztah · 2020-06-09T14:01:17Z

~~testing #41042 with API version negotiation enabled on the testutil client~~

fixes [roadmap] Support TLS1.3 #40495
carries Bump go-connections for TLS 1.3 support #41042
closes Bump go-connections for TLS 1.3 support #41042
closes [do not merge] vendor: github.com/akerouanton/go-connections #46739

thaJeztah · 2020-06-09T14:29:10Z

OK, so this "fixes" the failures, but it doesn't look right;


[2020-06-09T14:23:45.417Z] Running D:\gopath\src\github.com\docker\docker\integration\service
[2020-06-09T14:23:48.388Z] INFO: Windows Base image is  mcr.microsoft.com/windows/servercore:ltsc2019
[2020-06-09T14:23:48.388Z] INFO: Testing against a local daemon
[2020-06-09T14:23:48.388Z] ok  	github.com/docker/docker/integration/service	0.315s

...

[2020-06-09T14:23:48.388Z] === SKIP: github.com/docker/docker/integration/service TestCreateServiceCapabilities (0.00s)
[2020-06-09T14:23:48.388Z]     create_test.go:481: versions.LessThan(testEnv.DaemonAPIVersion(), "1.41"): setting service capabilities is unsupported before api v1.41

Why is the daemon it's testing against an older daemon? Is it testing against the pre-installed daemon on the host, instead of the one that was built (and should be used for tests)?

thaJeztah · 2020-06-09T14:34:37Z

Earlier, I see these debug lines;


[2020-06-09T14:15:29.442Z] INFO: go version go1.13.12 windows/amd64
[2020-06-09T14:15:29.442Z] INFO: Running the daemon under test in debug mode
[2020-06-09T14:15:29.442Z] INFO: Starting a daemon under test...
[2020-06-09T14:15:29.442Z] INFO: Args: -H tcp://0.0.0.0:2357 --data-root d:\CI\PR-41084\1\daemon --pidfile d:\CI\PR-41084\1\docker.pid -D
[2020-06-09T14:15:29.442Z] INFO: Process started successfully.
[2020-06-09T14:15:29.442Z] INFO: Start tailing logs of the daemon under tests
[2020-06-09T14:15:29.442Z] INFO: Waiting for the daemon under test to start...
[2020-06-09T14:15:32.417Z] INFO: Daemon under test started and replied!
[2020-06-09T14:15:32.417Z] INFO: Docker version of the daemon under test
[2020-06-09T14:15:32.417Z] 
[2020-06-09T14:15:32.417Z] Client:
[2020-06-09T14:15:32.417Z]  Version:      17.06.2-ce
[2020-06-09T14:15:32.417Z]  API version:  1.30
[2020-06-09T14:15:32.417Z]  Go version:   go1.8.3
[2020-06-09T14:15:32.417Z]  Git commit:   cec0b72
[2020-06-09T14:15:32.417Z]  Built:        Tue Sep  5 19:57:19 2017
[2020-06-09T14:15:32.417Z]  OS/Arch:      windows/amd64
[2020-06-09T14:15:32.417Z] 
[2020-06-09T14:15:32.417Z] Server:
[2020-06-09T14:15:32.417Z]  Version:      0.0.0-dev
[2020-06-09T14:15:32.417Z]  API version:  1.41 (minimum version 1.24)
[2020-06-09T14:15:32.417Z]  Go version:   go1.13.12
[2020-06-09T14:15:32.417Z]  Git commit:   896024cfc0
[2020-06-09T14:15:32.417Z]  Built:        06/09/2020 14:13:42
[2020-06-09T14:15:32.417Z]  OS/Arch:      windows/amd64
[2020-06-09T14:15:32.417Z]  Experimental: false

So it looks like either DOCKER_HOST should be set for these tests, or -H tcp://0.0.0.0:2357 should be passed to the client

thaJeztah · 2020-06-09T14:41:36Z

So... these are only used in Windows.ps1?

moby/hack/ci/windows.ps1

Lines 591 to 596 in 07e6b84

    
           $DASHH_CUT="tcp://127.0.0.1`:2357"    # Not a typo for 2375! 
        
           $DASHH_DUT="tcp://0.0.0.0:2357"       # Not a typo for 2375! 
        
           # Arguments for the daemon under test 
        
           $dutArgs=@() 
        
           $dutArgs += "-H $DASHH_DUT"

Further down, it's used to set DOCKER_HOST (in two places);

moby/hack/ci/windows.ps1

Lines 883 to 885 in 07e6b84

    
           $env:DOCKER_HOST=$DASHH_CUT   
        
           $env:PATH="$env:TEMP\binary;$env:PATH;"  # Force to use the test binaries, not the host ones. 
        
           Write-Host -ForegroundColor Green "INFO: DOCKER_HOST at $DASHH_CUT"

moby/hack/ci/windows.ps1

Lines 924 to 929 in 07e6b84

    
           # Location of the daemon under test. 
        
           $env:OrigDOCKER_HOST="$env:DOCKER_HOST" 
        
           # Make sure we are pointing at the DUT 
        
           $env:DOCKER_HOST=$DASHH_CUT   
        
           Write-Host -ForegroundColor Green "INFO: DOCKER_HOST at $DASHH_CUT"

And I see this printed in the logs;

[2020-06-09T14:22:27.249Z] INFO: Running integration tests at 06/09/2020 14:22:27...
[2020-06-09T14:22:27.249Z] INFO: DOCKER_HOST at tcp://127.0.0.1:2357
[2020-06-09T14:22:27.249Z] INFO: Integration API tests being run from the host:
[2020-06-09T14:22:27.249Z] INFO: make.ps1 starting at 06/09/2020 14:22:27
[2020-06-09T14:22:28.245Z] Running D:\gopath\src\github.com\docker\docker\integration\build
[2020-06-09T14:22:53.611Z] INFO: Windows Base image is  mcr.microsoft.com/windows/servercore:ltsc2019
[2020-06-09T14:22:53.612Z] INFO: Testing against a local daemon
[2020-06-09T14:23:08.597Z] FAIL
[2020-06-09T14:23:08.597Z] exit status 1
[2020-06-09T14:23:08.597Z] FAIL	github.com/docker/docker/integration/build	17.844s

So for some reason DOCKER_HOST is not used by the test client 🤔

thaJeztah · 2020-06-09T14:52:12Z

So, testutil creates the client here;

moby/testutil/environment/environment.go

Line 39 in 07e6b84

c, err := client.NewClientWithOpts(client.FromEnv)

Which uses FromEnv;

moby/client/options.go

Lines 45 to 49 in 07e6b84

    
           if host := os.Getenv("DOCKER_HOST"); host != "" { 
        
           	if err := WithHost(host)(c); err != nil { 
        
           		return err 
        
           	} 
        
           }

Which uses WithHost();

moby/client/options.go

Lines 89 to 91 in 07e6b84

    
           if transport, ok := c.client.Transport.(*http.Transport); ok { 
        
           	return sockets.ConfigureTransport(transport, c.proto, c.addr) 
        
           }

Which calls sockets.ConfigureTransport(transport, c.proto, c.addr)

I suspect this may be related docker/go-connections@fb772cf#diff-998ed58b58f0b5f52161ef8e4e27692d

docker/go-connections#61

@AkihiroSuda could you have a look?

thaJeztah · 2021-03-15T17:55:19Z

Reverting docker/go-connections#61 and docker/go-connections#58 resolves the problem on Windows, so we need to look at those

This restores the deprecated Transport.Dial, which were removed in commits: - 61039d0 (Replace deprecated Transport.Dial with Transport.DialContext) - fb772cf (Fix problems introduced by 61039d0) While we should still look at removing these, the moby code currently looks to be depending on their behavior. Removing them caused CI to fail, which blocked us from updating to the current version of this package. With those changes, Windows clients in CI were connecting with the wrong daemon, causing CI failures: Failed failed to get info from daemon: Error response from daemon: client version 1.41 is too new. Maximum supported API version is 1.40 exit status 1 FAIL github.com/docker/docker/integration/build 0.219s More details on moby/moby#41042 and moby/moby#41084 This patch restores the deprecated parts, but keeps the new variants as well, so that we can perform the migration in Moby when possible (after which they can be removed again) Signed-off-by: Sebastiaan van Stijn <[email protected]>

Fixes 40495 Signed-off-by: Sam Whited <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah mentioned this pull request Jun 9, 2020

Bump go-connections for TLS 1.3 support #41042

Closed

thaJeztah mentioned this pull request Jan 4, 2021

Request new tagged release docker/go-connections#76

Closed

thaJeztah mentioned this pull request Mar 15, 2021

[do not merge] testing reverting some changes docker/go-connections#80

Closed

thaJeztah force-pushed the 40495_update_goconnections_negotiate branch 3 times, most recently from 94ba20f to 1782471 Compare March 15, 2021 16:33

thaJeztah force-pushed the 40495_update_goconnections_negotiate branch from 1782471 to 0f08ef6 Compare July 21, 2021 14:00

thaJeztah mentioned this pull request Jul 22, 2021

socket: restore deprecated Transport.Dial (for now) docker/go-connections#90

Closed

thaJeztah force-pushed the 40495_update_goconnections_negotiate branch from 0f08ef6 to f83301c Compare July 22, 2021 11:58

thaJeztah force-pushed the 40495_update_goconnections_negotiate branch from f83301c to 323e0d8 Compare July 22, 2021 12:01

thaJeztah force-pushed the 40495_update_goconnections_negotiate branch from 323e0d8 to 5e9d608 Compare July 22, 2021 12:05

thaJeztah force-pushed the 40495_update_goconnections_negotiate branch from 5e9d608 to f68f795 Compare July 22, 2021 12:10

thaJeztah mentioned this pull request Jul 26, 2021

revert "configureNpipeTransport: use winio.DialPipeContext()" docker/go-connections#91

Closed

thaJeztah force-pushed the 40495_update_goconnections_negotiate branch 3 times, most recently from 6de2e87 to 1eb8123 Compare July 27, 2021 22:01

thaJeztah mentioned this pull request Jan 19, 2022

IPv6 address port mappings no longer works with v2 docker/compose#8721

Closed

jamesmoessis mentioned this pull request Jul 19, 2022

Unix transport used when HTTP host is specified docker/go-connections#99

Closed

thaJeztah mentioned this pull request Mar 8, 2023

Phase out weak ciphers? docker/go-connections#101

Closed

neersighted mentioned this pull request Jul 13, 2023

Consider supporting Unix Socket on Windows (starting with RS4) #36442

Closed

Bump go-connections for TLS 1.3 support

8074e7a

Fixes 40495 Signed-off-by: Sam Whited <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the 40495_update_goconnections_negotiate branch from 1eb8123 to 8074e7a Compare November 8, 2023 17:05

thaJeztah changed the title ~~[do not merge] Bump go-connections for TLS 1.3 support~~ Bump go-connections for TLS 1.3 support Nov 8, 2023

thaJeztah added status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. impact/changelog labels Nov 8, 2023

thaJeztah marked this pull request as ready for review November 9, 2023 19:15

corhere approved these changes Nov 9, 2023

View reviewed changes

cpuguy83 approved these changes Nov 9, 2023

View reviewed changes

cpuguy83 merged commit 63b2a21 into moby:master Nov 9, 2023

thaJeztah deleted the 40495_update_goconnections_negotiate branch November 9, 2023 22:34

thaJeztah added this to the 25.0.0 milestone Nov 9, 2023

thaJeztah mentioned this pull request Nov 10, 2023

vendor: update go-connections for TLS 1.3 support docker/cli#4651

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump go-connections for TLS 1.3 support #41084

Bump go-connections for TLS 1.3 support #41084
cpuguy83 merged 1 commit intomoby:masterfrom
thaJeztah:40495_update_goconnections_negotiate

thaJeztah commented Jun 9, 2020 •

edited

Loading

Uh oh!

thaJeztah commented Jun 9, 2020

Uh oh!

thaJeztah commented Jun 9, 2020

Uh oh!

thaJeztah commented Jun 9, 2020 •

edited

Loading

Uh oh!

thaJeztah commented Jun 9, 2020

Uh oh!

thaJeztah commented Mar 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

thaJeztah commented Jun 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Jun 9, 2020

Uh oh!

thaJeztah commented Jun 9, 2020

Uh oh!

thaJeztah commented Jun 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Jun 9, 2020

Uh oh!

thaJeztah commented Mar 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thaJeztah commented Jun 9, 2020 •

edited

Loading

thaJeztah commented Jun 9, 2020 •

edited

Loading