launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/gitlab-credentials into lp:launchpad.

Commit message:
Add optional GitLab credentials support, using personal access tokens.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/gitlab-credentials/+merge/364098

The structure of checkwatches.credentials is terrible for this as we have to pre-declare each possible instance, but it's hard to do better until we move this to the database.  In the meantime I preemptively declared a few major GitLab instances in case we need to set credentials for them in a hurry.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/gitlab-credentials into lp:launchpad.

=== modified file 'lib/lp/bugs/externalbugtracker/gitlab.py'
--- lib/lp/bugs/externalbugtracker/gitlab.py	2019-02-23 08:15:45 +0000
+++ lib/lp/bugs/externalbugtracker/gitlab.py	2019-03-07 15:36:47 +0000
@@ -29,6 +29,7 @@
     BugTaskStatus,
     )
 from lp.bugs.interfaces.externalbugtracker import UNKNOWN_REMOTE_IMPORTANCE
+from lp.services.config import config
 from lp.services.webapp.url import urlsplit
 
 
@@ -51,6 +52,16 @@
         super(GitLab, self).__init__(baseurl)
         self.cached_bugs = {}
 
+    @property
+    def credentials(self):
+        credentials_config = config["checkwatches.credentials"]
+        # lazr.config.Section doesn't support get().
+        try:
+            token = credentials_config["%s.token" % self.basehost]
+        except KeyError:
+            token = None
+        return {"token": token}
+
     def getModifiedRemoteBugs(self, bug_ids, last_accessed):
         """See `IExternalBugTracker`."""
         modified_bugs = self.getRemoteBugBatch(
@@ -132,6 +143,9 @@
             headers["If-Modified-Since"] = (
                 last_accessed.astimezone(pytz.UTC).strftime(
                     "%a, %d %b %Y %H:%M:%S GMT"))
+        token = self.credentials["token"]
+        if token is not None:
+            headers["Private-Token"] = token
         return super(GitLab, self).makeRequest(method, url, headers=headers)
 
     def _getCollection(self, base_page, last_accessed=None):

=== modified file 'lib/lp/bugs/externalbugtracker/tests/test_gitlab.py'
--- lib/lp/bugs/externalbugtracker/tests/test_gitlab.py	2019-02-23 08:15:45 +0000
+++ lib/lp/bugs/externalbugtracker/tests/test_gitlab.py	2019-03-07 15:36:47 +0000
@@ -18,8 +18,12 @@
     urlunsplit,
     )
 from testtools.matchers import (
+    Contains,
+    ContainsDict,
+    Equals,
     MatchesListwise,
     MatchesStructure,
+    Not,
     )
 import transaction
 from zope.component import getUtility
@@ -75,12 +79,15 @@
         responses.add(
             "GET", "https://gitlab.com/api/v4/projects/user%2Frepository/test";,
             json="success")
+        self.pushConfig(
+            "checkwatches.credentials", **{"gitlab.com.token": "sosekrit"})
         tracker = GitLab("https://gitlab.com/user/repository/issues";)
         self.assertEqual("success", tracker._getPage("test").json())
         requests = [call.request for call in responses.calls]
         self.assertThat(requests, MatchesListwise([
-            MatchesStructure.byEquality(
-                path_url="/api/v4/projects/user%2Frepository/test"),
+            MatchesStructure(
+                path_url=Equals("/api/v4/projects/user%2Frepository/test"),
+                headers=ContainsDict({"Private-Token": Equals("sosekrit")})),
             ]))
 
     @responses.activate
@@ -92,8 +99,9 @@
         self.assertEqual("success", tracker._getPage("test").json())
         requests = [call.request for call in responses.calls]
         self.assertThat(requests, MatchesListwise([
-            MatchesStructure.byEquality(
-                path_url="/api/v4/projects/user%2Frepository/test"),
+            MatchesStructure(
+                path_url=Equals("/api/v4/projects/user%2Frepository/test"),
+                headers=Not(Contains("Private-Token"))),
             ]))
 
     @responses.activate

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2018-11-22 16:35:28 +0000
+++ lib/lp/services/config/schema-lazr.conf	2019-03-07 15:36:47 +0000
@@ -169,6 +169,9 @@
 gcc.gnu.org.username: none
 gcc.gnu.org.password: none
 api.github.com.token: none
+gitlab.com.token: none
+gitlab.gnome.org.token: none
+salsa.debian.org.token: none
 
 
 [codebrowse]