Deprecate and cleanup use of status-url

In addition to the required web.root setting there is the web.status-url
that might cause some confusion for users as the difference is not
immediately clear.

In order to deprecate the web.status-url setting the following cases
need to be considered:

1. The Github reporter allows configuring the status-url as part of the
   pipeline configuration, but already uses the better item URL in
   `setCommitStatus()` instead. Here we can simply add a deprecation
   warning in case the status-url is configured.
2. The Pagure reporter also allows to configure the status-url and so
   far used it for setting the commit status. Analogous to the Github
   reporter we can replace this with the item URL and also add a
   deprecation warning when it is configured.
3. The no-job, enqueue and start messages still use the global
   `status_url` default (now unified to always use the setting from the
   system attributes and NOT from the local Zuul config). Since the
   `status_url` replacement attribute has already been deprecated in
   favor of `item_url` there isn't anything to do here.

After we've removed support for the `status_url` format attribute, we
can also remove the global `web.status_url` setting from the config and
the system attributes in Zookeeper.

Change-Id: If4ed1d7942dd2e7c2351a1eba9892f8503990f1f

doc/source/configuration.rst

@@ -27,7 +27,7 @@ An example ``zuul.conf``:
    password=MY_SECRET_PASSWORD
 
    [web]
-   status_url=https://zuul.example.com/status
+   root=https://zuul.example.com/
 
    [scheduler]
    log_config=/etc/zuul/scheduler-logging.yaml
@@ -244,11 +244,13 @@ The following sections of ``zuul.conf`` are used by the scheduler:
 
    .. attr:: status_url
 
+      .. warning:: This is deprecate and only used for the also deprecated
+                   ``status_url`` replacement field available in pipeline
+                   reporter messages.
+
       URL that will be posted in Zuul comments made to changes when
       starting jobs for a change.
 
-      .. TODO: is this effectively required?
-
 .. attr:: keystore
 
    .. _keystore-password:

doc/source/drivers/github.rst

@@ -510,19 +510,6 @@ itself. Status name, description, and context is taken from the pipeline.
       It is recommended to use `check` unless you have a specific
       reason to use the status API.
 
-   .. TODO support role markup in :default: so we can xref
-      :attr:`web.status_url` below
-
-   .. attr:: status-url
-      :default: link to the build status page
-      :type: string
-
-      URL to set in the Github status.
-
-      Defaults to a link to the build status or results page.  This
-      should probably be left blank unless there is a specific reason
-      to override it.
-
    .. attr:: check
       :type: string
 

doc/source/drivers/pagure.rst

@@ -212,12 +212,6 @@ is taken from the pipeline.
       String value (``pending``, ``success``, ``failure``) that the
       reporter should set as the commit status on Pagure.
 
-   .. attr:: status-url
-      :default: web.status_url or the empty string
-
-      String value for a link url to set in the Pagure status. Defaults to the
-      zuul server status_url, or the empty string if that is unset.
-
    .. attr:: comment
       :default: true
 

etc/zuul.conf-sample

@@ -33,7 +33,7 @@ prometheus_port=9093
 listen_address=127.0.0.1
 port=9000
 static_cache_expiry=0
-status_url=https://zuul.example.com/status
+root=https://zuul.example.com/
 prometheus_port=9094
 
 [webclient]

releasenotes/notes/status-url-f23c6b5173435d1a.yaml

@@ -0,0 +1,6 @@
+---
+deprecations:
+  - |
+    The ``status-url`` attribute of the Github and Pagure reporter is
+    deprecated. Reporters will currently ignore this setting and automatically
+    use the best URL for information about the item instead.

tests/fixtures/zuul-connections-merger.conf

@@ -1,5 +1,5 @@
 [web]
-status_url=http://zuul.example.com/status
+root=http://zuul.example.com/
 
 [merger]
 git_dir=/tmp/zuul-test/git

tests/fixtures/zuul-gitlab-driver-no-pool.conf

@@ -1,5 +1,5 @@
 [web]
-status_url=http://zuul.example.com/status/#{change.number},{change.patchset}
+root=http://zuul.example.com/
 
 [merger]
 git_dir=/tmp/zuul-test/git

tests/fixtures/zuul-gitlab-driver.conf

@@ -1,5 +1,5 @@
 [web]
-status_url=http://zuul.example.com/status/#{change.number},{change.patchset}
+root=http://zuul.example.com/
 
 [merger]
 git_dir=/tmp/zuul-test/git

tests/fixtures/zuul-pagure-driver-whitelist.conf

@@ -1,5 +1,5 @@
 [web]
-status_url=http://zuul.example.com/status/#{change.number},{change.patchset}
+root=http://zuul.example.com/
 
 [merger]
 git_dir=/tmp/zuul-test/git

tests/fixtures/zuul-pagure-driver.conf

@@ -1,5 +1,5 @@
 [web]
-status_url=http://zuul.example.com/status/#{change.number},{change.patchset}
+root=http://zuul.example.com/
 
 [merger]
 git_dir=/tmp/zuul-test/git

tests/fixtures/zuul-push-reqs.conf

@@ -1,5 +1,5 @@
 [web]
-status_url=http://zuul.example.com/status
+root=http://zuul.example.com/
 
 [merger]
 git_dir=/tmp/zuul-test/git

zuul/configloader.py

@@ -1498,7 +1498,7 @@ class PipelineParser(object):
                        reporter_name not in allowed_reporters:
                         raise UnknownConnection(reporter_name)
                     reporter = self.pcontext.connections.getReporter(
-                        reporter_name, pipeline, params)
+                        reporter_name, pipeline, params, self.pcontext)
                     reporter.setAction(conf_key)
                     reporter_set.append(reporter)
                     seen_connections.add(reporter_name)

zuul/driver/__init__.py

@@ -224,7 +224,8 @@ class ReporterInterface(object, metaclass=abc.ABCMeta):
     """
 
     @abc.abstractmethod
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         """Create and return a new Reporter object.
 
         This method is required by the interface.
@@ -236,6 +237,8 @@ class ReporterInterface(object, metaclass=abc.ABCMeta):
             reporter.
         :arg dict config: The configuration information supplied along
             with the reporter in the layout.
+        :arg ParseContext parse_context: The parse context during config
+            loading.
 
         :returns: A new Reporter object.
         :rtype: Reporter

zuul/driver/elasticsearch/__init__.py

@@ -23,7 +23,8 @@ class ElasticsearchDriver(Driver, ConnectionInterface, ReporterInterface):
     def getConnection(self, name, config):
         return elconnection.ElasticsearchConnection(self, name, config)
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return elreporter.ElasticsearchReporter(self, connection, config)
 
     def getReporterSchema(self):

zuul/driver/gerrit/__init__.py

@@ -66,7 +66,8 @@ class GerritDriver(Driver, ConnectionInterface, TriggerInterface,
     def getSource(self, connection):
         return gerritsource.GerritSource(self, connection)
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return gerritreporter.GerritReporter(self, connection, config)
 
     def getTriggerSchema(self):

zuul/driver/github/__init__.py

@@ -37,9 +37,10 @@ class GithubDriver(Driver, ConnectionInterface, TriggerInterface,
     def getSource(self, connection):
         return githubsource.GithubSource(self, connection)
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return githubreporter.GithubReporter(
-            self, connection, pipeline, config)
+            self, connection, pipeline, config, parse_context)
 
     def getTriggerSchema(self):
         return githubtrigger.getSchema()

zuul/driver/github/githubreporter.py

@@ -21,11 +21,17 @@ import time
 from zuul import model
 from zuul.lib.logutil import get_annotated_logger
 from zuul.reporter import BaseReporter
-from zuul.exceptions import MergeFailure
+from zuul.exceptions import DeprecationWarning, MergeFailure
 from zuul.driver.util import scalar_or_list
 from zuul.driver.github.githubsource import GithubSource
 
 
+class GithubStatusUrlDeprecation(DeprecationWarning):
+    zuul_error_name = 'Github status-url Deprecation'
+    zuul_error_message = """The 'status-url' reporter attribute
+is deprecated."""
+
+
 class GithubReporter(BaseReporter):
     """Sends off reports to Github."""
 
@@ -42,8 +48,10 @@ class GithubReporter(BaseReporter):
         model.MERGER_REBASE: 'rebase',
     }
 
-    def __init__(self, driver, connection, pipeline, config=None):
-        super(GithubReporter, self).__init__(driver, connection, config)
+    def __init__(self, driver, connection, pipeline, config=None,
+                 parse_context=None):
+        super(GithubReporter, self).__init__(
+            driver, connection, config, parse_context)
         self._commit_status = self.config.get('status', None)
         self._create_comment = self.config.get('comment', True)
         self._check = self.config.get('check', False)
@@ -58,6 +66,9 @@ class GithubReporter(BaseReporter):
             self._unlabels = [self._unlabels]
         self.context = "{}/{}".format(pipeline.tenant.name, pipeline.name)
 
+        if 'status-url' in self.config and parse_context:
+            parse_context.accumulator.addError(GithubStatusUrlDeprecation)
+
     def report(self, item, phase1=True, phase2=True):
         """Report on an event."""
         log = get_annotated_logger(self.log, item.event)
@@ -388,6 +399,7 @@ class GithubReporter(BaseReporter):
 def getSchema():
     github_reporter = v.Schema({
         'status': v.Any('pending', 'success', 'failure'),
+        # MODEL_API < 31
         'status-url': str,
         'comment': bool,
         'merge': bool,

zuul/driver/gitlab/__init__.py

@@ -37,7 +37,8 @@ class GitlabDriver(Driver, ConnectionInterface, TriggerInterface,
     def getSource(self, connection):
         return gitlabsource.GitlabSource(self, connection)
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return gitlabreporter.GitlabReporter(
             self, connection, pipeline, config)
 

zuul/driver/mqtt/__init__.py

@@ -23,7 +23,8 @@ class MQTTDriver(Driver, ConnectionInterface, ReporterInterface):
     def getConnection(self, name, config):
         return mqttconnection.MQTTConnection(self, name, config)
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return mqttreporter.MQTTReporter(self, connection, config)
 
     def getReporterSchema(self):

zuul/driver/pagure/__init__.py

@@ -37,9 +37,10 @@ class PagureDriver(Driver, ConnectionInterface, TriggerInterface,
     def getSource(self, connection):
         return paguresource.PagureSource(self, connection)
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return pagurereporter.PagureReporter(
-            self, connection, pipeline, config)
+            self, connection, pipeline, config, parse_context)
 
     def getTriggerSchema(self):
         return paguretrigger.getSchema()

zuul/driver/pagure/pagurereporter.py

@@ -15,26 +15,38 @@
 
 import time
 import logging
+
 import voluptuous as v
 
 from zuul.reporter import BaseReporter
-from zuul.exceptions import MergeFailure
+from zuul.exceptions import DeprecationWarning, MergeFailure
 from zuul.driver.pagure.paguresource import PagureSource
 
 
+class PagureStatusUrlDeprecation(DeprecationWarning):
+    zuul_error_name = 'Pagure status-url Deprecation'
+    zuul_error_message = """The 'status-url' reporter attribute
+is deprecated."""
+
+
 class PagureReporter(BaseReporter):
     """Sends off reports to Pagure."""
 
     name = 'pagure'
     log = logging.getLogger("zuul.PagureReporter")
 
-    def __init__(self, driver, connection, pipeline, config=None):
-        super(PagureReporter, self).__init__(driver, connection, config)
+    def __init__(self, driver, connection, pipeline, config=None,
+                 parse_context=None):
+        super(PagureReporter, self).__init__(driver, connection, config,
+                                             parse_context)
         self._commit_status = self.config.get('status', None)
         self._create_comment = self.config.get('comment', True)
         self._merge = self.config.get('merge', False)
         self.context = "{}/{}".format(pipeline.tenant.name, pipeline.name)
 
+        if 'status-url' in self.config and parse_context:
+            parse_context.accumulator.addError(PagureStatusUrlDeprecation)
+
     def report(self, item, phase1=True, phase2=True):
         """Report on an event."""
         for change in item.changes:
@@ -100,13 +112,7 @@ class PagureReporter(BaseReporter):
         state = self._commit_status
         change_number = change.number
 
-        url_pattern = self.config.get('status-url')
-        sched_config = self.connection.sched.config
-        if sched_config.has_option('web', 'status_url'):
-            url_pattern = sched_config.get('web', 'status_url')
-        url = item.formatUrlPattern(url_pattern) \
-            if url_pattern else 'https://sftests.com'
-
+        url = item.formatItemUrl()
         description = '%s status: %s (%s)' % (
             item.pipeline.name, self._commit_status, sha)
 
@@ -145,6 +151,7 @@ class PagureReporter(BaseReporter):
 def getSchema():
     pagure_reporter = v.Schema({
         'status': v.Any('pending', 'success', 'failure'),
+        # MODEL_API < 31
         'status-url': str,
         'comment': bool,
         'merge': bool,

zuul/driver/smtp/__init__.py

@@ -23,7 +23,8 @@ class SMTPDriver(Driver, ConnectionInterface, ReporterInterface):
     def getConnection(self, name, config):
         return smtpconnection.SMTPConnection(self, name, config)
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return smtpreporter.SMTPReporter(self, connection, config)
 
     def getReporterSchema(self):

zuul/driver/sql/__init__.py

@@ -31,7 +31,8 @@ class SQLDriver(Driver, ConnectionInterface, ReporterInterface):
     def getConnection(self, name, config):
         return sqlconnection.SQLConnection(self, name, config)
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return sqlreporter.SQLReporter(self, connection, config)
 
     def getReporterSchema(self):

zuul/driver/zuul/__init__.py

@@ -191,7 +191,8 @@ class ZuulDriver(Driver, TriggerInterface, ReporterInterface):
     def getTriggerEventClass(self):
         return zuulmodel.ZuulTriggerEvent
 
-    def getReporter(self, connection, pipeline, config=None):
+    def getReporter(self, connection, pipeline, config=None,
+                    parse_context=None):
         return zuulreporter.ZuulReporter(self, connection, pipeline, config)
 
     def getReporterSchema(self):

zuul/lib/connections.py

@@ -220,9 +220,11 @@ class ConnectionRegistry(object):
         return [c for c in self.connections.values()
                 if isinstance(c.driver, ProviderInterface)]
 
-    def getReporter(self, connection_name, pipeline, config=None):
+    def getReporter(self, connection_name, pipeline, config=None,
+                    parse_context=None):
         connection = self.connections[connection_name]
-        return connection.driver.getReporter(connection, pipeline, config)
+        return connection.driver.getReporter(
+            connection, pipeline, config, parse_context)
 
     def getTrigger(self, connection_name, config=None):
         connection = self.connections[connection_name]

zuul/model.py

@@ -7992,6 +7992,7 @@ class SystemAttributes:
         self.default_hold_expiration = 0
         self.default_ansible_version = None
         self.web_root = None
+        # TODO: Deprecated, remove after version 12
         self.web_status_url = ""
         self.websocket_url = None
 

zuul/reporter/__init__.py

@@ -15,7 +15,6 @@
 
 import abc
 import logging
-from zuul.lib.config import get_default
 
 
 class BaseReporter(object, metaclass=abc.ABCMeta):
@@ -26,7 +25,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
 
     log = logging.getLogger("zuul.reporter.BaseReporter")
 
-    def __init__(self, driver, connection, config=None):
+    def __init__(self, driver, connection, config=None, parse_context=None):
         self.driver = driver
         self.connection = connection
         self.config = config or {}
@@ -172,8 +171,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
         return ret
 
     def _formatItemReportEnqueue(self, item, with_jobs=True):
-        status_url = self.connection.sched.globals.web_status_url
-        if status_url:
+        if status_url := self.connection.sched.globals.web_status_url:
             status_url = item.formatUrlPattern(status_url)
 
         # change, changes, and status_url are deprecated
@@ -185,8 +183,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
             status_url=status_url)
 
     def _formatItemReportStart(self, item, with_jobs=True):
-        status_url = self.connection.sched.globals.web_status_url
-        if status_url:
+        if status_url := self.connection.sched.globals.web_status_url:
             status_url = item.formatUrlPattern(status_url)
 
         # change, changes, and status_url are deprecated
@@ -252,9 +249,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
         return msg
 
     def _formatItemReportNoJobs(self, item, with_jobs=True):
-        status_url = get_default(self.connection.sched.config,
-                                 'web', 'status_url', '')
-        if status_url:
+        if status_url := self.connection.sched.globals.web_status_url:
             status_url = item.formatUrlPattern(status_url)
 
         # change, changes, and status_url are deprecated