From 146d2b45186295cd9c07c1cab6f5184853f73d5c Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Thu, 23 Oct 2025 12:44:23 -0400 Subject: [PATCH 1/4] Add test for package renaming logic and fix to mainline code Signed-off-by: Geoff Wilson --- pulp_manager/app/services/pulp_manager.py | 2 +- .../tests/unit/services/test_pulp_manager.py | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/pulp_manager/app/services/pulp_manager.py b/pulp_manager/app/services/pulp_manager.py index 7cca788..5d22c97 100644 --- a/pulp_manager/app/services/pulp_manager.py +++ b/pulp_manager/app/services/pulp_manager.py @@ -180,7 +180,7 @@ def _process_package_name(self, name: str, base_url: str): package_rename_config = { "pattern": re.compile(pattern), - "replacement_rule": re.compile(replacement) + "replacement_rule": replacement } pattern = package_rename_config['pattern'] replacement_rule = package_rename_config['replacement_rule'] diff --git a/pulp_manager/tests/unit/services/test_pulp_manager.py b/pulp_manager/tests/unit/services/test_pulp_manager.py index c9feece..367ed7a 100644 --- a/pulp_manager/tests/unit/services/test_pulp_manager.py +++ b/pulp_manager/tests/unit/services/test_pulp_manager.py @@ -948,3 +948,55 @@ def new_pulp_client(pulp_server: PulpServer): self.pulp_manager.add_repos_from_pulp_server("source", None, None) assert mock_create_or_update_repository_source_pulp_server.call_count == 2 + + @patch.dict("pulp_manager.app.services.pulp_manager.CONFIG", { + "pulp": { + "package_name_replacement_pattern": "", + "package_name_replacement_rule": "" + } + }) + def test_process_package_name_no_pattern(self): + """Tests that when no pattern is configured, the original name with base_url is returned + """ + result = self.pulp_manager._process_package_name("test-package", "http://example.com/") + + assert result == "http://example.com/test-package" + + @patch.dict("pulp_manager.app.services.pulp_manager.CONFIG", { + "pulp": { + "package_name_replacement_pattern": "^prefix-(?P.+)$", + "package_name_replacement_rule": "{name}-suffix" + } + }) + def test_process_package_name_pattern_no_match(self): + """Tests that when pattern doesn't match, the original name with base_url is returned + """ + result = self.pulp_manager._process_package_name("no-prefix-package", "http://example.com/") + + assert result == "http://example.com/no-prefix-package" + + @patch.dict("pulp_manager.app.services.pulp_manager.CONFIG", { + "pulp": { + "package_name_replacement_pattern": "^prefix-(?P.+)$", + "package_name_replacement_rule": "{name}-suffix" + } + }) + def test_process_package_name_pattern_match(self): + """Tests that when pattern matches, the transformed name with base_url is returned + """ + result = self.pulp_manager._process_package_name("prefix-mypackage", "http://example.com/") + + assert result == "http://example.com/mypackage-suffix" + + @patch.dict("pulp_manager.app.services.pulp_manager.CONFIG", { + "pulp": { + "package_name_replacement_pattern": "^(?P[a-z]+)-(?P[a-z]+)-(?P.+)$", + "package_name_replacement_rule": "{env}/{org}/{pkg}" + } + }) + def test_process_package_name_complex_pattern(self): + """Tests package name transformation with multiple named groups + """ + result = self.pulp_manager._process_package_name("acme-prod-webserver", "http://example.com/") + + assert result == "http://example.com/prod/acme/webserver" From 72c7f2ea40e76c92b1407bd07c7032425e076b96 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Thu, 23 Oct 2025 13:29:31 -0400 Subject: [PATCH 2/4] Add tests for repo name prefixes (moved to func for easier testing) Signed-off-by: Geoff Wilson --- .../app/services/repo_config_register.py | 23 ++++++++-- .../services/test_repo_config_register.py | 43 +++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/pulp_manager/app/services/repo_config_register.py b/pulp_manager/app/services/repo_config_register.py index f915bd8..2ce1b11 100644 --- a/pulp_manager/app/services/repo_config_register.py +++ b/pulp_manager/app/services/repo_config_register.py @@ -157,6 +157,24 @@ def _generate_repo_config_from_file(self, file_path: str): return repo_config + def _apply_repo_name_prefix(self, name: str, root_path: str) -> str: + """Applies the appropriate prefix to a repo name based on whether it's remote or internal. + + :param name: The repo name + :type name: str + :param root_path: The root directory path containing the repo config file + :type root_path: str + :return: The repo name with appropriate prefix applied + :rtype: str + """ + if "remote" in root_path and not name.startswith("ext-"): + return f"ext-{name}" + elif "internal" in root_path: + prefix = CONFIG["pulp"]["internal_package_prefix"] + if not name.startswith(prefix): + return f"{prefix}{name}" + return name + def _parse_repo_config_files(self, repo_config_dir: str, regex_include: str, regex_exclude: str): """Parses the repo configs in the given repo_config_dir and returns a list of dicts @@ -180,10 +198,7 @@ def _parse_repo_config_files(self, repo_config_dir: str, regex_include: str, repo_config = json.loads(repo_config_file.read()) name = repo_config["name"] - if "remote" in root and not name.startswith("ext-"): - name = f"ext-{name}" - elif "internal" in root and not name.startswith(CONFIG["pulp"]["internal_package_prefix"]): - name = f"{CONFIG['pulp']['internal_package_prefix']}{name}" + name = self._apply_repo_name_prefix(name, root) if regex_exclude and re.search(regex_exclude, name): continue diff --git a/pulp_manager/tests/unit/services/test_repo_config_register.py b/pulp_manager/tests/unit/services/test_repo_config_register.py index fd93ac2..0e3187c 100644 --- a/pulp_manager/tests/unit/services/test_repo_config_register.py +++ b/pulp_manager/tests/unit/services/test_repo_config_register.py @@ -212,6 +212,49 @@ def open_side_effect(name, mode=None): assert len(parsed_repo_configs) == 1 assert parsed_repo_configs[0]["name"] == "ext-el8repo" + @patch.dict("pulp_manager.app.services.repo_config_register.CONFIG", { + "pulp": { + "internal_package_prefix": "int_" + } + }) + def test_apply_repo_name_prefix_remote(self): + """Tests that _apply_repo_name_prefix adds 'ext-' prefix for remote repos + """ + # Remote repo without prefix + result = self.repo_config_register._apply_repo_name_prefix("myrepo", "/path/remote/el7") + assert result == "ext-myrepo" + + # Remote repo already with prefix + result = self.repo_config_register._apply_repo_name_prefix("ext-myrepo", "/path/remote/el7") + assert result == "ext-myrepo" + + @patch.dict("pulp_manager.app.services.repo_config_register.CONFIG", { + "pulp": { + "internal_package_prefix": "int_" + } + }) + def test_apply_repo_name_prefix_internal(self): + """Tests that _apply_repo_name_prefix adds internal_package_prefix for internal repos + """ + # Internal repo without prefix + result = self.repo_config_register._apply_repo_name_prefix("myrepo", "/path/internal/el7") + assert result == "int_myrepo" + + # Internal repo already with prefix + result = self.repo_config_register._apply_repo_name_prefix("int_myrepo", "/path/internal/el7") + assert result == "int_myrepo" + + @patch.dict("pulp_manager.app.services.repo_config_register.CONFIG", { + "pulp": { + "internal_package_prefix": "int_" + } + }) + def test_apply_repo_name_prefix_neither(self): + """Tests that _apply_repo_name_prefix returns original name for repos not in remote or internal paths + """ + result = self.repo_config_register._apply_repo_name_prefix("myrepo", "/path/other/el7") + assert result == "myrepo" + @patch("pulp_manager.app.services.repo_config_register.Repo.clone_from") def test_create_repos_from_git_config_fail(self, mock_clone_from): """Tests logic flow that if they are errors an exception is raised From a0bff19c15d471e17543d6fc2044cbb644953a84 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Mon, 27 Oct 2025 13:21:21 -0400 Subject: [PATCH 3/4] Added documentation to README Signed-off-by: Geoff Wilson --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 33c2590..0caac11 100644 --- a/README.md +++ b/README.md @@ -248,6 +248,12 @@ Settings to apply to all pulp servers servers - `git_repo_config_dir`: Directory in `git_repo_config` which contains the pulp repo config +- `internal_package_prefix`: Prefix for indicating an internal package uploaded + directly to Pulp primary (no remote URL). +- `package_name_replacement_pattern`: Regex for matching packages to be + renamed. Use named matching groups for use in the format rule. +- `package_name_replacement_rule`: The new name pattern assigned to packages + which match the above. Reference named matching groups from above if needed. - `remote_tls_validation`: Boolean whether to require TLS validation of remote hosts - `use_https_for_sync`: Boolean whether to use HTTPS for repository sync URLs. From 99cabe2fd3a3939454e814ceebdb8a6afb3128d3 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Tue, 4 Nov 2025 12:47:47 -0500 Subject: [PATCH 4/4] New tests were wiping out config values needed by other tests -- now just sets the config keys as needed Signed-off-by: Geoff Wilson --- .../tests/unit/services/test_pulp_manager.py | 37 +++++++------------ .../services/test_repo_config_register.py | 22 ++++------- 2 files changed, 20 insertions(+), 39 deletions(-) diff --git a/pulp_manager/tests/unit/services/test_pulp_manager.py b/pulp_manager/tests/unit/services/test_pulp_manager.py index 367ed7a..97f389c 100644 --- a/pulp_manager/tests/unit/services/test_pulp_manager.py +++ b/pulp_manager/tests/unit/services/test_pulp_manager.py @@ -12,6 +12,7 @@ RpmRemote, RpmRepository, SigningService) from pulp3_bindings.pulp3.resources import Task as Pulp3Task +from pulp_manager.app.config import CONFIG from pulp_manager.app.database import engine, session from pulp_manager.app.exceptions import PulpManagerError, PulpManagerValueError from pulp_manager.app.models import PulpServer @@ -949,54 +950,42 @@ def new_pulp_client(pulp_server: PulpServer): self.pulp_manager.add_repos_from_pulp_server("source", None, None) assert mock_create_or_update_repository_source_pulp_server.call_count == 2 - @patch.dict("pulp_manager.app.services.pulp_manager.CONFIG", { - "pulp": { - "package_name_replacement_pattern": "", - "package_name_replacement_rule": "" - } - }) def test_process_package_name_no_pattern(self): """Tests that when no pattern is configured, the original name with base_url is returned """ + CONFIG["pulp"]["package_name_replacement_pattern"] = "" + CONFIG["pulp"]["package_name_replacement_rule"] = "" + result = self.pulp_manager._process_package_name("test-package", "http://example.com/") assert result == "http://example.com/test-package" - @patch.dict("pulp_manager.app.services.pulp_manager.CONFIG", { - "pulp": { - "package_name_replacement_pattern": "^prefix-(?P.+)$", - "package_name_replacement_rule": "{name}-suffix" - } - }) def test_process_package_name_pattern_no_match(self): """Tests that when pattern doesn't match, the original name with base_url is returned """ + CONFIG["pulp"]["package_name_replacement_pattern"] = "^prefix-(?P.+)$" + CONFIG["pulp"]["package_name_replacement_rule"] = "{name}-suffix" + result = self.pulp_manager._process_package_name("no-prefix-package", "http://example.com/") assert result == "http://example.com/no-prefix-package" - @patch.dict("pulp_manager.app.services.pulp_manager.CONFIG", { - "pulp": { - "package_name_replacement_pattern": "^prefix-(?P.+)$", - "package_name_replacement_rule": "{name}-suffix" - } - }) def test_process_package_name_pattern_match(self): """Tests that when pattern matches, the transformed name with base_url is returned """ + CONFIG["pulp"]["package_name_replacement_pattern"] = "^prefix-(?P.+)$" + CONFIG["pulp"]["package_name_replacement_rule"] = "{name}-suffix" + result = self.pulp_manager._process_package_name("prefix-mypackage", "http://example.com/") assert result == "http://example.com/mypackage-suffix" - @patch.dict("pulp_manager.app.services.pulp_manager.CONFIG", { - "pulp": { - "package_name_replacement_pattern": "^(?P[a-z]+)-(?P[a-z]+)-(?P.+)$", - "package_name_replacement_rule": "{env}/{org}/{pkg}" - } - }) def test_process_package_name_complex_pattern(self): """Tests package name transformation with multiple named groups """ + CONFIG["pulp"]["package_name_replacement_pattern"] = "^(?P[a-z]+)-(?P[a-z]+)-(?P.+)$" + CONFIG["pulp"]["package_name_replacement_rule"] = "{env}/{org}/{pkg}" + result = self.pulp_manager._process_package_name("acme-prod-webserver", "http://example.com/") assert result == "http://example.com/prod/acme/webserver" diff --git a/pulp_manager/tests/unit/services/test_repo_config_register.py b/pulp_manager/tests/unit/services/test_repo_config_register.py index 0e3187c..df45e8c 100644 --- a/pulp_manager/tests/unit/services/test_repo_config_register.py +++ b/pulp_manager/tests/unit/services/test_repo_config_register.py @@ -8,6 +8,7 @@ import pytest from mock import mock_open, patch +from pulp_manager.app.config import CONFIG from pulp_manager.app.database import engine, session from pulp_manager.app.services.repo_config_register import RepoConfigRegister @@ -212,14 +213,11 @@ def open_side_effect(name, mode=None): assert len(parsed_repo_configs) == 1 assert parsed_repo_configs[0]["name"] == "ext-el8repo" - @patch.dict("pulp_manager.app.services.repo_config_register.CONFIG", { - "pulp": { - "internal_package_prefix": "int_" - } - }) def test_apply_repo_name_prefix_remote(self): """Tests that _apply_repo_name_prefix adds 'ext-' prefix for remote repos """ + CONFIG["pulp"]["internal_package_prefix"] = "int_" + # Remote repo without prefix result = self.repo_config_register._apply_repo_name_prefix("myrepo", "/path/remote/el7") assert result == "ext-myrepo" @@ -228,14 +226,11 @@ def test_apply_repo_name_prefix_remote(self): result = self.repo_config_register._apply_repo_name_prefix("ext-myrepo", "/path/remote/el7") assert result == "ext-myrepo" - @patch.dict("pulp_manager.app.services.repo_config_register.CONFIG", { - "pulp": { - "internal_package_prefix": "int_" - } - }) def test_apply_repo_name_prefix_internal(self): """Tests that _apply_repo_name_prefix adds internal_package_prefix for internal repos """ + CONFIG["pulp"]["internal_package_prefix"] = "int_" + # Internal repo without prefix result = self.repo_config_register._apply_repo_name_prefix("myrepo", "/path/internal/el7") assert result == "int_myrepo" @@ -244,14 +239,11 @@ def test_apply_repo_name_prefix_internal(self): result = self.repo_config_register._apply_repo_name_prefix("int_myrepo", "/path/internal/el7") assert result == "int_myrepo" - @patch.dict("pulp_manager.app.services.repo_config_register.CONFIG", { - "pulp": { - "internal_package_prefix": "int_" - } - }) def test_apply_repo_name_prefix_neither(self): """Tests that _apply_repo_name_prefix returns original name for repos not in remote or internal paths """ + CONFIG["pulp"]["internal_package_prefix"] = "int_" + result = self.repo_config_register._apply_repo_name_prefix("myrepo", "/path/other/el7") assert result == "myrepo"