Merge pull request #2606 from coreos-inc/v2.3.3-branch

V2.3.3 changes

CHANGELOG.md

@@ -1,3 +1,13 @@
+### v2.3.3
+
+- Added: Prometheus metric for queued builds (#2596)
+
+- Fixed: Allow selection of Gitlab repository when Gitlab sends no permissions (#2601)
+- Fixed: Failure when viewing Gitlab repository with unexpected schema (#2599)
+- Fixed: LDAP stability fixes (#2598, #2584, #2595)
+- Fixed: Viewing of repositories with trust enabled caused a 500 (#2594, #2593)
+- Fixed: Failure in setup tool when time machine config is not set (#2589)
+
 ### v2.3.2
 
 - Added: Configuration of time machine in UI (#2516)

buildtrigger/gitlabhandler.py

@@ -309,9 +309,22 @@ class GitLabBuildTrigger(BuildTriggerHandler):
     def repo_view(repo):
       # Because *anything* can be None in GitLab API!
       permissions = repo.get('permissions') or {}
+      group_access = permissions.get('group_access') or {}
       project_access = permissions.get('project_access') or {}
-      access_level = project_access.get('access_level') or 0
+
+      missing_group_access = permissions.get('group_access') is None
+      missing_project_access = permissions.get('project_access') is None
+
+      access_level = max(group_access.get('access_level') or 0,
+                         project_access.get('access_level') or 0)
+
       has_admin_permission = _ACCESS_LEVEL_MAP.get(access_level, ("", False))[1]
+      if missing_group_access or missing_project_access:
+        # Default to has permission if we cannot check the permissions. This will allow our users
+        # to select the repository and then GitLab's own checks will ensure that the webhook is
+        # added only if allowed.
+        # TODO: Do we want to display this differently in the UI?
+        has_admin_permission = True
 
       view = {
         'name': repo['path'],

buildtrigger/test/gitlabmock.py

@@ -12,7 +12,7 @@ def get_gitlab_trigger(dockerfile_path=''):
     'username': 'knownuser'
   })
 
-  trigger._get_authorized_client = get_mock_gitlab(with_nullavatar=False)
+  trigger._get_authorized_client = get_mock_gitlab(with_nulls=False)
   return trigger
 
 def adddeploykey_mock(project_id, name, public_key):
@@ -29,10 +29,15 @@ def get_currentuser_mock():
 def project(namespace, name, is_org=False):
   project_access = None
 
-  if namespace == 'knownuser':
-    project_access = {
-      'access_level': 50,
-    }
+  if name != 'null':
+    if namespace == 'knownuser':
+      project_access = {
+        'access_level': 50,
+      }
+    else:
+      project_access = {
+        'access_level': 0,
+      }
 
   data = {
     'id': '%s/%s' % (namespace, name),
@@ -51,13 +56,14 @@ def project(namespace, name, is_org=False):
     'public': name != 'somerepo',
     'permissions': {
       'project_access': project_access,
+      'group_access': {'access_level': 0},
     },
     'owner': {
       'avatar_url': 'avatarurl',
     }
   }
 
-  if name == 'nullavatar':
+  if name == 'null':
     del data['owner']['avatar_url']
     data['namespace']['avatar'] = None
   elif is_org:
@@ -66,11 +72,11 @@ def project(namespace, name, is_org=False):
 
   return data
 
-def getprojects_mock(with_nullavatar=False):
-  if with_nullavatar:
+def getprojects_mock(with_nulls=False):
+  if with_nulls:
     def _getprojs(page=1, per_page=100):
       return [
-        project('someorg', 'nullavatar', is_org=True),
+        project('someorg', 'null', is_org=True),
       ]
     return _getprojs
 
@@ -183,7 +189,7 @@ def getrawfile_mock(repo_id, branch_name, path):
 
   return False
 
-def get_mock_gitlab(with_nullavatar=False):
+def get_mock_gitlab(with_nulls=False):
   def _get_mock():
     mock_gitlab = Mock()
     mock_gitlab.host = 'https://bitbucket.org'
@@ -191,7 +197,7 @@ def get_mock_gitlab(with_nullavatar=False):
     mock_gitlab.currentuser = Mock(side_effect=get_currentuser_mock)
     mock_gitlab.getusers = Mock(side_effect=getusers_mock)
 
-    mock_gitlab.getprojects = Mock(side_effect=getprojects_mock(with_nullavatar))
+    mock_gitlab.getprojects = Mock(side_effect=getprojects_mock(with_nulls))
     mock_gitlab.getproject = Mock(side_effect=getproject_mock)
     mock_gitlab.getbranches = Mock(side_effect=getbranches_mock)
 

buildtrigger/test/test_gitlabhandler.py

@@ -37,8 +37,15 @@ def test_lookup_user(email, expected_response, gitlab_trigger):
   assert gitlab_trigger.lookup_user(email) == expected_response
 
 
+def test_null_permissions(gitlab_trigger):
+  gitlab_trigger._get_authorized_client = get_mock_gitlab(with_nulls=True)
+  sources = gitlab_trigger.list_build_sources_for_namespace('someorg')
+  source = sources[0]
+  assert source['has_admin_permissions']
+
+
 def test_null_avatar(gitlab_trigger):
-  gitlab_trigger._get_authorized_client = get_mock_gitlab(with_nullavatar=True)
+  gitlab_trigger._get_authorized_client = get_mock_gitlab(with_nulls=True)
   namespace_data = gitlab_trigger.list_build_source_namespaces()
   expected = {
     'avatar_url': None,