diff --git a/scripts/src/submission/serializer.py b/scripts/src/submission/serializer.py index f6117be6..799b4e7a 100644 --- a/scripts/src/submission/serializer.py +++ b/scripts/src/submission/serializer.py @@ -12,23 +12,23 @@ class SubmissionEncoder(json.JSONEncoder): - def default(self, obj): - if isinstance(obj, submission.Submission): - obj_dict = copy.deepcopy(obj.__dict__) + def default(self, o): + if isinstance(o, submission.Submission): + obj_dict = copy.deepcopy(o.__dict__) obj_dict["chart"] = obj_dict["chart"].__dict__ obj_dict["report"] = obj_dict["report"].__dict__ obj_dict["source"] = obj_dict["source"].__dict__ obj_dict["tarball"] = obj_dict["tarball"].__dict__ return obj_dict - return json.JSONEncoder.default(self, obj) + return json.JSONEncoder.default(self, o) class SubmissionDecoder(json.JSONDecoder): def __init__(self, *args, **kwargs): - json.JSONDecoder.__init__(self, object_hook=self.object_hook, *args, **kwargs) + json.JSONDecoder.__init__(self, object_hook=self.custom_hook, *args, **kwargs) - def object_hook(self, dct): + def custom_hook(self, dct): if "chart" in dct: chart_obj = submission.Chart(**dct["chart"]) report_obj = submission.Report(**dct["report"]) diff --git a/scripts/src/submission/submission.py b/scripts/src/submission/submission.py index 165dc315..edd31291 100644 --- a/scripts/src/submission/submission.py +++ b/scripts/src/submission/submission.py @@ -1,6 +1,8 @@ +from dataclasses import dataclass, field import os import re import tarfile + import requests import semver import yaml @@ -10,7 +12,6 @@ except ImportError: from yaml import Loader -from dataclasses import dataclass, field from owners import owners_file from tools import gitutils @@ -19,44 +20,45 @@ xRateLimit = "X-RateLimit-Limit" xRateRemain = "X-RateLimit-Remaining" +REQUEST_TIMEOUT = 10 class SubmissionError(Exception): - """Root Exception for handling any error with the submission""" - - pass + """Root exception for handling any error with the submission""" class DuplicateChartError(SubmissionError): - """This Exception is to be raised when the user attempts to submit a PR with more than one chart""" - - pass + """This exception is raised when the user attempts to submit a PR with more than one chart""" class VersionError(SubmissionError): - """This Exception is to be raised when the version of the chart is not semver compatible""" - - pass + """This exception is raised when the version of the chart is not semver compatible""" class WebCatalogOnlyError(SubmissionError): - pass + """This exception is raised when there is an error when determining the WebCatalogOnly attribute + or if it is inconsistently set + + """ class HelmIndexError(SubmissionError): - pass + """This exception is raised when the chart is already present in the index, or if there was an error + downloading or reading the helm index + + """ class ReleaseTagError(SubmissionError): - pass + """This exception is raised when the chart's release already exists""" class ChartError(SubmissionError): - pass + """This exception is raised when the redhat prefix is incorrectly set""" class TarballContentError(SubmissionError): - pass + """This exception is raised when checking the provided tarball content""" @dataclass @@ -177,7 +179,7 @@ def check_release_tag(self, repository: str): "Authorization": f'Bearer {os.environ.get("BOT_TOKEN")}', } print(f"[INFO] checking tag: {tag_api}") - r = requests.head(tag_api, headers=headers) + r = requests.head(tag_api, headers=headers, timeout=REQUEST_TIMEOUT) if r.status_code == 200: msg = f"[ERROR] Helm chart release already exists in the GitHub Release/Tag: {tag_name}" raise ReleaseTagError(msg) @@ -232,10 +234,10 @@ class Submission: api_url: str modified_files: list[str] = None - chart: Chart = field(default_factory=lambda: Chart()) - report: Report = field(default_factory=lambda: Report()) - source: Source = field(default_factory=lambda: Source()) - tarball: Tarball = field(default_factory=lambda: Tarball()) + chart: Chart = field(default_factory=Chart) + report: Report = field(default_factory=Report) + source: Source = field(default_factory=Source) + tarball: Tarball = field(default_factory=Tarball) modified_owners: list[str] = field(default_factory=list) modified_unknown: list[str] = field(default_factory=list) is_web_catalog_only: bool = None @@ -274,7 +276,7 @@ def _get_modified_files(self): "get", files_api_query, os.environ.get("BOT_TOKEN") ) except SystemExit as e: - raise SubmissionError(e) + raise SubmissionError(e) from e files = r.json() page_size = len(files) @@ -643,7 +645,7 @@ def download_index_data( """ index_url = f"https://raw.githubusercontent.com/{repository}/{branch}/index.yaml" - r = requests.get(index_url) + r = requests.get(index_url, timeout=REQUEST_TIMEOUT) data = {"apiVersion": "v1", "entries": {}} if r.status_code != 200: diff --git a/scripts/src/submission/submission_test.py b/scripts/src/submission/submission_test.py index ffa98901..1a2deaf4 100644 --- a/scripts/src/submission/submission_test.py +++ b/scripts/src/submission/submission_test.py @@ -8,17 +8,17 @@ """ import contextlib -import pytest +from dataclasses import dataclass, field import os +from pathlib import Path import re -import responses import tarfile import tempfile -from dataclasses import dataclass, field -from reporegex import matchers +import pytest +import responses -from pathlib import Path +from reporegex import matchers from submission import submission # Define assets that are being reused accross tests @@ -132,10 +132,10 @@ def create_tarball(tarball_basedir: str, tarball_name: str, tarball_content: lis class SubmissionInitScenario: api_url: str modified_files: list[str] - tarball_content: list[str] = field(default_factory=lambda: list()) + tarball_content: list[str] = field(default_factory=list) expected_submission: submission.Submission = None excepted_exception: contextlib.ContextDecorator = field( - default_factory=lambda: contextlib.nullcontext() + default_factory=contextlib.nullcontext ) @@ -583,7 +583,7 @@ class WebCatalogOnlyScenario: report_web_catalog_only: str = None # Set to None to skip key creation in report expected_output: bool = None excepted_exception: contextlib.ContextDecorator = field( - default_factory=lambda: contextlib.nullcontext() + default_factory=contextlib.nullcontext ) @@ -706,34 +706,36 @@ def test_parse_web_catalog_only(test_scenario): # Populate OWNERS file if test_scenario.create_owners: - owners_file = open(os.path.join(owners_base_path, "OWNERS"), "w") - owners_file.write( - "publicPgpKey: unknown" - ) # Make sure OWNERS is not an empty file - if test_scenario.owners_web_catalog_only: + with open( + os.path.join(owners_base_path, "OWNERS"), "w", encoding="utf-8" + ) as owners_file: owners_file.write( - f"\nproviderDelivery: {test_scenario.owners_web_catalog_only}" - ) - owners_file.close() + "publicPgpKey: unknown" + ) # Make sure OWNERS is not an empty file + if test_scenario.owners_web_catalog_only: + owners_file.write( + f"\nproviderDelivery: {test_scenario.owners_web_catalog_only}" + ) # Populate report.yaml file if test_scenario.create_report: - report_file = open(os.path.join(chart_base_path, "report.yaml"), "w") - report_file.writelines( - [ - "apiversion: v1", - "\nkind: verify-report", - ] - ) - if test_scenario.report_web_catalog_only: + with open( + os.path.join(chart_base_path, "report.yaml"), "w", encoding="utf-8" + ) as report_file: report_file.writelines( [ - "\nmetadata:", - "\n tool:", - f"\n webCatalogOnly: {test_scenario.report_web_catalog_only}", + "apiversion: v1", + "\nkind: verify-report", ] ) - report_file.close() + if test_scenario.report_web_catalog_only: + report_file.writelines( + [ + "\nmetadata:", + "\n tool:", + f"\n webCatalogOnly: {test_scenario.report_web_catalog_only}", + ] + ) with test_scenario.excepted_exception: test_scenario.input_submission.parse_web_catalog_only(repo_path=temp_dir) @@ -818,23 +820,24 @@ def test_is_valid_web_catalog_only(test_scenario): # Populate report.yaml file if test_scenario.create_report: - report_file = open(os.path.join(chart_base_path, "report.yaml"), "w") - report_file.writelines( - [ - "apiversion: v1", - "\nkind: verify-report", - ] - ) - if test_scenario.report_has_digest: + with open( + os.path.join(chart_base_path, "report.yaml"), "w", encoding="utf-8" + ) as report_file: report_file.writelines( [ - "\nmetadata:", - "\n tool:", - "\n digests:", - "\n package: 7755e7cf43e55bbf2edafd9788b773b844fb15626c5ff8ff7a30a6d9034f3a33", + "apiversion: v1", + "\nkind: verify-report", ] ) - report_file.close() + if test_scenario.report_has_digest: + report_file.writelines( + [ + "\nmetadata:", + "\n tool:", + "\n digests:", + "\n package: 7755e7cf43e55bbf2edafd9788b773b844fb15626c5ff8ff7a30a6d9034f3a33", + ] + ) is_valid_web_catalog_only, reason = ( test_scenario.input_submission.is_valid_web_catalog_only(repo_path=temp_dir) @@ -846,12 +849,15 @@ def test_is_valid_web_catalog_only(test_scenario): assert test_scenario.expected_reason in reason -def create_new_index(charts: list[submission.Chart] = []): +def create_new_index(charts: list[submission.Chart] = None): """Create the JSON representation of a Helm chart index containing the provided list of charts The resulting index only contains the required information for the check_index to work. """ + if charts is None: + charts = [] + index = {"apiVersion": "v1", "entries": {}} for chart in charts: @@ -878,9 +884,9 @@ class CheckIndexScenario: version=expected_version, ) ) - index: dict = field(default_factory=lambda: create_new_index()) + index: dict = field(default_factory=create_new_index) excepted_exception: contextlib.ContextDecorator = field( - default_factory=lambda: contextlib.nullcontext() + default_factory=contextlib.nullcontext ) @@ -931,9 +937,9 @@ class CheckReleaseTagScenario: version=expected_version, ) ) - exising_tags: list[str] = field(default_factory=lambda: list()) + exising_tags: list[str] = field(default_factory=list) excepted_exception: contextlib.ContextDecorator = field( - default_factory=lambda: contextlib.nullcontext() + default_factory=contextlib.nullcontext ) @@ -965,7 +971,6 @@ def test_check_release_tag(test_scenario): if chart_release_tag not in test_scenario.exising_tags: responses.head( f"https://api.github.com/repos/my-fake-org/my-fake-repo/git/ref/tags/{chart_release_tag}", - # json=[{"filename": file} for file in test_scenario.modified_files], status=404, ) @@ -973,7 +978,6 @@ def test_check_release_tag(test_scenario): # Mock GitHub API responses.head( f"https://api.github.com/repos/my-fake-org/my-fake-repo/git/ref/tags/{tag}", - # json=[{"filename": file} for file in test_scenario.modified_files], ) with test_scenario.excepted_exception: diff --git a/scripts/src/submission/validate.py b/scripts/src/submission/validate.py index 602cad68..0098164b 100644 --- a/scripts/src/submission/validate.py +++ b/scripts/src/submission/validate.py @@ -14,13 +14,13 @@ def write_submission_to_file(s: submission.Submission, artifact_path: str): """Save a JSON representation of the Submission object to file.""" data = serializer.SubmissionEncoder().encode(s) - with open(artifact_path, "w") as f: + with open(artifact_path, "w", encoding="utf-8") as f: f.write(data) def read_submission_from_file(articact_path: str) -> submission.Submission: """Read and load the JSON representation of the Submission object from file.""" - with open(articact_path, "r") as f: + with open(articact_path, "r", encoding="utf-8") as f: s = json.load(f, cls=serializer.SubmissionDecoder) return s @@ -44,7 +44,7 @@ def craft_pr_content_error_msg( try: s.parse_modified_files(repo_path="pr-branch") except submission.SubmissionError as e: - raise ParseFilesError(str(e)) + raise ParseFilesError(str(e)) from e # Checks that this PR is a valid "Chart certification" PR is_valid, msg = s.is_valid_certification_submission(ignore_owners=True)