Address CVE-2024-41110/GHSA-v23v-6jw2-98fq by upgrading github.com/docker/docker and github.com/docker/cli
The Go docker SDK v24.0.9 is subject to CVE-2024-41110/GHSA-v23v-6jw2-98fq, which is a Critical vulnerability. The closest fixed version is v25.0.6, which is a majour version bump. The docker SDK implementation changed enough in the jump from v24 to v25 that it
- triggered a lot of
staticchecklinter violations - broke a number of tests
- most importantly changed the way
mac-addressis set on a container.
The staticcheck linter violations were all deprecation warnings, and were simple to fix.
WRT broken tests, the concrete type assigned to docker.client.Client.Transport changed from http.Transport to otelhttp.Transport. The later type does not expose the TLSClientConfig field, which we need to access to assert in the test below that TLS was set correctly in various scenarios. I resorted to injecting an http.Transport instance into the http.Client used by the docker.Client, and caching that transport instance for later access.
Now, setting a container's mac-address...
In the docker SDK v24, a container's mac-address was configured via container.Config.MacAddress. In SDK v25 that approach no longer works. The MacAddress field still exists in the container.Config struct, but it is ignored, so assigning executor.Config.Docker.MacAddress to it has no effect. This effectively breaks setting the build container's mac-address from the users POV.
The new implementation of executor.networkConfig compensates for this and attempts to minimize the changes required to the integration tests to make it pass again. There are two test cases that changed significantly ("host, *"); where this configuration used to result in this error creating the container:
conflicting options: mac-address and the network modeNow it succeeds, and the container's NetworkSettings' host entry contains the configured mac-address. This is, IMO, an expansion of capabilities so it does not represent a breaking change
Note the removal of the assertion
assert.Equal(t, macAddress, info.Config.MacAddress, "config")from all test cases. This member is also deprecated and while it is currently still being set, it will be removed in the future, so we may as well stop relying on it now.
Most unfortunate of all, this way of setting mac-address requires a bump in the docker API version from 1.43 to 1.44, so we'll need to update the version of docker being used in our own CI pipeline before we can merge this MR.
Best reviewer commit at a time.
See:
Closes https://gitlab.com/gitlab-org/gitlab-runner/-/issues/37919+