Merge "Support relative urls in success-url" into feature/zuulv3
diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst
index 5a5b7bd..c75085d 100644
--- a/doc/source/user/config.rst
+++ b/doc/source/user/config.rst
@@ -496,12 +496,21 @@
   different string.  Default: "FAILURE".
 
 **success-url**
-  When a job succeeds, this URL is reported along with the result.
+  When a job succeeds, this URL is reported along with the result.  If
+  this value is not supplied, Zuul uses the content of the job
+  :ref:`return value <return_values>` **zuul.log_url**.  This is
+  recommended as it allows the code which stores the URL to the job
+  artifacts to report exactly where they were stored.  To override
+  this value, or if it is not set, supply an absolute URL in this
+  field.  If a relative URL is supplied in this field, and
+  **zuul.log_url** is set, then the two will be combined to produce
+  the URL used for the report.  This can be used to specify that
+  certain jobs should "deep link" into the stored job artifacts.
   Default: none.
 
 **failure-url**
   When a job fails, this URL is reported along with the result.
-  Default: none.
+  Otherwise behaves the same as **success-url**.
 
 **hold-following-changes**
   In a dependent pipeline, this option may be used to indicate that no
diff --git a/doc/source/user/jobs.rst b/doc/source/user/jobs.rst
index 58f3371..78121bc 100644
--- a/doc/source/user/jobs.rst
+++ b/doc/source/user/jobs.rst
@@ -101,6 +101,8 @@
 
 .. TODO: describe standard lib and link to published docs for it.
 
+.. _return_values:
+
 Return Values
 -------------
 
diff --git a/tests/fixtures/config/data-return/git/common-config/playbooks/data-return.yaml b/tests/fixtures/config/data-return/git/common-config/playbooks/data-return.yaml
index b92ff5c..5e412c3 100644
--- a/tests/fixtures/config/data-return/git/common-config/playbooks/data-return.yaml
+++ b/tests/fixtures/config/data-return/git/common-config/playbooks/data-return.yaml
@@ -3,4 +3,4 @@
     - zuul_return:
         data:
           zuul:
-            log_url: test/log/url
+            log_url: http://example.com/test/log/url/
diff --git a/tests/fixtures/config/data-return/git/common-config/zuul.yaml b/tests/fixtures/config/data-return/git/common-config/zuul.yaml
index 8aea931..8d602f1 100644
--- a/tests/fixtures/config/data-return/git/common-config/zuul.yaml
+++ b/tests/fixtures/config/data-return/git/common-config/zuul.yaml
@@ -15,8 +15,14 @@
 - job:
     name: data-return
 
+- job:
+    name: data-return-relative
+    run: playbooks/data-return
+    success-url: docs/index.html
+
 - project:
     name: org/project
     check:
       jobs:
         - data-return
+        - data-return-relative
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 5d49d11..87eddc6 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -713,6 +713,10 @@
         self.waitUntilSettled()
         self.assertHistory([
             dict(name='data-return', result='SUCCESS', changes='1,1'),
-        ])
-        self.assertIn('- data-return test/log/url',
+            dict(name='data-return-relative', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
+        self.assertIn('- data-return http://example.com/test/log/url/',
+                      A.messages[-1])
+        self.assertIn('- data-return-relative '
+                      'http://example.com/test/log/url/docs/index.html',
                       A.messages[-1])
diff --git a/zuul/model.py b/zuul/model.py
index ffbb70c..4ab66ca 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -20,6 +20,7 @@
 import struct
 import time
 from uuid import uuid4
+import urllib.parse
 
 MERGER_MERGE = 1          # "git merge"
 MERGER_MERGE_RESOLVE = 2  # "git merge -s resolve"
@@ -1626,13 +1627,29 @@
                 result = job.failure_message
             if job.failure_url:
                 pattern = job.failure_url
-        url = None
+        url = None  # The final URL
+        default_url = build.result_data.get('zuul', {}).get('log_url')
         if pattern:
-            url = self.formatUrlPattern(pattern, job, build)
+            job_url = self.formatUrlPattern(pattern, job, build)
+        else:
+            job_url = None
+        try:
+            if job_url:
+                u = urllib.parse.urlparse(job_url)
+                if u.scheme:
+                    # The job success or failure url is absolute, so it's
+                    # our final url.
+                    url = job_url
+                else:
+                    # We have a relative job url.  Combine it with our
+                    # default url.
+                    if default_url:
+                        url = urllib.parse.urljoin(default_url, job_url)
+        except Exception:
+            self.log.exception("Error while parsing url for job %s:"
+                               % (job,))
         if not url:
-            url = build.result_data.get('zuul', {}).get('log_url')
-        if not url:
-            url = build.url or job.name
+            url = default_url or build.url or job.name
         return (result, url)
 
     def formatJSON(self):