Fix submit-git to respond with 422 (client) error if fail to git-clone control URL

5 files changed, 119 insertions, 39 deletions

diff --git a/brep/handler/submit/submit-git.bash.in b/brep/handler/submit/submit-git.bash.in
index 2fd26b0..c77f18d 100644
--- a/brep/handler/submit/submit-git.bash.in
+++ b/brep/handler/submit/submit-git.bash.in
@@ -299,15 +299,20 @@ function url_scheme () # <url>
   sed -n -e 's%^\(.*\)://.*$%\L\1%p' <<<"$1"
 }
 
-# Checks that the repository properly responds to the probing request before
+# Check that the repository properly responds to the probing request before
 # the timeout (in seconds). Noop for protocols other than HTTP(S).
 #
-function check_connectivity () # <repo-url> <timeout>
+# If the repository is other than ours (e.g., control) then don't log a
+# failure and respond with the 422 (client) rather than 503 (server) HTTP
+# error.
+#
+function check_connectivity () # <repo-url> <timeout> <ours>
 {
   trace_func "$@"
 
   local url="$1"
   local tmo="$2"
+  local our="$3"
 
   local s
   s="$(url_scheme "$url")"
@@ -324,11 +329,14 @@ function check_connectivity () # <repo-url> <timeout>
       u="$u/info/refs$q&service=git-upload-pack"
     fi
 
-    # This function is called on repositories other than ours (e.g., control)
-    # so we don't want a failure to be logged.
-    #
-    if ! run_silent curl -S -s --max-time "$tmo" "$u" >/dev/null; then
-      exit_with_manifest 503 "submission service temporarily unavailable"
+    local cmd=(curl -S -s --max-time "$tmo" "$u")
+
+    if [ "$our" ]; then
+      if ! run "${cmd[@]}" >/dev/null; then
+        exit_with_manifest 503 "submission service temporarily unavailable"
+      fi
+    elif ! run_silent "${cmd[@]}" >/dev/null; then
+      exit_with_manifest 422 "repository $url unavailable"
     fi
   fi
 }
diff --git a/brep/handler/submit/submit-git.in b/brep/handler/submit/submit-git.in
index 461aab4..a403377 100644
--- a/brep/handler/submit/submit-git.in
+++ b/brep/handler/submit/submit-git.in
@@ -387,18 +387,6 @@ function git_add () # <repo-dir> <path>...
   run git -C "$d" add $gvo "$@" >&2
 }
 
-function git_clone () # <repo-url> <repo-dir> <extra-options>...
-{
-  local url="$1"
-  shift
-
-  local dir="$1"
-  shift
-
-  check_connectivity "$url" "$git_timeout"
-  run git "${git_http_timeout[@]}" clone $gqo $gvo "$@" "$url" "$dir" >&2
-}
-
 # Dor now we make 10 re-tries to add the package and push to target. Push can
 # fail due to the target-to-reference information move race (see the above
 # notes for details) or because concurrent submissions. We may want to make it
@@ -411,7 +399,10 @@ for i in {1..11}; do
   # Clone the target repository.
   #
   tgt_dir="$data_dir/target"
-  git_clone "$tgt_repo" "$tgt_dir" --single-branch --depth 1
+  check_connectivity "$tgt_repo" "$git_timeout" true
+
+  run git "${git_http_timeout[@]}" clone $gqo $gvo --single-branch --depth 1 \
+"$tgt_repo" "$tgt_dir" >&2
 
   check_package_duplicate "$name" "$version" "$tgt_dir"
 
@@ -435,7 +426,7 @@ for i in {1..11}; do
 
     # Pull the reference repository.
     #
-    check_connectivity "$remote_url" "$git_timeout"
+    check_connectivity "$remote_url" "$git_timeout" true
     run git "${git_http_timeout[@]}" -C "$ref_repo" pull $gqo $gvo >&2
 
     # Check the package duplicate.
@@ -600,8 +591,12 @@ for i in {1..11}; do
     #
     ctl_dir="$data_dir/control"
 
-    git_clone "$control" "$ctl_dir" --single-branch --depth 1 \
- --branch "build2-control"
+    check_connectivity "$control" "$git_timeout" ""
+
+    if ! run_silent git "${git_http_timeout[@]}" clone $gqo $gvo --depth 1 \
+--single-branch --branch "build2-control" "$control" "$ctl_dir" >&2; then
+      exit_with_manifest 422 "failed to git-clone $control"
+    fi
 
     if [ ! -f "$ctl_dir/submit/${sha256sum:0:16}" ]; then
       exit_with_manifest 401 "package publishing authorization failed"
@@ -638,7 +633,7 @@ Add $name/$version to $s/$project
 $(cat "$data_dir/request.manifest")
 EOF
 
-  check_connectivity "$tgt_repo" "$git_timeout"
+  check_connectivity "$tgt_repo" "$git_timeout" true
 
   # Try to push the target modifications. If this succeeds then we are done.
   # Otherwise, drop the target directory and re-try the whole
diff --git a/tests/ci/ci-dir.test b/tests/ci/ci-dir.test
index 5b9e8c8..0e98f3a 100644
--- a/tests/ci/ci-dir.test
+++ b/tests/ci/ci-dir.test
@@ -29,7 +29,7 @@
   {
     $clone_root_data;
 
-    echo "simulate: success" >+$data_dir/request.manifest;
+    echo 'simulate: success' >+$data_dir/request.manifest;
 
     $* >>"EOO";
       : 1
@@ -65,7 +65,7 @@
   {
     $clone_root_data_clean;
 
-    echo "simulate: fly" >+$data_dir/request.manifest;
+    echo 'simulate: fly' >+$data_dir/request.manifest;
 
     $* >>"EOO"
       : 1
diff --git a/tests/submit/submit-dir.test b/tests/submit/submit-dir.test
index 055449a..a6a7e3e 100644
--- a/tests/submit/submit-dir.test
+++ b/tests/submit/submit-dir.test
@@ -29,7 +29,7 @@
   {
     $clone_root_data;
 
-    echo "simulate: success" >+$data_dir/request.manifest;
+    echo 'simulate: success' >+$data_dir/request.manifest;
 
     $* >>"EOO";
       : 1
@@ -65,7 +65,7 @@
   {
     $clone_root_data_clean;
 
-    echo "junk" >=$data_dir/libhello-0.1.0.tar.gz;
+    echo 'junk' >=$data_dir/libhello-0.1.0.tar.gz;
 
     $* >>"EOO"
       : 1
@@ -80,7 +80,7 @@
   {
     $clone_root_data_clean;
 
-    echo "simulate: fly" >+$data_dir/request.manifest;
+    echo 'simulate: fly' >+$data_dir/request.manifest;
 
     $* >>"EOO"
       : 1
diff --git a/tests/submit/submit-git.test b/tests/submit/submit-git.test
index 5bfa4d4..f96f79d 100644
--- a/tests/submit/submit-git.test
+++ b/tests/submit/submit-git.test
@@ -151,7 +151,7 @@ pkg_ctl="$prj_ctl/hello.git"
       %.*
       EOO
 
-      git -C tgt log -1 "--pretty=format:%an %ae %cn %ce" >>:EOO
+      git -C tgt log -1 '--pretty=format:%an %ae %cn %ce' >>:EOO
         User Name user@example.org Commiter commiter@example.com
         EOO
   }
@@ -257,7 +257,7 @@ pkg_ctl="$prj_ctl/hello.git"
   :
   {
     $clone_root_data;
-    sed -i -e "s%^author-.+\$%%" $data_dir/request.manifest;
+    sed -i -e 's%^author-.+$%%' $data_dir/request.manifest;
 
     $clone_root_ref;
     $g clone ref.git &ref/***;
@@ -304,7 +304,7 @@ pkg_ctl="$prj_ctl/hello.git"
     #
     $g -C tgt pull;
 
-    git -C tgt log -1 "--pretty=format:%an %ae %cn %ce" >>:EOO
+    git -C tgt log -1 '--pretty=format:%an %ae %cn %ce' >>:EOO
       Submission Handler noreply@example.com Submission Handler noreply@example.com
       EOO
   }
@@ -424,7 +424,7 @@ pkg_ctl="$prj_ctl/hello.git"
   :
   {
     $clone_root_data;
-    sed -i -e "s%^\(section:\) .+\$%\\1 delta%" $data_dir/request.manifest;
+    sed -i -e 's%^(section:) .+$%\1 delta%' $data_dir/request.manifest;
 
     $clone_root_tgt;
     $g clone tgt.git &tgt/***;
@@ -438,7 +438,7 @@ pkg_ctl="$prj_ctl/hello.git"
       owners=owners
       EOI
 
-    $g -C tgt commit -am "Add section name fallback";
+    $g -C tgt commit -am 'Add section name fallback';
     $g -C tgt push;
 
     $* "file:///$~/tgt.git" $root_ref_dir $data_dir >>"EOO"
@@ -448,6 +448,24 @@ pkg_ctl="$prj_ctl/hello.git"
       reference: $checksum
       EOO
   }
+
+  : simulate
+  :
+  : Test that the simulated submission still succeeds given no control URL.
+  :
+  {
+    $clone_root_data;
+    sed -i -e 's%^control: .+$%simulate: success%' $data_dir/request.manifest;
+
+    $clone_root_tgt;
+
+    $* "file:///$~/tgt.git" $data_dir >>"EOO"
+      : 1
+      status: 200
+      message: libhello/0.1.0 submission is queued
+      reference: $checksum
+      EOO
+  }
 }
 
 : failure
@@ -457,7 +475,7 @@ pkg_ctl="$prj_ctl/hello.git"
   :
   {
     $clone_root_data_clean;
-    sed -i -e "s%^author-.+\$%%" $data_dir/request.manifest;
+    sed -i -e 's%^author-.+$%%' $data_dir/request.manifest;
 
     $clone_root_tgt;
     tgt_url = "file:///$~/tgt.git";
@@ -474,7 +492,7 @@ pkg_ctl="$prj_ctl/hello.git"
   :
   {
     $clone_root_data_clean;
-    sed -i -e "s%^author-email.+\$%%" $data_dir/request.manifest;
+    sed -i -e 's%^author-email.+$%%' $data_dir/request.manifest;
 
     $clone_root_tgt;
     tgt_url = "file:///$~/tgt.git";
@@ -491,7 +509,7 @@ pkg_ctl="$prj_ctl/hello.git"
   :
   {
     $clone_root_data_clean;
-    sed -i -e "s%^author-name.+\$%%" $data_dir/request.manifest;
+    sed -i -e 's%^author-name.+$%%' $data_dir/request.manifest;
 
     $clone_root_tgt;
     tgt_url = "file:///$~/tgt.git";
@@ -838,7 +856,7 @@ pkg_ctl="$prj_ctl/hello.git"
   {
     $clone_root_data_clean;
 
-    sed -i -e "s%^\(sha256sum:\) .+\$%\\1 59941e842667%" \
+    sed -i -e 's%^(sha256sum:) .+$%\1 59941e842667%' \
 	$data_dir/request.manifest;
 
     $clone_root_tgt;
@@ -856,7 +874,7 @@ pkg_ctl="$prj_ctl/hello.git"
   :
   {
     $clone_root_data_clean;
-    sed -i -e "s%^\(section:\) .+\$%\\1 delta%" $data_dir/request.manifest;
+    sed -i -e 's%^(section:) .+$%\1 delta%' $data_dir/request.manifest;
 
     $clone_root_tgt;
 
@@ -867,4 +885,63 @@ pkg_ctl="$prj_ctl/hello.git"
       reference: $checksum
       EOO
   }
+
+  : control-unavailable
+  :
+  {
+    $clone_root_data_clean;
+    sed -i -e 's%^(control:) .+$%\1 http://non-existent-host/path/rep.git%' \
+	$data_dir/request.manifest;
+
+    $clone_root_tgt;
+
+    $* "file:///$~/tgt.git" $data_dir >>"EOO"
+      : 1
+      status: 422
+      message: repository http://non-existent-host/path/rep.git unavailable
+      reference: $checksum
+      EOO
+  }
+
+  : control-clone-failed
+  :
+  {
+    $clone_root_data_clean;
+    sed -i -e 's%^(control:) .+$%\1 http://example.com/path/rep.git%' \
+	$data_dir/request.manifest;
+
+    $clone_root_tgt;
+
+    $* "file:///$~/tgt.git" $data_dir >>"EOO"
+      : 1
+      status: 422
+      message: failed to git-clone http://example.com/path/rep.git
+      reference: $checksum
+      EOO
+  }
+
+  : target-unavailable
+  :
+  {
+    $clone_root_data_clean;
+
+    $* 'http://non-existent-host/path/rep.git' $data_dir >>"EOO" 2>>~%EOE%
+      : 1
+      status: 503
+      message: submission service temporarily unavailable
+      reference: $checksum
+      EOO
+      %curl: .+%
+      EOE
+  }
+
+  : target-clone-failed
+  :
+  {
+    $clone_root_data_clean;
+
+    $* 'http://example.com/path/rep.git' $data_dir 2>>~%EOE% != 0
+      %fatal: .+%
+      EOE
+  }
 }