From f344802c8ec0d1e56151d69a25fa16da35788800 Mon Sep 17 00:00:00 2001
From: Peter Amstutz <peter.amstutz@curii.com>
Date: Mon, 29 Jun 2020 14:55:55 -0400
Subject: [PATCH] Restore check for 'id' field for CWL v1.0 only

On further research, I realized why the check for 'id' on embedded
process objects was in place (to catch potential id conflicts earlier
instead of later), however the underlying problem is resolved in cwl
1.1+

refs #16382

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>
---
 sdk/cwl/arvados_cwl/runner.py              |  2 ++
 sdk/cwl/tests/arvados-tests.yml            |  9 +++++++-
 sdk/cwl/tests/wf-defaults/default-dir4.cwl |  2 +-
 sdk/cwl/tests/wf-defaults/default-dir8.cwl | 24 ++++++++++++++++++++++
 sdk/cwl/tests/wf-defaults/wf4.cwl          |  2 +-
 sdk/cwl/tests/wf-defaults/wf8.cwl          | 17 +++++++++++++++
 6 files changed, 53 insertions(+), 3 deletions(-)
 create mode 100644 sdk/cwl/tests/wf-defaults/default-dir8.cwl
 create mode 100644 sdk/cwl/tests/wf-defaults/wf8.cwl

diff --git a/sdk/cwl/arvados_cwl/runner.py b/sdk/cwl/arvados_cwl/runner.py
index a47b34acc0..71e499ebca 100644
--- a/sdk/cwl/arvados_cwl/runner.py
+++ b/sdk/cwl/arvados_cwl/runner.py
@@ -460,6 +460,8 @@ def packed_workflow(arvrunner, tool, merged_map):
     def visit(v, cur_id):
         if isinstance(v, dict):
             if v.get("class") in ("CommandLineTool", "Workflow"):
+                if tool.metadata["cwlVersion"] == "v1.0" and "id" not in v:
+                    raise SourceLine(v, None, Exception).makeError("Embedded process object is missing required 'id' field, add an 'id' or use to cwlVersion: v1.1")
                 if "id" in v:
                     cur_id = rewrite_to_orig.get(v["id"], v["id"])
             if "path" in v and "location" not in v:
diff --git a/sdk/cwl/tests/arvados-tests.yml b/sdk/cwl/tests/arvados-tests.yml
index 786dc643ca..c4c0968756 100644
--- a/sdk/cwl/tests/arvados-tests.yml
+++ b/sdk/cwl/tests/arvados-tests.yml
@@ -218,7 +218,14 @@
   output:
     out: null
   tool: wf-defaults/wf4.cwl
-  doc: default in embedded subworkflow missing 'id' field
+  doc: default in embedded subworkflow missing 'id' field, v1.0
+  should_fail: true
+
+- job: null
+  output:
+    out: null
+  tool: wf-defaults/wf8.cwl
+  doc: default in embedded subworkflow missing 'id' field, v1.1
   should_fail: false
 
 - job: null
diff --git a/sdk/cwl/tests/wf-defaults/default-dir4.cwl b/sdk/cwl/tests/wf-defaults/default-dir4.cwl
index a5b9c2f3ec..bd92782488 100644
--- a/sdk/cwl/tests/wf-defaults/default-dir4.cwl
+++ b/sdk/cwl/tests/wf-defaults/default-dir4.cwl
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
-cwlVersion: v1.1
+cwlVersion: v1.0
 class: Workflow
 inputs: []
 outputs: []
diff --git a/sdk/cwl/tests/wf-defaults/default-dir8.cwl b/sdk/cwl/tests/wf-defaults/default-dir8.cwl
new file mode 100644
index 0000000000..a5b9c2f3ec
--- /dev/null
+++ b/sdk/cwl/tests/wf-defaults/default-dir8.cwl
@@ -0,0 +1,24 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+cwlVersion: v1.1
+class: Workflow
+inputs: []
+outputs: []
+$namespaces:
+  arv: "http://arvados.org/cwl#"
+steps:
+  step1:
+    in: []
+    out: []
+    run:
+      class: CommandLineTool
+      inputs:
+        inp2:
+          type: Directory
+          default:
+            class: Directory
+            location: inp1
+      outputs: []
+      arguments: [echo, $(inputs.inp2)]
diff --git a/sdk/cwl/tests/wf-defaults/wf4.cwl b/sdk/cwl/tests/wf-defaults/wf4.cwl
index 27abea290c..3f498fdffb 100644
--- a/sdk/cwl/tests/wf-defaults/wf4.cwl
+++ b/sdk/cwl/tests/wf-defaults/wf4.cwl
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
-cwlVersion: v1.1
+cwlVersion: v1.0
 class: Workflow
 inputs: []
 outputs: []
diff --git a/sdk/cwl/tests/wf-defaults/wf8.cwl b/sdk/cwl/tests/wf-defaults/wf8.cwl
new file mode 100644
index 0000000000..2548faec1c
--- /dev/null
+++ b/sdk/cwl/tests/wf-defaults/wf8.cwl
@@ -0,0 +1,17 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+cwlVersion: v1.1
+class: Workflow
+inputs: []
+outputs: []
+$namespaces:
+  arv: "http://arvados.org/cwl#"
+requirements:
+  SubworkflowFeatureRequirement: {}
+steps:
+  step1:
+    in: []
+    out: []
+    run: default-dir8.cwl
-- 
2.30.2