From dbf61ad6922ad883f60de9799cef94396f37311b Mon Sep 17 00:00:00 2001 From: Matthias Goerens Date: Tue, 10 Dec 2024 15:57:12 +0100 Subject: [PATCH] Refactor get-verifiy-param to use Submission This commit doesn't change the behavior of the get-verify-param step. It builds upon the new Submission data structure to avoid additional calls to the GitHub API and improve some readability aspects of the code. This commit also introduces a slight modification to the Submission data structure. Previously, the path related to the chart's source pointed to the "./src/Chart.yaml" file, it now points to the "./src" directory. This is useful as this is one of the return value of generate_verify_options. Finally, this commit also changes the SUBMISSION_PATH environment variable to an absolute path. This way, steps that run from within the "./pr-branch" directory such as get-verify-param can easily load the submission artifact. Note that $GITHUB_WORKSPACE is the default path under which load an artifact is downloaded by the download-artifact action. Signed-off-by: Matthias Goerens --- .github/workflows/build.yml | 13 +-- scripts/src/report/get_verify_params.py | 96 ++++++++--------------- scripts/src/submission/submission.py | 18 ++++- scripts/src/submission/submission_test.py | 4 +- scripts/src/updateindex/updateindex.py | 3 +- 5 files changed, 60 insertions(+), 74 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 50ac1a3f..b6252116 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -7,9 +7,10 @@ on: env: # Path to the submission artifact. # This artifact is created in the 'validate-submission' job. It contains a JSON representation of - # the Submision object. It can be read by other jobs to access those information rather than + # the Submission object. It can be read by other jobs to access those information rather than # fetch them from the GitHub API a second time. - SUBMISSION_PATH: "submission.json" + SUBMISSION_PATH: "${{ github.workspace }}/submission.json" + SUBMISSION_ARTIFACT_NAME: submission jobs: setup: @@ -159,7 +160,7 @@ jobs: uses: actions/upload-artifact@v4 if: ${{ always() && steps.validate-submission.outputs.submission_file_present == 'true' }} with: - name: submission + name: ${{ env.SUBMISSION_ARTIFACT_NAME }} path: ${{ env.SUBMISSION_PATH }} - name: Add 'content-ok' label @@ -233,7 +234,7 @@ jobs: - name: Download submission information uses: actions/download-artifact@v4 with: - name: submission + name: ${{ env.SUBMISSION_ARTIFACT_NAME }} - name: Remove 'authorized-request' label from PR uses: actions/github-script@v7 @@ -264,7 +265,7 @@ jobs: BOT_TOKEN: ${{ secrets.BOT_TOKEN }} working-directory: ./pr-branch run: | - ../ve1/bin/get-verify-params --directory=pr --api-url=${{ github.event.pull_request._links.self.href }} + ../ve1/bin/get-verify-params --directory=pr - name: Install oc id: install-oc @@ -429,7 +430,7 @@ jobs: - name: Download submission information uses: actions/download-artifact@v4 with: - name: submission + name: ${{ env.SUBMISSION_ARTIFACT_NAME }} # Chart report errors are only present if an error has occured during the chart_report - name: Download chart_report errors diff --git a/scripts/src/report/get_verify_params.py b/scripts/src/report/get_verify_params.py index a105dbae..a487e238 100644 --- a/scripts/src/report/get_verify_params.py +++ b/scripts/src/report/get_verify_params.py @@ -2,84 +2,64 @@ import os import sys +from submission import submission, validate + sys.path.append("../") from chartprreview import chartprreview from signedchart import signedchart from tools import gitutils -def get_report_full_path(category, organization, chart, version): - return os.path.join( - os.getcwd(), get_report_relative_path(category, organization, chart, version) - ) - - -def get_report_relative_path(category, organization, chart, version): - return os.path.join("charts", category, organization, chart, version, "report.yaml") +def generate_verify_options(directory: str, s: submission.Submission): + """Prepare a set of GitHub outputs, including the options for chart-verifier verify + Args: + directory (str): Local directory in which to write the error logs + s (Submission): Submitted GitHub Pull Request -def generate_verify_options(directory, category, organization, chart, version): - print("[INFO] Generate verify options. %s, %s, %s" % (organization, chart, version)) - src = os.path.join( - os.getcwd(), "charts", category, organization, chart, version, "src" - ) - report_path = get_report_full_path(category, organization, chart, version) - tar = os.path.join( - os.getcwd(), - "charts", - category, - organization, - chart, - version, - f"{chart}-{version}.tgz", - ) + Returns: + str: The flags used by chert-verifier verify command + str: The path to the chart (tarball or source) + bool: Whether a report needs to be generated + bool: Whether a cluster is needed - print(f"[INF0] report path exists = {os.path.exists(report_path)} : {report_path}") - print(f"[INF0] src path exists = {os.path.exists(src)} : {src}") - print(f"[INF0] tarball path = {os.path.exists(tar)} : {tar}") + """ + print("[INFO] Generate verify options") - flags = f"--set profile.vendortype={category}" + flags = f"--set profile.vendortype={s.chart.category}" cluster_needed = True - report_provided = False - if os.path.exists(report_path): + if s.report.found: print("[INFO] report is included") flags = f"{flags} -e has-readme" cluster_needed = False - report_provided = True - if os.path.exists(src) and not os.path.exists(tar): + if s.source.found and not s.tarball.found: print("[INFO] chart src included") - return flags, src, True, cluster_needed, report_provided - elif os.path.exists(tar) and not os.path.exists(src): + src_full_path = os.path.join(os.getcwd(), s.source.path) + return flags, src_full_path, True, cluster_needed + elif s.tarball.found and not s.source.found: print("[INFO] tarball included") - if not os.path.exists(report_path): - owners_file = os.path.join( - os.getcwd(), "charts", category, organization, chart, "OWNERS" + tarball_full_path = os.path.join(os.getcwd(), s.tarball.path) + if not s.report.found: + full_owners_path = os.path.join(os.getcwd(), s.chart.get_owners_path()) + signed_flags = signedchart.get_verifier_flags( + tarball_full_path, full_owners_path, directory ) - signed_flags = signedchart.get_verifier_flags(tar, owners_file, directory) if signed_flags: print(f"[INFO] include flags for signed chart: {signed_flags}") flags = f"{flags} {signed_flags}" - return flags, tar, True, cluster_needed, report_provided - elif os.path.exists(tar) and os.path.exists(src): + return flags, tarball_full_path, True, cluster_needed + elif s.tarball.found and s.source.found: msg = "[ERROR] Both chart source directory and tarball should not exist" chartprreview.write_error_log(directory, msg) sys.exit(1) else: print("[INFO] report only") - return "", "", False, False, report_provided + return "", "", False, False def main(): parser = argparse.ArgumentParser() - parser.add_argument( - "-u", - "--api-url", - dest="api_url", - type=str, - required=True, - help="API URL for the pull request", - ) parser.add_argument( "-d", "--directory", @@ -91,22 +71,14 @@ def main(): args = parser.parse_args() - category, organization, chart, version = chartprreview.get_modified_charts( - args.directory, args.api_url - ) + submission_path = os.environ.get("SUBMISSION_PATH") + s = validate.read_submission_from_file(articact_path=submission_path) - ( - flags, - chart_uri, - report_needed, - cluster_needed, - report_provided, - ) = generate_verify_options(args.directory, category, organization, chart, version) - gitutils.add_output("report_provided", report_provided) - gitutils.add_output( - "provided_report_relative_path", - get_report_relative_path(category, organization, chart, version), + flags, chart_uri, report_needed, cluster_needed = generate_verify_options( + args.directory, s ) + gitutils.add_output("report_provided", s.report.found) + gitutils.add_output("provided_report_relative_path", s.report.path) gitutils.add_output("report_needed", report_needed) gitutils.add_output("cluster_needed", cluster_needed) if report_needed: diff --git a/scripts/src/submission/submission.py b/scripts/src/submission/submission.py index 45d22cef..a0aaab77 100644 --- a/scripts/src/submission/submission.py +++ b/scripts/src/submission/submission.py @@ -180,21 +180,35 @@ def check_release_tag(self, repository: str): @dataclass class Report: + """Contains metadata about the report.yaml""" + + # Whether the PR contains a report.yaml found: bool = False + # Whether the report.yaml is signed (signaled by the presence or absence of report.yaml.asc) signed: bool = False + # The path to the report.yaml, e.g. charts/parnters/hashicorp/vault/0.29.1/report.yaml path: str = None @dataclass class Source: + """Contains metadata about the chart's source""" + + # Whether the PR contains the Helm chart's source found: bool = False - path: str = None # Path to the Chart.yaml + # The path to the base directory containing the source, e.g. charts/parnters/hashicorp/vault/0.29.1/src + path: str = None @dataclass class Tarball: + """Contains metadata about the tarball""" + + # Whether the PR contains a tarball found: bool = False + # The path to the tarball, e.g. charts/parnters/hashicorp/vault/0.29.1/vault-0.29.1.tgz path: str = None + # The name of the provenance file, if provided provenance: str = None @@ -334,7 +348,7 @@ def set_source(self, file_path: str): """ if os.path.basename(file_path) == "Chart.yaml": self.source.found = True - self.source.path = file_path + self.source.path = os.path.dirname(file_path) def set_tarball(self, file_path: str, tarball_match: re.Match[str]): """Action to take when a file related to the tarball is found. diff --git a/scripts/src/submission/submission_test.py b/scripts/src/submission/submission_test.py index be303b5a..45f32a60 100644 --- a/scripts/src/submission/submission_test.py +++ b/scripts/src/submission/submission_test.py @@ -137,7 +137,7 @@ class SubmissionInitScenario: chart=expected_chart, source=submission.Source( found=True, - path=f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/src/Chart.yaml", + path=f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/src", ), ), ), @@ -305,7 +305,7 @@ class CertificationScenario: expected_is_valid_certification=False, expected_reason="[ERROR] Send OWNERS file by itself in a separate PR.", ), - # Invalid certification Submission contains OWNERS and report file, but ignore_owners is set to True + # Valid certification Submission contains OWNERS and report file, but ignore_owners is set to True CertificationScenario( input_submission=submission.Submission( api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/1", diff --git a/scripts/src/updateindex/updateindex.py b/scripts/src/updateindex/updateindex.py index e0f89ff2..3b20033c 100644 --- a/scripts/src/updateindex/updateindex.py +++ b/scripts/src/updateindex/updateindex.py @@ -1,5 +1,4 @@ -"""This files downloads and updates the Helm repository index data -""" +"""This files downloads and updates the Helm repository index data""" import argparse import base64