From b09882687238f35c74e7bbca28dd3e822e601e07 Mon Sep 17 00:00:00 2001 From: rthallisey Date: Tue, 8 Oct 2019 09:29:38 -0400 Subject: [PATCH 1/2] Add Regex support to OMPS The current pattern for registry string replacement only matches on whole strings. This pattern is very broad and lacks the precision to handle corner cases like strings that only end or begin with '/'. Regexes provide the precision needed to expand pattern matching to handle more precise string replacement. --- README.md | 18 +++++++++-- example.test.env.yaml | 1 + omps/quay.py | 7 ++++- tests/api/test_common.py | 2 +- .../replace_registry/replace_registry_test.py | 2 +- tests/test_quay.py | 16 ++++++++-- tests/test_settings.py | 30 +++++++++++++++++++ 7 files changed, 68 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 09926b0..3ff0130 100644 --- a/README.md +++ b/README.md @@ -80,13 +80,25 @@ organization: { "old": "quay.io", "new": "example.com", + "regexp": "", }, ] ``` -all specified `old` registries will be replaced by `new` in all manifests yaml -files for that organization. Replacement happen during pushing manifests into -application registry. +All specified `old` registries will be replaced by `new` in all manifests yaml +files for that organization. +You can pattern match registry strings with the regexp field instead of matching +whole strings with 'old': +``` +"replace_registry": [ + { + "old": "", + "new": "example.com", + "regexp": "quay.io$", + }, +] +``` +Replacements occur when pushing manifests into the application registry. ### Greenwave integration diff --git a/example.test.env.yaml b/example.test.env.yaml index 6e0427b..26bd395 100644 --- a/example.test.env.yaml +++ b/example.test.env.yaml @@ -42,6 +42,7 @@ private_org: replace_registry: - old: registry.stage.redhat.io new: registry.redhat.io + regex: "" # Config of the instance tested to check if Greenwave policies are met for the # NVR greenwave: diff --git a/omps/quay.py b/omps/quay.py index 5903023..d7076d6 100644 --- a/omps/quay.py +++ b/omps/quay.py @@ -259,7 +259,12 @@ def replace_registries(self, text): for conf in self._replace_registry_conf: old = conf['old'] new = conf['new'] - if old in text: + regexp = conf['regexp'] + + if regexp is not "": + self.logger.info("Replacing registries with regexp '%s'", regexp) + text = re.sub(regexp, new, text, flags=re.MULTILINE) + elif old in text: self.logger.info("Replacing registry '%s' with '%s'", old, new) text = text.replace(old, new) diff --git a/tests/api/test_common.py b/tests/api/test_common.py index d0e5619..4beb0bb 100644 --- a/tests/api/test_common.py +++ b/tests/api/test_common.py @@ -22,7 +22,7 @@ def _yield_yaml_files(dir_path): old = 'quay.io' new = 'example.com' qo = QuayOrganization('testorg', 'random token', replace_registry_conf=[ - {'old': old, 'new': new} + {'old': old, 'new': new, 'regexp': ""} ]) should_be_replaced = set() diff --git a/tests/integration/replace_registry/replace_registry_test.py b/tests/integration/replace_registry/replace_registry_test.py index 5a7dfc8..3e9506d 100644 --- a/tests/integration/replace_registry/replace_registry_test.py +++ b/tests/integration/replace_registry/replace_registry_test.py @@ -41,7 +41,7 @@ def koji_registries(replace_conf, path): the manifest at 'path' Args: - replace_conf: List of {'old': ..., 'new': ...} dictionaries. + replace_conf: List of {'old': ..., 'new': ..., 'regexp': ...} dictionaries. path: Path of the manifest. Returns: diff --git a/tests/test_quay.py b/tests/test_quay.py index 3c9fd24..88d345e 100644 --- a/tests/test_quay.py +++ b/tests/test_quay.py @@ -446,7 +446,7 @@ def test_publish_repo_error(self): def test_registry_replacing_enabled(self, enabled): """Test if property returns correct value""" if enabled: - replace_conf = [{'old': 'reg_old', 'new': 'reg_new'}] + replace_conf = [{'old': 'reg_old', 'new': 'reg_new', 'regexp': ''}] else: replace_conf = None @@ -468,7 +468,7 @@ def test_registry_replacing_enabled(self, enabled): ]) def test_replace_registries(self, text, expected): """Test if registries are replaced properly""" - replace_conf = [{'old': 'reg_old', 'new': 'reg_new'}] + replace_conf = [{'old': 'reg_old', 'new': 'reg_new', 'regexp': ''}] org = 'testorg' qo = QuayOrganization(org, TOKEN, replace_registry_conf=replace_conf) assert qo.replace_registries(text) == expected @@ -483,6 +483,18 @@ def test_replace_registries_unconfigured(self): assert res == text assert id(res) == id(text) + @pytest.mark.parametrize('text,expected', [ + ( + 'Registry reg_old will be replaced using a regexp: reg_old', + 'Registry reg_old will be replaced using a regexp: reg_new', + ) + ]) + def test_regexp_replace_registries(self, text, expected): + """Test if registries are replaced properly""" + replace_conf = [{'old': '', 'new': 'reg_new', 'regexp': 'reg_old$'}] + org = 'testorg' + qo = QuayOrganization(org, TOKEN, replace_registry_conf=replace_conf) + assert qo.replace_registries(text) == expected class TestOrgManager: """Tets for OrgManager class""" diff --git a/tests/test_settings.py b/tests/test_settings.py index 946011d..79c9924 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -118,6 +118,30 @@ def test_organizations(): { 'old': 'quay.io', 'new': 'example.com', + 'regexp': '', + }, + ] + } + } + + class ConfClass(DefaultConfig): + ORGANIZATIONS = expected + + conf = Config(ConfClass) + assert conf.organizations == expected + + +def test_regexp(): + """Regexp Test""" + expected = { + 'myorg': { + 'public': False, + 'oauth_token': 'token', + 'replace_registry': [ + { + 'old': 'quay.io/', + 'new': 'example.com', + 'regexp': 'quay.io$', }, ] } @@ -161,6 +185,12 @@ class ConfClass(DefaultConfig): {'old': 'expected new too'} ] } + }, { + 'organization': { + 'replace_registry': [ + {'regexp': 'expected regexp too'} + ] + } } ]) From 208b6d1cb7cfcabfeb2ea99986accd77dd47029c Mon Sep 17 00:00:00 2001 From: rthallisey Date: Tue, 8 Oct 2019 15:27:18 -0400 Subject: [PATCH 2/2] Make regexp backwards compatible --- README.md | 11 ++++++----- omps/quay.py | 14 +++++++++----- tests/test_quay.py | 9 +++++---- tests/test_settings.py | 11 +++++------ 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 3ff0130..a5d6289 100644 --- a/README.md +++ b/README.md @@ -80,21 +80,22 @@ organization: { "old": "quay.io", "new": "example.com", - "regexp": "", }, ] ``` All specified `old` registries will be replaced by `new` in all manifests yaml files for that organization. -You can pattern match registry strings with the regexp field instead of matching -whole strings with 'old': +You can pattern match and replace registry strings with the regexp field instead +of matching whole strings. Both `old` and `new` will be evalutated as regexes +when `regexp` is set to `True`. If `regexp` is missing it defaults to `False`. +Here's an example: ``` "replace_registry": [ { - "old": "", + "old": "quay.io$", "new": "example.com", - "regexp": "quay.io$", + "regexp": "True", }, ] ``` diff --git a/omps/quay.py b/omps/quay.py index d7076d6..999ca0f 100644 --- a/omps/quay.py +++ b/omps/quay.py @@ -257,13 +257,17 @@ def replace_registries(self, text): return text for conf in self._replace_registry_conf: - old = conf['old'] new = conf['new'] - regexp = conf['regexp'] + old = conf['old'] + regexp = False + + if 'regexp' in conf.keys(): + regexp = conf['regexp'] - if regexp is not "": - self.logger.info("Replacing registries with regexp '%s'", regexp) - text = re.sub(regexp, new, text, flags=re.MULTILINE) + if regexp: + if re.search(old, text, flags=re.MULTILINE): + self.logger.info("Replacing registries with regexp '%s'", regexp) + text = re.sub(old, new, text, flags=re.MULTILINE) elif old in text: self.logger.info("Replacing registry '%s' with '%s'", old, new) text = text.replace(old, new) diff --git a/tests/test_quay.py b/tests/test_quay.py index 88d345e..d3abfe5 100644 --- a/tests/test_quay.py +++ b/tests/test_quay.py @@ -446,7 +446,7 @@ def test_publish_repo_error(self): def test_registry_replacing_enabled(self, enabled): """Test if property returns correct value""" if enabled: - replace_conf = [{'old': 'reg_old', 'new': 'reg_new', 'regexp': ''}] + replace_conf = [{'old': 'reg_old', 'new': 'reg_new'}] else: replace_conf = None @@ -468,7 +468,7 @@ def test_registry_replacing_enabled(self, enabled): ]) def test_replace_registries(self, text, expected): """Test if registries are replaced properly""" - replace_conf = [{'old': 'reg_old', 'new': 'reg_new', 'regexp': ''}] + replace_conf = [{'old': 'reg_old', 'new': 'reg_new'}] org = 'testorg' qo = QuayOrganization(org, TOKEN, replace_registry_conf=replace_conf) assert qo.replace_registries(text) == expected @@ -490,12 +490,13 @@ def test_replace_registries_unconfigured(self): ) ]) def test_regexp_replace_registries(self, text, expected): - """Test if registries are replaced properly""" - replace_conf = [{'old': '', 'new': 'reg_new', 'regexp': 'reg_old$'}] + """Test if registries are replaced properly with regexp""" + replace_conf = [{'old': 'reg_old$', 'new': 'reg_new', 'regexp': 'True'}] org = 'testorg' qo = QuayOrganization(org, TOKEN, replace_registry_conf=replace_conf) assert qo.replace_registries(text) == expected + class TestOrgManager: """Tets for OrgManager class""" diff --git a/tests/test_settings.py b/tests/test_settings.py index 79c9924..b8e480e 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -118,7 +118,6 @@ def test_organizations(): { 'old': 'quay.io', 'new': 'example.com', - 'regexp': '', }, ] } @@ -131,17 +130,17 @@ class ConfClass(DefaultConfig): assert conf.organizations == expected -def test_regexp(): - """Regexp Test""" +def test_organziation_regexp(): + """Organziation with regexp Test""" expected = { 'myorg': { 'public': False, 'oauth_token': 'token', 'replace_registry': [ { - 'old': 'quay.io/', + 'old': 'quay.io/$', 'new': 'example.com', - 'regexp': 'quay.io$', + 'regexp': 'True', }, ] } @@ -188,7 +187,7 @@ class ConfClass(DefaultConfig): }, { 'organization': { 'replace_registry': [ - {'regexp': 'expected regexp too'} + {'regexp': 'False'} ] } }