diff --git a/.golangci.yml b/.golangci.yml index 8114921f..428426f1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -37,12 +37,8 @@ linters: rules: main: deny: - - pkg: io/ioutil - desc: use os or io instead - pkg: golang.org/x/exp desc: it's experimental and unreliable - - pkg: github.com/pkg/errors - desc: use builtin errors package instead nolintlint: allow-unused: false require-explanation: true diff --git a/act/container/docker_cli.go b/act/container/docker_cli.go index ca8e7b5a..d1066737 100644 --- a/act/container/docker_cli.go +++ b/act/container/docker_cli.go @@ -4,8 +4,10 @@ //go:build !(WITHOUT_DOCKER || !(linux || darwin || windows || netbsd)) -// This file is exact copy of https://github.com/docker/cli/blob/9a471180cb7d39c236d090399a9d362c3f5a8ebd/cli/command/container/opts.go -// appended with license information. +// This file is exact copy of https://github.com/docker/cli/blob/dfc4efb1e2ab8c06d70d2a1366ad448d2f917e90/cli/command/container/opts.go with: +// * appended with license information +// * regexp and loader.ParseVolume in place of the import-restricted internal/lazyregexp and internal/volumespec +// * invalidParameter from the package's errors.go, and convertPortSet/convertPortMap for the callers in docker_run.go // // docker/cli is licensed under the Apache License, Version 2.0. // See DOCKER_LICENSE for the full license text. @@ -30,6 +32,7 @@ import ( "strings" "time" + cerrdefs "github.com/containerd/errdefs" "github.com/docker/cli/cli/compose/loader" "github.com/docker/cli/opts" "github.com/docker/go-connections/nat" @@ -380,7 +383,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con var binds []string volumes := copts.volumes.GetMap() // add any bind targets to the list of container volumes - for bind := range copts.volumes.GetMap() { + for bind := range volumes { parsed, err := loader.ParseVolume(bind) if err != nil { return nil, err @@ -515,13 +518,13 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con // collect all the environment variables for the container envVariables, err := opts.ReadKVEnvStrings(copts.envFile.GetSlice(), copts.env.GetSlice()) if err != nil { - return nil, err + return nil, fmt.Errorf("--env-file: %w", err) } // collect all the labels for the container labels, err := opts.ReadKVStrings(copts.labelsFile.GetSlice(), copts.labels.GetSlice()) if err != nil { - return nil, err + return nil, fmt.Errorf("--label-file: %w", err) } pidMode := container.PidMode(copts.pidMode) @@ -1164,16 +1167,17 @@ func toNetipAddrSlice(ips []string) []netip.Addr { } // invalidParameter wraps an error to indicate it was caused by invalid input. -// This is a local replacement for docker/docker/errdefs.InvalidParameter. -type invalidParameterError struct{ error } +// This is a copy of docker/cli's cli/command/container/errors.go, which is not importable. +type invalidParameterErr struct{ error } -func (e invalidParameterError) InvalidParameter() {} +func (invalidParameterErr) InvalidParameter() {} +func (e invalidParameterErr) Unwrap() error { return e.error } func invalidParameter(err error) error { - if err == nil { - return nil + if err == nil || cerrdefs.IsInvalidArgument(err) { + return err } - return invalidParameterError{err} + return invalidParameterErr{err} } func convertPortSet(ports nat.PortSet) (network.PortSet, error) { diff --git a/act/container/docker_cli_test.go b/act/container/docker_cli_test.go index 08146246..6b1f4e77 100644 --- a/act/container/docker_cli_test.go +++ b/act/container/docker_cli_test.go @@ -2,20 +2,22 @@ // Copyright 2022 The nektos/act Authors. All rights reserved. // SPDX-License-Identifier: MIT -// This file is exact copy of https://github.com/docker/cli/blob/9ac8584acfd501c3f4da0e845e3a40ed15c85041/cli/command/container/opts_test.go with: +// This file is exact copy of https://github.com/docker/cli/blob/dfc4efb1e2ab8c06d70d2a1366ad448d2f917e90/cli/command/container/opts_test.go with: // * appended with license information -// * commented out case 'invalid-mixed-network-types' in test TestParseNetworkConfig +// * added tests for the locally changed parseDevice, validateDevice and invalidParameter // // docker/cli is licensed under the Apache License, Version 2.0. // See DOCKER_LICENSE for the full license text. // -//nolint:depguard,gocritic // verbatim copy from docker/cli tests +//nolint:gocritic // verbatim copy from docker/cli tests package container import ( + "errors" "fmt" "io" + "net" "net/netip" "os" "runtime" @@ -23,18 +25,23 @@ import ( "testing" "time" - "github.com/docker/go-connections/nat" - "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/moby/moby/api/types/container" networktypes "github.com/moby/moby/api/types/network" - "github.com/pkg/errors" "github.com/spf13/pflag" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/skip" ) +func mustParseMAC(s string) networktypes.HardwareAddr { + mac, err := net.ParseMAC(s) + if err != nil { + panic(err) + } + return networktypes.HardwareAddr(mac) +} + func TestValidateAttach(t *testing.T) { valid := []string{ "stdin", @@ -64,12 +71,12 @@ func parseRun(args []string) (*container.Config, *container.HostConfig, *network if err := flags.Parse(args); err != nil { return nil, nil, nil, err } - // TODO: fix tests to accept ContainerConfig - containerConfig, err := parse(flags, copts, runtime.GOOS) + // TODO(dnephin): fix tests to accept ContainerConfig; see https://github.com/moby/moby/pull/31621 + containerCfg, err := parse(flags, copts, runtime.GOOS) if err != nil { return nil, nil, nil, err } - return containerConfig.Config, containerConfig.HostConfig, containerConfig.NetworkingConfig, err + return containerCfg.Config, containerCfg.HostConfig, containerCfg.NetworkingConfig, err } func setupRunFlags() (*pflag.FlagSet, *containerOptions) { @@ -82,20 +89,81 @@ func setupRunFlags() (*pflag.FlagSet, *containerOptions) { func mustParse(t *testing.T, args string) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig) { t.Helper() - config, hostConfig, networkingConfig, err := parseRun(append(strings.Split(args, " "), "ubuntu", "bash")) + config, hostConfig, nwConfig, err := parseRun(append(strings.Split(args, " "), "ubuntu", "bash")) assert.NilError(t, err) - return config, hostConfig, networkingConfig + return config, hostConfig, nwConfig } func TestParseRunLinks(t *testing.T) { - if _, hostConfig, _ := mustParse(t, "--link a:b"); len(hostConfig.Links) == 0 || hostConfig.Links[0] != "a:b" { - t.Fatalf("Error parsing links. Expected []string{\"a:b\"}, received: %v", hostConfig.Links) + tests := []struct { + name string + input string + expHostConfigLinks []string + expNetConfigLinks map[string][]string + }{ + // Default bridge - legacy links ... + { + name: "default/onelink", + input: "--link a:b", + expHostConfigLinks: []string{"a:b"}, + expNetConfigLinks: map[string][]string{"default": nil}, + }, + { + name: "default/twolinks", + input: "--link a:b --link c:d", + expHostConfigLinks: []string{"a:b", "c:d"}, + expNetConfigLinks: map[string][]string{"default": nil}, + }, + { + name: "bridge/onelink", + input: "--network bridge --link a:b", + expHostConfigLinks: []string{"a:b"}, + // expNetConfigLinks - no EndpointsConfig is created for a single named network with no options set. + // See the "For backward compatibility" comment in parseNetworkOpts(). + }, + { + name: "default/nolinks", + expNetConfigLinks: map[string][]string{"default": nil}, + }, + + // User-defined bridge - links become DNS aliases ... + { + name: "userdefnet/onelink", + input: "--network userdefnet --link a:b", + expHostConfigLinks: []string{"a:b"}, + expNetConfigLinks: map[string][]string{"userdefnet": {"a:b"}}, + }, + { + name: "userdefnet/twolinks", + input: "--network userdefnet --link a:b --link c:d", + expHostConfigLinks: []string{"a:b", "c:d"}, + expNetConfigLinks: map[string][]string{"userdefnet": {"a:b", "c:d"}}, + }, + { + name: "userdefnet/nolinks", + input: "--network userdefnet", + }, + { + // Link options are applied to the first network (and there's no "advanced syntax" + // link key, like "--network name=userdefnet,link=a:b"). + name: "links apply to the first network", + input: "--network userdefnet --link a:b --network bar --link c:d", + expHostConfigLinks: []string{"a:b", "c:d"}, + expNetConfigLinks: map[string][]string{"userdefnet": {"a:b", "c:d"}, "bar": nil}, + }, } - if _, hostConfig, _ := mustParse(t, "--link a:b --link c:d"); len(hostConfig.Links) < 2 || hostConfig.Links[0] != "a:b" || hostConfig.Links[1] != "c:d" { - t.Fatalf("Error parsing links. Expected []string{\"a:b\", \"c:d\"}, received: %v", hostConfig.Links) - } - if _, hostConfig, _ := mustParse(t, ""); len(hostConfig.Links) != 0 { - t.Fatalf("Error parsing links. No link expected, received: %v", hostConfig.Links) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, hostConfig, netConfig := mustParse(t, tc.input) + assert.Check(t, is.DeepEqual(hostConfig.Links, tc.expHostConfigLinks)) + assert.Check(t, is.Len(netConfig.EndpointsConfig, len(tc.expNetConfigLinks))) + for netName, expLinks := range tc.expNetConfigLinks { + nc, ok := netConfig.EndpointsConfig[netName] + assert.Assert(t, ok) + assert.Check(t, is.DeepEqual(nc.Links, expLinks)) + } + }) } } @@ -294,37 +362,7 @@ func compareRandomizedStrings(a, b, c, d string) error { if a == d && b == c { return nil } - return errors.Errorf("strings don't match") -} - -func mustNetworkPort(t *testing.T, value string) networktypes.Port { - t.Helper() - - port, err := networktypes.ParsePort(value) - if err != nil { - t.Fatalf("failed to parse network port %q: %v", value, err) - } - return port -} - -func mustAddr(t *testing.T, value string) netip.Addr { - t.Helper() - - addr, err := netip.ParseAddr(value) - if err != nil { - t.Fatalf("failed to parse address %q: %v", value, err) - } - return addr -} - -func mustAddrs(t *testing.T, values ...string) []netip.Addr { - t.Helper() - - addrs := make([]netip.Addr, 0, len(values)) - for _, value := range values { - addrs = append(addrs, mustAddr(t, value)) - } - return addrs + return errors.New("strings don't match") } // Simple parse with MacAddress validation @@ -334,10 +372,11 @@ func TestParseWithMacAddress(t *testing.T) { if _, _, _, err := parseRun([]string{invalidMacAddress, "img", "cmd"}); err != nil && err.Error() != "invalidMacAddress is not a valid mac address" { t.Fatalf("Expected an error with %v mac-address, got %v", invalidMacAddress, err) } - _, hostConfig, networkingConfig := mustParse(t, validMacAddress) - endpoint := networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)] - assert.Check(t, endpoint != nil) - assert.Equal(t, "92:d0:c6:0a:29:33", endpoint.MacAddress.String()) + _, hostConfig, nwConfig := mustParse(t, validMacAddress) + defaultNw := hostConfig.NetworkMode.NetworkName() + if nwConfig.EndpointsConfig[defaultNw].MacAddress.String() != "92:d0:c6:0a:29:33" { + t.Fatalf("Expected the default endpoint to have the MacAddress '92:d0:c6:0a:29:33' set, got '%v'", nwConfig.EndpointsConfig[defaultNw].MacAddress) + } } func TestRunFlagsParseWithMemory(t *testing.T) { @@ -408,93 +447,144 @@ func TestParseHostnameDomainname(t *testing.T) { } func TestParseWithExpose(t *testing.T) { - invalids := []string{ - ":", - "8080:9090", - "/tcp", - "/udp", - "NaN/tcp", - "NaN-NaN/tcp", - "8080-NaN/tcp", - "1234567890-8080/tcp", - } - valids := map[string][]nat.Port{ - "8080/tcp": {"8080/tcp"}, - "8080/udp": {"8080/udp"}, - "8080/ncp": {"8080/ncp"}, - "8080-8080/udp": {"8080/udp"}, - "8080-8082/tcp": {"8080/tcp", "8081/tcp", "8082/tcp"}, - } - for _, expose := range invalids { - if _, _, _, err := parseRun([]string{fmt.Sprintf("--expose=%v", expose), "img", "cmd"}); err == nil { - t.Fatalf("Expected error with '--expose=%v', got none", expose) + t.Run("invalid", func(t *testing.T) { + tests := map[string]string{ + ":": `invalid range format for --expose: invalid start port ':': invalid syntax`, + "8080:9090": `invalid range format for --expose: invalid start port '8080:9090': invalid syntax`, + "/tcp": `invalid range format for --expose: invalid start port '': value is empty`, + "/udp": `invalid range format for --expose: invalid start port '': value is empty`, + "NaN/tcp": `invalid range format for --expose: invalid start port 'NaN': invalid syntax`, + "NaN-NaN/tcp": `invalid range format for --expose: invalid start port 'NaN': invalid syntax`, + "8080-NaN/tcp": `invalid range format for --expose: invalid end port 'NaN': invalid syntax`, + "1234567890-8080/tcp": `invalid range format for --expose: invalid start port '1234567890': value out of range`, } - } - for expose, exposedPorts := range valids { - config, _, _, err := parseRun([]string{fmt.Sprintf("--expose=%v", expose), "img", "cmd"}) - if err != nil { - t.Fatal(err) + for expose, expectedError := range tests { + t.Run(expose, func(t *testing.T) { + _, _, _, err := parseRun([]string{fmt.Sprintf("--expose=%v", expose), "img", "cmd"}) + assert.Error(t, err, expectedError) + }) } - if len(config.ExposedPorts) != len(exposedPorts) { - t.Fatalf("Expected %v exposed port, got %v", len(exposedPorts), len(config.ExposedPorts)) + }) + t.Run("valid", func(t *testing.T) { + tests := map[string][]networktypes.Port{ + "8080/tcp": {networktypes.MustParsePort("8080/tcp")}, + "8080/udp": {networktypes.MustParsePort("8080/udp")}, + "8080/ncp": {networktypes.MustParsePort("8080/ncp")}, + "8080-8080/udp": {networktypes.MustParsePort("8080/udp")}, + "8080-8082/tcp": {networktypes.MustParsePort("8080/tcp"), networktypes.MustParsePort("8081/tcp"), networktypes.MustParsePort("8082/tcp")}, } - for _, port := range exposedPorts { - if _, ok := config.ExposedPorts[mustNetworkPort(t, string(port))]; !ok { - t.Fatalf("Expected %v, got %v", exposedPorts, config.ExposedPorts) - } + for expose, exposedPorts := range tests { + t.Run(expose, func(t *testing.T) { + config, _, _, err := parseRun([]string{fmt.Sprintf("--expose=%v", expose), "img", "cmd"}) + assert.NilError(t, err) + for _, port := range exposedPorts { + _, ok := config.ExposedPorts[port] + assert.Check(t, ok, "missing port %q in exposed ports: %#+v", port, config.ExposedPorts[port]) + } + }) } - } - // Merge with actual published port - config, _, _, err := parseRun([]string{"--publish=80", "--expose=80-81/tcp", "img", "cmd"}) - if err != nil { - t.Fatal(err) - } - if len(config.ExposedPorts) != 2 { - t.Fatalf("Expected 2 exposed ports, got %v", config.ExposedPorts) - } - ports := []nat.Port{"80/tcp", "81/tcp"} - for _, port := range ports { - if _, ok := config.ExposedPorts[mustNetworkPort(t, string(port))]; !ok { - t.Fatalf("Expected %v, got %v", ports, config.ExposedPorts) + }) + + t.Run("merge with published", func(t *testing.T) { + // Merge with actual published port + config, _, _, err := parseRun([]string{"--publish=80", "--expose=80-81/tcp", "img", "cmd"}) + assert.NilError(t, err) + assert.Check(t, is.Len(config.ExposedPorts, 2)) + ports := []networktypes.Port{networktypes.MustParsePort("80/tcp"), networktypes.MustParsePort("81/tcp")} + for _, port := range ports { + _, ok := config.ExposedPorts[port] + assert.Check(t, ok, "missing port %q in exposed ports: %#+v", port, config.ExposedPorts[port]) } - } + }) } func TestParseDevice(t *testing.T) { skip.If(t, runtime.GOOS != "linux") // Windows and macOS validate server-side - valids := map[string]container.DeviceMapping{ - "/dev/snd": { - PathOnHost: "/dev/snd", - PathInContainer: "/dev/snd", - CgroupPermissions: "rwm", + testCases := []struct { + devices []string + deviceMapping *container.DeviceMapping + deviceRequests []container.DeviceRequest + }{ + { + devices: []string{"/dev/snd"}, + deviceMapping: &container.DeviceMapping{ + PathOnHost: "/dev/snd", + PathInContainer: "/dev/snd", + CgroupPermissions: "rwm", + }, }, - "/dev/snd:rw": { - PathOnHost: "/dev/snd", - PathInContainer: "/dev/snd", - CgroupPermissions: "rw", + { + devices: []string{"/dev/snd:rw"}, + deviceMapping: &container.DeviceMapping{ + PathOnHost: "/dev/snd", + PathInContainer: "/dev/snd", + CgroupPermissions: "rw", + }, }, - "/dev/snd:/something": { - PathOnHost: "/dev/snd", - PathInContainer: "/something", - CgroupPermissions: "rwm", + { + devices: []string{"/dev/snd:/something"}, + deviceMapping: &container.DeviceMapping{ + PathOnHost: "/dev/snd", + PathInContainer: "/something", + CgroupPermissions: "rwm", + }, }, - "/dev/snd:/something:rw": { - PathOnHost: "/dev/snd", - PathInContainer: "/something", - CgroupPermissions: "rw", + { + devices: []string{"/dev/snd:/something:rw"}, + deviceMapping: &container.DeviceMapping{ + PathOnHost: "/dev/snd", + PathInContainer: "/something", + CgroupPermissions: "rw", + }, + }, + { + devices: []string{"vendor.com/class=name"}, + deviceMapping: nil, + deviceRequests: []container.DeviceRequest{ + { + Driver: "cdi", + DeviceIDs: []string{"vendor.com/class=name"}, + }, + }, + }, + { + devices: []string{"vendor.com/class=name", "/dev/snd:/something:rw"}, + deviceMapping: &container.DeviceMapping{ + PathOnHost: "/dev/snd", + PathInContainer: "/something", + CgroupPermissions: "rw", + }, + deviceRequests: []container.DeviceRequest{ + { + Driver: "cdi", + DeviceIDs: []string{"vendor.com/class=name"}, + }, + }, }, } - for device, deviceMapping := range valids { - _, hostconfig, _, err := parseRun([]string{fmt.Sprintf("--device=%v", device), "img", "cmd"}) - if err != nil { - t.Fatal(err) - } - if len(hostconfig.Devices) != 1 { - t.Fatalf("Expected 1 devices, got %v", hostconfig.Devices) - } - if hostconfig.Devices[0] != deviceMapping { - t.Fatalf("Expected %v, got %v", deviceMapping, hostconfig.Devices) - } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("%s", tc.devices), func(t *testing.T) { + var args []string + for _, d := range tc.devices { + args = append(args, fmt.Sprintf("--device=%v", d)) + } + args = append(args, "img", "cmd") + + _, hostconfig, _, err := parseRun(args) + + assert.NilError(t, err) + + if tc.deviceMapping != nil { + if assert.Check(t, is.Len(hostconfig.Devices, 1)) { + assert.Check(t, is.DeepEqual(*tc.deviceMapping, hostconfig.Devices[0])) + } + } else { + assert.Check(t, is.Len(hostconfig.Devices, 0)) + } + + assert.Check(t, is.DeepEqual(tc.deviceRequests, hostconfig.DeviceRequests)) + }) } } @@ -573,23 +663,23 @@ func TestParseDeviceByServerOS(t *testing.T) { func TestParseNetworkConfig(t *testing.T) { tests := []struct { - name string - flags []string - expected map[string]*networktypes.EndpointSettings - expectedCfg container.HostConfig - expectedErr string + name string + flags []string + expected map[string]*networktypes.EndpointSettings + expectedHostCfg container.HostConfig + expectedErr string }{ { - name: "single-network-legacy", - flags: []string{"--network", "net1"}, - expected: map[string]*networktypes.EndpointSettings{}, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + name: "single-network-legacy", + flags: []string{"--network", "net1"}, + expected: map[string]*networktypes.EndpointSettings{}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { - name: "single-network-advanced", - flags: []string{"--network", "name=net1"}, - expected: map[string]*networktypes.EndpointSettings{}, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + name: "single-network-advanced", + flags: []string{"--network", "name=net1"}, + expected: map[string]*networktypes.EndpointSettings{}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "single-network-legacy-with-options", @@ -607,15 +697,15 @@ func TestParseNetworkConfig(t *testing.T) { expected: map[string]*networktypes.EndpointSettings{ "net1": { IPAMConfig: &networktypes.EndpointIPAMConfig{ - IPv4Address: mustAddr(t, "172.20.88.22"), - IPv6Address: mustAddr(t, "2001:db8::8822"), - LinkLocalIPs: mustAddrs(t, "169.254.2.2", "fe80::169:254:2:2"), + IPv4Address: netip.MustParseAddr("172.20.88.22"), + IPv6Address: netip.MustParseAddr("2001:db8::8822"), + LinkLocalIPs: []netip.Addr{netip.MustParseAddr("169.254.2.2"), netip.MustParseAddr("fe80::169:254:2:2")}, }, Links: []string{"foo:bar", "bar:baz"}, Aliases: []string{"web1", "web2"}, }, }, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "multiple-network-advanced-mixed", @@ -631,14 +721,15 @@ func TestParseNetworkConfig(t *testing.T) { "--network-alias", "web2", "--network", "net2", "--network", "name=net3,alias=web3,driver-opt=field3=value3,ip=172.20.88.22,ip6=2001:db8::8822", + "--network", "name=net4,mac-address=02:32:1c:23:00:04,link-local-ip=169.254.169.254", }, expected: map[string]*networktypes.EndpointSettings{ "net1": { DriverOpts: map[string]string{"field1": "value1"}, IPAMConfig: &networktypes.EndpointIPAMConfig{ - IPv4Address: mustAddr(t, "172.20.88.22"), - IPv6Address: mustAddr(t, "2001:db8::8822"), - LinkLocalIPs: mustAddrs(t, "169.254.2.2", "fe80::169:254:2:2"), + IPv4Address: netip.MustParseAddr("172.20.88.22"), + IPv6Address: netip.MustParseAddr("2001:db8::8822"), + LinkLocalIPs: []netip.Addr{netip.MustParseAddr("169.254.2.2"), netip.MustParseAddr("fe80::169:254:2:2")}, }, Links: []string{"foo:bar", "bar:baz"}, Aliases: []string{"web1", "web2"}, @@ -647,17 +738,23 @@ func TestParseNetworkConfig(t *testing.T) { "net3": { DriverOpts: map[string]string{"field3": "value3"}, IPAMConfig: &networktypes.EndpointIPAMConfig{ - IPv4Address: mustAddr(t, "172.20.88.22"), - IPv6Address: mustAddr(t, "2001:db8::8822"), + IPv4Address: netip.MustParseAddr("172.20.88.22"), + IPv6Address: netip.MustParseAddr("2001:db8::8822"), }, Aliases: []string{"web3"}, }, + "net4": { + MacAddress: mustParseMAC("02:32:1c:23:00:04"), + IPAMConfig: &networktypes.EndpointIPAMConfig{ + LinkLocalIPs: []netip.Addr{netip.MustParseAddr("169.254.169.254")}, + }, + }, }, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "single-network-advanced-with-options", - flags: []string{"--network", "name=net1,alias=web1,alias=web2,driver-opt=field1=value1,driver-opt=field2=value2,ip=172.20.88.22,ip6=2001:db8::8822"}, + flags: []string{"--network", "name=net1,alias=web1,alias=web2,driver-opt=field1=value1,driver-opt=field2=value2,ip=172.20.88.22,ip6=2001:db8::8822,mac-address=02:32:1c:23:00:04"}, expected: map[string]*networktypes.EndpointSettings{ "net1": { DriverOpts: map[string]string{ @@ -665,19 +762,31 @@ func TestParseNetworkConfig(t *testing.T) { "field2": "value2", }, IPAMConfig: &networktypes.EndpointIPAMConfig{ - IPv4Address: mustAddr(t, "172.20.88.22"), - IPv6Address: mustAddr(t, "2001:db8::8822"), + IPv4Address: netip.MustParseAddr("172.20.88.22"), + IPv6Address: netip.MustParseAddr("2001:db8::8822"), }, - Aliases: []string{"web1", "web2"}, + Aliases: []string{"web1", "web2"}, + MacAddress: mustParseMAC("02:32:1c:23:00:04"), }, }, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { - name: "multiple-networks", - flags: []string{"--network", "net1", "--network", "name=net2"}, - expected: map[string]*networktypes.EndpointSettings{"net1": {}, "net2": {}}, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + name: "multiple-networks", + flags: []string{"--network", "net1", "--network", "name=net2"}, + expected: map[string]*networktypes.EndpointSettings{"net1": {}, "net2": {}}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, + }, + { + name: "advanced-options-with-standalone-mac-address-flag", + flags: []string{"--network=name=net1,alias=foobar", "--mac-address", "52:0f:f3:dc:50:10"}, + expected: map[string]*networktypes.EndpointSettings{ + "net1": { + Aliases: []string{"foobar"}, + MacAddress: mustParseMAC("52:0f:f3:dc:50:10"), + }, + }, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "conflict-network", @@ -699,13 +808,26 @@ func TestParseNetworkConfig(t *testing.T) { flags: []string{"--network", "name=net1,ip=172.20.88.22,ip6=2001:db8::8822", "--ip6", "2001:db8::8822"}, expectedErr: `conflicting options: cannot specify both --ip6 and per-network IPv6 address`, }, - // case is skipped as it fails w/o any change - // - //{ - // name: "invalid-mixed-network-types", - // flags: []string{"--network", "name=host", "--network", "net1"}, - // expectedErr: `conflicting options: cannot attach both user-defined and non-user-defined network-modes`, - //}, + { + name: "invalid-mixed-network-types", + flags: []string{"--network", "name=host", "--network", "net1"}, + expectedErr: `conflicting options: cannot attach both user-defined and non-user-defined network-modes`, + }, + { + name: "conflict-options-link-local-ip", + flags: []string{"--network", "name=net1,link-local-ip=169.254.169.254", "--link-local-ip", "169.254.10.8"}, + expectedErr: `conflicting options: cannot specify both --link-local-ip and per-network link-local IP addresses`, + }, + { + name: "conflict-options-mac-address", + flags: []string{"--network", "name=net1,mac-address=02:32:1c:23:00:04", "--mac-address", "02:32:1c:23:00:04"}, + expectedErr: `conflicting options: cannot specify both --mac-address and per-network MAC address`, + }, + { + name: "invalid-mac-address", + flags: []string{"--network", "name=net1,mac-address=foobar"}, + expectedErr: "foobar is not a valid mac address", + }, } for _, tc := range tests { @@ -718,10 +840,8 @@ func TestParseNetworkConfig(t *testing.T) { } assert.NilError(t, err) - assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedCfg.NetworkMode) - if diff := cmp.Diff(tc.expected, nwConfig.EndpointsConfig, cmpopts.EquateComparable(netip.Addr{})); diff != "" { - t.Fatalf("unexpected endpoints (-want +got):\n%s", diff) - } + assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedHostCfg.NetworkMode) + assert.DeepEqual(t, nwConfig.EndpointsConfig, tc.expected, cmpopts.EquateComparable(netip.Addr{})) }) } } @@ -770,42 +890,84 @@ func TestRunFlagsParseShmSize(t *testing.T) { } func TestParseRestartPolicy(t *testing.T) { - invalids := map[string]string{ - "always:2:3": "invalid restart policy format: maximum retry count must be an integer", - "on-failure:invalid": "invalid restart policy format: maximum retry count must be an integer", - } - valids := map[string]container.RestartPolicy{ - "": {}, - "always": { - Name: "always", - MaximumRetryCount: 0, + tests := []struct { + input string + expected container.RestartPolicy + expectedErr string + }{ + { + input: "", }, - "on-failure:1": { - Name: "on-failure", - MaximumRetryCount: 1, + { + input: "no", + expected: container.RestartPolicy{ + Name: container.RestartPolicyDisabled, + }, + }, + { + input: ":1", + expectedErr: "invalid restart policy format: no policy provided before colon", + }, + { + input: "always", + expected: container.RestartPolicy{ + Name: container.RestartPolicyAlways, + }, + }, + { + input: "always:2:3", + expectedErr: "invalid restart policy format: maximum retry count must be an integer", + }, + { + input: "on-failure:1", + expected: container.RestartPolicy{ + Name: container.RestartPolicyOnFailure, + MaximumRetryCount: 1, + }, + }, + { + input: "on-failure:invalid", + expectedErr: "invalid restart policy format: maximum retry count must be an integer", + }, + { + input: "unless-stopped", + expected: container.RestartPolicy{ + Name: container.RestartPolicyUnlessStopped, + }, + }, + { + input: "unless-stopped:invalid", + expectedErr: "invalid restart policy format: maximum retry count must be an integer", + }, + + // Unknown / invalid combinations: validation is handled by the daemon> + { + input: "anything:123", + expected: container.RestartPolicy{Name: "anything", MaximumRetryCount: 123}, + }, + { + input: "negative:-123", + expected: container.RestartPolicy{Name: "negative", MaximumRetryCount: -123}, }, } - for restart, expectedError := range invalids { - if _, _, _, err := parseRun([]string{"--restart=" + restart, "img", "cmd"}); err == nil || err.Error() != expectedError { - t.Fatalf("Expected an error with message '%v' for %v, got %v", expectedError, restart, err) - } - } - for restart, expected := range valids { - _, hostconfig, _, err := parseRun([]string{fmt.Sprintf("--restart=%v", restart), "img", "cmd"}) - if err != nil { - t.Fatal(err) - } - if hostconfig.RestartPolicy != expected { - t.Fatalf("Expected %v, got %v", expected, hostconfig.RestartPolicy) - } + for _, tc := range tests { + t.Run(tc.input, func(t *testing.T) { + _, hostConfig, _, err := parseRun([]string{"--restart=" + tc.input, "img", "cmd"}) + if tc.expectedErr != "" { + assert.Check(t, is.Error(err, tc.expectedErr)) + assert.Check(t, is.Nil(hostConfig)) + } else { + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(hostConfig.RestartPolicy, tc.expected)) + } + }) } } func TestParseRestartPolicyAutoRemove(t *testing.T) { _, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"}) //nolint:dogsled // verbatim copy from docker/cli tests - if err == nil { - t.Fatal("Expected error for conflicting --restart and --rm, but got none") - } + const expected = "conflicting options: cannot specify both --restart and --rm" + assert.Check(t, is.Error(err, expected)) } func TestParseHealth(t *testing.T) { @@ -841,8 +1003,8 @@ func TestParseHealth(t *testing.T) { checkError("--no-healthcheck conflicts with --health-* options", "--no-healthcheck", "--health-cmd=/check.sh -q", "img", "cmd") - health = checkOk("--health-timeout=2s", "--health-retries=3", "--health-interval=4.5s", "--health-start-period=5s", "img", "cmd") - if health.Timeout != 2*time.Second || health.Retries != 3 || health.Interval != 4500*time.Millisecond || health.StartPeriod != 5*time.Second { + health = checkOk("--health-timeout=2s", "--health-retries=3", "--health-interval=4.5s", "--health-start-period=5s", "--health-start-interval=1s", "img", "cmd") + if health.Timeout != 2*time.Second || health.Retries != 3 || health.Interval != 4500*time.Millisecond || health.StartPeriod != 5*time.Second || health.StartInterval != 1*time.Second { t.Fatalf("--health-*: got %#v", health) } } @@ -863,13 +1025,13 @@ func TestParseLoggingOpts(t *testing.T) { } func TestParseEnvfileVariables(t *testing.T) { //nolint:dupl // verbatim copy from docker/cli tests - e := "open nonexistent: no such file or directory" + expErr := "--env-file: open nonexistent: no such file or directory" if runtime.GOOS == "windows" { - e = "open nonexistent: The system cannot find the file specified." + expErr = "--env-file: open nonexistent: The system cannot find the file specified." } // env ko - if _, _, _, err := parseRun([]string{"--env-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != e { - t.Fatalf("Expected an error with message '%s', got %v", e, err) + if _, _, _, err := parseRun([]string{"--env-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != expErr { + t.Fatalf("Expected an error with message '%s', got %v", expErr, err) } // env ok config, _, _, err := parseRun([]string{"--env-file=testdata/valid.env", "img", "cmd"}) @@ -905,7 +1067,7 @@ func TestParseEnvfileVariablesWithBOMUnicode(t *testing.T) { } // UTF16 with BOM - e := "invalid env file" + e := "invalid utf8 bytes at line" if _, _, _, err := parseRun([]string{"--env-file=testdata/utf16.env", "img", "cmd"}); err == nil || !strings.Contains(err.Error(), e) { t.Fatalf("Expected an error with message '%s', got %v", e, err) } @@ -916,13 +1078,13 @@ func TestParseEnvfileVariablesWithBOMUnicode(t *testing.T) { } func TestParseLabelfileVariables(t *testing.T) { //nolint:dupl // verbatim copy from docker/cli tests - e := "open nonexistent: no such file or directory" + expErr := "--label-file: open nonexistent: no such file or directory" if runtime.GOOS == "windows" { - e = "open nonexistent: The system cannot find the file specified." + expErr = "--label-file: open nonexistent: The system cannot find the file specified." } // label ko - if _, _, _, err := parseRun([]string{"--label-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != e { - t.Fatalf("Expected an error with message '%s', got %v", e, err) + if _, _, _, err := parseRun([]string{"--label-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != expErr { + t.Fatalf("Expected an error with message '%s', got %v", expErr, err) } // label ok config, _, _, err := parseRun([]string{"--label-file=testdata/valid.label", "img", "cmd"}) @@ -943,12 +1105,8 @@ func TestParseLabelfileVariables(t *testing.T) { //nolint:dupl // verbatim copy func TestParseEntryPoint(t *testing.T) { config, _, _, err := parseRun([]string{"--entrypoint=anything", "cmd", "img"}) - if err != nil { - t.Fatal(err) - } - if len(config.Entrypoint) != 1 && config.Entrypoint[0] != "anything" { - t.Fatalf("Expected entrypoint 'anything', got %v", config.Entrypoint) - } + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(config.Entrypoint, []string{"anything"})) } func TestValidateDevice(t *testing.T) { @@ -995,10 +1153,8 @@ func TestValidateDevice(t *testing.T) { for path, expectedError := range invalid { if _, err := validateDevice(path, runtime.GOOS); err == nil { t.Fatalf("ValidateDevice(`%q`) should have failed validation", path) - } else { - if err.Error() != expectedError { - t.Fatalf("ValidateDevice(`%q`) error should contain %q, got %q", path, expectedError, err.Error()) - } + } else if err.Error() != expectedError { + t.Fatalf("ValidateDevice(`%q`) error should contain %q, got %q", path, expectedError, err.Error()) } } } @@ -1073,10 +1229,12 @@ func TestDeviceCgroupRulesAndInvalidParameter(t *testing.T) { if invalidParameter(nil) != nil { t.Fatal("invalidParameter(nil) should be nil") } - err = invalidParameter(errors.New("bad input")) - assert.Assert(t, err != nil) + cause := errors.New("bad input") + err = invalidParameter(cause) var invalid interface{ InvalidParameter() } assert.Assert(t, errors.As(err, &invalid)) + assert.Assert(t, errors.Is(err, cause)) + assert.Equal(t, invalidParameter(err), err) // already invalid, so not wrapped twice } func TestParseSystemPaths(t *testing.T) { diff --git a/act/container/docker_run.go b/act/container/docker_run.go index 73237af9..bbd160e6 100644 --- a/act/container/docker_run.go +++ b/act/container/docker_run.go @@ -27,7 +27,6 @@ import ( "gitea.com/gitea/runner/act/filecollector" "dario.cat/mergo" - "github.com/Masterminds/semver" cerrdefs "github.com/containerd/errdefs" "github.com/docker/cli/cli/compose/loader" "github.com/docker/cli/cli/connhelper" @@ -42,6 +41,7 @@ import ( "github.com/moby/moby/api/types/network" "github.com/moby/moby/api/types/system" "github.com/moby/moby/client" + "github.com/moby/moby/client/pkg/versions" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" ) @@ -92,19 +92,11 @@ func (cr *containerReference) connectToNetwork(name string, aliases []string) co // supportsContainerImagePlatform returns true if the underlying Docker server // API version is 1.41 and beyond func supportsContainerImagePlatform(ctx context.Context, cli client.APIClient) bool { - logger := common.Logger(ctx) ver, err := cli.ServerVersion(ctx, client.ServerVersionOptions{}) if err != nil { - logger.Panicf("Failed to get Docker API Version: %s", err) - return false + common.Logger(ctx).Panicf("Failed to get Docker API Version: %s", err) } - sv, err := semver.NewVersion(ver.APIVersion) - if err != nil { - logger.Panicf("Failed to unmarshal Docker Version: %s", err) - return false - } - constraint, _ := semver.NewConstraint(">= 1.41") - return constraint.Check(sv) + return versions.GreaterThanOrEqualTo(ver.APIVersion, "1.41") } func (cr *containerReference) Create(capAdd, capDrop []string) common.Executor { @@ -588,7 +580,7 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor { } var platSpecs *specs.Platform - if supportsContainerImagePlatform(ctx, cr.cli) && cr.input.Platform != "" { + if cr.input.Platform != "" && supportsContainerImagePlatform(ctx, cr.cli) { platSpecs, err = parsePlatform(cr.input.Platform) if err != nil { return err diff --git a/act/container/docker_stub.go b/act/container/docker_stub.go index bcd804d7..0db4febe 100644 --- a/act/container/docker_stub.go +++ b/act/container/docker_stub.go @@ -8,13 +8,13 @@ package container import ( "context" + "errors" "runtime" "time" "gitea.com/gitea/runner/act/common" "github.com/moby/moby/api/types/system" - "github.com/pkg/errors" ) // ImageExistsLocally returns a boolean indicating if an image with the diff --git a/go.mod b/go.mod index 3939b248..e5751573 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ require ( connectrpc.com/connect v1.20.0 dario.cat/mergo v1.0.2 gitea.dev/actions-proto-go v0.6.0 - github.com/Masterminds/semver v1.5.0 github.com/avast/retry-go/v5 v5.0.0 github.com/containerd/errdefs v1.0.0 github.com/creack/pty v1.1.24 @@ -27,7 +26,6 @@ require ( github.com/moby/patternmatcher v0.6.1 github.com/opencontainers/image-spec v1.1.1 github.com/opencontainers/selinux v1.15.1 - github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.24.0 github.com/prometheus/client_model v0.6.2 github.com/rhysd/actionlint v1.7.12 diff --git a/go.sum b/go.sum index 02862ba5..fd4350e0 100644 --- a/go.sum +++ b/go.sum @@ -8,8 +8,6 @@ gitea.dev/actions-proto-go v0.6.0 h1:gjllYQ5vmwlkqOeofTQu5qKTZpmf7kWsafoHvoPCSzY gitea.dev/actions-proto-go v0.6.0/go.mod h1:p4RX+D9oqiEEzzkPMXscw2CmaGuYFPWFc6xIOmDNDqs= github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6 h1:He8afgbRMd7mFxO99hRNu+6tazq8nFF9lIwo9JFroBk= github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8= -github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= -github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=