From 153540fdd0e3b6f00050550abed67cae16299cbe Mon Sep 17 00:00:00 2001 From: Alex Collins Date: Tue, 5 Apr 2022 08:47:20 -0700 Subject: [PATCH] feat: Remove binaries from argoexec image. Fixes #7486 (#8292) Signed-off-by: Alex Collins --- Dockerfile | 54 +++----- workflow/artifacts/git/git.go | 165 ++++++++---------------- workflow/artifacts/git/git_test.go | 183 +++++++++++++++------------ workflow/artifacts/http/http.go | 46 ++++--- workflow/artifacts/http/http_test.go | 30 ++--- workflow/executor/executor.go | 43 ++++++- 6 files changed, 246 insertions(+), 275 deletions(-) diff --git a/Dockerfile b/Dockerfile index f3d953f1ff90..9bcaafd949dc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,11 +1,5 @@ #syntax=docker/dockerfile:1.2 -ARG DOCKER_CHANNEL=stable -ARG DOCKER_VERSION=20.10.14 -# NOTE: kubectl version should be one minor version less than https://storage.googleapis.com/kubernetes-release/release/stable.txt -ARG KUBECTL_VERSION=1.22.3 -ARG JQ_VERSION=1.6 - FROM golang:1.17 as builder RUN apt-get update && apt-get --no-install-recommends install -y \ @@ -15,9 +9,7 @@ RUN apt-get update && apt-get --no-install-recommends install -y \ apt-transport-https \ ca-certificates \ wget \ - gcc \ - libcap2-bin \ - zip && \ + gcc && \ apt-get clean \ && rm -rf \ /var/lib/apt/lists/* \ @@ -37,33 +29,6 @@ RUN go mod download COPY . . -#################################################################################################### - -FROM alpine:3 as argoexec-base - -ARG DOCKER_CHANNEL -ARG DOCKER_VERSION -ARG KUBECTL_VERSION - -RUN apk --no-cache add curl git tar jq - -COPY hack/arch.sh hack/os.sh /bin/ - -RUN if [ $(arch.sh) = ppc64le ] || [ $(arch.sh) = s390x ]; then \ - curl -o docker.tgz https://download.docker.com/$(os.sh)/static/${DOCKER_CHANNEL}/$(uname -m)/docker-18.06.3-ce.tgz; \ - else \ - curl -o docker.tgz https://download.docker.com/$(os.sh)/static/${DOCKER_CHANNEL}/$(uname -m)/docker-${DOCKER_VERSION}.tgz; \ - fi && \ - tar --extract --file docker.tgz --strip-components 1 --directory /usr/local/bin/ && \ - rm docker.tgz -RUN curl -o /usr/local/bin/kubectl https://storage.googleapis.com/kubernetes-release/release/v${KUBECTL_VERSION}/bin/$(os.sh)/$(arch.sh)/kubectl && \ - chmod +x /usr/local/bin/kubectl -RUN rm /bin/arch.sh /bin/os.sh - -COPY hack/ssh_known_hosts /etc/ssh/ -COPY hack/nsswitch.conf /etc/ - - #################################################################################################### FROM node:16 as argo-ui @@ -81,6 +46,15 @@ RUN NODE_OPTIONS="--max-old-space-size=2048" JOBS=max yarn --cwd ui build FROM builder as argoexec-build +COPY hack/arch.sh hack/os.sh /bin/ + +# NOTE: kubectl version should be one minor version less than https://storage.googleapis.com/kubernetes-release/release/stable.txt +RUN curl -o /usr/local/bin/kubectl https://storage.googleapis.com/kubernetes-release/release/v1.22.3/bin/$(os.sh)/$(arch.sh)/kubectl && \ + chmod +x /usr/local/bin/kubectl + +RUN curl -o /usr/local/bin/jq https://github.com/stedolan/jq/releases/download/jq-1.6/jq-linux64 && \ + chmod +x /usr/local/bin/jq + # Tell git to forget about all of the files that were not included because of .dockerignore in order to ensure that # the git state is "clean" even though said .dockerignore files are not present RUN cat .dockerignore >> .gitignore @@ -118,10 +92,14 @@ RUN --mount=type=cache,target=/root/.cache/go-build make dist/argo #################################################################################################### -FROM argoexec-base as argoexec +FROM gcr.io/distroless/static as argoexec -COPY --from=argoexec-build /go/src/github.com/argoproj/argo-workflows/dist/argoexec /usr/local/bin/ +COPY --from=argoexec-build /usr/local/bin/kubectl /bin/ +COPY --from=argoexec-build /usr/local/bin/jq /bin/ +COPY --from=argoexec-build /go/src/github.com/argoproj/argo-workflows/dist/argoexec /bin/ COPY --from=argoexec-build /etc/mime.types /etc/mime.types +COPY hack/ssh_known_hosts /etc/ssh/ +COPY hack/nsswitch.conf /etc/ ENTRYPOINT [ "argoexec" ] diff --git a/workflow/artifacts/git/git.go b/workflow/artifacts/git/git.go index de5187c3e501..3d9931bc6cdf 100644 --- a/workflow/artifacts/git/git.go +++ b/workflow/artifacts/git/git.go @@ -5,10 +5,7 @@ import ( "fmt" "io/ioutil" "os" - "os/exec" - "path/filepath" "regexp" - "strings" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" @@ -45,65 +42,33 @@ func GetUser(url string) string { return "git" } -func (g *ArtifactDriver) auth(sshUser string) (func(), transport.AuthMethod, []string, error) { +func (g *ArtifactDriver) auth(sshUser string) (func(), transport.AuthMethod, error) { if g.SSHPrivateKey != "" { signer, err := ssh.ParsePrivateKey([]byte(g.SSHPrivateKey)) if err != nil { - return nil, nil, nil, err + return nil, nil, err } privateKeyFile, err := ioutil.TempFile("", "id_rsa.") if err != nil { - return nil, nil, nil, err + return nil, nil, err } err = ioutil.WriteFile(privateKeyFile.Name(), []byte(g.SSHPrivateKey), 0o600) if err != nil { - return nil, nil, nil, err + return nil, nil, err } auth := &ssh2.PublicKeys{User: sshUser, Signer: signer} if g.InsecureIgnoreHostKey { auth.HostKeyCallback = ssh.InsecureIgnoreHostKey() } - args := []string{"ssh", "-i", privateKeyFile.Name()} - if g.InsecureIgnoreHostKey { - args = append(args, "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null") - } else { - args = append(args, "-o", "StrictHostKeyChecking=yes", "-o") - } - env := []string{"GIT_SSH_COMMAND=" + strings.Join(args, " ")} if g.InsecureIgnoreHostKey { auth.HostKeyCallback = ssh.InsecureIgnoreHostKey() - env = append(env, "GIT_SSL_NO_VERIFY=true") } - return func() { _ = os.Remove(privateKeyFile.Name()) }, - auth, - env, - nil + return func() { _ = os.Remove(privateKeyFile.Name()) }, auth, nil } if g.Username != "" || g.Password != "" { - filename := filepath.Join(os.TempDir(), "git-ask-pass.sh") - _, err := os.Stat(filename) - if os.IsNotExist(err) { - //nolint:gosec - err := ioutil.WriteFile(filename, []byte(`#!/bin/sh -case "$1" in -Username*) echo "${GIT_USERNAME}" ;; -Password*) echo "${GIT_PASSWORD}" ;; -esac -`), 0o755) - if err != nil { - return nil, nil, nil, err - } - } - return func() {}, - &http.BasicAuth{Username: g.Username, Password: g.Password}, - []string{ - "GIT_ASKPASS=" + filename, - "GIT_USERNAME=" + g.Username, - "GIT_PASSWORD=" + g.Password, - }, - nil + return func() {}, &http.BasicAuth{Username: g.Username, Password: g.Password}, nil } - return func() {}, nil, nil, nil + return func() {}, nil, nil } // Save is unsupported for git output artifacts @@ -112,111 +77,87 @@ func (g *ArtifactDriver) Save(string, *wfv1.Artifact) error { } func (g *ArtifactDriver) Load(inputArtifact *wfv1.Artifact, path string) error { - sshUser := GetUser(inputArtifact.Git.Repo) - closer, auth, env, err := g.auth(sshUser) + a := inputArtifact.Git + sshUser := GetUser(a.Repo) + closer, auth, err := g.auth(sshUser) if err != nil { return err } defer closer() - - var recurseSubmodules = git.DefaultSubmoduleRecursionDepth - if inputArtifact.Git.DisableSubmodules { - log.Info("Recursive cloning of submodules is disabled") - recurseSubmodules = git.NoRecurseSubmodules - } - repo, err := git.PlainClone(path, false, &git.CloneOptions{ - URL: inputArtifact.Git.Repo, - RecurseSubmodules: recurseSubmodules, - Auth: auth, - Depth: inputArtifact.Git.GetDepth(), - }) + depth := a.GetDepth() + r, err := git.PlainClone(path, false, &git.CloneOptions{URL: a.Repo, Auth: auth, Depth: depth}) switch err { case transport.ErrEmptyRemoteRepository: - log.Info("Cloned an empty repository ") + log.Info("Cloned an empty repository") r, err := git.PlainInit(path, false) if err != nil { - return err + return fmt.Errorf("failed to plain init: %w", err) } - if _, err := r.CreateRemote(&config.RemoteConfig{Name: git.DefaultRemoteName, URLs: []string{inputArtifact.Git.Repo}}); err != nil { - return err + if _, err := r.CreateRemote(&config.RemoteConfig{Name: git.DefaultRemoteName, URLs: []string{a.Repo}}); err != nil { + return fmt.Errorf("failed to create remote %q: %w", a.Repo, err) } - branchName := inputArtifact.Git.Revision + branchName := a.Revision if branchName == "" { branchName = "master" } if err = r.CreateBranch(&config.Branch{Name: branchName, Remote: git.DefaultRemoteName, Merge: plumbing.Master}); err != nil { - return err + return fmt.Errorf("failed to create branch %q: %w", branchName, err) } return nil - default: - return err case nil: // fallthrough ... + default: + return fmt.Errorf("failed to clone %q: %w", a.Repo, err) } - if inputArtifact.Git.Fetch != nil { - refSpecs := make([]config.RefSpec, len(inputArtifact.Git.Fetch)) - for i, spec := range inputArtifact.Git.Fetch { + if len(a.Fetch) > 0 { + refSpecs := make([]config.RefSpec, len(a.Fetch)) + for i, spec := range a.Fetch { refSpecs[i] = config.RefSpec(spec) } - fetchOptions := git.FetchOptions{ - Auth: auth, - RefSpecs: refSpecs, - Depth: inputArtifact.Git.GetDepth(), + opts := &git.FetchOptions{Auth: auth, RefSpecs: refSpecs, Depth: depth} + if err := opts.Validate(); err != nil { + return fmt.Errorf("failed to validate fetch %v: %w", refSpecs, err) + } + if err = r.Fetch(opts); isFetchErr(err) { + return fmt.Errorf("failed to fetch %v: %w", refSpecs, err) + } + } + w, err := r.Worktree() + if err != nil { + return fmt.Errorf("failed to get work tree: %w", err) + } + if a.Revision != "" { + if err := r.Fetch(&git.FetchOptions{RefSpecs: []config.RefSpec{"refs/heads/*:refs/heads/*"}}); isFetchErr(err) { + return fmt.Errorf("failed to fatch refs: %w", err) } - err = fetchOptions.Validate() + h, err := r.ResolveRevision(plumbing.Revision(a.Revision)) if err != nil { - return err + return fmt.Errorf("failed to get resolve revision: %w", err) } - err = repo.Fetch(&fetchOptions) - if isAlreadyUpToDateErr(err) { - return err + if err := w.Checkout(&git.CheckoutOptions{Hash: plumbing.NewHash(h.String())}); err != nil { + return fmt.Errorf("failed to checkout %q: %w", h, err) } } - if inputArtifact.Git.Revision != "" { - // We still rely on forking git for checkout, since go-git does not have a reliable - // way of resolving revisions (e.g. mybranch, HEAD^, v1.2.3) - rev := getRevisionForCheckout(inputArtifact.Git.Revision) - log.Info("Checking out revision ", rev) - cmd := exec.Command("git", "checkout", rev, "--") - cmd.Dir = path - cmd.Env = env - output, err := cmd.Output() + if !a.DisableSubmodules { + s, err := w.Submodules() if err != nil { - return g.error(err, cmd) - } - log.Infof("`%s` stdout:\n%s", cmd.Args, string(output)) - if !inputArtifact.Git.DisableSubmodules { - submodulesCmd := exec.Command("git", "submodule", "update", "--init", "--recursive", "--force") - submodulesCmd.Dir = path - submodulesCmd.Env = env - submoduleOutput, err := submodulesCmd.Output() - if err != nil { - return g.error(err, cmd) - } - log.Infof("`%s` stdout:\n%s", cmd.Args, string(submoduleOutput)) + return fmt.Errorf("failed to get submodules: %w", err) + } + if err := s.Update(&git.SubmoduleUpdateOptions{ + Init: true, + RecurseSubmodules: git.DefaultSubmoduleRecursionDepth, + Auth: auth, + }); err != nil { + return fmt.Errorf("failed to update submodules: %w", err) } } return nil } -// getRevisionForCheckout trims "refs/heads/" from the revision name (if present) -// so that `git checkout` will succeed. -func getRevisionForCheckout(revision string) string { - return strings.TrimPrefix(revision, "refs/heads/") -} - -func isAlreadyUpToDateErr(err error) bool { +func isFetchErr(err error) bool { return err != nil && err.Error() != "already up-to-date" } -func (g *ArtifactDriver) error(err error, cmd *exec.Cmd) error { - if exErr, ok := err.(*exec.ExitError); ok { - log.Errorf("`%s` stderr:\n%s", cmd.Args, string(exErr.Stderr)) - return errors.New(strings.Split(string(exErr.Stderr), "\n")[0]) - } - return err -} - func (g *ArtifactDriver) ListObjects(artifact *wfv1.Artifact) ([]string, error) { return nil, fmt.Errorf("ListObjects is currently not supported for this artifact type, but it will be in a future version") } diff --git a/workflow/artifacts/git/git_test.go b/workflow/artifacts/git/git_test.go index 8897ab9bdccc..5ff342078005 100644 --- a/workflow/artifacts/git/git_test.go +++ b/workflow/artifacts/git/git_test.go @@ -1,105 +1,128 @@ package git import ( - "io/ioutil" "os" "testing" + "k8s.io/client-go/util/homedir" + "github.com/stretchr/testify/assert" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" ) -var d = uint64(1) - func TestGitArtifactDriver_Save(t *testing.T) { driver := &ArtifactDriver{} err := driver.Save("", nil) assert.Error(t, err) } -func TestGitArtifactDriverLoad_HTTPS(t *testing.T) { - for _, tt := range []struct { - url string - }{ - {"https://github.com/argoproj/empty.git"}, - } { - if os.Getenv("GITHUB_TOKEN") == "" { - t.Skip("not running an GITHUB_TOKEN not set") - } - _ = os.Remove("git-ask-pass.sh") - tmp := t.TempDir() - driver := &ArtifactDriver{Username: os.Getenv("GITHUB_TOKEN")} - assert.NotEmpty(t, driver.Username) - err := driver.Load(&wfv1.Artifact{ - ArtifactLocation: wfv1.ArtifactLocation{ - Git: &wfv1.GitArtifact{ - Repo: tt.url, - Fetch: []string{"+refs/heads/*:refs/remotes/origin/*"}, - Revision: "HEAD", - Depth: &d, - }, - }, - }, tmp) - assert.NoError(t, err) - println(tmp) - } -} - -func TestGitArtifactDriverLoad_SSL(t *testing.T) { - t.SkipNow() - for _, tt := range []struct { - name string - insecure bool - url string - }{ - {"Insecure", true, "https://github.com/argoproj/argo-workflows.git"}, - {"Secure", false, "https://github.com/argoproj/argo-workflows.git"}, - {"Insecure", true, "https://github.com/argoproj/empty.git"}, - {"Secure", false, "https://github.com/argoproj/empty.git"}, - } { - t.Run(tt.name, func(t *testing.T) { - _ = os.Remove("git-ask-pass.sh") - key := os.Getenv("HOME") + "/.ssh/id_rsa" - data, err := ioutil.ReadFile(key) - if err != nil && os.IsNotExist(err) { - t.Skip(key + " does not exist") +func TestGitArtifactDriver_Load(t *testing.T) { + t.Run("EmptyRepo", func(t *testing.T) { + driver := &ArtifactDriver{} + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/empty-test-repo.git"})) + assert.DirExists(t, path) + }) + t.Run("PrivateRepo", func(t *testing.T) { + t.Run("SSH", func(t *testing.T) { + if os.Getenv("CI") == "true" { + t.SkipNow() } + privateKey, err := os.ReadFile(homedir.HomeDir() + "/.ssh/id_rsa") assert.NoError(t, err) - tmp := t.TempDir() - println(tmp) - driver := &ArtifactDriver{SSHPrivateKey: string(data)} - err = driver.Load(&wfv1.Artifact{ - ArtifactLocation: wfv1.ArtifactLocation{ - Git: &wfv1.GitArtifact{ - Repo: tt.url, - Fetch: []string{"+refs/heads/*:refs/remotes/origin/*"}, - Revision: "HEAD", - InsecureIgnoreHostKey: tt.insecure, - Depth: &d, - }, - }, - }, tmp) - assert.NoError(t, err) + driver := &ArtifactDriver{SSHPrivateKey: string(privateKey)} + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "git@github.com:argoproj-labs/private-test-repo.git"})) + assert.FileExists(t, path+"/README.md") }) - } + t.Run("HTTPS", func(t *testing.T) { + token := os.Getenv("PERSONAL_ACCESS_TOKEN") + if token == "" { + t.SkipNow() + } + driver := &ArtifactDriver{Username: "alexec", Password: token} + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/private-test-repo.git"})) + assert.FileExists(t, path+"/README.md") + }) + }) + t.Run("PublicRepo", func(t *testing.T) { + driver := &ArtifactDriver{} + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git"})) + assert.FileExists(t, path+"/README.md") + }) + t.Run("Depth", func(t *testing.T) { + driver := &ArtifactDriver{} + var depth uint64 = 1 + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Depth: &depth})) + assert.FileExists(t, path+"/README.md") + }) + t.Run("FetchRefs", func(t *testing.T) { + driver := &ArtifactDriver{} + t.Run("Garbage", func(t *testing.T) { + assert.Error(t, load(driver, &wfv1.GitArtifact{ + Repo: "https://github.com/argoproj-labs/test-repo.git", + Fetch: []string{"garbage"}, + })) + }) + t.Run("Valid", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{ + Repo: "https://github.com/argoproj-labs/test-repo.git", + Fetch: []string{"+refs/heads/*:refs/remotes/origin/*"}, + })) + assert.FileExists(t, path+"/README.md") + }) + }) + t.Run("Revision", func(t *testing.T) { + driver := &ArtifactDriver{} + t.Run("Garbage", func(t *testing.T) { + assert.Error(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Revision: "garbage"})) + }) + t.Run("Hash", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Revision: "6093d6a"})) + assert.FileExists(t, path+"/README.md") + }) + t.Run("HEAD", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Revision: "HEAD"})) + assert.FileExists(t, path+"/README.md") + }) + t.Run("HEAD~1", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Revision: "HEAD~1"})) + assert.FileExists(t, path+"/README.md") + }) + t.Run("Main", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Revision: "main"})) + assert.FileExists(t, path+"/README.md") + }) + t.Run("RemoteBranch", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Revision: "origin/my-branch"})) + assert.FileExists(t, path+"/my-branch") + }) + t.Run("LocalBranch", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Revision: "my-branch"})) + assert.FileExists(t, path+"/my-branch") + }) + t.Run("Tag", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo.git", Revision: "v0.0.0"})) + assert.FileExists(t, path+"/README.md") + }) + }) + t.Run("Submodules", func(t *testing.T) { + driver := &ArtifactDriver{} + t.Run("Disabled", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo-w-submodule.git", DisableSubmodules: true})) + assert.FileExists(t, path+"/README.md") + }) + t.Run("Enabled", func(t *testing.T) { + assert.NoError(t, load(driver, &wfv1.GitArtifact{Repo: "https://github.com/argoproj-labs/test-repo-w-submodule.git"})) + assert.FileExists(t, path+"/test-repo/README.md") + }) + }) } -func TestGetCheckoutRevision(t *testing.T) { - for _, tt := range []struct { - in string - expected string - }{ - {"my-branch", "my-branch"}, - {"refs/heads/my-branch", "my-branch"}, - {"refs/tags/1.0.0", "refs/tags/1.0.0"}, - {"ae7b5432cfa15577d4740fb047762254be3652db", "ae7b5432cfa15577d4740fb047762254be3652db"}, - } { - t.Run(tt.in, func(t *testing.T) { - result := getRevisionForCheckout(tt.in) - assert.Equal(t, result, tt.expected) - }) - } +const path = "/tmp/repo" + +func load(driver *ArtifactDriver, git *wfv1.GitArtifact) error { + _ = os.RemoveAll(path) + return driver.Load(&wfv1.Artifact{ArtifactLocation: wfv1.ArtifactLocation{Git: git}}, path) } func TestGetUser(t *testing.T) { diff --git a/workflow/artifacts/http/http.go b/workflow/artifacts/http/http.go index 6088d82d5475..c9d0f4f71e17 100644 --- a/workflow/artifacts/http/http.go +++ b/workflow/artifacts/http/http.go @@ -2,10 +2,9 @@ package http import ( "fmt" - "os/exec" - "strings" - - log "github.com/sirupsen/logrus" + "io" + "net/http" + "os" "github.com/argoproj/argo-workflows/v3/errors" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" @@ -20,28 +19,33 @@ var _ common.ArtifactDriver = &ArtifactDriver{} // Load download artifacts from an HTTP URL func (h *ArtifactDriver) Load(inputArtifact *wfv1.Artifact, path string) error { // Download the file to a local file path - args := []string{"-fsS", "-L", "--create-dirs", "--proto-default", "https", "-o", path, inputArtifact.HTTP.URL} - headers := inputArtifact.HTTP.Headers - for _, v := range headers { - // Build curl -H string for each key-value header parameter - args = append(args, "-H", fmt.Sprintf("%s: %s", v.Name, v.Value)) + req, err := http.NewRequest("GET", inputArtifact.HTTP.URL, nil) + if err != nil { + return err + } + for _, h := range inputArtifact.HTTP.Headers { + req.Header.Add(h.Name, h.Value) } - log.Info(strings.Join(append([]string{"curl"}, args...), " ")) - cmd := exec.Command("curl", args...) - output, err := cmd.CombinedOutput() - log.Info(string(output)) + resp, err := http.DefaultClient.Do(req) if err != nil { - log.WithError(err).Error() - if exitErr, ok := err.(*exec.ExitError); ok { - // https://ec.haxx.se/usingcurl/usingcurl-returns - // 22 - HTTP page not retrieved. - if exitErr.ExitCode() == 22 { - return errors.New(errors.CodeNotFound, exitErr.Error()) - } + return err + } + defer resp.Body.Close() + if resp.StatusCode >= 400 { + switch resp.StatusCode { + case http.StatusNotFound: + return errors.New(errors.CodeNotFound, "no found") + default: + return fmt.Errorf("%s", resp.Status) } + } + f, err := os.Create(path) + if err != nil { return err } - return nil + defer f.Close() + _, err = io.Copy(f, resp.Body) + return err } func (h *ArtifactDriver) Save(string, *wfv1.Artifact) error { diff --git a/workflow/artifacts/http/http_test.go b/workflow/artifacts/http/http_test.go index 4dae1781b5cc..d66d41b1574a 100644 --- a/workflow/artifacts/http/http_test.go +++ b/workflow/artifacts/http/http_test.go @@ -1,26 +1,15 @@ package http import ( - "bytes" "os" - "regexp" "testing" - log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/argoproj/argo-workflows/v3/errors" wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1" ) -func captureOutput(f func()) string { - var buf bytes.Buffer - log.SetOutput(&buf) - f() - log.SetOutput(os.Stderr) - return buf.String() -} - func TestHTTPArtifactDriver_Load(t *testing.T) { driver := &ArtifactDriver{} a := &wfv1.HTTPArtifact{ @@ -39,17 +28,14 @@ func TestHTTPArtifactDriver_Load(t *testing.T) { h1 := wfv1.Header{Name: "Accept", Value: "application/json"} h2 := wfv1.Header{Name: "Authorization", Value: "Bearer foo-bar"} a.Headers = []wfv1.Header{h1, h2} - output := captureOutput(func() { - err := driver.Load(&wfv1.Artifact{ - ArtifactLocation: wfv1.ArtifactLocation{HTTP: a}, - }, "/tmp/found-with-request-headers") - if assert.NoError(t, err) { - _, err := os.Stat("/tmp/found-with-request-headers") - assert.NoError(t, err) - } - }) - curl := "curl -fsS -L --create-dirs --proto-default https -o /tmp/found-with-request-headers https://github.com/argoproj/argo-workflows -H Accept: application/json -H Authorization: Bearer foo-bar" - assert.Regexp(t, regexp.MustCompile(curl), output) + err := driver.Load(&wfv1.Artifact{ + ArtifactLocation: wfv1.ArtifactLocation{HTTP: a}, + }, "/tmp/found-with-request-headers") + if assert.NoError(t, err) { + _, err := os.Stat("/tmp/found-with-request-headers") + assert.NoError(t, err) + } + assert.FileExists(t, "/tmp/found-with-request-headers") }) t.Run("NotFound", func(t *testing.T) { err := driver.Load(&wfv1.Artifact{ diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index 1bd7cd9a2ca4..1b6d14e57c01 100644 --- a/workflow/executor/executor.go +++ b/workflow/executor/executor.go @@ -22,6 +22,8 @@ import ( "syscall" "time" + "github.com/argoproj/argo-workflows/v3/util/file" + argofile "github.com/argoproj/pkg/file" log "github.com/sirupsen/logrus" apiv1 "k8s.io/api/core/v1" @@ -809,8 +811,45 @@ func isTarball(filePath string) (bool, error) { // renaming it to the desired location func untar(tarPath string, destPath string) error { decompressor := func(src string, dest string) error { - _, err := common.RunCommand("tar", "-xf", src, "-C", dest) - return err + f, err := os.Open(src) + if err != nil { + return err + } + defer f.Close() + gzr, err := file.GetGzipReader(f) + if err != nil { + return err + } + defer gzr.Close() + tr := tar.NewReader(gzr) + for { + header, err := tr.Next() + switch { + case err == io.EOF: + return nil + case err != nil: + return err + case header == nil: + continue + } + target := filepath.Join(dest, header.Name) + if err := os.MkdirAll(filepath.Dir(target), 0o700); err != nil && os.IsExist(err) { + return err + } + switch header.Typeflag { + case tar.TypeReg: + f, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) + if err != nil { + return err + } + if _, err := io.Copy(f, tr); err != nil { + return err + } + if err := f.Close(); err != nil { + return err + } + } + } } return unpack(tarPath, destPath, decompressor)