Repository: argoproj/argo (go)

Issue:
open artifact : permission denied
## Summary 

**What happened:**
I installed the quick-start-postgres.yaml, submitted the  example using the argo UI. After the example succeeded,I want to see the logs in Archived Workflows, but it shows "Internal Server Error". Then  I clicked on" logs from the artifacts", but it shows "open artifact788239015: permission denied". I clicked on "OUTPUT ARTIFACTS" and I can see the file"main-logs", but when I clicked on it, stil, "open artifact083636081: permission denied"


**What version of Argo Workflows are you running?** 
v2.11.6

argo-server logs
```
time="2020-10-21T13:09:37Z" level=info msg="Download artifact" artifactName=main-logs nodeId=delightful-poochenheimer uid=1b4ba4cd-a7a0-4db8-a5f6-b0b71d4b7fc4
time="2020-10-21T13:13:31Z" level=info msg="Alloc=9032 TotalAlloc=45921 Sys=72384 NumGC=23 Goroutines=79"
```
workflow-controller logs
```
time="2020-10-21T12:58:29Z" level=info msg="Alloc=6195 TotalAlloc=63291 Sys=71872 NumGC=63 Goroutines=181"
time="2020-10-21T12:58:39Z" level=info msg="Performing periodic workflow GC"
time="2020-10-21T12:58:39Z" level=info msg="Zero old offloads, nothing to do"
time="2020-10-21T12:59:50Z" level=info msg="Processing workflow" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T12:59:50Z" level=info msg="Updated phase  -> Running" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T12:59:50Z" level=info msg="Pod node delightful-poochenheimer initialized Pending" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T12:59:50Z" level=info msg="Created pod: delightful-poochenheimer (delightful-poochenheimer)" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T12:59:50Z" level=info msg="Workflow update successful" namespace=argo phase=Running resourceVersion=161374914 workflow=delightful-poochenheimer
time="2020-10-21T12:59:50Z" level=info msg="Processing workflow" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T12:59:50Z" level=info msg="Skipped pod delightful-poochenheimer (delightful-poochenheimer) creation: already exists" namespace=argo podPhase=Pending workflow=delightful-poochenheimer
time="2020-10-21T12:59:50Z" level=info msg="Workflow update successful" namespace=argo phase=Running resourceVersion=161374917 workflow=delightful-poochenheimer
time="2020-10-21T12:59:51Z" level=info msg="Processing workflow" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T12:59:51Z" level=info msg="Updating node delightful-poochenheimer message: ContainerCreating"
time="2020-10-21T12:59:51Z" level=info msg="Skipped pod delightful-poochenheimer (delightful-poochenheimer) creation: already exists" namespace=argo podPhase=Pending workflow=delightful-poochenheimer
time="2020-10-21T12:59:51Z" level=info msg="Workflow update successful" namespace=argo phase=Running resourceVersion=161374944 workflow=delightful-poochenheimer
time="2020-10-21T12:59:52Z" level=info msg="Processing workflow" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T12:59:52Z" level=info msg="Skipped pod delightful-poochenheimer (delightful-poochenheimer) creation: already exists" namespace=argo podPhase=Pending workflow=delightful-poochenheimer
time="2020-10-21T13:00:07Z" level=info msg="Processing workflow" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T13:00:07Z" level=info msg="Setting node delightful-poochenheimer outputs"
time="2020-10-21T13:00:07Z" level=info msg="Updating node delightful-poochenheimer status Pending -> Running"
time="2020-10-21T13:00:07Z" level=info msg="Skipped pod delightful-poochenheimer (delightful-poochenheimer) creation: already exists" namespace=argo podPhase=Running workflow=delightful-poochenheimer
time="2020-10-21T13:00:07Z" level=info msg="Workflow update successful" namespace=argo phase=Running resourceVersion=161375296 workflow=delightful-poochenheimer
time="2020-10-21T13:00:07Z" level=info msg="Processing workflow" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T13:00:07Z" level=info msg="Skipped pod delightful-poochenheimer (delightful-poochenheimer) creation: already exists" namespace=argo podPhase=Running workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="Processing workflow" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="Updating node delightful-poochenheimer status Running -> Succeeded"
time="2020-10-21T13:00:08Z" level=info msg="Updated phase Running -> Succeeded" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="Marking workflow completed" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="Marking workflow as pending archiving" namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="Checking daemoned children of " namespace=argo workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="Workflow update successful" namespace=argo phase=Succeeded resourceVersion=161375322 workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="archiving workflow" namespace=argo uid=1b4ba4cd-a7a0-4db8-a5f6-b0b71d4b7fc4 workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="Delete pod for gc successfully" namespace=argo pod=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="archiving workflow" namespace=argo uid=1b4ba4cd-a7a0-4db8-a5f6-b0b71d4b7fc4 workflow=delightful-poochenheimer
time="2020-10-21T13:00:08Z" level=info msg="Found Workflow argo/delightful-poochenheimer set expire at 2020-10-21 13:05:08 +0000 UTC (4m59.240014881s from now)"
time="2020-10-21T13:00:08Z" level=info msg="Queueing workflow argo/delightful-poochenheimer for delete in 4m59.240014881s"
time="2020-10-21T13:00:08Z" level=info msg="Found Workflow argo/delightful-poochenheimer set expire at 2020-10-21 13:05:08 +0000 UTC (4m59.239699386s from now)"
time="2020-10-21T13:00:08Z" level=info msg="Queueing workflow argo/delightful-poochenheimer for delete in 4m59.239699386s"
time="2020-10-21T13:03:29Z" level=info msg="Alloc=6258 TotalAlloc=66707 Sys=72128 NumGC=66 Goroutines=181"
time="2020-10-21T13:03:39Z" level=info msg="Performing periodic workflow GC"
time="2020-10-21T13:03:39Z" level=info msg="Zero old offloads, nothing to do"
time="2020-10-21T13:05:08Z" level=info msg="Deleting TTL expired workflow argo/delightful-poochenheimer"
time="2020-10-21T13:05:08Z" level=info msg="Successfully deleted 'argo/delightful-poochenheimer'"
time="2020-10-21T13:05:08Z" level=info msg="Found Workflow argo/delightful-poochenheimer set expire at 2020-10-21 13:05:08 +0000 UTC (-36.460904ms from now)"
time="2020-10-21T13:05:08Z" level=info msg="Queueing workflow argo/delightful-poochenheimer for delete in 0s"
time="2020-10-21T13:08:29Z" level=info msg="Alloc=6248 TotalAlloc=67136 Sys=72128 NumGC=68 Goroutines=181"
time="2020-10-21T13:08:39Z" level=info msg="Performing periodic workflow GC"
time="2020-10-21T13:08:39Z" level=info msg="Zero old offloads, nothing to do"
time="2020-10-21T13:13:29Z" level=info msg="Alloc=6140 TotalAlloc=67444 Sys=72128 NumGC=71 Goroutines=181"
time="2020-10-21T13:13:39Z" level=info msg="Performing periodic workflow GC"
time="2020-10-21T13:13:39Z" level=info msg="Zero old offloads, nothing to do"
```
---
<!-- Issue Author: Don't delete this message to encourage other users to support your issue! -->
**Message from the maintainers**:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.


Pull request description:
fix(server): Download artifacts from UI. Fixes #4338
Signed-off-by: Alex Collins <alex_collins@intuit.com>

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] My organization is added to [USERS.md](https://github.com/argoproj/argo/blob/master/USERS.md).
* [x] I've signed the CLA.
* [ ] ~~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. 

Fixes #4338

Patch:
diff --git a/Makefile b/Makefile
index cb243d05a..ea0e5859c 100644
--- a/Makefile
+++ b/Makefile
@@ -1,9 +1,6 @@
 export SHELL:=/bin/bash
 export SHELLOPTS:=$(if $(SHELLOPTS),$(SHELLOPTS):)pipefail:errexit
 
-# This means we only one shell, and therefore you can use `trap` to clean-up
-.ONESHELL:
-
 # https://stackoverflow.com/questions/4122831/disable-make-builtin-rules-and-variables-from-inside-the-make-file
 MAKEFLAGS += --no-builtin-rules
 .SUFFIXES:
@@ -95,6 +92,7 @@ endif
 ARGOEXEC_PKGS    := $(shell echo cmd/argoexec            && go list -f '{{ join .Deps "\n" }}' ./cmd/argoexec/            | grep 'argoproj/argo' | cut -c 26-)
 CLI_PKGS         := $(shell echo cmd/argo                && go list -f '{{ join .Deps "\n" }}' ./cmd/argo/                | grep 'argoproj/argo' | cut -c 26-)
 CONTROLLER_PKGS  := $(shell echo cmd/workflow-controller && go list -f '{{ join .Deps "\n" }}' ./cmd/workflow-controller/ | grep 'argoproj/argo' | cut -c 26-)
+MANIFESTS        := $(shell find manifests -mindepth 2 -type f)
 E2E_MANIFESTS    := $(shell find test/e2e/manifests -mindepth 2 -type f)
 E2E_EXECUTOR ?= pns
 TYPES := $(shell find pkg/apis/workflow/v1alpha1 -type f -name '*.go' -not -name openapi_generated.go -not -name '*generated*' -not -name '*test.go')
@@ -112,7 +110,6 @@ PROTO_BINARIES := $(GOPATH)/bin/protoc-gen-gogo $(GOPATH)/bin/protoc-gen-gogofas
 
 # go_install,path
 define go_install
-	trap 'rm -Rf vendor' EXIT
 	[ -e vendor ] || go mod vendor
 	go install -mod=vendor ./vendor/$(1)
 endef
@@ -120,7 +117,6 @@ endef
 # protoc,my.proto
 define protoc
 	# protoc $(1)
-	trap 'rm -Rf vendor' EXIT
     [ -e vendor ] || go mod vendor
     protoc \
       -I /usr/local/include \
@@ -274,6 +270,7 @@ codegen: \
 	$(GOPATH)/bin/mockery
 	# `go generate ./...` takes around 10s, so we only run on specific packages.
 	go generate ./persist/sqldb ./pkg/apiclient/workflow ./server/auth ./server/auth/sso ./workflow/executor
+	rm -Rf vendor
 
 $(GOPATH)/bin/mockery:
 	./hack/recurl.sh dist/mockery.tar.gz https://github.com/vektra/mockery/releases/download/v1.1.1/mockery_1.1.1_$(shell uname -s)_$(shell uname -m).tar.gz
@@ -311,7 +308,6 @@ $(GOPATH)/bin/goimports:
 	go get golang.org/x/tools/cmd/goimports@v0.0.0-20200630154851-b2d8b0336632
 
 pkg/apis/workflow/v1alpha1/generated.proto: $(GOPATH)/bin/go-to-protobuf $(PROTO_BINARIES) $(TYPES)
-	trap 'rm -Rf vendor' EXIT
 	[ -e vendor ] || go mod vendor
 	${GOPATH}/bin/go-to-protobuf \
 		--go-header-file=./hack/custom-boilerplate.go.txt \
@@ -435,11 +431,11 @@ endif
 	./hack/port-forward.sh
 	# Check dex, minio, postgres and mysql are in hosts file
 ifeq ($(AUTH_MODE),sso)
-	grep '127.0.0.1 *dex' /etc/hosts
+	grep '127.0.0.1[[:blank:]]*dex' /etc/hosts
 endif
-	grep '127.0.0.1 *minio' /etc/hosts
-	grep '127.0.0.1 *postgres' /etc/hosts
-	grep '127.0.0.1 *mysql' /etc/hosts
+	grep '127.0.0.1[[:blank:]]*minio' /etc/hosts
+	grep '127.0.0.1[[:blank:]]*postgres' /etc/hosts
+	grep '127.0.0.1[[:blank:]]*mysql' /etc/hosts
 ifeq ($(RUN_MODE),local)
 	env SECURE=$(SECURE) ALWAYS_OFFLOAD_NODE_STATUS=$(ALWAYS_OFFLOAD_NODE_STATUS) LOG_LEVEL=$(LOG_LEVEL) UPPERIO_DB_DEBUG=$(UPPERIO_DB_DEBUG) VERSION=$(VERSION) AUTH_MODE=$(AUTH_MODE) NAMESPACED=$(NAMESPACED) NAMESPACE=$(KUBE_NAMESPACE) $(GOPATH)/bin/goreman -set-ports=false -logtime=false start
 endif
diff --git a/USERS.md b/USERS.md
index bb66900f5..f9437b65e 100644
--- a/USERS.md
+++ b/USERS.md
@@ -93,6 +93,7 @@ Currently, the following organizations are **officially** using Argo Workflows:
 1. [Threekit](https://www.threekit.com/)
 1. [Tiger Analytics](https://www.tigeranalytics.com/)
 1. [Tulip](https://tulip.com/)
+1. [UFirstGroup](https://www.ufirstgroup.com)
 1. [Wavefront](https://www.wavefront.com/)
 1. [Wellcome Trust](https://wellcome.ac.uk/)
 1. [Helio](https://helio.exchange)
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
index 715f3aa53..633d7d0be 100644
--- a/api/openapi-spec/swagger.json
+++ b/api/openapi-spec/swagger.json
@@ -2352,7 +2352,7 @@
           "type": "string"
         },
         "failedJobsHistoryLimit": {
-          "description": "FailedJobsHistoryLimit is the number of successful jobs to be kept at a time",
+          "description": "FailedJobsHistoryLimit is the number of failed jobs to be kept at a time",
           "type": "integer",
           "format": "int32"
         },
diff --git a/docs/fields.md b/docs/fields.md
index a610e9a8b..8f1a32b82 100644
--- a/docs/fields.md
+++ b/docs/fields.md
@@ -968,7 +968,7 @@ CronWorkflowSpec is the specification of a CronWorkflow
 | Field Name | Field Type | Description   |
 |:----------:|:----------:|---------------|
 |`concurrencyPolicy`|`string`|ConcurrencyPolicy is the K8s-style concurrency policy that will be used|
-|`failedJobsHistoryLimit`|`int32`|FailedJobsHistoryLimit is the number of successful jobs to be kept at a time|
+|`failedJobsHistoryLimit`|`int32`|FailedJobsHistoryLimit is the number of failed jobs to be kept at a time|
 |`schedule`|`string`|Schedule is a schedule to run the Workflow in Cron format|
 |`startingDeadlineSeconds`|`int64`|StartingDeadlineSeconds is the K8s-style deadline that will limit the time a CronWorkflow will be run after its original scheduled time if it is missed.|
 |`successfulJobsHistoryLimit`|`int32`|SuccessfulJobsHistoryLimit is the number of successful jobs to be kept at a time|
diff --git a/manifests/base/argo-server/argo-server-deployment.yaml b/manifests/base/argo-server/argo-server-deployment.yaml
index e4e7c24f2..cdbfe42d9 100644
--- a/manifests/base/argo-server/argo-server-deployment.yaml
+++ b/manifests/base/argo-server/argo-server-deployment.yaml
@@ -26,6 +26,12 @@ spec:
               path: /
             initialDelaySeconds: 10
             periodSeconds: 20
+          volumeMounts:
+            - mountPath: /tmp
+              name: tmp
+      volumes:
+        - name: tmp
+          emptyDir: { }
       securityContext:
         runAsNonRoot: true
       nodeSelector:
diff --git a/manifests/install.yaml b/manifests/install.yaml
index 3a11b0dd9..096c32205 100644
--- a/manifests/install.yaml
+++ b/manifests/install.yaml
@@ -470,11 +470,17 @@ spec:
             scheme: HTTP
           initialDelaySeconds: 10
           periodSeconds: 20
+        volumeMounts:
+        - mountPath: /tmp
+          name: tmp
       nodeSelector:
         kubernetes.io/os: linux
       securityContext:
         runAsNonRoot: true
       serviceAccountName: argo-server
+      volumes:
+      - emptyDir: {}
+        name: tmp
 ---
 apiVersion: apps/v1
 kind: Deployment
diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml
index 60fe78dfa..387c6af73 100644
--- a/manifests/namespace-install.yaml
+++ b/manifests/namespace-install.yaml
@@ -364,11 +364,17 @@ spec:
             scheme: HTTP
           initialDelaySeconds: 10
           periodSeconds: 20
+        volumeMounts:
+        - mountPath: /tmp
+          name: tmp
       nodeSelector:
         kubernetes.io/os: linux
       securityContext:
         runAsNonRoot: true
       serviceAccountName: argo-server
+      volumes:
+      - emptyDir: {}
+        name: tmp
 ---
 apiVersion: apps/v1
 kind: Deployment
diff --git a/manifests/quick-start-minimal.yaml b/manifests/quick-start-minimal.yaml
index 59583fcf6..a0cf84702 100644
--- a/manifests/quick-start-minimal.yaml
+++ b/manifests/quick-start-minimal.yaml
@@ -602,11 +602,17 @@ spec:
             scheme: HTTP
           initialDelaySeconds: 10
           periodSeconds: 20
+        volumeMounts:
+        - mountPath: /tmp
+          name: tmp
       nodeSelector:
         kubernetes.io/os: linux
       securityContext:
         runAsNonRoot: true
       serviceAccountName: argo-server
+      volumes:
+      - emptyDir: {}
+        name: tmp
 ---
 apiVersion: apps/v1
 kind: Deployment
diff --git a/manifests/quick-start-mysql.yaml b/manifests/quick-start-mysql.yaml
index 2b0e76d7b..4a1472bda 100644
--- a/manifests/quick-start-mysql.yaml
+++ b/manifests/quick-start-mysql.yaml
@@ -646,11 +646,17 @@ spec:
             scheme: HTTP
           initialDelaySeconds: 10
           periodSeconds: 20
+        volumeMounts:
+        - mountPath: /tmp
+          name: tmp
       nodeSelector:
         kubernetes.io/os: linux
       securityContext:
         runAsNonRoot: true
       serviceAccountName: argo-server
+      volumes:
+      - emptyDir: {}
+        name: tmp
 ---
 apiVersion: apps/v1
 kind: Deployment
diff --git a/manifests/quick-start-postgres.yaml b/manifests/quick-start-postgres.yaml
index 89da612bd..07d208ff0 100644
--- a/manifests/quick-start-postgres.yaml
+++ b/manifests/quick-start-postgres.yaml
@@ -646,11 +646,17 @@ spec:
             scheme: HTTP
           initialDelaySeconds: 10
           periodSeconds: 20
+        volumeMounts:
+        - mountPath: /tmp
+          name: tmp
       nodeSelector:
         kubernetes.io/os: linux
       securityContext:
         runAsNonRoot: true
       serviceAccountName: argo-server
+      volumes:
+      - emptyDir: {}
+        name: tmp
 ---
 apiVersion: apps/v1
 kind: Deployment
diff --git a/pkg/apis/workflow/v1alpha1/cron_workflow_types.go b/pkg/apis/workflow/v1alpha1/cron_workflow_types.go
index 77a7e23a4..38121822b 100644
--- a/pkg/apis/workflow/v1alpha1/cron_workflow_types.go
+++ b/pkg/apis/workflow/v1alpha1/cron_workflow_types.go
@@ -63,7 +63,7 @@ type CronWorkflowSpec struct {
 	StartingDeadlineSeconds *int64 `json:"startingDeadlineSeconds,omitempty" protobuf:"varint,5,opt,name=startingDeadlineSeconds"`
 	// SuccessfulJobsHistoryLimit is the number of successful jobs to be kept at a time
 	SuccessfulJobsHistoryLimit *int32 `json:"successfulJobsHistoryLimit,omitempty" protobuf:"varint,6,opt,name=successfulJobsHistoryLimit"`
-	// FailedJobsHistoryLimit is the number of successful jobs to be kept at a time
+	// FailedJobsHistoryLimit is the number of failed jobs to be kept at a time
 	FailedJobsHistoryLimit *int32 `json:"failedJobsHistoryLimit,omitempty" protobuf:"varint,7,opt,name=failedJobsHistoryLimit"`
 	// Timezone is the timezone against which the cron schedule will be calculated, e.g. "Asia/Tokyo". Default is machine's local time.
 	Timezone string `json:"timezone,omitempty" protobuf:"bytes,8,opt,name=timezone"`
diff --git a/pkg/apis/workflow/v1alpha1/generated.proto b/pkg/apis/workflow/v1alpha1/generated.proto
index 8a4f76c9d..105ae6e63 100644
--- a/pkg/apis/workflow/v1alpha1/generated.proto
+++ b/pkg/apis/workflow/v1alpha1/generated.proto
@@ -241,7 +241,7 @@ message CronWorkflowSpec {
   // SuccessfulJobsHistoryLimit is the number of successful jobs to be kept at a time
   optional int32 successfulJobsHistoryLimit = 6;
 
-  // FailedJobsHistoryLimit is the number of successful jobs to be kept at a time
+  // FailedJobsHistoryLimit is the number of failed jobs to be kept at a time
   optional int32 failedJobsHistoryLimit = 7;
 
   // Timezone is the timezone against which the cron schedule will be calculated, e.g. "Asia/Tokyo". Default is machine's local time.
diff --git a/pkg/apis/workflow/v1alpha1/openapi_generated.go b/pkg/apis/workflow/v1alpha1/openapi_generated.go
index 316e95df7..6b0bf39c1 100644
--- a/pkg/apis/workflow/v1alpha1/openapi_generated.go
+++ b/pkg/apis/workflow/v1alpha1/openapi_generated.go
@@ -856,7 +856,7 @@ func schema_pkg_apis_workflow_v1alpha1_CronWorkflowSpec(ref common.ReferenceCall
 					},
 					"failedJobsHistoryLimit": {
 						SchemaProps: spec.SchemaProps{
-							Description: "FailedJobsHistoryLimit is the number of successful jobs to be kept at a time",
+							Description: "FailedJobsHistoryLimit is the number of failed jobs to be kept at a time",
 							Type:        []string{"integer"},
 							Format:      "int32",
 						},
diff --git a/server/artifacts/artifact_server.go b/server/artifacts/artifact_server.go
index a60d87392..98458dacb 100644
--- a/server/artifacts/artifact_server.go
+++ b/server/artifacts/artifact_server.go
@@ -137,8 +137,7 @@ func (a *ArtifactServer) getArtifact(ctx context.Context, wf *wfv1.Workflow, nod
 	if err != nil {
 		return nil, err
 	}
-
-	tmp, err := ioutil.TempFile(".", "artifact")
+	tmp, err := ioutil.TempFile("/tmp", "artifact")
 	if err != nil {
 		return nil, err
 	}
@@ -154,7 +153,6 @@ func (a *ArtifactServer) getArtifact(ctx context.Context, wf *wfv1.Workflow, nod
 	if err != nil {
 		return nil, err
 	}
-
 	log.WithFields(log.Fields{"size": len(file)}).Debug("Artifact file size")
 
 	return file, nil
diff --git a/workflow/common/util.go b/workflow/common/util.go
index ffca81d8a..389f00c75 100644
--- a/workflow/common/util.go
+++ b/workflow/common/util.go
@@ -20,6 +20,7 @@ import (
 	apierr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/remotecommand"
@@ -434,20 +435,39 @@ func RunShellCommand(arg ...string) ([]byte, error) {
 	return RunCommand(name, arg...)
 }
 
-const patchRetries = 5
+// Run	Seconds
+// 0	0.000
+// 1	1.000
+// 2	2.000
+// 3	3.000
+// 4	4.000
+var defaultPatchBackoff = wait.Backoff{
+	Steps:    5,
+	Duration: 1 * time.Second,
+	Factor:   1,
+}
 
 // AddPodAnnotation adds an annotation to pod
-func AddPodAnnotation(c kubernetes.Interface, podName, namespace, key, value string) error {
-	return addPodMetadata(c, "annotations", podName, namespace, key, value)
+func AddPodAnnotation(c kubernetes.Interface, podName, namespace, key, value string, options ...interface{}) error {
+	backoff := defaultPatchBackoff
+	for _, option := range options {
+		switch v := option.(type) {
+		case wait.Backoff:
+			backoff = v
+		default:
+			panic("unknown option type")
+		}
+	}
+	return addPodMetadata(c, "annotations", podName, namespace, key, value, backoff)
 }
 
 // AddPodLabel adds an label to pod
 func AddPodLabel(c kubernetes.Interface, podName, namespace, key, value string) error {
-	return addPodMetadata(c, "labels", podName, namespace, key, value)
+	return addPodMetadata(c, "labels", podName, namespace, key, value, defaultPatchBackoff)
 }
 
 // addPodMetadata is helper to either add a pod label or annotation to the pod
-func addPodMetadata(c kubernetes.Interface, field, podName, namespace, key, value string) error {
+func addPodMetadata(c kubernetes.Interface, field, podName, namespace, key, value string, backoff wait.Backoff) error {
 	metadata := map[string]interface{}{
 		"metadata": map[string]interface{}{
 			field: map[string]string{
@@ -455,23 +475,14 @@ func addPodMetadata(c kubernetes.Interface, field, podName, namespace, key, valu
 			},
 		},
 	}
-	var err error
 	patch, err := json.Marshal(metadata)
 	if err != nil {
 		return errors.InternalWrapError(err)
 	}
-	for attempt := 0; attempt < patchRetries; attempt++ {
+	return wait.ExponentialBackoff(backoff, func() (bool, error) {
 		_, err = c.CoreV1().Pods(namespace).Patch(podName, types.MergePatchType, patch)
-		if err != nil {
-			if !apierr.IsConflict(err) {
-				return err
-			}
-		} else {
-			break
-		}
-		time.Sleep(100 * time.Millisecond)
-	}
-	return err
+		return err == nil, err
+	})
 }
 
 const deleteRetries = 3
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index 140385a68..55b3e822d 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -741,7 +741,7 @@ func (woc *wfOperationCtx) processNodeRetries(node *wfv1.NodeStatus, retryStrate
 		if err != nil {
 			return nil, false, err
 		}
-		if *retryStrategyBackoffFactor > 0 {
+		if retryStrategyBackoffFactor != nil && *retryStrategyBackoffFactor > 0 {
 			// Formula: timeToWait = duration * factor^retry_number
 			// Note that timeToWait should equal to duration for the first retry attempt.
 			timeToWait = baseDuration * time.Duration(math.Pow(float64(*retryStrategyBackoffFactor), float64(len(node.Children)-1)))
@@ -797,7 +797,7 @@ func (woc *wfOperationCtx) processNodeRetries(node *wfv1.NodeStatus, retryStrate
 	if err != nil {
 		return nil, false, err
 	}
-	if retryStrategy.Limit != nil && int32(len(node.Children)) > *limit {
+	if retryStrategy.Limit != nil && limit != nil && int32(len(node.Children)) > *limit {
 		woc.log.Infoln("No more retries left. Failing...")
 		return woc.markNodePhase(node.Name, lastChildNode.Phase, "No more retries left"), true, nil
 	}
diff --git a/workflow/cron/controller.go b/workflow/cron/controller.go
index fa14c2429..cc65b4f60 100644
--- a/workflow/cron/controller.go
+++ b/workflow/cron/controller.go
@@ -4,10 +4,8 @@ import (
 	"context"
 	"fmt"
 	"reflect"
-	"sync"
 	"time"
 
-	"github.com/robfig/cron/v3"
 	log "github.com/sirupsen/logrus"
 	apiv1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -39,8 +37,7 @@ type Controller struct {
 	namespace            string
 	managedNamespace     string
 	instanceId           string
-	cron                 *cron.Cron
-	nameEntryIDMap       sync.Map
+	cron                 *cronFacade
 	wfClientset          versioned.Interface
 	wfInformer           cache.SharedIndexInformer
 	wfLister             util.WorkflowLister
@@ -65,10 +62,9 @@ func NewCronController(wfclientset versioned.Interface, restConfig *rest.Config,
 		namespace:            namespace,
 		managedNamespace:     managedNamespace,
 		instanceId:           instanceId,
-		cron:                 cron.New(),
+		cron:                 newCronFacade(),
 		restConfig:           restConfig,
 		dynamicInterface:     dynamicInterface,
-		nameEntryIDMap:       sync.Map{},
 		wfQueue:              workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "wf_cron_queue"),
 		cronWfQueue:          workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "cron_wf_queue"),
 		metrics:              metrics,
@@ -133,11 +129,8 @@ func (cc *Controller) processNextCronItem() bool {
 		return true
 	}
 	if !exists {
-		if entryId, ok := cc.nameEntryIDMap.Load(key.(string)); ok {
-			logCtx.Infof("Deleting '%s'", key)
-			cc.cron.Remove(entryId.(cron.EntryID))
-			cc.nameEntryIDMap.Delete(key.(string))
-		}
+		logCtx.Infof("Deleting '%s'", key)
+		cc.cron.Delete(key.(string))
 		return true
 	}
 
@@ -169,22 +162,18 @@ func (cc *Controller) processNextCronItem() bool {
 	}
 
 	// The job is currently scheduled, remove it and re add it.
-	if entryId, ok := cc.nameEntryIDMap.Load(key.(string)); ok {
-		cc.cron.Remove(entryId.(cron.EntryID))
-		cc.nameEntryIDMap.Delete(key.(string))
-	}
+	cc.cron.Delete(key.(string))
 
 	cronSchedule := cronWf.Spec.Schedule
 	if cronWf.Spec.Timezone != "" {
 		cronSchedule = "CRON_TZ=" + cronWf.Spec.Timezone + " " + cronSchedule
 	}
 
-	entryId, err := cc.cron.AddJob(cronSchedule, cronWorkflowOperationCtx)
+	err = cc.cron.AddJob(key.(string), cronSchedule, cronWorkflowOperationCtx)
 	if err != nil {
 		logCtx.WithError(err).Error("could not schedule CronWorkflow")
 		return true
 	}
-	cc.nameEntryIDMap.Store(key.(string), entryId)
 
 	logCtx.Infof("CronWorkflow %s added", key.(string))
 
@@ -237,15 +226,9 @@ func (cc *Controller) processNextWorkflowItem() bool {
 
 	// Workflows are run in the same namespace as CronWorkflow
 	nameEntryIdMapKey := wf.Namespace + "/" + wf.OwnerReferences[0].Name
-	var woc *cronWfOperationCtx
-	if entryId, ok := cc.nameEntryIDMap.Load(nameEntryIdMapKey); ok {
-		woc, ok = cc.cron.Entry(entryId.(cron.EntryID)).Job.(*cronWfOperationCtx)
-		if !ok {
-			log.Errorf("Parent CronWorkflow '%s' is malformed", nameEntryIdMapKey)
-			return true
-		}
-	} else {
-		log.Warnf("Parent CronWorkflow '%s' no longer exists", nameEntryIdMapKey)
+	woc, err := cc.cron.Load(nameEntryIdMapKey)
+	if err != nil {
+		log.Warnf("Parent CronWorkflow '%s' is bad: %v", nameEntryIdMapKey, err)
 		return true
 	}
 
diff --git a/workflow/cron/cron_facade.go b/workflow/cron/cron_facade.go
new file mode 100644
index 000000000..d070f34e1
--- /dev/null
+++ b/workflow/cron/cron_facade.go
@@ -0,0 +1,69 @@
+package cron
+
+import (
+	"fmt"
+	"reflect"
+	"sync"
+
+	"github.com/robfig/cron/v3"
+)
+
+// cronFacade allows the client to operate using key rather than cron.EntryID,
+// as well as providing sync guarantees
+type cronFacade struct {
+	mu       sync.Mutex
+	cron     *cron.Cron
+	entryIDs map[string]cron.EntryID
+}
+
+func newCronFacade() *cronFacade {
+	return &cronFacade{
+		cron:     cron.New(),
+		entryIDs: make(map[string]cron.EntryID),
+	}
+}
+
+func (f *cronFacade) Start() {
+	f.cron.Start()
+}
+
+func (f *cronFacade) Stop() {
+	f.cron.Stop()
+}
+
+func (f *cronFacade) Delete(key string) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	entryID, ok := f.entryIDs[key]
+	if !ok {
+		return
+	}
+	f.cron.Remove(entryID)
+	delete(f.entryIDs, key)
+}
+
+func (f *cronFacade) AddJob(key, schedule string, cwoc *cronWfOperationCtx) error {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	entryID, err := f.cron.AddJob(schedule, cwoc)
+	if err != nil {
+		return err
+	}
+	f.entryIDs[key] = entryID
+	return nil
+}
+
+func (f *cronFacade) Load(key string) (*cronWfOperationCtx, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	entryID, ok := f.entryIDs[key]
+	if !ok {
+		return nil, fmt.Errorf("entry ID for %s not found", key)
+	}
+	entry := f.cron.Entry(entryID).Job
+	cwoc, ok := entry.(*cronWfOperationCtx)
+	if !ok {
+		return nil, fmt.Errorf("job entry ID for %s was not a *cronWfOperationCtx, was %v", key, reflect.TypeOf(entry))
+	}
+	return cwoc, nil
+}
diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go
index b1c6c1622..f4e06897e 100644
--- a/workflow/executor/executor.go
+++ b/workflow/executor/executor.go
@@ -40,6 +40,12 @@ import (
 )
 
 // ExecutorRetry is a retry backoff settings for WorkflowExecutor
+// Run	Seconds
+// 0	0.000
+// 1	1.000
+// 2	2.600
+// 3	5.160
+// 4	9.256
 var ExecutorRetry = wait.Backoff{
 	Steps:    5,
 	Duration: 1 * time.Second,
@@ -829,7 +835,7 @@ func (we *WorkflowExecutor) AddError(err error) {
 
 // AddAnnotation adds an annotation to the workflow pod
 func (we *WorkflowExecutor) AddAnnotation(key, value string) error {
-	return common.AddPodAnnotation(we.ClientSet, we.PodName, we.Namespace, key, value)
+	return common.AddPodAnnotation(we.ClientSet, we.PodName, we.Namespace, key, value, ExecutorRetry)
 }
 
 // isTarball returns whether or not the file is a tarball
diff --git a/workflow/sync/semaphore.go b/workflow/sync/semaphore.go
index 277b1636b..a9834dda7 100644
--- a/workflow/sync/semaphore.go
+++ b/workflow/sync/semaphore.go
@@ -152,6 +152,10 @@ func (s *PrioritySemaphore) tryAcquire(holderKey string) (bool, string) {
 		item := s.pending.peek()
 		nextKey = fmt.Sprintf("%v", item.key)
 		if holderKey != nextKey {
+			// Enqueue the front workflow if lock is available
+			if len(s.lockHolder) < s.limit {
+				s.nextWorkflow(nextKey)
+			}
 			return false, waitingMsg
 		}
 	}
diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go
index 153bd0427..722d9f9a0 100644
--- a/workflow/validate/validate.go
+++ b/workflow/validate/validate.go
@@ -708,7 +708,7 @@ func (ctx *templateValidationCtx) validateLeaf(scope map[string]interface{}, tmp
 		if !intstr.IsValidIntOrArgoVariable(tmpl.ActiveDeadlineSeconds) {
 			return errors.Errorf(errors.CodeBadRequest, "templates.%s.activeDeadlineSeconds must be a positive integer > 0 or an argo variable", tmpl.Name)
 		}
-		if i, err := intstr.Int(tmpl.ActiveDeadlineSeconds); err == nil && *i < 0 {
+		if i, err := intstr.Int(tmpl.ActiveDeadlineSeconds); err == nil && i != nil && *i < 0 {
 			return errors.Errorf(errors.CodeBadRequest, "templates.%s.activeDeadlineSeconds must be a positive integer > 0 or an argo variable", tmpl.Name)
 		}
 	}
@@ -1190,6 +1190,7 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl
 		if task.Depends != "" {
 			usingDepends = true
 		}
+
 		nameToTask[task.Name] = task
 	}
 
@@ -1203,6 +1204,10 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl
 	// Verify dependencies for all tasks can be resolved as well as template names
 	for _, task := range tmpl.DAG.Tasks {
 
+		if usingDepends && '0' <= task.Name[0] && task.Name[0] <= '9' {
+			return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s name cannot begin with a digit when using 'depends'", tmpl.Name, task.Name)
+		}
+
 		if usingDepends && len(task.Dependencies) > 0 {
 			return errors.Errorf(errors.CodeBadRequest, "templates.%s cannot use both 'depends' and 'dependencies' in the same DAG template", tmpl.Name)
 		}


Test patch:
diff --git a/workflow/validate/validate_dag_test.go b/workflow/validate/validate_dag_test.go
index 2dbe9915c..334630e29 100644
--- a/workflow/validate/validate_dag_test.go
+++ b/workflow/validate/validate_dag_test.go
@@ -689,3 +689,57 @@ func TestDependsAndContinueOn(t *testing.T) {
 	_, err := validate(dagDependsAndContinueOn)
 	assert.Error(t, err, "templates.dag-target cannot use 'continueOn' when using 'depends'. Instead use 'dep-task.Failed'/'dep-task.Errored'")
 }
+
+var dagDependsDigit = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: dag-diamond-
+spec:
+  entrypoint: diamond
+  templates:
+    - name: diamond
+      dag:
+        tasks:
+          - name: 5A
+            template: pass
+          - name: B
+            depends: 5A
+            template: pass
+          - name: C
+            depends: 5A
+            template: fail
+          - name: should-execute-1
+            depends: "'5A' && (C.Succeeded || C.Failed)"   # For more information about this depends field, see: docs/enhanced-depends-logic.md
+            template: pass
+          - name: should-execute-2
+            depends: B || C
+            template: pass
+          - name: should-not-execute
+            depends: B && C
+            template: pass
+          - name: should-execute-3
+            depends: should-execute-2.Succeeded || should-not-execute
+            template: pass
+    - name: pass
+      container:
+        image: alpine:3.7
+        command:
+          - sh
+          - -c
+          - exit 0
+    - name: fail
+      container:
+        image: alpine:3.7
+        command:
+          - sh
+          - -c
+          - exit 1
+`
+
+func TestDAGDependsDigit(t *testing.T) {
+	_, err := validate(dagDependsDigit)
+	if assert.NotNil(t, err) {
+		assert.Contains(t, err.Error(), "templates.diamond.tasks.5A name cannot begin with a digit when using 'depends'")
+	}
+}