From ff5e2898041230978cef9af37dd87c09b07a77e8 Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Wed, 12 Oct 2022 18:30:56 +0200 Subject: [PATCH] test: add test for networking setup in act (#1375) * test: add test for networking setup in act This test makes sure that the hostname inside of act is resolvable. * fix: only merge existing container options When merging parsed container options without options being set in a job, the default docker options are returned and will override the expected defaults by act (e.g. network mode). This is a first attempt to mitigate this behavior and only merge settings if something was requested on a job. * refactor: split config merging into own function Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- pkg/container/docker_run.go | 78 +++++++++++++++---------- pkg/runner/runner_test.go | 1 + pkg/runner/testdata/networking/push.yml | 14 +++++ 3 files changed, 63 insertions(+), 30 deletions(-) create mode 100644 pkg/runner/testdata/networking/push.yml diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index 79a44d1..b935166 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -374,6 +374,52 @@ func (cr *containerReference) remove() common.Executor { } } +func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig, error) { + logger := common.Logger(ctx) + input := cr.input + + if input.Options == "" { + return config, hostConfig, nil + } + + // parse configuration from CLI container.options + flags := pflag.NewFlagSet("container_flags", pflag.ContinueOnError) + copts := addFlags(flags) + + optionsArgs, err := shellquote.Split(input.Options) + if err != nil { + return nil, nil, fmt.Errorf("Cannot split container options: '%s': '%w'", input.Options, err) + } + + err = flags.Parse(optionsArgs) + if err != nil { + return nil, nil, fmt.Errorf("Cannot parse container options: '%s': '%w'", input.Options, err) + } + + containerConfig, err := parse(flags, copts, "") + if err != nil { + return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err) + } + + logger.Debugf("Custom container.Config from options ==> %+v", containerConfig.Config) + + err = mergo.Merge(config, containerConfig.Config, mergo.WithOverride) + if err != nil { + return nil, nil, fmt.Errorf("Cannot merge container.Config options: '%s': '%w'", input.Options, err) + } + logger.Debugf("Merged container.Config ==> %+v", config) + + logger.Debugf("Custom container.HostConfig from options ==> %+v", containerConfig.HostConfig) + + err = mergo.Merge(hostConfig, containerConfig.HostConfig, mergo.WithOverride) + if err != nil { + return nil, nil, fmt.Errorf("Cannot merge container.HostConfig options: '%s': '%w'", input.Options, err) + } + logger.Debugf("Merged container.HostConfig ==> %+v", hostConfig) + + return config, hostConfig, nil +} + func (cr *containerReference) create(capAdd []string, capDrop []string) common.Executor { return func(ctx context.Context) error { if cr.id != "" { @@ -383,25 +429,6 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E isTerminal := term.IsTerminal(int(os.Stdout.Fd())) input := cr.input - // parse configuration from CLI container.options - flags := pflag.NewFlagSet("container_flags", pflag.ContinueOnError) - copts := addFlags(flags) - - optionsArgs, err := shellquote.Split(input.Options) - if err != nil { - return fmt.Errorf("Cannot split container options: '%s': '%w'", input.Options, err) - } - - err = flags.Parse(optionsArgs) - if err != nil { - return fmt.Errorf("Cannot parse container options: '%s': '%w'", input.Options, err) - } - - containerConfig, err := parse(flags, copts, "") - if err != nil { - return fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err) - } - config := &container.Config{ Image: input.Image, WorkingDir: input.WorkingDir, @@ -409,13 +436,6 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E Tty: isTerminal, } logger.Debugf("Common container.Config ==> %+v", config) - logger.Debugf("Custom container.Config from options ==> %+v", containerConfig.Config) - - err = mergo.Merge(config, containerConfig.Config, mergo.WithOverride) - if err != nil { - return fmt.Errorf("Cannot merge container.Config options: '%s': '%w'", input.Options, err) - } - logger.Debugf("Merged container.Config ==> %+v", config) if len(input.Cmd) != 0 { config.Cmd = input.Cmd @@ -458,13 +478,11 @@ func (cr *containerReference) create(capAdd []string, capDrop []string) common.E UsernsMode: container.UsernsMode(input.UsernsMode), } logger.Debugf("Common container.HostConfig ==> %+v", hostConfig) - logger.Debugf("Custom container.HostConfig from options ==> %+v", containerConfig.HostConfig) - err = mergo.Merge(hostConfig, containerConfig.HostConfig, mergo.WithOverride) + config, hostConfig, err := cr.mergeContainerConfigs(ctx, config, hostConfig) if err != nil { - return fmt.Errorf("Cannot merge container.HostConfig options: '%s': '%w'", input.Options, err) + return err } - logger.Debugf("Merged container.HostConfig ==> %+v", hostConfig) resp, err := cr.cli.ContainerCreate(ctx, config, hostConfig, nil, platSpecs, input.Name) if err != nil { diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 86bd5c3..f2610a1 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -173,6 +173,7 @@ func TestRunEvent(t *testing.T) { {workdir, "env-and-path", "push", "", platforms}, {workdir, "non-existent-action", "push", "Job 'nopanic' failed", platforms}, {workdir, "outputs", "push", "", platforms}, + {workdir, "networking", "push", "", platforms}, {workdir, "steps-context/conclusion", "push", "", platforms}, {workdir, "steps-context/outcome", "push", "", platforms}, {workdir, "job-status-check", "push", "job 'fail' failed", platforms}, diff --git a/pkg/runner/testdata/networking/push.yml b/pkg/runner/testdata/networking/push.yml new file mode 100644 index 0000000..2b1bf9a --- /dev/null +++ b/pkg/runner/testdata/networking/push.yml @@ -0,0 +1,14 @@ +name: test network setup +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Install tools + run: | + apt update + apt install -y bind9-host + - name: Run hostname test + run: | + hostname -f + host $(hostname -f)