Skip to content

Commit

Permalink
Fix display of robots when setting up a build trigger
Browse files Browse the repository at this point in the history 
Fixes https://issues.redhat.com/browse/PROJQUAY-825
  • Loading branch information
Joseph Schorr committed Jul 7, 2020
1 parent 75276fd commit cfc2a2d
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 33 deletions.
102 changes: 74 additions & 28 deletions endpoints/api/test/test_trigger_analyzer.py
View file
Expand Up @@ -5,6 +5,8 @@
from data import model
from endpoints.api.trigger_analyzer import TriggerAnalyzer
from util import dockerfileparse
from test.fixtures import *
from app import app as real_app

BAD_PATH = '"server_hostname/" is not a valid Quay repository path'

Expand All @@ -14,7 +16,7 @@

BAD_CONF = {"context": "context", "dockerfile_path": "dockerfile_path"}

ONE_ROBOT = {"can_read": False, "is_robot": True, "kind": "user", "name": "name"}
ONE_ROBOT = {"can_read": False, "is_robot": True, "kind": "user", "name": "namespace+name"}

DOCKERFILE_NOT_CHILD = "Dockerfile, context, is not a child of the context, dockerfile_path."

Expand All @@ -41,7 +43,7 @@ def can_read_fn(base_namespace, base_repository):

def patch_list_namespace_robots(monkeypatch):
my_mock = Mock()
my_mock.configure_mock(**{"username": "name"})
my_mock.configure_mock(**{"username": "namespace+name"})
return_value = [my_mock]

def return_list_mocks(namesapce):
Expand Down Expand Up @@ -121,7 +123,7 @@ def return_path():
@pytest.mark.parametrize(
"handler_fn, config_dict, admin_org_permission, status, message, get_base_image, robots, server_hostname, get_repository, can_read, namespace, name",
[
(
pytest.param(
return_none,
EMPTY_CONF,
False,
Expand All @@ -134,8 +136,9 @@ def return_path():
False,
"namespace",
None,
id="test1",
),
(
pytest.param(
return_none,
EMPTY_CONF,
True,
Expand All @@ -148,8 +151,9 @@ def return_path():
False,
"namespace",
None,
id="test2",
),
(
pytest.param(
return_content,
BAD_CONF,
False,
Expand All @@ -162,8 +166,9 @@ def return_path():
False,
"namespace",
None,
id="test3",
),
(
pytest.param(
return_none,
EMPTY_CONF,
False,
Expand All @@ -176,8 +181,9 @@ def return_path():
False,
"namespace",
None,
id="test4",
),
(
pytest.param(
return_none,
EMPTY_CONF,
True,
Expand All @@ -190,8 +196,9 @@ def return_path():
False,
"namespace",
None,
id="test5",
),
(
pytest.param(
return_content,
BAD_CONF,
False,
Expand All @@ -204,8 +211,9 @@ def return_path():
False,
"namespace",
None,
id="test6",
),
(
pytest.param(
return_content,
GOOD_CONF,
False,
Expand All @@ -218,8 +226,9 @@ def return_path():
False,
"namespace",
None,
id="test7",
),
(
pytest.param(
return_content,
GOOD_CONF,
False,
Expand All @@ -232,8 +241,9 @@ def return_path():
False,
"namespace",
None,
id="test8",
),
(
pytest.param(
return_content,
GOOD_CONF,
False,
Expand All @@ -246,8 +256,9 @@ def return_path():
False,
"namespace",
None,
id="test9",
),
(
pytest.param(
return_content,
GOOD_CONF,
False,
Expand All @@ -260,8 +271,9 @@ def return_path():
False,
"namespace",
None,
id="test10",
),
(
pytest.param(
return_content,
GOOD_CONF,
False,
Expand All @@ -274,22 +286,9 @@ def return_path():
False,
"namespace",
None,
id="test11",
),
(
return_content,
GOOD_CONF,
False,
"requiresrobot",
None,
return_path,
[],
"server_hostname",
"nonpublic",
True,
"path",
"file",
),
(
pytest.param(
return_content,
GOOD_CONF,
False,
Expand All @@ -302,6 +301,7 @@ def return_path():
True,
"path",
"file",
id="test12",
),
],
)
Expand All @@ -319,6 +319,7 @@ def test_trigger_analyzer(
namespace,
name,
get_monkeypatch,
client,
):
patch_list_namespace_robots(get_monkeypatch)
patch_get_all_repo_users_transitive(get_monkeypatch)
Expand All @@ -338,3 +339,48 @@ def test_trigger_analyzer(
"message": message,
"is_admin": admin_org_permission,
}


class FakeHandler(object):
def load_dockerfile_contents(self):
return """
FROM localhost:5000/devtable/simple
"""


def test_inherited_robot_accounts(get_monkeypatch, initialized_db, client):
patch_permissions(get_monkeypatch, False)
analyzer = TriggerAnalyzer(FakeHandler(), "anothernamespace", "localhost:5000", {}, True)
assert analyzer.analyze_trigger()["status"] == "error"


def test_inherited_robot_accounts_same_namespace(get_monkeypatch, initialized_db, client):
patch_permissions(get_monkeypatch, True)
analyzer = TriggerAnalyzer(FakeHandler(), "devtable", "localhost:5000", {}, True)

result = analyzer.analyze_trigger()
assert result["status"] == "requiresrobot"
for robot in result["robots"]:
assert robot["name"].startswith("devtable+")
assert "token" not in robot


def test_inherited_robot_accounts_same_namespace_no_read_permission(
get_monkeypatch, initialized_db, client
):
patch_permissions(get_monkeypatch, False)
analyzer = TriggerAnalyzer(FakeHandler(), "devtable", "localhost:5000", {}, True)

result = analyzer.analyze_trigger()
assert analyzer.analyze_trigger()["status"] == "error"


def test_inherited_robot_accounts_same_namespace_not_org_admin(
get_monkeypatch, initialized_db, client
):
patch_permissions(get_monkeypatch, True)
analyzer = TriggerAnalyzer(FakeHandler(), "devtable", "localhost:5000", {}, False)

result = analyzer.analyze_trigger()
assert result["status"] == "requiresrobot"
assert not result["robots"]
18 changes: 13 additions & 5 deletions endpoints/api/trigger_analyzer.py
View file
Expand Up @@ -29,7 +29,7 @@ def is_parent(context, dockerfile_path):
return normalized_subdir.startswith(normalized_context)


class TriggerAnalyzer:
class TriggerAnalyzer(object):
"""
This analyzes triggers and returns the appropriate trigger and robot view to the frontend.
"""
Expand Down Expand Up @@ -110,7 +110,10 @@ def analyze_trigger(self):

# If the repository is private and the user cannot see that repo, then
# mark it as not found.
can_read = permissions.ReadRepositoryPermission(base_namespace, base_repository)
can_read = False
if base_namespace == self.namespace_name:
can_read = permissions.ReadRepositoryPermission(base_namespace, base_repository)

if found_repository.visibility.name != "public" and not can_read:
return self.analyze_view(
self.namespace_name,
Expand All @@ -127,7 +130,7 @@ def analyze_trigger(self):
def analyze_view(self, image_namespace, image_repository, status, message=None):
# Retrieve the list of robots and mark whether they have read access already.
robots = []
if self.admin_org_permission:
if self.admin_org_permission and self.namespace_name == image_namespace:
if image_repository is not None:
perm_query = model.user.get_all_repo_users_transitive(
image_namespace, image_repository
Expand All @@ -137,15 +140,20 @@ def analyze_view(self, image_namespace, image_repository, status, message=None):
user_ids_with_permission = set()

def robot_view(robot):
return {
assert robot.username.startswith(self.namespace_name + "+"), (
"Expected robot under namespace %s, Found: %s"
% (self.namespace_name, robot.username)
)
result = {
"name": robot.username,
"kind": "user",
"is_robot": True,
"can_read": robot.id in user_ids_with_permission,
}
return result

robots = [
robot_view(robot) for robot in model.user.list_namespace_robots(image_namespace)
robot_view(robot) for robot in model.user.list_namespace_robots(self.namespace_name)
]

return {
Expand Down

0 comments on commit cfc2a2d

Please sign in to comment.