From db2c29028dfd47d30e349cadb3ea0af3bf69d3e7 Mon Sep 17 00:00:00 2001 From: Jake Coffman Date: Fri, 22 Dec 2023 06:10:04 -0600 Subject: [PATCH 1/5] more script test fixes --- testdata/scripts/input.txt | 3 +++ tests/dependabot_test.go | 48 +++++++------------------------------- 2 files changed, 11 insertions(+), 40 deletions(-) diff --git a/testdata/scripts/input.txt b/testdata/scripts/input.txt index f4e6922..4b59be9 100644 --- a/testdata/scripts/input.txt +++ b/testdata/scripts/input.txt @@ -9,6 +9,9 @@ stderr '"repo":"dependabot/cli"' dependabot update go_modules dependabot/cli --commit 1278c8d7503f9881eb969959446e2c3a5a0cce2d --updater-image input-verify-updater stderr '"commit":"1278c8d7503f9881eb969959446e2c3a5a0cce2d"' +dependabot update go_modules dependabot/cli --branch cool-branch --updater-image input-verify-updater +stderr '"branch":"cool-branch"' + ! dependabot update go_modules dependabot/cli --commit unknown --updater-image input-verify-updater stderr 'commit must be a SHA, or not provided' diff --git a/tests/dependabot_test.go b/tests/dependabot_test.go index 58292be..a78714f 100644 --- a/tests/dependabot_test.go +++ b/tests/dependabot_test.go @@ -1,20 +1,20 @@ package tests import ( - "bytes" "context" - "fmt" "os" "os/exec" + "path/filepath" "rsc.io/script" "rsc.io/script/scripttest" "testing" + "time" ) func TestDependabot(t *testing.T) { - err := exec.Command("go", "build", "../cmd/dependabot/dependabot.go").Run() + err := exec.Command("go", "build", "dependabot.go").Run() if err != nil { - panic("failed to build dependabot") + t.Fatal("failed to build dependabot") } t.Cleanup(func() { os.Remove("dependabot") @@ -29,7 +29,7 @@ func TestDependabot(t *testing.T) { env := []string{ "PATH=" + os.Getenv("PATH"), } - scripttest.Test(t, ctx, engine, env, "../testdata/scripts/*.txt") + scripttest.Test(t, ctx, engine, env, "../../testdata/scripts/*.txt") } // Commands returns the commands that can be used in the scripts. @@ -40,43 +40,11 @@ func TestDependabot(t *testing.T) { // from the scripttest package. func Commands() map[string]script.Cmd { commands := scripttest.DefaultCmds() + wd, _ := os.Getwd() + dependabot := filepath.Join(wd, "dependabot") // additional Dependabot commands - commands["dependabot"] = Dependabot() + commands["dependabot"] = script.Program(dependabot, nil, 100*time.Millisecond) return commands } - -// Dependabot runs the Dependabot CLI. -func Dependabot() script.Cmd { - return script.Command( - script.CmdUsage{ - Summary: "runs the Dependabot CLI", - Args: "[ | -f ] [flags]", - }, - func(s *script.State, args ...string) (script.WaitFunc, error) { - if len(args) == 0 { - return nil, script.ErrUsage - } - - os.Link("dependabot", s.Getwd()+"/dependabot") - - execCmd := exec.Command("./dependabot", args...) - - var execOut, execErr bytes.Buffer - execCmd.Dir = s.Getwd() - execCmd.Env = s.Environ() - execCmd.Stdout = &execOut - execCmd.Stderr = &execErr - - if err := execCmd.Start(); err != nil { - return nil, fmt.Errorf("failed to run dependabot: %w", err) - } - - wait := func(*script.State) (stdout, stderr string, err error) { - err = execCmd.Wait() - return execOut.String(), execErr.String(), err - } - return wait, nil - }) -} From d37cc64b9e5b1512483cc359e3fb89831eface6b Mon Sep 17 00:00:00 2001 From: Jake Coffman Date: Fri, 22 Dec 2023 06:12:48 -0600 Subject: [PATCH 2/5] move to prevent cached test after modification --- {tests => cmd/dependabot}/dependabot_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename {tests => cmd/dependabot}/dependabot_test.go (98%) diff --git a/tests/dependabot_test.go b/cmd/dependabot/dependabot_test.go similarity index 98% rename from tests/dependabot_test.go rename to cmd/dependabot/dependabot_test.go index a78714f..cb8e050 100644 --- a/tests/dependabot_test.go +++ b/cmd/dependabot/dependabot_test.go @@ -1,4 +1,4 @@ -package tests +package main import ( "context" From 18c1cd3471774c043fcac9b307de6a3bd3b72407 Mon Sep 17 00:00:00 2001 From: Jake Coffman Date: Fri, 22 Dec 2023 06:16:39 -0600 Subject: [PATCH 3/5] fix comment --- cmd/dependabot/dependabot_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/dependabot/dependabot_test.go b/cmd/dependabot/dependabot_test.go index cb8e050..13a8a89 100644 --- a/cmd/dependabot/dependabot_test.go +++ b/cmd/dependabot/dependabot_test.go @@ -34,10 +34,8 @@ func TestDependabot(t *testing.T) { // Commands returns the commands that can be used in the scripts. // Each line of the scripts are -// So if you enter "dependabot update go_modules rsc/quote", it will run -// the Dependabot() function with args "update go_modules rsc/quote". -// When you use "echo" in the scripts it's actually running the echo command -// from the scripttest package. +// When you use "echo" in the scripts it's actually running script.Echo +// not the echo binary on your system. func Commands() map[string]script.Cmd { commands := scripttest.DefaultCmds() wd, _ := os.Getwd() From 8c44f9d39733eb7719264ccc51d3b769f86df538 Mon Sep 17 00:00:00 2001 From: Jake Coffman Date: Fri, 22 Dec 2023 06:45:52 -0600 Subject: [PATCH 4/5] add more tests --- cmd/dependabot/internal/cmd/update.go | 4 ++++ testdata/scripts/basic.txt | 2 ++ testdata/scripts/input.txt | 5 +++++ testdata/scripts/local.txt | 2 ++ testdata/scripts/timeout.txt | 24 ++++++++++++++++++++++++ 5 files changed, 37 insertions(+) create mode 100644 testdata/scripts/timeout.txt diff --git a/cmd/dependabot/internal/cmd/update.go b/cmd/dependabot/internal/cmd/update.go index 9aa9b09..ed0200a 100644 --- a/cmd/dependabot/internal/cmd/update.go +++ b/cmd/dependabot/internal/cmd/update.go @@ -2,6 +2,7 @@ package cmd import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -91,6 +92,9 @@ func NewUpdateCommand() *cobra.Command { Writer: writer, ApiUrl: flags.apiUrl, }); err != nil { + if errors.Is(err, context.DeadlineExceeded) { + log.Fatalf("update timed out after %s", flags.timeout) + } log.Fatalf("failed to run updater: %v", err) } diff --git a/testdata/scripts/basic.txt b/testdata/scripts/basic.txt index c248722..5bf82ce 100644 --- a/testdata/scripts/basic.txt +++ b/testdata/scripts/basic.txt @@ -8,6 +8,8 @@ dependabot update go_modules dependabot/cli --updater-image dummy-updater stderr 'bin/run arguments: fetch_files' stderr 'bin/run arguments: update_files' +exec docker rmi -f dummy-updater + -- Dockerfile -- FROM ubuntu:22.04 diff --git a/testdata/scripts/input.txt b/testdata/scripts/input.txt index 4b59be9..0e7c620 100644 --- a/testdata/scripts/input.txt +++ b/testdata/scripts/input.txt @@ -18,6 +18,11 @@ stderr 'commit must be a SHA, or not provided' dependabot update go_modules dependabot/cli --dep golang.org/x/image --updater-image input-verify-updater stderr '"allowed-updates":\[\{"dependency-name":"golang.org/x/image"\}\]' +dependabot update go_modules dependabot/cli --directory /code --updater-image input-verify-updater +stderr '"directory":"\/code"' + +exec docker rmi -f input-verify-updater + -- Dockerfile -- FROM ubuntu:22.04 diff --git a/testdata/scripts/local.txt b/testdata/scripts/local.txt index ffcb23d..1e3fc0f 100644 --- a/testdata/scripts/local.txt +++ b/testdata/scripts/local.txt @@ -11,6 +11,8 @@ dependabot update go_modules dependabot-fixtures/go-modules-lib --updater-image stderr \.git stderr hello.txt +exec docker rmi -f local-updater + -- Dockerfile -- FROM ubuntu:22.04 diff --git a/testdata/scripts/timeout.txt b/testdata/scripts/timeout.txt new file mode 100644 index 0000000..7d501ca --- /dev/null +++ b/testdata/scripts/timeout.txt @@ -0,0 +1,24 @@ +exec docker build -t sleepy-updater . + +! dependabot update go_modules dependabot/cli --timeout 1s --updater-image sleepy-updater +stderr 'update timed out after 1s' + +exec docker rmi -f sleepy-updater + +-- Dockerfile -- +FROM ubuntu:22.04 + +RUN useradd dependabot + +COPY --chown=dependabot --chmod=755 update-ca-certificates /usr/bin/update-ca-certificates +COPY --chown=dependabot --chmod=755 run bin/run + +-- update-ca-certificates -- +#!/usr/bin/env bash + +echo "Updated those certificates for ya" + +-- run -- +#!/usr/bin/env bash + +sleep 10 From 28dd6853d4a0c1b978b9a2ef036d17b7b5e133ef Mon Sep 17 00:00:00 2001 From: Jake Coffman Date: Fri, 22 Dec 2023 09:15:47 -0600 Subject: [PATCH 5/5] add proxy integration tests (#212) --- internal/infra/proxy.go | 26 +++++--- internal/infra/proxy_test.go | 117 ----------------------------------- internal/infra/run.go | 18 +++--- testdata/scripts/proxy.txt | 58 +++++++++++++++++ 4 files changed, 88 insertions(+), 131 deletions(-) delete mode 100644 internal/infra/proxy_test.go create mode 100644 testdata/scripts/proxy.txt diff --git a/internal/infra/proxy.go b/internal/infra/proxy.go index 122e072..f64a305 100644 --- a/internal/infra/proxy.go +++ b/internal/infra/proxy.go @@ -45,7 +45,6 @@ func NewProxy(ctx context.Context, cli *client.Client, params *RunParams, nets * } hostCfg := &container.HostConfig{ - AutoRemove: true, ExtraHosts: []string{ "host.docker.internal:host-gateway", }, @@ -169,13 +168,26 @@ func (p *Proxy) TailLogs(ctx context.Context, cli *client.Client) { _, _ = stdcopy.StdCopy(w, w, out) } -func (p *Proxy) Close() error { +func (p *Proxy) Close() (err error) { + defer func() { + removeErr := p.cli.ContainerRemove(context.Background(), p.containerID, types.ContainerRemoveOptions{Force: true}) + if removeErr != nil { + err = fmt.Errorf("failed to remove proxy container: %w", removeErr) + } + }() + + // Check the error code if the container has already exited, so we can pass it along to the caller. If the proxy + //crashes we want the CLI to error out. Unlike the Updater it should never stop on its own. + containerInfo, inspectErr := p.cli.ContainerInspect(context.Background(), p.containerID) + if inspectErr != nil { + return fmt.Errorf("failed to inspect proxy container: %w", inspectErr) + } + if containerInfo.State.ExitCode != 0 { + return fmt.Errorf("proxy container exited with non-zero exit code: %d", containerInfo.State.ExitCode) + } + timeout := 5 _ = p.cli.ContainerStop(context.Background(), p.containerID, container.StopOptions{Timeout: &timeout}) - err := p.cli.ContainerRemove(context.Background(), p.containerID, types.ContainerRemoveOptions{Force: true}) - if err != nil { - return fmt.Errorf("failed to remove proxy container: %w", err) - } - return nil + return err } diff --git a/internal/infra/proxy_test.go b/internal/infra/proxy_test.go deleted file mode 100644 index 138fd68..0000000 --- a/internal/infra/proxy_test.go +++ /dev/null @@ -1,117 +0,0 @@ -package infra - -import ( - "archive/tar" - "bytes" - "context" - "errors" - "io" - "net/http" - "os" - "testing" - "time" - - "github.com/docker/docker/api/types" - "github.com/moby/moby/client" -) - -// This tests the Proxy's ability to use a custom cert for outbound calls. -// It creates a custom proxy image to test with, passes it a cert, and uses it to -// communicate with a test server using the certs. -func TestNewProxy_customCert(t *testing.T) { - ctx := context.Background() - - CertSubject.CommonName = "host.docker.internal" - ca, err := GenerateCertificateAuthority() - if err != nil { - t.Fatal(err) - } - - cert, err := os.CreateTemp(os.TempDir(), "cert.pem") - key, err2 := os.CreateTemp(os.TempDir(), "key.pem") - if err != nil || err2 != nil { - t.Fatal(err, err2) - } - _, _ = cert.WriteString(ca.Cert) - _, _ = key.WriteString(ca.Key) - _ = cert.Close() - _ = key.Close() - - successChan := make(chan struct{}) - addr := "127.0.0.1:8765" - if os.Getenv("CI") != "" { - t.Log("detected running in actions") - addr = "0.0.0.0:8765" - } - testServer := &http.Server{ - ReadHeaderTimeout: time.Second, - Addr: addr, - Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte("SUCCESS")) - successChan <- struct{}{} - }), - } - defer func() { - _ = testServer.Shutdown(ctx) - }() - go func() { - t.Log("Starting HTTPS server") - if err = testServer.ListenAndServeTLS(cert.Name(), key.Name()); err != nil && !errors.Is(err, http.ErrServerClosed) { - panic(err) - } - }() - - cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) - if err != nil { - t.Fatal(err) - } - - // build the test image - var buildContext bytes.Buffer - tw := tar.NewWriter(&buildContext) - _ = addFileToArchive(tw, "/Dockerfile", 0644, proxyTestDockerfile) - _ = tw.Close() - - proxyImageName := "curl-test" - resp, err := cli.ImageBuild(ctx, &buildContext, types.ImageBuildOptions{Tags: []string{proxyImageName}}) - if err != nil { - t.Fatal(err) - } - _, _ = io.Copy(io.Discard, resp.Body) - _ = resp.Body.Close() - - defer func() { - _, _ = cli.ImageRemove(ctx, proxyImageName, types.ImageRemoveOptions{}) - }() - - proxy, err := NewProxy(ctx, cli, &RunParams{ - ProxyCertPath: cert.Name(), - ProxyImage: proxyImageName, - }, nil) - if err != nil { - panic(err) - } - defer func() { - _ = proxy.Close() - }() - - t.Log("Starting proxy") - - go proxy.TailLogs(ctx, cli) - - select { - case <-successChan: - t.Log("Success!") - case <-time.After(5 * time.Second): - t.Errorf("Not able to contact the test server") - } -} - -const proxyTestDockerfile = ` -FROM ghcr.io/github/dependabot-update-job-proxy/dependabot-update-job-proxy:latest -RUN apk add --no-cache curl -RUN echo "#!/bin/sh" > /update-job-proxy -RUN echo "CURLing host.docker.internal" >> /update-job-proxy -RUN echo "curl -s https://host.docker.internal:8765" >> /update-job-proxy -RUN chmod +x /update-job-proxy -` diff --git a/internal/infra/run.go b/internal/infra/run.go index 6095f14..30772cb 100644 --- a/internal/infra/run.go +++ b/internal/infra/run.go @@ -15,14 +15,13 @@ import ( "syscall" "time" + "github.com/dependabot/cli/internal/model" + "github.com/dependabot/cli/internal/server" + "github.com/docker/docker/api/types" "github.com/docker/docker/pkg/archive" "github.com/hexops/gotextdiff" "github.com/hexops/gotextdiff/myers" "github.com/hexops/gotextdiff/span" - - "github.com/dependabot/cli/internal/model" - "github.com/dependabot/cli/internal/server" - "github.com/docker/docker/api/types" "github.com/moby/moby/api/types/registry" "github.com/moby/moby/client" "gopkg.in/yaml.v3" @@ -330,8 +329,9 @@ func generateIgnoreConditions(params *RunParams, actual *model.Scenario) error { return nil } -func runContainers(ctx context.Context, params RunParams) error { - cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) +func runContainers(ctx context.Context, params RunParams) (err error) { + var cli *client.Client + cli, err = client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) if err != nil { return fmt.Errorf("failed to create Docker client: %w", err) } @@ -365,7 +365,11 @@ func runContainers(ctx context.Context, params RunParams) error { if err != nil { return err } - defer prox.Close() + defer func() { + if proxyErr := prox.Close(); proxyErr != nil { + err = proxyErr + } + }() // proxy logs interfere with debugging output if !params.Debug { diff --git a/testdata/scripts/proxy.txt b/testdata/scripts/proxy.txt new file mode 100644 index 0000000..731e8ba --- /dev/null +++ b/testdata/scripts/proxy.txt @@ -0,0 +1,58 @@ +# Tests related to running the Proxy + +exec docker build -qt proxy-updater . +exec docker build -qt dummy-proxy -f Dockerfile.proxy . + +dependabot update go_modules dependabot/cli --updater-image proxy-updater --proxy-image dummy-proxy +stderr 'proxy \| Proxy is running' +stderr 'updater \| Updater is running' +! stderr 'proxy \| custom-ca-cert\.crt' + +dependabot update go_modules dependabot/cli --proxy-cert my-cert --updater-image proxy-updater --proxy-image dummy-proxy +stderr 'proxy \| custom-ca-cert\.crt' +stderr 'proxy \| I am a certificate' + +# Test that the CLI exits with non-zero if the proxy does too. +! dependabot update go_modules dependabot/cli --proxy-cert crash --updater-image proxy-updater --proxy-image dummy-proxy --proxy-username user --proxy-password pass + +exec docker rmi -f proxy-updater dummy-proxy + +-- crash -- +crash + +-- my-cert -- +I am a certificate + +-- Dockerfile.proxy -- +FROM ubuntu:22.04 + +COPY --chmod=755 update-ca-certificates /usr/bin/update-ca-certificates +COPY --chmod=755 update-job-proxy /update-job-proxy + +-- update-job-proxy -- +#!/usr/bin/env bash + +echo "Proxy is running" +echo "$(