Replace regex matching/replacement and re-reading of generated files with a simple parser, and struct to remember and manipulate the file content.
Annotate the generated file with a header comment saying the file is generated, but can be modified, and a trailing comment describing how the file was generated and listing external nameservers.
Always start with the host's resolv.conf file, whether generating config for host networking, or with/without an internal resolver - rather than editing a file previously generated for a different use-case.
Resolves an issue where rewrites of the generated file resulted in default IPv6 nameservers being unnecessarily added to the config.
This test was added in 27ef09a46ffeb8ba42548de937b68351009f30ea, which changed the Ping handling to ignore internal server errors. That case is tested in TestPingFail, which verifies that we accept the Ping response if a 500 status code was received.
The TestPingWithError test was added to verify behavior if a protocol (connection) error occurred; however the mock-client returned both a response, and an error; the error returned would only happen if a connection error occurred, which means that the server would not provide a reply.
Running the test also shows that returning a response is unexpected, and ignored:
=== RUN TestPingWithError 2024/02/23 14:16:49 RoundTripper returned a response & error; ignoring response 2024/02/23 14:16:49 RoundTripper returned a response & error; ignoring response --- PASS: TestPingWithError (0.00s) PASS
This patch updates the test to remove the response.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
client: NegotiateAPIVersion: do not ignore (connection) errors from Ping
NegotiateAPIVersion was ignoring errors returned by Ping. The intent here was to handle API responses from a daemon that may be in an unhealthy state, however this case is already handled by Ping itself.
Ping only returns an error when either failing to connect to the API (daemon not running or permissions errors), or when failing to parse the API response.
Neither of those should be ignored in this code, or considered a successful "ping", so update the code to return
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
client: doRequest: make sure we return a connection-error
This function has various errors that are returned when failing to make a connection (due to permission issues, TLS mis-configuration, or failing to resolve the TCP address).
The errConnectionFailed error is currently used as a special case when processing Ping responses. The current code did not consistently treat connection errors, and because of that could either absorb the error, or process the empty response.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
client: fix connection-errors being shadowed by API version mismatch errors
Commit e6907243af215a90fe36b377d89a49e3a2eded0a applied a fix for situations where the client was configured with API-version negotiation, but did not yet negotiate a version.
However, the checkVersion() function that was implemented copied the semantics of cli.NegotiateAPIVersion, which ignored connection failures with the assumption that connection errors would still surface further down.
However, when using the result of a failed negotiation for NewVersionError, an API version mismatch error would be produced, masking the actual connection error.
This patch changes the signature of checkVersion to return unexpected errors, including failures to connect to the API.
Before this patch:
docker -H unix:///no/such/socket.sock secret ls "secret list" requires API version 1.25, but the Docker daemon API version is 1.24
With this patch applied:
docker -H unix:///no/such/socket.sock secret ls Cannot connect to the Docker daemon at unix:///no/such/socket.sock. Is the docker daemon running?
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Don't use all `*.json` files blindly, take only these that are likely to be reports from go test. Also, use `find ... -exec` instead of piping results to `xargs`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
integration: Reset `OTEL_EXPORTER_OTLP_ENDPOINT` for sub-daemons
When creating a new daemon in the `TestDaemonProxy`, reset the `OTEL_EXPORTER_OTLP_ENDPOINT` to an empty value to disable OTEL collection to avoid it hitting the proxy.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This patch disables pulling legacy (schema1 and schema 2, version 1) images by default.
A `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE` environment-variable is introduced to allow re-enabling this feature, aligning with the environment variable used in containerd 2.0 (`CONTAINERD_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE`).
With this patch, attempts to pull a legacy image produces an error:
With graphdrivers:
docker pull docker:1.0 1.0: Pulling from library/docker [DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/
With the containerd image store enabled, output is slightly different as it returns the error before printing the `1.0: pulling ...`:
docker pull docker:1.0 Error response from daemon: [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/
Using the "distribution" endpoint to resolve the digest for an image also produces an error:
curl -v --unix-socket /var/run/docker.sock http://foo/distribution/docker.io/library/docker:1.0/json * Trying /var/run/docker.sock:0... * Connected to foo (/var/run/docker.sock) port 80 (#0) > GET /distribution/docker.io/library/docker:1.0/json HTTP/1.1 > Host: foo > User-Agent: curl/7.88.1 > Accept: */* > < HTTP/1.1 400 Bad Request < Api-Version: 1.45 < Content-Type: application/json < Docker-Experimental: false < Ostype: linux < Server: Docker/dev (linux) < Date: Tue, 27 Feb 2024 16:09:42 GMT < Content-Length: 354 < {"message":"[DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/"} * Connection #0 to host foo left intact
Starting the daemon with the `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE` env-var set to a non-empty value allows pulling the image;
docker pull docker:1.0 [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/ b0a0e6710d13: Already exists d193ad713811: Already exists ba7268c3149b: Already exists c862d82a67a2: Already exists Digest: sha256:5e7081837926c7a40e58881bbebc52044a95a62a2ea52fb240db3fc539212fe5 Status: Image is up to date for docker:1.0 docker.io/library/docker:1.0
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This message accidentally changed in ac2a028dcc05532109e14f8af105ca42c0abf1f3 because my IDE's "refactor tool" was a bit over-enthusiastic. It also went and updated the tests accordingly, so CI didn't catch this :)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
libn/cnmallocator: migrate tests to gotest.tools/v3
Apply command gotest.tools/v3/assert/cmd/gty-migrate-from-testify to the cnmallocator package to be consistent with the assertion library used elsewhere in moby.
While github.com/stretchr/testify is not used directly by any of the repository code, it is a transitive dependency via Swarmkit and therefore still easy to use without having to revendor. Add lint rules to ban importing testify packages to make sure nobody does.
Replace regex matching/replacement and re-reading of generated files with a simple parser, and struct to remember and manipulate the file content.
Annotate the generated file with a header comment saying the file is generated, but can be modified, and a trailing comment describing how the file was generated and listing external nameservers.
Always start with the host's resolv.conf file, whether generating config for host networking, or with/without an internal resolver - rather than editing a file previously generated for a different use-case.
Resolves an issue where rewrites of the generated file resulted in default IPv6 nameservers being unnecessarily added to the config.
This test was added in 27ef09a46ffeb8ba42548de937b68351009f30ea, which changed the Ping handling to ignore internal server errors. That case is tested in TestPingFail, which verifies that we accept the Ping response if a 500 status code was received.
The TestPingWithError test was added to verify behavior if a protocol (connection) error occurred; however the mock-client returned both a response, and an error; the error returned would only happen if a connection error occurred, which means that the server would not provide a reply.
Running the test also shows that returning a response is unexpected, and ignored:
=== RUN TestPingWithError 2024/02/23 14:16:49 RoundTripper returned a response & error; ignoring response 2024/02/23 14:16:49 RoundTripper returned a response & error; ignoring response --- PASS: TestPingWithError (0.00s) PASS
This patch updates the test to remove the response.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
client: NegotiateAPIVersion: do not ignore (connection) errors from Ping
NegotiateAPIVersion was ignoring errors returned by Ping. The intent here was to handle API responses from a daemon that may be in an unhealthy state, however this case is already handled by Ping itself.
Ping only returns an error when either failing to connect to the API (daemon not running or permissions errors), or when failing to parse the API response.
Neither of those should be ignored in this code, or considered a successful "ping", so update the code to return
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
client: doRequest: make sure we return a connection-error
This function has various errors that are returned when failing to make a connection (due to permission issues, TLS mis-configuration, or failing to resolve the TCP address).
The errConnectionFailed error is currently used as a special case when processing Ping responses. The current code did not consistently treat connection errors, and because of that could either absorb the error, or process the empty response.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
client: fix connection-errors being shadowed by API version mismatch errors
Commit e6907243af215a90fe36b377d89a49e3a2eded0a applied a fix for situations where the client was configured with API-version negotiation, but did not yet negotiate a version.
However, the checkVersion() function that was implemented copied the semantics of cli.NegotiateAPIVersion, which ignored connection failures with the assumption that connection errors would still surface further down.
However, when using the result of a failed negotiation for NewVersionError, an API version mismatch error would be produced, masking the actual connection error.
This patch changes the signature of checkVersion to return unexpected errors, including failures to connect to the API.
Before this patch:
docker -H unix:///no/such/socket.sock secret ls "secret list" requires API version 1.25, but the Docker daemon API version is 1.24
With this patch applied:
docker -H unix:///no/such/socket.sock secret ls Cannot connect to the Docker daemon at unix:///no/such/socket.sock. Is the docker daemon running?
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Don't use all `*.json` files blindly, take only these that are likely to be reports from go test. Also, use `find ... -exec` instead of piping results to `xargs`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
integration: Reset `OTEL_EXPORTER_OTLP_ENDPOINT` for sub-daemons
When creating a new daemon in the `TestDaemonProxy`, reset the `OTEL_EXPORTER_OTLP_ENDPOINT` to an empty value to disable OTEL collection to avoid it hitting the proxy.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This patch disables pulling legacy (schema1 and schema 2, version 1) images by default.
A `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE` environment-variable is introduced to allow re-enabling this feature, aligning with the environment variable used in containerd 2.0 (`CONTAINERD_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE`).
With this patch, attempts to pull a legacy image produces an error:
With graphdrivers:
docker pull docker:1.0 1.0: Pulling from library/docker [DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/
With the containerd image store enabled, output is slightly different as it returns the error before printing the `1.0: pulling ...`:
docker pull docker:1.0 Error response from daemon: [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/
Using the "distribution" endpoint to resolve the digest for an image also produces an error:
curl -v --unix-socket /var/run/docker.sock http://foo/distribution/docker.io/library/docker:1.0/json * Trying /var/run/docker.sock:0... * Connected to foo (/var/run/docker.sock) port 80 (#0) > GET /distribution/docker.io/library/docker:1.0/json HTTP/1.1 > Host: foo > User-Agent: curl/7.88.1 > Accept: */* > < HTTP/1.1 400 Bad Request < Api-Version: 1.45 < Content-Type: application/json < Docker-Experimental: false < Ostype: linux < Server: Docker/dev (linux) < Date: Tue, 27 Feb 2024 16:09:42 GMT < Content-Length: 354 < {"message":"[DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/"} * Connection #0 to host foo left intact
Starting the daemon with the `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE` env-var set to a non-empty value allows pulling the image;
docker pull docker:1.0 [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/ b0a0e6710d13: Already exists d193ad713811: Already exists ba7268c3149b: Already exists c862d82a67a2: Already exists Digest: sha256:5e7081837926c7a40e58881bbebc52044a95a62a2ea52fb240db3fc539212fe5 Status: Image is up to date for docker:1.0 docker.io/library/docker:1.0
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This message accidentally changed in ac2a028dcc05532109e14f8af105ca42c0abf1f3 because my IDE's "refactor tool" was a bit over-enthusiastic. It also went and updated the tests accordingly, so CI didn't catch this :)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
libn/cnmallocator: migrate tests to gotest.tools/v3
Apply command gotest.tools/v3/assert/cmd/gty-migrate-from-testify to the cnmallocator package to be consistent with the assertion library used elsewhere in moby.
While github.com/stretchr/testify is not used directly by any of the repository code, it is a transitive dependency via Swarmkit and therefore still easy to use without having to revendor. Add lint rules to ban importing testify packages to make sure nobody does.