From 1108810bdefd0d880517b274acd6a3bd0d4156e0 Mon Sep 17 00:00:00 2001 From: Ondrej Nosek Date: Tue, 21 Mar 2023 02:44:04 +0100 Subject: [PATCH 07/12] More robust spec file presence checking Some commands (verrel, sources, prep, import, ...) need to check whether the dist-git repository is in the correct state. It means at least the presence of the specfile. In the beginning, rpkg detects layouts. Layouts determine the file structure of the repository. For example, most commands can't be executed for the RetiredLayout (there is no specfile). When the repository directory exists, some layout can be always detected. Therefore '--path' argument is now checked for a valid directory. The timeout change in the request fixes the new bandit's finding. Fixes: #663 JIRA: RHELCMP-11387 Signed-off-by: Ondrej Nosek --- pyrpkg/__init__.py | 9 ++++--- pyrpkg/cli.py | 8 +++--- pyrpkg/layout/__init__.py | 4 +-- pyrpkg/utils.py | 14 ++++++++++ tests/commands/test_push.py | 54 +++++++++++++++++++------------------ tests/test_cli.py | 12 ++++++--- 6 files changed, 63 insertions(+), 38 deletions(-) diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index 776cb21..028d195 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -923,9 +923,8 @@ class Commands(object): def load_spec(self): """This sets the spec attribute""" - if self.layout is None: + if self.layout is None or isinstance(self.layout, layout.IncompleteLayout): raise rpkgError('Spec file is not available') - if self.is_retired(): raise rpkgError('This package or module is retired. The action has stopped.') @@ -1166,8 +1165,10 @@ class Commands(object): @property def sources_filename(self): - if self.layout is None: - return os.path.join(self.path, 'sources') + if self.layout is None or isinstance(self.layout, layout.IncompleteLayout): + raise rpkgError('Spec file is not available') + if self.is_retired(): + raise rpkgError('This package or module is retired. The action has stopped.') return os.path.join( self.path, self.layout.sources_file_template.replace("{0.repo_name}", self.repo_name)) diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index c3672b3..1bcf6e4 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -386,7 +386,7 @@ class cliClient(object): help='Run Koji commands as a different user') # Let the user define a path to work in rather than cwd self.parser.add_argument('--path', default=None, - type=utils.u, + type=utils.validate_path, help='Define the directory to work in ' '(defaults to cwd)') # Verbosity @@ -911,8 +911,9 @@ class cliClient(object): if 'path' in args: # Without "path", we can't really test... url = '%(protocol)s://%(host)s/%(path)s/info/refs?service=git-receive-pack' % args - resp = requests.head(url, auth=HTTPBasicAuth(args['username'], - args['password'])) + resp = requests.head(url, + auth=HTTPBasicAuth(args['username'], args['password']), + timeout=15) if resp.status_code == 401: return self.oidc_client.report_token_issue() @@ -2363,6 +2364,7 @@ class cliClient(object): def import_srpm(self): uploadfiles = self.cmd.import_srpm(self.args.srpm) + self.load_cmd() # to reload layouts - because a specfile could appear during import if uploadfiles: try: self.cmd.upload(uploadfiles, replace=True, offline=self.args.offline) diff --git a/pyrpkg/layout/__init__.py b/pyrpkg/layout/__init__.py index 762af0d..850ddc2 100644 --- a/pyrpkg/layout/__init__.py +++ b/pyrpkg/layout/__init__.py @@ -12,8 +12,8 @@ from pyrpkg.errors import LayoutError from .base import MetaLayout -from .layouts import (DistGitLayout, IncompleteLayout, # noqa: F401 - RetiredLayout, SRPMLayout) +from .layouts import (DistGitLayout, DistGitResultsDirLayout, # noqa: F401 + IncompleteLayout, RetiredLayout, SRPMLayout) def build(path, hint=None): diff --git a/pyrpkg/utils.py b/pyrpkg/utils.py index ceb4906..3337bdb 100644 --- a/pyrpkg/utils.py +++ b/pyrpkg/utils.py @@ -26,11 +26,25 @@ if six.PY3: def u(s): return s + def validate_path(s): + abspath = os.path.abspath(s) + if os.path.exists(abspath): + return s + else: + raise argparse.ArgumentTypeError('given path \'{0}\' doesn\'t exist'.format(abspath)) + getcwd = os.getcwd else: def u(s): return s.decode('utf-8') + def validate_path(s): + abspath = os.path.abspath(s.decode('utf-8')) + if os.path.exists(abspath): + return s.decode('utf-8') + else: + raise argparse.ArgumentTypeError('given path \'{0}\' doesn\'t exist'.format(abspath)) + getcwd = os.getcwdu diff --git a/tests/commands/test_push.py b/tests/commands/test_push.py index ef8057a..79c3a8b 100644 --- a/tests/commands/test_push.py +++ b/tests/commands/test_push.py @@ -1,9 +1,13 @@ # -*- coding: utf-8 -*- import os +import subprocess import git +import pyrpkg +from pyrpkg.sources import SourcesFile + from . import CommandTestCase SPECFILE_TEMPLATE = """Name: test @@ -22,11 +26,6 @@ Test %%install rm -f $RPM_BUILD_ROOT%%{_sysconfdir}/""" -CLONE_CONFIG = ''' - bz.default-component %(module)s - sendemail.to %(module)s-owner@fedoraproject.org -''' - class CommandPushTestCase(CommandTestCase): @@ -45,28 +44,30 @@ class CommandPushTestCase(CommandTestCase): self.make_new_git(self.module) - import pyrpkg - cmd = pyrpkg.Commands(self.path, self.lookaside, - self.lookasidehash, - self.lookaside_cgi, self.gitbaseurl, - self.anongiturl, self.branchre, self.kojiprofile, - self.build_client, self.user, self.dist, - self.target, self.quiet) - cmd.clone_config_rpms = CLONE_CONFIG - cmd.clone(self.module, anon=True) - cmd.path = os.path.join(self.path, self.module) - os.chdir(os.path.join(self.path, self.module)) + moduledir = os.path.join(self.gitroot, self.module) + subprocess.check_call(['git', 'clone', 'file://%s' % moduledir], + cwd=self.path, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + self.cloned_dir = os.path.join(self.path, self.module) + self.cmd = pyrpkg.Commands(self.cloned_dir, self.lookaside, + self.lookasidehash, + self.lookaside_cgi, self.gitbaseurl, + self.anongiturl, self.branchre, self.kojiprofile, + self.build_client, self.user, self.dist, + self.target, self.quiet) + os.chdir(self.cloned_dir) spec_file = 'module.spec' with open(spec_file, 'w') as f: f.write(SPECFILE_TEMPLATE % '') - cmd.repo.index.add([spec_file]) - cmd.repo.index.commit("add SPEC") + self.cmd.repo.index.add([spec_file]) + self.cmd.repo.index.commit("add SPEC") # Now, change directory to parent and test the push os.chdir(self.path) - cmd.push(no_verify=True) + self.cmd.push(no_verify=True) class TestPushWithPatches(CommandTestCase): @@ -76,18 +77,20 @@ class TestPushWithPatches(CommandTestCase): self.make_new_git(self.module) - import pyrpkg - self.cmd = pyrpkg.Commands(self.path, self.lookaside, + moduledir = os.path.join(self.gitroot, self.module) + subprocess.check_call(['git', 'clone', 'file://%s' % moduledir], + cwd=self.path, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + self.cloned_dir = os.path.join(self.path, self.module) + self.cmd = pyrpkg.Commands(self.cloned_dir, self.lookaside, self.lookasidehash, self.lookaside_cgi, self.gitbaseurl, self.anongiturl, self.branchre, self.kojiprofile, self.build_client, self.user, self.dist, self.target, self.quiet) - self.cmd.clone_config_rpms = CLONE_CONFIG - self.cmd.clone(self.module, anon=True) - self.cmd.path = os.path.join(self.path, self.module) - os.chdir(os.path.join(self.path, self.module)) + os.chdir(self.cloned_dir) # Track SPEC and a.patch in git spec_file = 'module.spec' @@ -103,7 +106,6 @@ Patch3: d.path f.write(patch_file) # Track c.patch in sources - from pyrpkg.sources import SourcesFile sources_file = SourcesFile(self.cmd.sources_filename, self.cmd.source_entry_type) file_hash = self.cmd.lookasidecache.hash_file('c.patch') diff --git a/tests/test_cli.py b/tests/test_cli.py index df053aa..868ad1f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1841,9 +1841,11 @@ class TestMockbuild(CliTestCase): @patch('pyrpkg.Commands._config_dir_basic') @patch('pyrpkg.Commands._config_dir_other') @patch('os.path.exists', return_value=False) + @patch('pyrpkg.utils.validate_path') def test_use_mock_config_got_from_koji( - self, exists, config_dir_other, config_dir_basic): + self, validate_path, exists, config_dir_other, config_dir_basic): mock_layout = layout.DistGitLayout(root_dir=self.cloned_repo_path) + validate_path.return_value = self.cloned_repo_path with patch('pyrpkg.layout.build', return_value=mock_layout): config_dir_basic.return_value = '/path/to/config-dir' @@ -1859,9 +1861,11 @@ class TestMockbuild(CliTestCase): @patch('pyrpkg.Commands._config_dir_basic') @patch('os.path.exists', return_value=False) + @patch('pyrpkg.utils.validate_path') def test_fail_to_store_mock_config_in_created_config_dir( - self, exists, config_dir_basic): + self, validate_path, exists, config_dir_basic): config_dir_basic.side_effect = rpkgError + validate_path.return_value = self.cloned_repo_path cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-7', 'mockbuild'] @@ -1870,10 +1874,12 @@ class TestMockbuild(CliTestCase): @patch('pyrpkg.Commands._config_dir_basic') @patch('pyrpkg.Commands._config_dir_other') @patch('os.path.exists', return_value=False) + @patch('pyrpkg.utils.validate_path') def test_fail_to_populate_mock_config( - self, exists, config_dir_other, config_dir_basic): + self, validate_path, exists, config_dir_other, config_dir_basic): config_dir_basic.return_value = '/path/to/config-dir' config_dir_other.side_effect = rpkgError + validate_path.return_value = self.cloned_repo_path cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-7', 'mockbuild'] -- 2.39.2