commit 9e1f8d646e changed this to a
"nolint" comment due to a regression in GoSec. That regression was
fixed, so we can go back to use the more fine-grained "nosec" comment.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is a follow-up to 0e73168b7e
This repository is not yet a module (i.e., does not have a `go.mod`). This
is not problematic when building the code in GOPATH or "vendor" mode, but
when using the code as a module-dependency (in module-mode), different semantics
are applied since Go1.21, which switches Go _language versions_ on a per-module,
per-package, or even per-file base.
A condensed summary of that logic [is as follows][1]:
- For modules that have a go.mod containing a go version directive; that
version is considered a minimum _required_ version (starting with the
go1.19.13 and go1.20.8 patch releases: before those, it was only a
recommendation).
- For dependencies that don't have a go.mod (not a module), go language
version go1.16 is assumed.
- Likewise, for modules that have a go.mod, but the file does not have a
go version directive, go language version go1.16 is assumed.
- If a go.work file is present, but does not have a go version directive,
language version go1.17 is assumed.
When switching language versions, Go _downgrades_ the language version,
which means that language features (such as generics, and `any`) are not
available, and compilation fails. For example:
# github.com/docker/cli/cli/context/store
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/storeconfig.go:6:24: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
/go/pkg/mod/github.com/docker/cli@v25.0.0-beta.2+incompatible/cli/context/store/store.go:74:12: predeclared any requires go1.18 or later (-lang was set to go1.16; check go.mod)
Note that these fallbacks are per-module, per-package, and can even be
per-file, so _(indirect) dependencies_ can still use modern language
features, as long as their respective go.mod has a version specified.
Unfortunately, these failures do not occur when building locally (using
vendor / GOPATH mode), but will affect consumers of the module.
Obviously, this situation is not ideal, and the ultimate solution is to
move to go modules (add a go.mod), but this comes with a non-insignificant
risk in other areas (due to our complex dependency tree).
We can revert to using go1.16 language features only, but this may be
limiting, and may still be problematic when (e.g.) matching signatures
of dependencies.
There is an escape hatch: adding a `//go:build` directive to files that
make use of go language features. From the [go toolchain docs][2]:
> The go line for each module sets the language version the compiler enforces
> when compiling packages in that module. The language version can be changed
> on a per-file basis by using a build constraint.
>
> For example, a module containing code that uses the Go 1.21 language version
> should have a `go.mod` file with a go line such as `go 1.21` or `go 1.21.3`.
> If a specific source file should be compiled only when using a newer Go
> toolchain, adding `//go:build go1.22` to that source file both ensures that
> only Go 1.22 and newer toolchains will compile the file and also changes
> the language version in that file to Go 1.22.
This patch adds `//go:build` directives to those files using recent additions
to the language. It's currently using go1.19 as version to match the version
in our "vendor.mod", but we can consider being more permissive ("any" requires
go1.18 or up), or more "optimistic" (force go1.21, which is the version we
currently use to build).
For completeness sake, note that any file _without_ a `//go:build` directive
will continue to use go1.16 language version when used as a module.
[1]: 58c28ba286/src/cmd/go/internal/gover/version.go (L9-L56)
[2]; https://go.dev/doc/toolchain#:~:text=The%20go%20line%20for,file%20to%20Go%201.22
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/compose/types/types.go:568:17: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return []byte(fmt.Sprintf("%v", e.External)), nil
^
cli/command/formatter/buildcache.go:174:9: fmt.Sprintf can be replaced with faster strconv.Itoa (perfsprint)
return fmt.Sprintf("%d", c.v.UsageCount)
^
cli/command/formatter/buildcache.go:178:9: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return fmt.Sprintf("%t", c.v.InUse)
^
cli/command/formatter/buildcache.go:182:9: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return fmt.Sprintf("%t", c.v.Shared)
^
cli/command/formatter/image.go:259:9: fmt.Sprintf can be replaced with faster strconv.FormatInt (perfsprint)
return fmt.Sprintf("%d", c.i.Containers)
^
cli/command/formatter/tabwriter/tabwriter_test.go:698:9: fmt.Sprintf can be replaced with faster strconv.Itoa (perfsprint)
b.Run(fmt.Sprintf("%d", x), func(b *testing.B) {
^
cli/command/formatter/tabwriter/tabwriter_test.go:720:9: fmt.Sprintf can be replaced with faster strconv.Itoa (perfsprint)
b.Run(fmt.Sprintf("%d", h), func(b *testing.B) {
^
cli/command/image/prune.go:62:31: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
pruneFilters.Add("dangling", fmt.Sprintf("%v", !options.all))
^
cli/command/network/formatter.go:92:9: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return fmt.Sprintf("%v", c.n.EnableIPv6)
^
cli/command/network/formatter.go:96:9: fmt.Sprintf can be replaced with faster strconv.FormatBool (perfsprint)
return fmt.Sprintf("%v", c.n.Internal)
^
cli/command/service/formatter.go:745:9: fmt.Sprintf can be replaced with faster strconv.FormatUint (perfsprint)
pub = fmt.Sprintf("%d", pr.pStart)
^
cli/command/service/formatter.go:750:9: fmt.Sprintf can be replaced with faster strconv.FormatUint (perfsprint)
tgt = fmt.Sprintf("%d", pr.tStart)
^
cli/command/service/opts.go:49:10: fmt.Sprintf can be replaced with faster strconv.FormatUint (perfsprint)
return fmt.Sprintf("%v", *i.value)
^
cli/compose/loader/loader.go:720:36: fmt.Sprint can be replaced with faster strconv.Itoa (perfsprint)
v, err := toServicePortConfigs(fmt.Sprint(value))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test was skipped if the host was not using UTC timezone, because the output
of timestamps would be different, causing the test to fail.
This patch overrides the TZ env-var to make the test use UTC, so that we don't
have to skip the test.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The BuildKit dockerignore package was migrated to the patternmatcher
repository / module. This patch updates our uses of the BuildKit package
with its new location.
A small local change was made to keep the format of the existing error message,
because the "ignorefile" package is slightly more agnostic in that respect
and doesn't include ".dockerignore" in the error message.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This attempts to make it clearer that the --cgroup-parent option is only used
for the containers used during build. Instead of mentioning "build container",
I opted for using "RUN instructions" (to match the --network description),
although this may not be ideal (as it assumes the "Dockerfile" front-end, which
of course may not be the case).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Both these functions took the whole DockerCLI as argument, but only needed
the ConfigFile. ResolveAuthConfig also had an unused context.Context as
argument.
This patch updates both functions to accept a ConfigFile, and removes the
unused context.Context.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When passing a Dockerfile through stdin, it's not possible to specify the
name of the Dockerfile (using the `-f` option). When building with BuildKit
enabled, an error is already produced for this case, but the classic builder
silently ignored it.
This patch adds an error for this situation:
echo -e 'FROM busybox' | DOCKER_BUILDKIT=0 docker build -f some.Dockerfile -
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
environment-variable.
unable to prepare context: ambiguous Dockerfile source: both stdin and flag correspond to Dockerfiles
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This error was only used in a single location, so no need to define a
package-level variable for this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
the "golang.org/x/sys/execabs" package was introduced to address a security
issue on Windows, and changing the default behavior of os/exec was considered
a breaking change. go1.19 applied the behavior that was previously implemented
in the execabs package;
from the release notes: https://go.dev/doc/go1.19#os-exec-path
> Command and LookPath no longer allow results from a PATH search to be found
> relative to the current directory. This removes a common source of security
> problems but may also break existing programs that depend on using, say,
> exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe)
> in the current directory. See the os/exec package documentation for information
> about how best to update such programs.
>
> On Windows, Command and LookPath now respect the NoDefaultCurrentDirectoryInExePath
> environment variable, making it possible to disable the default implicit search
> of “.” in PATH lookups on Windows systems.
With those changes, we no longer need to use the execabs package, and we can
switch back to os/exec.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
None of the client will return the old error-types, so there's no need
to keep the compatibility code. We can consider deprecating this function
in favor of the errdefs equivalent this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This makes it possible to update the image loaded for e2e tests without
modifying all tests that use them.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Replace uses of this function in favor of the implementation in the
API types, so that we have a single, canonical implementation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/command/image/build/context_test.go:21:19: unused-parameter: parameter 't' seems to be unused, consider removing or renaming it as _ (revive)
func prepareEmpty(t *testing.T) string {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This code depended on the registry Service interface, which has been removed,
so needed to be refactored. Digging further into the reason this code existed,
it looked like the Class=plugin was previously required on Docker Hub to handle
plugins, but this requirement is no longer there, so we can remove this special
handling.
This patch removes the special handling to both remove the use of the registry.Service
interface, as well as removing complexity that is no longer needed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Tests mocking the output of GET images/json with fakeClient used an
array with one empty element as an empty response.
Change it to just an empty array.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Looks like the linter uses an explicit -lang, which (for go1.19)
results in some additional formatting for octal values.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also removing redundant defer for env.PatchAll(), which is now automatically
handled in t.Cleanup()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Paweł Gronowski
Sebastiaan van Stijn
Brian Goff
Sebastiaan van Stijn
CrazyMax
Jason Hall