From 05eeac08b63449925cc2e12d2fdaf937d5fa1bbc Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 28 Aug 2018 17:34:24 +0300 Subject: Fix submit-git to respond with 422 (client) error if fail to git-clone control URL --- brep/handler/submit/submit-git.bash.in | 22 +++++--- brep/handler/submit/submit-git.in | 29 +++++----- tests/ci/ci-dir.test | 4 +- tests/submit/submit-dir.test | 6 +-- tests/submit/submit-git.test | 97 ++++++++++++++++++++++++++++++---- 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 () # 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 () # +# 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 () # { trace_func "$@" local url="$1" local tmo="$2" + local our="$3" local s s="$(url_scheme "$url")" @@ -324,11 +329,14 @@ function check_connectivity () # 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 () # ... run git -C "$d" add $gvo "$@" >&2 } -function git_clone () # ... -{ - 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 + } } -- cgit v1.1