Skip to content

Rework branch names check [RHELDST-29966] #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rbikar merged 1 commit into from
Apr 23, 2025
Merged

Rework branch names check [RHELDST-29966] #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions tests/ubiconfig/test_ubi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def branches():
"ubi7.1": "2189cbc2e447f796fe354f8d784d76b0a2620248",
"ubi7": "c99cb8d7dae2e78e8cc7e720d3f950d1c5a0b51f",
"ubi8": "26d24af7859df3c4d361bd33cd57984d03abe206",
"ga-cleanup": "117d4af7859df3c4d361bd33cdab6784d03abe104",
}


Expand Down Expand Up @@ -290,19 +291,6 @@ def test_load_file_without_providing_version(
ubi.get_loader().load("rhel-7-server.yaml")


@patch("requests.Session")
@patch("ubiconfig._impl.loaders._GitlabLoader._pre_load")
@patch("ubiconfig._impl.loaders._GitlabLoader._get_branches")
def test_load_invalid_branch_name(
mocked_get_branches, mocked_pre_load, mocked_session, branches, files_branch_map
):
mocked_get_branches.return_value = branches
mocked_pre_load.return_value = files_branch_map

with pytest.raises(ValueError):
ubi.get_loader().load("rhel-7-server.yaml", "invalid_branch")


@patch("requests.Session")
@patch("ubiconfig._impl.loaders._GitlabLoader._pre_load")
@patch("ubiconfig._impl.loaders._GitlabLoader._get_branches")
Expand Down Expand Up @@ -421,6 +409,10 @@ def test_get_branches(mocked_session, branches, caplog):
"commit": {"id": "2189cbc2e447f796fe354f8d784d76b0a2620248"},
},
{"name": "ubi8", "commit": {"id": "26d24af7859df3c4d361bd33cd57984d03abe206"}},
{
"name": "ga-cleanup",
"commit": {"id": "117d4af7859df3c4d361bd33cdab6784d03abe104"},
},
]
headers = {"Content-Length": "629", "X-Total-Pages": "1", "X-Per-Page": "20"}
mocked_session.return_value.request.return_value.headers = headers
Expand All @@ -438,17 +430,19 @@ def test_get_branches(mocked_session, branches, caplog):

@patch("requests.Session")
@patch("ubiconfig._impl.loaders._GitlabLoader._get_branches")
def test_pre_load(mocked_get_branches, mocked_session, files_branch_map):
def test_pre_load(mocked_get_branches, mocked_session, files_branch_map, caplog):
branch_sha1 = OrderedDict(
[
("ubi7.1", "2189cbc2e447f796fe354f8d784d76b0a2620248"),
("ubi7", "c99cb8d7dae2e78e8cc7e720d3f950d1c5a0b51f"),
("ubi8", "26d24af7859df3c4d361bd33cd57984d03abe206"),
("ga-cleanup", "117d4af7859df3c4d361bd33cdab6784d03abe104"),
]
)
mocked_get_branches.return_value = branch_sha1
headers = {"Content-Length": "629", "X-Total-Pages": "1", "X-Per-Page": "20"}
mocked_session.return_value.request.return_value.headers = headers

file_list = [
[
{"name": "rhel-7-server.yaml", "path": "rhel-7-server.yaml"},
Expand All @@ -459,10 +453,16 @@ def test_pre_load(mocked_get_branches, mocked_session, files_branch_map):
[{"name": "rhel-8-for-power-le.yaml", "path": "rhel-8-for-power-le.yaml"}],
]
mocked_session.return_value.request.return_value.json.side_effect = file_list

loader = ubi.get_loader()
expected_map = files_branch_map
actual_files_branch_map = loader._files_branch_map
assert expected_map == actual_files_branch_map
assert "ga-cleanup" not in actual_files_branch_map
assert (
"Skipping branch ga-cleanup (name does not match with required format)"
in caplog.text
)


def test_ubi_config(ubi7_1_config_file1):
Expand Down
20 changes: 13 additions & 7 deletions ubiconfig/_impl/loaders/gitlab.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,9 @@ def load(self, file_name, version=None):
"Couldn't find file %s from remote repo %s" % (file_name, self._url)
)

match = re.match(PREFIX_VERSION_RE, version)
if not match:
raise ValueError("Invalid version (branch name) %s" % version)

prefix = match.group("prefix")
default_version = match.group("default_version")
match = re.match(PREFIX_VERSION_RE, version) # type: ignore
prefix = match.group("prefix") # type: ignore
default_version = match.group("default_version") # type: ignore

default_branch = f"{prefix}{default_version}"

Expand Down Expand Up @@ -135,12 +132,21 @@ def load_all(self):
return ubi_configs

def _pre_load(self):
"""Iterate all branches to get a mapping of {file_path: (branch, sha1)...}"""
"""
Iterate all branches to get a mapping of {file_path: (branch, sha1)...}
Log a warning when invalid branch names are used.
"""
files_branch_map = {}

LOG.debug("Loading config files from all branches")

for branch, sha1 in self._branches.items():
if not re.match(PREFIX_VERSION_RE, branch):
LOG.warning(
"Skipping branch %s (name does not match with required format)",
branch,
)
continue
page = 1
while True:
file_list_api = self._repo_api.get_file_list_api(branch=sha1, page=page)
Expand Down