Remove hack MalformedHostHeaderOverride by thaJeztah · Pull Request #39076 · moby/moby

thaJeztah · 2019-04-15T08:20:16Z

This hack was added to fix a compatibility with clients
that were built using Go 1.5 and older (added in 3d6f598)

This hack causes some problems with current clients; with Go 1.5 and older
no longer being supported for some time, and being several years old, it
should now be ok to remove this hack altogether.

People using tools that are built with those versions of Go wouldn't have
updated those for years, and are probably out of date anyway; that's not
something we can continue taking into account.

This will affect docker clients (the docker cli) for docker 1.12 and older.
Those versions have reached EOL a long time ago (and have known unpatched
vulnerabilities), so should no longer be used anyway, but We should add
a nebtuib in the release notes, just in case someone, somewhere, still
has such old tools.

For those affected, using a more recent client (and if needed, setting
the DOCKER_API_VERSION environment variable to the needed API version)
should provide a way out.

This reverts the changes originally made in; #22000 and #22888,
which were to address #20865.

thaJeztah · 2019-04-15T08:23:26Z

ping @lifubang @cpuguy83 @kolyshkin

@runcom @ibuildthecloud FYI

thaJeztah · 2019-04-18T19:10:18Z

Same failures again; looks related to this change https://jenkins.dockerproject.org/job/Docker-PRs/53854/console

Failed: DID NOT RAISE <class 'socket.error'>

@shin- @ulyssessouza I could use some help on this one; wondering why this causes docker-py to fail

Failure is produced by this helper https://github.com/docker/docker-py/blob/7252086054904dfbf1ab3abe6ba494cec2db6181/tests/helpers.py#L111-L135

def assert_cat_socket_detached_with_keys(sock, inputs):
    if six.PY3 and hasattr(sock, '_sock'):
        sock = sock._sock

    for i in inputs:
        sock.sendall(i)
        time.sleep(0.5)

    # If we're using a Unix socket, the sock.send call will fail with a
    # BrokenPipeError ; INET sockets will just stop receiving / sending data
    # but will not raise an error
    if getattr(sock, 'family', -9) == getattr(socket, 'AF_UNIX', -1):
        with pytest.raises(socket.error):
            sock.sendall(b'make sure the socket is closed\n')
    elif isinstance(sock, paramiko.Channel):
        with pytest.raises(OSError):
            sock.sendall(b'make sure the socket is closed\n')
    else:
        sock.sendall(b"make sure the socket is closed\n")
        data = sock.recv(128)
        # New in 18.06: error message is broadcast over the socket when reading
        # after detach
        assert data == b'' or data.startswith(
            b'exec attach failed: error on attach stdin: read escape sequence'
        )

shin- · 2019-04-18T20:14:52Z

Removing the first if clause would probably work, though we'll still need it for older versions of the server. Shouldn't be too hard - I can take a look by EOW.

thaJeztah · 2019-04-18T22:22:11Z

Thanks @shin- !

shin- · 2019-04-23T01:05:26Z

@thaJeztah can you try docker/docker-py#2317 (commit: a16a2051a6a30e9a44e64ad2333c1a49b4f5b580) and let me know if that fixes it?

codecov · 2019-04-23T11:58:53Z

Codecov Report

❗ No coverage uploaded for pull request base (master@9b67b40). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #39076   +/-   ##
=========================================
  Coverage          ?   37.28%           
=========================================
  Files             ?      608           
  Lines             ?    45201           
  Branches          ?        0           
=========================================
  Hits              ?    16855           
  Misses            ?    26063           
  Partials          ?     2283

thaJeztah · 2019-04-23T11:59:16Z

Thanks! Added a temporary commit to switch to that commit 👍

thaJeztah · 2019-05-21T09:53:09Z

Janky failed on #39243

thaJeztah · 2019-07-14T11:59:30Z

~~temporarily rebased on top of #39068 (which has the docker-py bump)~~

thaJeztah · 2019-07-16T13:31:01Z

Wow; lots of weird failures on s390x; https://jenkins.dockerproject.org/job/Docker-PRs-s390x/14838/console

12:40:08 java.nio.file.FileSystemException: /home/jenkins/workspace/Docker-PRs-s390x/bundles/test-integration/DockerDaemonSuite.TestCLIProxyProxyTCPSock/dab712fc04589: Operation not permitted
12:40:08 	at sun.nio.fs.UnixException.translateToIOException(UnixException.java:91)

thaJeztah · 2019-07-16T13:33:14Z

I wonder what version of Docker is installed on those s390x machines

This hack was added to fix a compatibility with clients that were built using Go 1.5 and older (added in 3d6f598) This hack causes some problems with current clients; with Go 1.5 and older no longer being supported for some time, and being several years old, it should now be ok to remove this hack altogether. People using tools that are built with those versions of Go wouldn't have updated those for years, and are probably out of date anyway; that's not something we can continue taking into account. This will affect docker clients (the docker cli) for docker 1.12 and older. Those versions have reached EOL a long time ago (and have known unpatched vulnerabilities), so should no longer be used anyway, but We should add a nebtuib in the release notes, just in case someone, somewhere, still has such old tools. For those affected, using a more recent client (and if needed, setting the DOCKER_API_VERSION environment variable to the needed API version) should provide a way out. This reverts the changes originally made in; moby#22000 and moby#22888, which were to address moby#20865. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2019-07-18T19:26:05Z

Rebased, to get rid of #39068

This should be ready for review

kolyshkin

LGTM

These tests are 'docker.oldclient' and 'google.kubernetes.basic.docker.*'. 'docker.oldclient' tries to run docker cli 1.9 against daemon in the image, and fails with: --- FAIL: docker.oldclient (29.22s) cluster.go:117: Error response from daemon: 400 Bad Request: malformed Host header cluster.go:130: "/home/core/docker-1.9.1 run echo echo 'IT WORKED'" failed: output , status Process exited with status 1 This is related to moby/moby#39076, merged into 20.10 which removed some backwards compatibility. The 'google.kubernetes.basic.docker.*' tests fails with the following message in journal: Jul 15 14:17:42.446942 kubelet[4663]: F0715 14:17:42.446505 4663 server.go:274] failed to run Kubelet: misconfiguration: kubelet cgroup driver: "cgroupfs" is different from docker cgroup driver: "systemd" Kubernetes release 1.19 is the first one that properly supports the unified cgroup hierarchy. We also have other tests that test that kubernetes works (kubeadm) so we can disable the legacy ones. The old tests should be removed once the docker 20.10 upgrade has propagated to all channels. See also flatcar-archive/coreos-overlay#931 Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>

thaJeztah added area/api API status/2-code-review impact/api impact/changelog impact/deprecation labels Apr 15, 2019

GordonTheTurtle added the status/0-triage label Apr 15, 2019

thaJeztah force-pushed the remove_hack_malformed_host_header branch from 7aa0319 to 6685829 Compare April 15, 2019 08:21

This was referenced Apr 15, 2019

Refactor Read function in malformed_host_override.go #39046

Closed

fixes: #38745 lost UnixConn features in listener socket read hack #38928

Closed

thaJeztah added rebuild/janky and removed status/0-triage labels Apr 15, 2019

GordonTheTurtle removed the rebuild/janky label Apr 15, 2019

thaJeztah added the rebuild/janky label Apr 16, 2019

GordonTheTurtle removed the rebuild/janky label Apr 16, 2019

This comment has been minimized.

Sign in to view

thaJeztah added the rebuild/janky label Apr 16, 2019

GordonTheTurtle removed the rebuild/janky label Apr 16, 2019

thaJeztah added the rebuild/janky label Apr 18, 2019

GordonTheTurtle removed the rebuild/janky label Apr 18, 2019

shin- mentioned this pull request Apr 23, 2019

Fix socket detach helper docker/docker-py#2317

Merged

thaJeztah force-pushed the remove_hack_malformed_host_header branch from 6685829 to 8f45459 Compare April 23, 2019 11:58

thaJeztah requested a review from tianon as a code owner April 23, 2019 11:58

thaJeztah changed the title ~~Remove hack MalformedHostHeaderOverride~~ [WIP] Remove hack MalformedHostHeaderOverride Apr 23, 2019

GordonTheTurtle removed the rebuild/janky label May 20, 2019

thaJeztah force-pushed the remove_hack_malformed_host_header branch from c14641d to db057c9 Compare July 14, 2019 11:59

thaJeztah force-pushed the remove_hack_malformed_host_header branch from db057c9 to b211094 Compare July 14, 2019 12:01

This comment has been minimized.

Sign in to view

thaJeztah added the rebuild/janky label Jul 14, 2019

GordonTheTurtle removed the rebuild/janky label Jul 14, 2019

thaJeztah added the rebuild/janky label Jul 14, 2019

GordonTheTurtle removed the rebuild/janky label Jul 14, 2019

thaJeztah added the rebuild/janky label Jul 15, 2019

GordonTheTurtle removed the rebuild/janky label Jul 15, 2019

thaJeztah force-pushed the remove_hack_malformed_host_header branch from b211094 to 43ec501 Compare July 16, 2019 12:39

thaJeztah force-pushed the remove_hack_malformed_host_header branch from 43ec501 to f6b1f01 Compare July 18, 2019 19:25

thaJeztah changed the title ~~[WIP] Remove hack MalformedHostHeaderOverride~~ Remove hack MalformedHostHeaderOverride Jul 18, 2019

tiborvass approved these changes Jul 19, 2019

View reviewed changes

kolyshkin approved these changes Jul 19, 2019

View reviewed changes

thaJeztah merged commit 48da116 into moby:master Jul 20, 2019

thaJeztah deleted the remove_hack_malformed_host_header branch July 20, 2019 00:15

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

jepio mentioned this pull request Jul 14, 2021

Move to unified cgroupv2 hierarchy and upgrade docker to 20.10 flatcar-archive/coreos-overlay#931

Merged

7 tasks

thaJeztah mentioned this pull request Jul 12, 2023

Golang client fails to attach to streams with "http: invalid Host header" with go1.20.6, go1.19.11 #45935

Closed

Conversation

thaJeztah commented Apr 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Apr 15, 2019

Uh oh!

This comment has been minimized.

thaJeztah commented Apr 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shin- commented Apr 18, 2019

Uh oh!

thaJeztah commented Apr 18, 2019

Uh oh!

shin- commented Apr 23, 2019

Uh oh!

codecov bot commented Apr 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thaJeztah commented Apr 23, 2019

Uh oh!

thaJeztah commented May 21, 2019

Uh oh!

thaJeztah commented Jul 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

thaJeztah commented Jul 16, 2019

Uh oh!

thaJeztah commented Jul 16, 2019

Uh oh!

thaJeztah commented Jul 18, 2019

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

thaJeztah commented Apr 15, 2019 •

edited

Loading

thaJeztah commented Apr 18, 2019 •

edited

Loading

codecov bot commented Apr 23, 2019 •

edited

Loading

thaJeztah commented Jul 14, 2019 •

edited

Loading