From 33d23d942078341a4a9d0da9f8788bc610e997c3 Mon Sep 17 00:00:00 2001 From: Jan Staněk Date: Aug 10 2016 12:53:12 +0000 Subject: [PATCH 1/7] Rewrite; Add dist-git sanity check --- diff --git a/modlint b/modlint index 0cc9a94..b27aa65 100755 --- a/modlint +++ b/modlint @@ -1,23 +1,85 @@ #!/usr/bin/python3 -import modulemd + + +import argparse +from typing import Generator import sys -def usage(): - print("Usage: modlint ") - sys.exit() +import requests +from modulemd import ModuleMetadata + + +def existing_content(mmd: ModuleMetadata) -> Generator[str, None, None]: + """Check that rpm content points to existing repo (commit). + + For each package in the module, this function queries the dist-git + and use the status code for determining if the package (and + optionally commit) exists. + + Keyword arguments: + mmd -- The checked module metadata object. + + Yields: + Error messages for invalid packages. + """ + + ERRORS = { + 400: 'Bad hash', + 404: 'Nonexistent rpm', + } + + # TODO: Get URL from the metadata + CGIT_URL_TEMPLATE = 'http://pkgs.fedoraproject.org/cgit/rpms/{name}.git/commit' + + try: + packages = mmd.components.rpms.packages + except AttributeError: + return None + + for package, details in packages.items(): + if 'commit' in details: + payload = {'id': details['commit']} + else: + payload = {} + + response = requests.head( + CGIT_URL_TEMPLATE.format(name=package), + params=payload + ) + + # XXX: Maybe yield tuple instead? + if response.status_code != requests.codes.ok: + yield '{pkg [{hash}]: {msg}}'.format( + pkg=package, + hash=(details['commit'] if 'commit' in details else 'HEAD'), + msg=ERRORS[response.status_code] + ) + + +if __name__ == '__main__': + parser = argparse.ArgumentParser(description='Validate module metadata.') + + # Positional arguments + parser.add_argument('file', help='Input metadata file') + + args = parser.parse_args() + + metadata = ModuleMetadata() -if (len(sys.argv) != 2): - usage() + all_went_well = True -filename = sys.argv[1] -modulemd = modulemd.ModuleMetadata() + try: + metadata.load(args.file) + metadata.validate() + except Exception as exc: + all_went_well = False + print('ERROR:', str(exc), file=sys.stderr) -try: - modulemd.load(filename) - modulemd.validate() -except Exception as e: - # raise - print("ERROR: ", str(e)) - sys.exit() + for error in existing_content(metadata): + all_went_well = False + print('RPM CONTENT ERROR:', error, file=sys.stderr) -print("Everything is OK!") + if all_went_well: + print('Everything OK') + else: + raise SystemExit(1) From c0eb120c940a37d4a900f74cc6c40c045bbffc40 Mon Sep 17 00:00:00 2001 From: Jan Staněk Date: Aug 11 2016 08:32:06 +0000 Subject: [PATCH 2/7] Get default hash using builtin method - Fix typo in format string --- diff --git a/modlint b/modlint index b27aa65..df7c30c 100755 --- a/modlint +++ b/modlint @@ -49,9 +49,9 @@ def existing_content(mmd: ModuleMetadata) -> Generator[str, None, None]: # XXX: Maybe yield tuple instead? if response.status_code != requests.codes.ok: - yield '{pkg [{hash}]: {msg}}'.format( + yield '{pkg} [{hash}]: {msg}'.format( pkg=package, - hash=(details['commit'] if 'commit' in details else 'HEAD'), + hash=details.get('commit', 'HEAD'), msg=ERRORS[response.status_code] ) From 082b885a5e77e7efdd38d9684956720686f88e55 Mon Sep 17 00:00:00 2001 From: Jan Staněk Date: Aug 11 2016 08:34:23 +0000 Subject: [PATCH 3/7] Split metadata loading and validation to separate steps --- diff --git a/modlint b/modlint index df7c30c..fec3e39 100755 --- a/modlint +++ b/modlint @@ -70,10 +70,16 @@ if __name__ == '__main__': try: metadata.load(args.file) + except ValueError as invalid_input_metadata: + message = 'ERROR: Invalid input: {!s}'.format(invalid_input_metadata) + raise SystemExit(message) + + try: metadata.validate() - except Exception as exc: + except (TypeError, ValueError) as invalid_metadata_structure: all_went_well = False - print('ERROR:', str(exc), file=sys.stderr) + print('ERROR: Invalid structure:', str(invalid_metadata_structure), + file=sys.stderr) for error in existing_content(metadata): all_went_well = False From 4fb5a70ea0d4c15798b941d37b3e80e89c5f5486 Mon Sep 17 00:00:00 2001 From: Jan Staněk Date: Aug 11 2016 12:18:28 +0000 Subject: [PATCH 4/7] Report rpm content errors with exceptions - Introduce dedicated exception for RPM content (`RpmContentError`) - Rename existing_content to verify_rpm_content and limit its scope to single SRPM package - Add rpm_content generator for easier access to the content - Change `SystemExit` to `sys.exit` - Drop typing module --- diff --git a/modlint b/modlint index fec3e39..4f42bc6 100755 --- a/modlint +++ b/modlint @@ -2,58 +2,92 @@ import argparse -from typing import Generator import sys import requests from modulemd import ModuleMetadata -def existing_content(mmd: ModuleMetadata) -> Generator[str, None, None]: - """Check that rpm content points to existing repo (commit). +class RpmContentError(ValueError): + """Metadata contains invalid/nonexistent srpm or commit.""" - For each package in the module, this function queries the dist-git - and use the status code for determining if the package (and - optionally commit) exists. + REASONS = { + 400: 'Bad hash', + 404: 'Nonexistent rpm', + } + """Mapping of HTTP status codes to actual failure reasons.""" + + def __init__(self, package, commit, status_code): + super().__init__(self, package, commit, status_code) + + self.package = package + self.commit = commit + self.status_code = status_code + + def __repr__(self): + return 'RpmContentError({pkg}, {commit}, {status})'.format( + pkg=self.package, + commit=self.commit, + status=self.status_code + ) + + def __str__(self): + return '{pkg} [{commit}]: {reason}'.format( + pkg=self.package, + commit=self.commit, + reason=self.REASONS.get(self.status_code, 'Unknown error') + ) + + +def verify_rpm_content(package, metadata, *, requests=requests): + """Verify the existence of dist-git repo for described package. Keyword arguments: - mmd -- The checked module metadata object. + package -- Name of the package to verify (key in + modulemd packages) + metadata -- Metadata associated with the package + (value in modulemd packages) - Yields: - Error messages for invalid packages. - """ + requests -- Dependency injection of networking library. - ERRORS = { - 400: 'Bad hash', - 404: 'Nonexistent rpm', - } + Returns: + None if the associated repo exists. + + Raises: + RpmContentError -- when the package (or commit) does not exist + in dist-git. + """ # TODO: Get URL from the metadata CGIT_URL_TEMPLATE = 'http://pkgs.fedoraproject.org/cgit/rpms/{name}.git/commit' + commit = metadata.get('commit', 'HEAD') + + response = requests.head( + CGIT_URL_TEMPLATE.format(name=package), + params={'id': commit} + ) + + if response.status_code != requests.codes.ok: + raise RpmContentError(package, commit, response.status_code) + + +def rpm_content(mmd): + """Generate (package, metadata) pairs from module. + + Keyword arguments: + mmd -- ModuleMetadata to be inspected. + + Yields: + (package, metadata) from mmd, if any. + """ + try: packages = mmd.components.rpms.packages except AttributeError: return None - for package, details in packages.items(): - if 'commit' in details: - payload = {'id': details['commit']} - else: - payload = {} - - response = requests.head( - CGIT_URL_TEMPLATE.format(name=package), - params=payload - ) - - # XXX: Maybe yield tuple instead? - if response.status_code != requests.codes.ok: - yield '{pkg} [{hash}]: {msg}'.format( - pkg=package, - hash=details.get('commit', 'HEAD'), - msg=ERRORS[response.status_code] - ) + yield from packages.items() if __name__ == '__main__': @@ -65,27 +99,31 @@ if __name__ == '__main__': args = parser.parse_args() metadata = ModuleMetadata() - + metadata_errors = TypeError, ValueError all_went_well = True try: metadata.load(args.file) - except ValueError as invalid_input_metadata: + except metadata_errors as invalid_input_metadata: message = 'ERROR: Invalid input: {!s}'.format(invalid_input_metadata) - raise SystemExit(message) + sys.exit(message) try: metadata.validate() - except (TypeError, ValueError) as invalid_metadata_structure: + except metadata_errors as invalid_metadata_structure: all_went_well = False print('ERROR: Invalid structure:', str(invalid_metadata_structure), file=sys.stderr) - for error in existing_content(metadata): - all_went_well = False - print('RPM CONTENT ERROR:', error, file=sys.stderr) + for package in rpm_content(metadata): + try: + verify_rpm_content(*package) + except RpmContentError as invalid_rpm: + all_went_well = False + print('RPM CONTENT ERROR:', str(invalid_rpm), + file=sys.stderr) if all_went_well: print('Everything OK') else: - raise SystemExit(1) + sys.exit(1) From f06b635d44f9769deee35016f691e7683f528aa8 Mon Sep 17 00:00:00 2001 From: Jan Staněk Date: Aug 23 2016 07:16:08 +0000 Subject: [PATCH 5/7] Change attribute docstring to documentation comment --- diff --git a/modlint b/modlint index 4f42bc6..75c83ac 100755 --- a/modlint +++ b/modlint @@ -11,11 +11,11 @@ from modulemd import ModuleMetadata class RpmContentError(ValueError): """Metadata contains invalid/nonexistent srpm or commit.""" + #: Mapping of HTTP status codes to actual failure reasons. REASONS = { 400: 'Bad hash', 404: 'Nonexistent rpm', } - """Mapping of HTTP status codes to actual failure reasons.""" def __init__(self, package, commit, status_code): super().__init__(self, package, commit, status_code) From 7517e89cae8d74b6220c5763073e209e3ee68f2c Mon Sep 17 00:00:00 2001 From: Jan Staněk Date: Aug 23 2016 07:17:38 +0000 Subject: [PATCH 6/7] Add proper quoting to __repr__ string --- diff --git a/modlint b/modlint index 75c83ac..2a30127 100755 --- a/modlint +++ b/modlint @@ -25,7 +25,7 @@ class RpmContentError(ValueError): self.status_code = status_code def __repr__(self): - return 'RpmContentError({pkg}, {commit}, {status})'.format( + return 'RpmContentError({pkg!r}, {commit!r}, {status!r})'.format( pkg=self.package, commit=self.commit, status=self.status_code From eb7b15f7a3937fd4e0b0266b142169b4022e5396 Mon Sep 17 00:00:00 2001 From: Jan Staněk Date: Aug 23 2016 07:18:44 +0000 Subject: [PATCH 7/7] Add explanation for returning from generator early --- diff --git a/modlint b/modlint index 2a30127..d77d9d9 100755 --- a/modlint +++ b/modlint @@ -85,6 +85,7 @@ def rpm_content(mmd): try: packages = mmd.components.rpms.packages except AttributeError: + # Module has no packages -- end generation with no output return None yield from packages.items()