Repository: argoproj/argo (go)

Issue:
Cannot use `optional` with input artifacts/S3
Checklist:

 * [x] I've included the version. - master
 * [x] I've included reproduction steps.
 * [ ] I've included the workflow YAML.
 * [x] I've included the logs.
 
**What happened**:

Started a step with an input artifact where the bucket does not exist, step failed.

**What you expected to happen**:

Artifact was marked optional - should pass.

**How to reproduce it (as minimally and precisely as possible)**:

**Anything else we need to know?**:

**Environment**:

- Argo version:
```
$ argo version
```
- Kubernetes version :
```
$ kubectl version -o yaml
```

**Other debugging information (if applicable)**:

- workflow result:

```
argo --loglevel DEBUG get <workflowname>
```

- executor logs:

```
kubectl logs <failedpodname> -c init
kubectl logs <failedpodname> -c wait
```

- workflow-controller logs:

```
kubectl logs -n argo $(kubectl get pods -l app=workflow-controller -n argo -o name)
```

**Logs**

```
argo get <workflowname>
kubectl logs <failedpodname> -c init
kubectl logs <failedpodname> -c wait
kubectl logs -n argo $(kubectl get pods -l app=workflow-controller -n argo -o name)
```

---
<!-- Issue Author: Don't delete this message to encourage other users to support your issue! -->
**Message from the maintainers**:

If you are impacted by this bug please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.


Pull request description:
fix(executor): Optional input artifacts. Fixes #2990
Checklist:

* [x] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
* [x] The title of the PR is (a) [conventional](https://www.conventionalcommits.org/en/v1.0.0/), (b) states what changed, and (c) suffixes the related issues number. E.g. `"fix(controller): Updates such and such. Fixes #1234"`.  
* [x] I've signed the CLA.
* [x] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
* [x] My builds are green. Try syncing with master if they are not. 
* [x] My organization is added to USERS.md.

# Changes

* Each artifact repository that can output an artifact can now support optional input artifacts.
* HTTP also - as this was easy to test.

# Testing

| Name | Change | How |
|---|---|---|
| Artifactory | Translate 404 error | Ask @jessesuen or community |
| GCS | Bug fixes. Translate `ErrObjectNotExists`| Ask author @whynowy |
| Git | - | N/A |
| HDFS | Translate ErrNotExists | Ask @sarabala1979 and @dtaniwaki |
| HTTP | Translate exit code 22 | Automated |
| OSS | Bug fix | Ask @merlintang  |
| Raw | -| N/A |
| S3 | Translate error | Automated |

# Prototypes

https://bit.ly/argo-wf-protos 

Patch:
diff --git a/cmd/argoexec/commands/wait.go b/cmd/argoexec/commands/wait.go
index 1f25ba7bc..9a39b0788 100644
--- a/cmd/argoexec/commands/wait.go
+++ b/cmd/argoexec/commands/wait.go
@@ -43,32 +43,32 @@ func waitContainer() error {
 		// do not return here so we can still try to kill sidecars & save outputs
 	}
 
-	// Saving logs
-	logArt, err := wfExecutor.SaveLogs()
+	// Capture output script result
+	err := wfExecutor.CaptureScriptResult()
 	if err != nil {
 		wfExecutor.AddError(err)
 		return err
 	}
-	// Saving output parameters
-	err = wfExecutor.SaveParameters()
+	// Capture output script exit code
+	err = wfExecutor.CaptureScriptExitCode()
 	if err != nil {
 		wfExecutor.AddError(err)
 		return err
 	}
-	// Saving output artifacts
-	err = wfExecutor.SaveArtifacts()
+	// Saving logs
+	logArt, err := wfExecutor.SaveLogs()
 	if err != nil {
 		wfExecutor.AddError(err)
 		return err
 	}
-	// Capture output script result
-	err = wfExecutor.CaptureScriptResult()
+	// Saving output parameters
+	err = wfExecutor.SaveParameters()
 	if err != nil {
 		wfExecutor.AddError(err)
 		return err
 	}
-	// Capture output script exit code
-	err = wfExecutor.CaptureScriptExitCode()
+	// Saving output artifacts
+	err = wfExecutor.SaveArtifacts()
 	if err != nil {
 		wfExecutor.AddError(err)
 		return err
@@ -85,4 +85,4 @@ func waitContainer() error {
 	}
 
 	return nil
-}
+}
\ No newline at end of file
diff --git a/docs/configure-artifact-repository.md b/docs/configure-artifact-repository.md
index 15f763cc3..08d32561f 100644
--- a/docs/configure-artifact-repository.md
+++ b/docs/configure-artifact-repository.md
@@ -5,6 +5,17 @@ repository. Argo supports any S3 compatible artifact repository such as AWS, GCS
 and Minio. This section shows how to configure the artifact repository.
 Subsequent sections will show how to use it.
 
+| Name | Inputs | Outputs | Usage (Feb 2020) |
+|---|---|---|---|
+| Artifactory | Yes | Yes | 11% |
+| GCS | Yes | Yes | - |
+| Git | Yes | No | - |
+| HDFS | Yes | Yes | 3% |
+| HTTP | Yes | No | 2% |
+| OSS | Yes | Yes | - |
+| Raw | Yes | No | 5% |
+| S3 | Yes | Yes | 86% | 
+
 ## Configuring Minio
 
 ```
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go
index 06b644e05..2429afe0c 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types.go
@@ -697,6 +697,18 @@ type ArtifactLocation struct {
 	GCS *GCSArtifact `json:"gcs,omitempty" protobuf:"bytes,9,opt,name=gcs"`
 }
 
+// HasLocation whether or not an artifact has a location defined
+func (a *ArtifactLocation) HasLocation() bool {
+	return a.S3.HasLocation() ||
+		a.Git.HasLocation() ||
+		a.HTTP.HasLocation() ||
+		a.Artifactory.HasLocation() ||
+		a.Raw.HasLocation() ||
+		a.HDFS.HasLocation() ||
+		a.OSS.HasLocation() ||
+		a.GCS.HasLocation()
+}
+
 type ArtifactRepositoryRef struct {
 	ConfigMap string `json:"configMap,omitempty" protobuf:"bytes,1,opt,name=configMap"`
 	Key       string `json:"key,omitempty" protobuf:"bytes,2,opt,name=key"`
@@ -1295,7 +1307,7 @@ type S3Artifact struct {
 }
 
 func (s *S3Artifact) HasLocation() bool {
-	return s != nil && s.Bucket != ""
+	return s != nil && s.Endpoint != "" && s.Bucket != "" && s.Key != ""
 }
 
 // GitArtifact is the location of an git artifact
@@ -1708,18 +1720,6 @@ func (args *Arguments) GetParameterByName(name string) *Parameter {
 	return nil
 }
 
-// HasLocation whether or not an artifact has a location defined
-func (a *Artifact) HasLocation() bool {
-	return a.S3.HasLocation() ||
-		a.Git.HasLocation() ||
-		a.HTTP.HasLocation() ||
-		a.Artifactory.HasLocation() ||
-		a.Raw.HasLocation() ||
-		a.HDFS.HasLocation() ||
-		a.OSS.HasLocation() ||
-		a.GCS.HasLocation()
-}
-
 // GetTemplateByName retrieves a defined template by its name
 func (wf *Workflow) GetTemplateByName(name string) *Template {
 	for _, t := range wf.Spec.Templates {
diff --git a/workflow/artifacts/artifactory/artifactory.go b/workflow/artifacts/artifactory/artifactory.go
index e5c626f63..f18fb2c9d 100644
--- a/workflow/artifacts/artifactory/artifactory.go
+++ b/workflow/artifacts/artifactory/artifactory.go
@@ -37,6 +37,9 @@ func (a *ArtifactoryArtifactDriver) Load(artifact *wfv1.Artifact, path string) e
 	defer func() {
 		_ = res.Body.Close()
 	}()
+	if res.StatusCode == 404 {
+		return errors.New(errors.CodeNotFound, res.Status)
+	}
 	if res.StatusCode < 200 || res.StatusCode >= 300 {
 		return errors.InternalErrorf("loading file from artifactory failed with reason:%s", res.Status)
 	}
diff --git a/workflow/artifacts/gcs/gcs.go b/workflow/artifacts/gcs/gcs.go
index 89702c513..f310f3659 100644
--- a/workflow/artifacts/gcs/gcs.go
+++ b/workflow/artifacts/gcs/gcs.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/argoproj/pkg/file"
 
+	"github.com/argoproj/argo/errors"
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 )
 
@@ -65,13 +66,13 @@ func (g *ArtifactDriver) Load(inputArtifact *wfv1.Artifact, path string) error {
 			gcsClient, err := g.newGCSClient()
 			if err != nil {
 				log.Warnf("Failed to create new GCS client: %v", err)
-				return false, nil
+				return false, err
 			}
 			defer gcsClient.Close()
 			err = downloadObjects(gcsClient, inputArtifact.GCS.Bucket, inputArtifact.GCS.Key, path)
 			if err != nil {
 				log.Warnf("Failed to download objects from GCS: %v", err)
-				return false, nil
+				return false, err
 			}
 			return true, nil
 		})
@@ -87,7 +88,7 @@ func downloadObjects(client *storage.Client, bucket, key, path string) error {
 	for _, objName := range objNames {
 		err = downloadObject(client, bucket, key, objName, path)
 		if err != nil {
-			return fmt.Errorf("download object: %v", err)
+			return err
 		}
 	}
 	return nil
@@ -107,6 +108,9 @@ func downloadObject(client *storage.Client, bucket, key, objName, path string) e
 	ctx := context.Background()
 	rc, err := client.Bucket(bucket).Object(objName).NewReader(ctx)
 	if err != nil {
+		if err == storage.ErrObjectNotExist {
+			return errors.New(errors.CodeNotFound, err.Error())
+		}
 		return fmt.Errorf("new bucket reader: %v", err)
 	}
 	defer rc.Close()
@@ -152,13 +156,12 @@ func (g *ArtifactDriver) Save(path string, outputArtifact *wfv1.Artifact) error
 			log.Infof("GCS Save path: %s, key: %s", path, outputArtifact.GCS.Key)
 			client, err := g.newGCSClient()
 			if err != nil {
-				log.Warnf("Failed to create new GCS client: %v", err)
-				return false, nil
+				return false, err
 			}
 			defer client.Close()
 			err = uploadObjects(client, outputArtifact.GCS.Bucket, outputArtifact.GCS.Key, path)
 			if err != nil {
-				log.Warnf("Failed to upload objects: %v", err)
+				return false, err
 			}
 			return true, nil
 		})
diff --git a/workflow/artifacts/git/git.go b/workflow/artifacts/git/git.go
index ae7554ebd..98c31e43c 100644
--- a/workflow/artifacts/git/git.go
+++ b/workflow/artifacts/git/git.go
@@ -49,7 +49,7 @@ func (g *GitArtifactDriver) Load(inputArtifact *wfv1.Artifact, path string) erro
 }
 
 // Save is unsupported for git output artifacts
-func (g *GitArtifactDriver) Save(path string, outputArtifact *wfv1.Artifact) error {
+func (g *GitArtifactDriver) Save(string, *wfv1.Artifact) error {
 	return errors.Errorf(errors.CodeBadRequest, "Git output artifacts unsupported")
 }
 
diff --git a/workflow/artifacts/hdfs/hdfs.go b/workflow/artifacts/hdfs/hdfs.go
index 59c17765c..73ce77af0 100644
--- a/workflow/artifacts/hdfs/hdfs.go
+++ b/workflow/artifacts/hdfs/hdfs.go
@@ -131,7 +131,7 @@ func CreateDriver(ci resource.Interface, art *wfv1.HDFSArtifact) (*ArtifactDrive
 }
 
 // Load downloads artifacts from HDFS compliant storage
-func (driver *ArtifactDriver) Load(inputArtifact *wfv1.Artifact, path string) error {
+func (driver *ArtifactDriver) Load(_ *wfv1.Artifact, path string) error {
 	hdfscli, err := createHDFSClient(driver.Addresses, driver.HDFSUser, driver.KrbOptions)
 	if err != nil {
 		return err
@@ -169,7 +169,14 @@ func (driver *ArtifactDriver) Load(inputArtifact *wfv1.Artifact, path string) er
 		}
 	}
 
-	return hdfscli.CopyToLocal(driver.Path, path)
+	err = hdfscli.CopyToLocal(driver.Path, path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return errors.New(errors.CodeNotFound, err.Error())
+		}
+		return err
+	}
+	return nil
 }
 
 // Save saves an artifact to HDFS compliant storage
diff --git a/workflow/artifacts/http/http.go b/workflow/artifacts/http/http.go
index 10f12ff4a..5547ad453 100644
--- a/workflow/artifacts/http/http.go
+++ b/workflow/artifacts/http/http.go
@@ -1,9 +1,13 @@
 package http
 
 import (
+	"os/exec"
+	"strings"
+
+	log "github.com/sirupsen/logrus"
+
 	"github.com/argoproj/argo/errors"
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
-	"github.com/argoproj/argo/workflow/common"
 )
 
 // HTTPArtifactDriver is the artifact driver for a HTTP URL
@@ -12,9 +16,25 @@ type HTTPArtifactDriver struct{}
 // Load download artifacts from an HTTP URL
 func (h *HTTPArtifactDriver) Load(inputArtifact *wfv1.Artifact, path string) error {
 	// Download the file to a local file path
-	return common.RunCommand("curl", "-sS", "-L", "-o", path, inputArtifact.HTTP.URL)
+	args := []string{"-fsS", "-L", "-o", path, inputArtifact.HTTP.URL}
+	log.Info(strings.Join(append([]string{"curl"}, args...), " "))
+	cmd := exec.Command("curl", args...)
+	output, err := cmd.CombinedOutput()
+	log.Info(string(output))
+	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
+	}
+	return nil
 }
 
-func (h *HTTPArtifactDriver) Save(path string, outputArtifact *wfv1.Artifact) error {
+func (h *HTTPArtifactDriver) Save(string, *wfv1.Artifact) error {
 	return errors.Errorf(errors.CodeBadRequest, "HTTP output artifacts unsupported")
 }
diff --git a/workflow/artifacts/oss/oss.go b/workflow/artifacts/oss/oss.go
index 50802691c..497f956c7 100644
--- a/workflow/artifacts/oss/oss.go
+++ b/workflow/artifacts/oss/oss.go
@@ -34,8 +34,7 @@ func (ossDriver *OSSArtifactDriver) Load(inputArtifact *wfv1.Artifact, path stri
 			log.Infof("OSS Load path: %s, key: %s", path, inputArtifact.OSS.Key)
 			osscli, err := ossDriver.newOSSClient()
 			if err != nil {
-				log.Warnf("Failed to create new OSS client: %v", err)
-				return false, nil
+				return false, err
 			}
 			bucketName := inputArtifact.OSS.Bucket
 			bucket, err := osscli.Bucket(bucketName)
diff --git a/workflow/artifacts/raw/raw.go b/workflow/artifacts/raw/raw.go
index 3742701d0..9cbadbbe0 100644
--- a/workflow/artifacts/raw/raw.go
+++ b/workflow/artifacts/raw/raw.go
@@ -25,6 +25,6 @@ func (a *RawArtifactDriver) Load(artifact *wfv1.Artifact, path string) error {
 }
 
 // Save is unsupported for raw output artifacts
-func (g *RawArtifactDriver) Save(path string, outputArtifact *wfv1.Artifact) error {
+func (g *RawArtifactDriver) Save(string, *wfv1.Artifact) error {
 	return errors.Errorf(errors.CodeBadRequest, "Raw output artifacts unsupported")
 }
diff --git a/workflow/artifacts/s3/s3.go b/workflow/artifacts/s3/s3.go
index a43f7dba1..ac9630a77 100644
--- a/workflow/artifacts/s3/s3.go
+++ b/workflow/artifacts/s3/s3.go
@@ -10,6 +10,7 @@ import (
 	"github.com/argoproj/pkg/file"
 	argos3 "github.com/argoproj/pkg/s3"
 
+	"github.com/argoproj/argo/errors"
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 	"github.com/argoproj/argo/workflow/common"
 )
@@ -66,7 +67,7 @@ func (s3Driver *S3ArtifactDriver) Load(inputArtifact *wfv1.Artifact, path string
 			}
 			if !isDir {
 				// It's neither a file, nor a directory. Return the original NoSuchKey error
-				return false, origErr
+				return false, errors.New(errors.CodeNotFound, origErr.Error())
 			}
 
 			if err = s3cli.GetDirectory(inputArtifact.S3.Bucket, inputArtifact.S3.Key, path); err != nil {
diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go
index 38bc46911..f8a10079e 100644
--- a/workflow/executor/executor.go
+++ b/workflow/executor/executor.go
@@ -164,6 +164,10 @@ func (we *WorkflowExecutor) LoadArtifacts() error {
 		tempArtPath := artPath + ".tmp"
 		err = artDriver.Load(&art, tempArtPath)
 		if err != nil {
+			if art.Optional && errors.IsCode(errors.CodeNotFound, err) {
+				log.Infof("Skipping optional input artifact that was not found: %s", art.Name)
+				continue
+			}
 			return err
 		}
 


Test patch:
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types_test.go b/pkg/apis/workflow/v1alpha1/workflow_types_test.go
index 6ed418b77..1b7409fca 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types_test.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types_test.go
@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
 )
 
 func TestWorkflows(t *testing.T) {
@@ -24,6 +25,19 @@ func TestWorkflows(t *testing.T) {
 	assert.Equal(t, "3", wfs[3].Name)
 }
 
+func TestArtifactLocation_HasLocation(t *testing.T) {
+	assert.False(t, (&ArtifactLocation{}).HasLocation())
+	assert.False(t, (&ArtifactLocation{ArchiveLogs: pointer.BoolPtr(true)}).HasLocation())
+	assert.True(t, (&ArtifactLocation{S3: &S3Artifact{Key: "my-key", S3Bucket: S3Bucket{Endpoint: "my-endpoint", Bucket: "my-bucket"}}}).HasLocation())
+	assert.True(t, (&ArtifactLocation{Git: &GitArtifact{Repo: "my-repo"}}).HasLocation())
+	assert.True(t, (&ArtifactLocation{HTTP: &HTTPArtifact{URL: "my-url"}}).HasLocation())
+	assert.True(t, (&ArtifactLocation{Artifactory: &ArtifactoryArtifact{URL: "my-url"}}).HasLocation())
+	assert.True(t, (&ArtifactLocation{Raw: &RawArtifact{Data: "my-data"}}).HasLocation())
+	assert.True(t, (&ArtifactLocation{HDFS: &HDFSArtifact{HDFSConfig: HDFSConfig{Addresses: []string{"my-address"}}}}).HasLocation())
+	assert.True(t, (&ArtifactLocation{OSS: &OSSArtifact{Key: "my-key", OSSBucket: OSSBucket{Endpoint: "my-endpoint", Bucket: "my-bucket"}}}).HasLocation())
+	assert.True(t, (&ArtifactLocation{GCS: &GCSArtifact{Key: "my-key", GCSBucket: GCSBucket{Bucket: "my-bucket"}}}).HasLocation())
+}
+
 func TestNodes_FindByDisplayName(t *testing.T) {
 	assert.Nil(t, Nodes{}.FindByDisplayName(""))
 	assert.NotNil(t, Nodes{"": NodeStatus{DisplayName: "foo"}}.FindByDisplayName("foo"))
diff --git a/test/e2e/functional_test.go b/test/e2e/functional_test.go
index c081414ce..311853abc 100644
--- a/test/e2e/functional_test.go
+++ b/test/e2e/functional_test.go
@@ -507,6 +507,18 @@ func (s *FunctionalSuite) TestSameInputOutputPathOptionalArtifact() {
 		})
 }
 
+func (s *FunctionalSuite) TestOptionalInputArtifacts() {
+	s.Given().
+		Workflow("@testdata/input-artifacts.yaml").
+		When().
+		SubmitWorkflow().
+		WaitForWorkflow(30 * time.Second).
+		Then().
+		ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
+			assert.Equal(t, wfv1.NodeSucceeded, status.Phase)
+		})
+}
+
 func TestFunctionalSuite(t *testing.T) {
 	suite.Run(t, new(FunctionalSuite))
 }
diff --git a/test/e2e/testdata/input-artifacts.yaml b/test/e2e/testdata/input-artifacts.yaml
new file mode 100644
index 000000000..160639613
--- /dev/null
+++ b/test/e2e/testdata/input-artifacts.yaml
@@ -0,0 +1,33 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: input-artifacts
+  labels:
+    argo-e2e: true
+spec:
+  entrypoint: main
+  templates:
+    - name: main
+      container:
+        image: argoproj/argosay:v2
+      inputs:
+        artifacts:
+          - name: my-http-artifact
+            path: /my-http-artifact
+            optional: true
+            http:
+              url: https://github.com/argoproj/argo/not-found
+          - name: my-s3-artifact
+            path: /my-s3-artifact
+            optional: true
+            s3:
+              bucket: my-bucket
+              key: my-key
+              endpoint: minio:9000
+              insecure: true
+              accessKeySecret:
+                name: my-minio-cred
+                key: accesskey
+              secretKeySecret:
+                name: my-minio-cred
+                key: secretkey
\ No newline at end of file
diff --git a/workflow/artifacts/artifactory/artifactory_test.go b/workflow/artifacts/artifactory/artifactory_test.go
index fba6926ae..5d43a860b 100644
--- a/workflow/artifacts/artifactory/artifactory_test.go
+++ b/workflow/artifacts/artifactory/artifactory_test.go
@@ -1,61 +1,39 @@
-package artifactory_test
+package artifactory
 
 import (
-	"io/ioutil"
 	"os"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/argoproj/argo/errors"
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
-	art "github.com/argoproj/argo/workflow/artifacts/artifactory"
 )
 
-const (
-	LoadFileName string = "argo_artifactory_test_load.txt"
-	SaveFileName string = "argo_artifactory_test_save.txt"
-	RepoName     string = "generic-local"
-	URL          string = "http://localhost:8081/artifactory/" + RepoName + "/" + LoadFileName
-	Username     string = "admin"
-	Password     string = "password"
-)
-
-func TestSaveAndLoad(t *testing.T) {
-
-	t.Skip("This test is skipped since it depends on external service")
-	fileContent := "time: " + string(time.Now().UnixNano())
-
-	// create file to test save
-	lf, err := ioutil.TempFile("", LoadFileName)
-	assert.NoError(t, err)
-	defer os.Remove(lf.Name())
-	// load file with test content
-	content := []byte(fileContent)
-	_, err = lf.Write(content)
-	assert.NoError(t, err)
-	err = lf.Close()
-	assert.NoError(t, err)
-
-	// create file to test load
-	sf, err := ioutil.TempFile("", SaveFileName)
-	assert.NoError(t, err)
-	defer os.Remove(sf.Name())
-
-	artL := &wfv1.Artifact{}
-	artL.Artifactory = &wfv1.ArtifactoryArtifact{
-		URL: URL,
-	}
-	driver := &art.ArtifactoryArtifactDriver{
-		Username: Username,
-		Password: Password,
-	}
-	err = driver.Save(lf.Name(), artL)
-	assert.NoError(t, err)
-	err = driver.Load(artL, sf.Name())
-	assert.NoError(t, err)
-
-	dat, err := ioutil.ReadFile(sf.Name())
-	assert.NoError(t, err)
-	assert.Equal(t, fileContent, string(dat))
+func TestArtifactoryArtifactDriver_Load(t *testing.T) {
+	driver := &ArtifactoryArtifactDriver{}
+	t.Run("NotFound", func(t *testing.T) {
+		err := driver.Load(&wfv1.Artifact{
+			ArtifactLocation: wfv1.ArtifactLocation{
+				Artifactory: &wfv1.ArtifactoryArtifact{URL: "https://github.com/argoproj/argo/not-found"},
+			},
+		}, "/tmp/not-found")
+		if assert.Error(t, err) {
+			argoError, ok := err.(errors.ArgoError)
+			if assert.True(t, ok) {
+				assert.Equal(t, errors.CodeNotFound, argoError.Code())
+			}
+		}
+	})
+	t.Run("Found", func(t *testing.T) {
+		err := driver.Load(&wfv1.Artifact{
+			ArtifactLocation: wfv1.ArtifactLocation{
+				Artifactory: &wfv1.ArtifactoryArtifact{URL: "https://github.com/argoproj/argo"},
+			},
+		}, "/tmp/found")
+		if assert.NoError(t, err) {
+			_, err := os.Stat("/tmp/found")
+			assert.NoError(t, err)
+		}
+	})
 }
diff --git a/workflow/artifacts/git/git_test.go b/workflow/artifacts/git/git_test.go
new file mode 100644
index 000000000..9b5d04e40
--- /dev/null
+++ b/workflow/artifacts/git/git_test.go
@@ -0,0 +1,35 @@
+package git
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
+)
+
+func TestGitArtifactDriver_Load(t *testing.T) {
+	driver := &GitArtifactDriver{}
+	t.Run("Found", func(t *testing.T) {
+		path := "/tmp/git-found"
+		assert.NoError(t, os.RemoveAll(path))
+		assert.NoError(t, os.MkdirAll(path, 0777))
+		d := uint64(1)
+		err := driver.Load(&wfv1.Artifact{
+			ArtifactLocation: wfv1.ArtifactLocation{
+				Git: &wfv1.GitArtifact{Repo: "https://github.com/argoproj/argoproj.git", Revision: "master", Depth: &d},
+			},
+		}, path)
+		if assert.NoError(t, err) {
+			_, err := os.Stat(path)
+			assert.NoError(t, err)
+		}
+	})
+}
+
+func TestGitArtifactDriver_Save(t *testing.T) {
+	driver := &GitArtifactDriver{}
+	err := driver.Save("", nil)
+	assert.Error(t, err)
+}
diff --git a/workflow/artifacts/http/http_test.go b/workflow/artifacts/http/http_test.go
new file mode 100644
index 000000000..fc2a7923c
--- /dev/null
+++ b/workflow/artifacts/http/http_test.go
@@ -0,0 +1,44 @@
+package http
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/argoproj/argo/errors"
+	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
+)
+
+func TestHTTPArtifactDriver_Load(t *testing.T) {
+	driver := &HTTPArtifactDriver{}
+	t.Run("NotFound", func(t *testing.T) {
+		err := driver.Load(&wfv1.Artifact{
+			ArtifactLocation: wfv1.ArtifactLocation{
+				HTTP: &wfv1.HTTPArtifact{URL: "https://github.com/argoproj/argo/not-found"},
+			},
+		}, "/tmp/not-found")
+		if assert.Error(t, err) {
+			argoError, ok := err.(errors.ArgoError)
+			if assert.True(t, ok) {
+				assert.Equal(t, errors.CodeNotFound, argoError.Code())
+			}
+		}
+	})
+	t.Run("Found", func(t *testing.T) {
+		err := driver.Load(&wfv1.Artifact{
+			ArtifactLocation: wfv1.ArtifactLocation{
+				HTTP: &wfv1.HTTPArtifact{URL: "https://github.com/argoproj/argo"},
+			},
+		}, "/tmp/found")
+		if assert.NoError(t, err) {
+			_, err := os.Stat("/tmp/found")
+			assert.NoError(t, err)
+		}
+	})
+}
+
+func TestHTTPArtifactDriver_Save(t *testing.T) {
+	driver := &HTTPArtifactDriver{}
+	assert.Error(t, driver.Save("", nil))
+}