diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9c5ea9edf..50ac1a3f4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,6 +4,13 @@ on: pull_request_target: types: [opened, synchronize, reopened, edited, ready_for_review, labeled] +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 + # fetch them from the GitHub API a second time. + SUBMISSION_PATH: "submission.json" + jobs: setup: name: Setup CI @@ -94,20 +101,19 @@ jobs: fi echo "insecure_skip_tls_verify=true" >> $GITHUB_OUTPUT - - chart-verifier: - name: Run chart-verifier + validate-submission: + name: Extract and validate PR content runs-on: ubuntu-22.04 needs: [setup] outputs: - report_content: ${{ steps.check_report.outputs.report_content }} - redhat_to_community: ${{ steps.check_report.outputs.redhat_to_community }} - message_file: ${{ steps.pr_comment.outputs.message-file }} - message_text_base64: ${{ steps.encode_pr_comment.outputs.message-text-base64 }} - web_catalog_only: ${{ steps.check_pr_content.outputs.web_catalog_only }} - chart_entry_name: ${{ steps.check_pr_content.outputs.chart-entry-name }} - release_tag: ${{ steps.check_pr_content.outputs.release_tag }} + pr-content-error-message: ${{ steps.validate-submission.outputs.pr-content-error-message }} + owners-error-message: ${{ steps.validate-submission.outputs.owners-error-message }} + validate-submission-outcome: ${{ steps.validate-submission.outcome }} + chart_entry_name: ${{ steps.validate-submission.outputs.chart_entry_name }} + release_tag: ${{ steps.validate-submission.outputs.release_tag }} + web_catalog_only: ${{ steps.validate-submission.outputs.web_catalog_only }} + vendor_type: ${{ steps.validate-submission.outputs.vendor_type }} steps: - name: Checkout @@ -133,21 +139,31 @@ jobs: ../ve1/bin/pip3 install . cd .. - - name: Check PR Content - id: check_pr_content - if: ${{ needs.setup.outputs.run_build == 'true' }} - continue-on-error: true + - name: Extract PR information + id: validate-submission env: - GITHUB_REF: ${{ github.ref }} BOT_TOKEN: ${{ secrets.BOT_TOKEN }} run: | - INDEX_BRANCH=$(if [ "${GITHUB_REF}" = "refs/heads/main" ]; then echo "refs/heads/gh-pages"; else echo "${GITHUB_REF}-gh-pages"; fi) - ./ve1/bin/check-pr-content --index-branch=${INDEX_BRANCH} --repository=${{ github.repository }} --api-url=${{ github.event.pull_request._links.self.href }} + # Sandbox repo does not have an helm repository index + if [ "$GITHUB_REPOSITORY" == "openshift-helm-charts/sandbox" ]; then + CLI_FLAG="--ignore_missing_helm_index" + fi + + ./ve1/bin/validate-submission \ + --api_url "${{ github.event.pull_request._links.self.href }}" \ + --output "$SUBMISSION_PATH" \ + --repository ${{ github.repository }} \ + ${CLI_FLAG} + + - name: Upload PR information + uses: actions/upload-artifact@v4 + if: ${{ always() && steps.validate-submission.outputs.submission_file_present == 'true' }} + with: + name: submission + path: ${{ env.SUBMISSION_PATH }} - name: Add 'content-ok' label uses: actions/github-script@v7 - if: ${{ steps.check_pr_content.outcome == 'success'}} - continue-on-error: true with: github-token: ${{secrets.GITHUB_TOKEN}} script: | @@ -160,8 +176,7 @@ jobs: - name: Remove 'content-ok' label uses: actions/github-script@v7 - if: ${{ steps.check_pr_content.outcome == 'failure' && contains( github.event.pull_request.labels.*.name, 'content-ok') }} - continue-on-error: true + if: ${{ failure() && contains( github.event.pull_request.labels.*.name, 'content-ok') }} with: github-token: ${{secrets.GITHUB_TOKEN}} script: | @@ -172,11 +187,53 @@ jobs: name: 'content-ok' }) - - name: Reflect on PR Content check - if: ${{ steps.check_pr_content.outcome == 'failure'}} + + chart-verifier: + name: Run chart-verifier + runs-on: ubuntu-22.04 + needs: [setup, validate-submission] + + outputs: + report_content: ${{ steps.check_report.outputs.report_content }} + redhat_to_community: ${{ steps.check_report.outputs.redhat_to_community }} + community_manual_review_required: ${{ steps.check_report.outputs.community_manual_review_required }} + install-oc-outcome: ${{ steps.install-oc.outcome }} + verifier_error_message: ${{ steps.check-verifier-result.outputs.verifier_error_message }} + run-verifier-outcome: ${{ steps.run-verifier.outcome }} + check_report-outcome: ${{ steps.check_report.outcome }} + upload-chart-report-errors-outcome: ${{ steps.upload-chart-report-errors.outcome }} + ocp-version-range: ${{ steps.get-ocp-range.outputs.ocp-version-range }} + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Checkout PR Branch + if: ${{ needs.setup.outputs.run_build == 'true' }} + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.ref }} + repository: ${{ github.event.pull_request.head.repo.full_name }} + path: "pr-branch" + + - name: Set up Python 3.x Part 1 + uses: actions/setup-python@v5 + with: + python-version: "3.10" + + - name: Set up Python 3.x Part 2 run: | - echo "The 'PR Content check' step has failed." - exit 1 + # set up python + python3 -m venv ve1 + cd scripts + ../ve1/bin/pip3 install -r requirements.txt + ../ve1/bin/pip3 install . + cd .. + + - name: Download submission information + uses: actions/download-artifact@v4 + with: + name: submission - name: Remove 'authorized-request' label from PR uses: actions/github-script@v7 @@ -243,6 +300,7 @@ jobs: echo "delete_namespace=true" >> $GITHUB_OUTPUT echo $KUBECONFIG + # Run chart-verifier against the provided chart, and generate a report.yaml. Runs only if a report.yaml hasn't been provided by the submitter. - uses: redhat-actions/chart-verifier@v1 id: run-verifier if: ${{ steps.verify_requires.outputs.report_needed == 'true' }} @@ -288,13 +346,15 @@ jobs: echo "::error file=.github/workflows/build.yaml::Failure in get-ocp-range, mandatory for profile version ${{ steps.get-profile-version.outputs.result }}" exit 1 + # This steps checks that the report.yaml generated by the chart-verifier action is not containing any error. + # If the user has provided a report.yaml in their submission, this report is checked instead (chart-verifier does not run in that case as signaled by report_needed=False) - name: Check Report id: check_report if: ${{ needs.setup.outputs.run_build == 'true' }} env: BOT_TOKEN: ${{ secrets.BOT_TOKEN }} - VENDOR_TYPE: ${{ steps.check_pr_content.outputs.category }} - WEB_CATALOG_ONLY: ${{ steps.check_pr_content.outputs.web_catalog_only }} + VENDOR_TYPE: ${{ needs.validate-submission.outputs.vendor_type }} + WEB_CATALOG_ONLY: ${{ needs.validate-submission.outputs.web_catalog_only }} REPORT_GENERATED: ${{ steps.verify_requires.outputs.report_needed }} GENERATED_REPORT_PATH: ${{ steps.run-verifier.outputs.report_file }} REPORT_SUMMARY_PATH: ${{ steps.run-verifier.outputs.report_info_file }} @@ -308,6 +368,16 @@ jobs: --api-url=${{ github.event.pull_request._links.self.href }} cd .. + - name: Upload chart_report errors + id: upload-chart-report-errors + uses: actions/upload-artifact@v4 + if: ${{ always() && steps.check_report.outcome == 'failure' }} + with: + name: chart_report_errors + path: pr/errors + if-no-files-found: error + continue-on-error: true + - name: Delete Namespace if: ${{ always() && steps.oc_login.conclusion == 'success' }} env: @@ -317,37 +387,84 @@ jobs: oc login --token=${{ secrets.CLUSTER_TOKEN }} --server=${API_SERVER} --insecure-skip-tls-verify=${{ needs.setup.outputs.insecure_skip_tls_verify }} ve1/bin/sa-for-chart-testing --delete charts-${{ github.event.number }} - - name: Save PR artifact - env: - BOT_TOKEN: ${{ secrets.BOT_TOKEN }} - if: ${{ always() && needs.setup.outputs.run_build == 'true' }} + manage-gh-pr: + name: Comment and merge PR + runs-on: ubuntu-22.04 + needs: [setup, validate-submission, chart-verifier] + + outputs: + message_file: ${{ steps.pr_comment.outputs.message-file }} + message_text_base64: ${{ steps.encode_pr_comment.outputs.message-text-base64 }} + + # Run manage-gh-pr as long as setup was successful, independently from potential errors in validate-submission or chart-verifier + if: ${{ always() && needs.setup.result == 'success' }} + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Checkout PR Branch + if: ${{ needs.setup.outputs.run_build == 'true' }} + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.ref }} + repository: ${{ github.event.pull_request.head.repo.full_name }} + path: "pr-branch" + + - name: Set up Python 3.x Part 1 + uses: actions/setup-python@v5 + with: + python-version: "3.10" + + - name: Set up Python 3.x Part 2 run: | - ve1/bin/pr-artifact --directory=./pr --pr-number=${{ github.event.number }} --api-url=${{ github.event.pull_request._links.self.href }} + # set up python + python3 -m venv ve1 + cd scripts + ../ve1/bin/pip3 install -r requirements.txt + ../ve1/bin/pip3 install . + cd .. + + # Submission information should always be present + - name: Download submission information + uses: actions/download-artifact@v4 + with: + name: submission + + # Chart report errors are only present if an error has occured during the chart_report + - name: Download chart_report errors + uses: actions/download-artifact@v4 + if: ${{ needs.chart-verifier.outputs.upload-chart-report-errors-outcome == 'success' }} + with: + name: chart_report_errors + path: pr/ - name: Prepare PR comment id: pr_comment - if: ${{ always() && needs.setup.outputs.run_build == 'true' }} + if: ${{ needs.setup.outputs.run_build == 'true' }} env: BOT_TOKEN: ${{ secrets.BOT_TOKEN }} - PR_CONTENT_ERROR_MESSAGE: ${{ steps.check_pr_content.outputs.pr-content-error-message }} - OWNERS_ERROR_MESSAGE: ${{ steps.check_pr_content.outputs.owners-error-message }} - COMMUNITY_MANUAL_REVIEW: ${{ steps.check_report.outputs.community_manual_review_required }} - OC_INSTALL_RESULT: ${{ steps.install-oc.outcome }} - VERIFIER_ERROR_MESSAGE: ${{ steps.check-verifier-result.outputs.verifier_error_message }} + PR_CONTENT_ERROR_MESSAGE: ${{ needs.validate-submission.outputs.pr-content-error-message }} + OWNERS_ERROR_MESSAGE: ${{ needs.validate-submission.outputs.owners-error-message }} + COMMUNITY_MANUAL_REVIEW: ${{ needs.chart-verifier.outputs.community_manual_review_required }} + OC_INSTALL_RESULT: ${{ needs.chart-verifier.outputs.install-oc-outcome || 'skipped' }} + VERIFIER_ERROR_MESSAGE: ${{ needs.chart-verifier.outputs.verifier_error_message }} run: | - ve1/bin/pr-comment ${{ steps.check_pr_content.outcome }} ${{ steps.run-verifier.outcome }} ${{ steps.check_report.conclusion }} + ve1/bin/pr-comment ${{ needs.validate-submission.outputs.validate-submission-outcome }} \ + ${{ needs.chart-verifier.outputs.run-verifier-outcome || 'skipped' }} \ + ${{ needs.chart-verifier.outputs.check_report-outcome || 'skipped'}} # Note(komish): This step is a temporary fix for the metrics step in the next job # which expects the PR comment to exist at the specified filesystem location. - name: Encode PR Comment for Metrics id: encode_pr_comment - if: ${{ always() && needs.setup.outputs.run_build == 'true' }} + if: ${{ needs.setup.outputs.run_build == 'true' }} run: | commentBase64=$(base64 --wrap=0 ${{ steps.pr_comment.outputs.message-file }}) echo "message-text-base64=${commentBase64}" | tee -a $GITHUB_OUTPUT - name: Comment on PR - if: ${{ always() && needs.setup.outputs.run_build == 'true' }} + if: ${{ needs.setup.outputs.run_build == 'true' }} uses: actions/github-script@v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} @@ -362,27 +479,26 @@ jobs: body: comment }); + # The steps running chart-verifier may be skipped in some valid case, for instance if the user + # provided a valid report.yaml - name: Add 'authorized-request' label to PR - if: ${{ always() && steps.check_pr_content.outcome == 'success' && steps.run-verifier.outcome != 'failure' && needs.setup.outputs.run_build == 'true' }} + if: ${{ needs.validate-submission.outputs.validate-submission-outcome == 'success' && + needs.chart-verifier.outputs.run-verifier-outcome != 'failure' && + needs.setup.outputs.run_build == 'true' }} uses: actions/github-script@v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | - var fs = require('fs'); - var issue_number = ${{ github.event.number }}; - var vendor_label = fs.readFileSync('./pr/vendor'); - var chart_name = fs.readFileSync('./pr/chart'); - if (vendor_label.toString() !== "" && chart_name.toString() !== "") { - github.rest.issues.addLabels({ - issue_number: Number(issue_number), - owner: context.repo.owner, - repo: context.repo.repo, - labels: ['authorized-request'] - })}; + github.rest.issues.addLabels({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + labels: ['authorized-request'] + }) - name: Approve PR id: approve_pr - if: ${{ steps.check_report.conclusion == 'success' }} + if: ${{ needs.chart-verifier.outputs.check_report-outcome == 'success' }} uses: hmarr/auto-approve-action@v4 with: # The token we use for this changes for the Sandbox repository because the sandbox repository @@ -392,7 +508,7 @@ jobs: - name: Merge PR id: merge_pr - if: ${{ steps.approve_pr.conclusion == 'success' }} + if: ${{ steps.approve_pr.outcome == 'success' }} uses: pascalgn/automerge-action@v0.16.2 env: GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }} @@ -400,7 +516,7 @@ jobs: MERGE_LABELS: "" - name: Check for PR merge - if: ${{ needs.setup.outputs.run_build == 'true' }} + if: ${{ steps.merge_pr.outcome == 'success' }} env: BOT_TOKEN: ${{ secrets.BOT_TOKEN }} run: | @@ -410,7 +526,7 @@ jobs: release: name: Release Chart runs-on: ubuntu-22.04 - needs: [setup, chart-verifier] + needs: [setup, validate-submission, chart-verifier, manage-gh-pr] steps: - name: Checkout @@ -461,8 +577,8 @@ jobs: BOT_TOKEN: ${{ secrets.BOT_TOKEN }} REPORT_CONTENT: ${{ needs.chart-verifier.outputs.report_content }} REDHAT_TO_COMMUNITY: ${{ needs.chart-verifier.outputs.redhat_to_community }} - WEB_CATALOG_ONLY: ${{ needs.chart-verifier.outputs.web_catalog_only }} - OCP_VERSION_RANGE: ${{ steps.get-ocp-range.outputs.ocp-version-range }} + WEB_CATALOG_ONLY: ${{ needs.validate-submission.outputs.web_catalog_only }} + OCP_VERSION_RANGE: ${{ needs.chart-verifier.outputs.ocp-version-range }} id: prepare-chart-release run: | cd pr-branch @@ -474,11 +590,11 @@ jobs: # Only the report file is always included. # The release tag format is -- - name: Create GitHub release - if: ${{ needs.chart-verifier.outputs.web_catalog_only == 'False' }} + if: ${{ needs.validate-submission.outputs.web_catalog_only == 'False' }} uses: softprops/action-gh-release@v2 with: token: ${{ secrets.GITHUB_TOKEN }} - tag_name: ${{ needs.chart-verifier.outputs.release_tag }} + tag_name: ${{ needs.validate-submission.outputs.release_tag }} files: | ${{ steps.prepare-chart-release.outputs.report_file }} ${{ steps.prepare-chart-release.outputs.public_key_file }} @@ -489,9 +605,9 @@ jobs: - name: Update Helm repository index if: ${{ needs.setup.outputs.run_build == 'true' }} env: - CHART_ENTRY_NAME: ${{ needs.chart-verifier.outputs.chart_entry_name }} - WEB_CATALOG_ONLY: ${{ needs.chart-verifier.outputs.web_catalog_only }} - RELEASE_TAG: ${{ needs.chart-verifier.outputs.release_tag }} + CHART_ENTRY_NAME: ${{ needs.validate-submission.outputs.chart_entry_name }} + WEB_CATALOG_ONLY: ${{ needs.validate-submission.outputs.web_catalog_only }} + RELEASE_TAG: ${{ needs.validate-submission.outputs.release_tag }} run: | INDEX_BRANCH=$(if [ "${GITHUB_REF}" = "refs/heads/main" ]; then echo "gh-pages"; else echo "${GITHUB_REF##*/}-gh-pages"; fi) @@ -551,8 +667,8 @@ jobs: - name: Retrieve PR comment for metrics if: ${{ always() && needs.setup.outputs.run_build == 'true' && github.repository != 'openshift-helm-charts/sandbox' }} run: | - mkdir -p $(dirname ${{ needs.chart-verifier.outputs.message_file }}) - echo ${{ needs.chart-verifier.outputs.message_text_base64 }} | base64 -d | tee ${{ needs.chart-verifier.outputs.message_file }} + mkdir -p $(dirname ${{ needs.manage-gh-pr.outputs.message_file }}) + echo ${{ needs.manage-gh-pr.outputs.message_text_base64 }} | base64 -d | tee ${{ needs.manage-gh-pr.outputs.message_file }} - name: Add metrics id: add_metrics @@ -575,7 +691,7 @@ jobs: echo "add PR run metric" ve1/bin/metrics --write-key="${WRITE_KEY}" \ --metric-type="pull_request" \ - --message-file="${{ needs.chart-verifier.outputs.message_file }}" \ + --message-file="${{ needs.manage-gh-pr.outputs.message_file }}" \ --pr-number="${{ github.event.number }}" \ --pr-action="${{ github.event.action }}" \ --repository="${GITHUB_REPOSITORY}" \ diff --git a/scripts/setup.cfg b/scripts/setup.cfg index 0fc80aefc..22be6cf01 100644 --- a/scripts/setup.cfg +++ b/scripts/setup.cfg @@ -33,7 +33,7 @@ where = src console_scripts = chart-repo-manager = chartrepomanager.chartrepomanager:main chart-pr-review = chartprreview.chartprreview:main - check-pr-content = checkprcontent.checkpr:main + validate-submission = submission.validate:main pr-artifact = pullrequest.prartifact:main pr-comment = pullrequest.prepare_pr_comment:main sa-for-chart-testing = saforcharttesting.saforcharttesting:main diff --git a/scripts/src/checkprcontent/__init__.py b/scripts/src/checkprcontent/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/scripts/src/checkprcontent/checkpr.py b/scripts/src/checkprcontent/checkpr.py deleted file mode 100644 index 8190a15a7..000000000 --- a/scripts/src/checkprcontent/checkpr.py +++ /dev/null @@ -1,311 +0,0 @@ -import argparse -import json -import os -import re -import sys - -import requests -import semver -import yaml -from reporegex import matchers - -try: - from yaml import CLoader as Loader -except ImportError: - from yaml import Loader - -sys.path.append("../") -from owners import owners_file -from pullrequest import prartifact -from report import verifier_report -from tools import gitutils - -ALLOW_CI_CHANGES = "allow/ci-changes" - - -def check_web_catalog_only(report_in_pr, num_files_in_pr, report_file_match): - print(f"[INFO] report in PR {report_in_pr}") - print(f"[INFO] num files in PR {num_files_in_pr}") - - category, organization, chart, version = report_file_match.groups() - - print(f"read owners file : {category}/{organization}/{chart}") - found_owners, owner_data = owners_file.get_owner_data(category, organization, chart) - - if found_owners: - owner_web_catalog_only = owners_file.get_web_catalog_only(owner_data) - print( - f"[INFO] webCatalogOnly/providerDelivery from OWNERS : {owner_web_catalog_only}" - ) - else: - msg = "[ERROR] OWNERS file was not found." - print(msg) - gitutils.add_output("owners-error-message", msg) - sys.exit(1) - - if report_in_pr: - report_file_path = os.path.join( - "pr-branch", "charts", category, organization, chart, version, "report.yaml" - ) - print(f"read report file : {report_file_path}") - found_report, report_data = verifier_report.get_report_data(report_file_path) - - if found_report: - report_web_catalog_only = verifier_report.get_web_catalog_only(report_data) - print( - f"[INFO] webCatalogOnly/providerDelivery from report : {report_web_catalog_only}" - ) - else: - msg = f"[ERROR] Failed tp open report: {report_file_path}." - print(msg) - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - - web_catalog_only = False - if report_in_pr and num_files_in_pr > 1: - if report_web_catalog_only or owner_web_catalog_only: - msg = "[ERROR] The web catalog distribution method requires the pull request to be report only." - print(msg) - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - elif report_in_pr: - if report_web_catalog_only and owner_web_catalog_only: - if verifier_report.get_package_digest(report_data): - web_catalog_only = True - else: - msg = "[ERROR] The web catalog distribution method requires a package digest in the report." - print(msg) - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - elif report_web_catalog_only: - msg = "[ERROR] Report indicates web catalog only but the distribution method set for the chart is not web catalog only." - print(msg) - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - elif owner_web_catalog_only: - msg = "[ERROR] The web catalog distribution method is set for the chart but is not set in the report." - print(msg) - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - - if web_catalog_only: - print("[INFO] webCatalogOnly/providerDelivery is a go") - gitutils.add_output("web_catalog_only", "True") - else: - gitutils.add_output("web_catalog_only", "False") - print("[INFO] webCatalogOnly/providerDelivery is a no-go") - - -def get_file_match_compiled_patterns(): - """Return a tuple of patterns, where the first can be used to match any file in a chart PR - and the second can be used to match a valid report file within a chart PR. The patterns - match based on the relative path of a file to the base repository - - Both patterns capture chart type, chart vendor, chart name and chart version from the file path.. - - Examples of valid file paths are: - - charts/partners/hashicorp/vault/0.20.0/ - charts/partners/hashicorp/vault/0.20.0//report.yaml - """ - - base = matchers.submission_path_matcher() - - pattern = re.compile(base + r"/.*") - reportpattern = re.compile(base + r"/report.yaml") - tarballpattern = re.compile(base + r"/(.*\.tgz)") - return pattern, reportpattern, tarballpattern - - -def ensure_only_chart_is_modified(api_url, repository, branch): - label_names = prartifact.get_labels(api_url) - for label_name in label_names: - if label_name == ALLOW_CI_CHANGES: - return - - files = prartifact.get_modified_files(api_url) - pattern, reportpattern, tarballpattern = get_file_match_compiled_patterns() - matches_found = 0 - report_found = False - none_chart_files = {} - - for file_path in files: - match = pattern.match(file_path) - if not match: - file_name = os.path.basename(file_path) - none_chart_files[file_name] = file_path - else: - matches_found += 1 - if reportpattern.match(file_path): - print(f"[INFO] Report found: {file_path}") - gitutils.add_output("report-exists", "true") - report_found = True - else: - tar_match = tarballpattern.match(file_path) - if tar_match: - print(f"[INFO] tarball found: {file_path}") - _, _, chart_name, chart_version, tar_name = tar_match.groups() - expected_tar_name = f"{chart_name}-{chart_version}.tgz" - if tar_name != expected_tar_name: - msg = f"[ERROR] the tgz file is named incorrectly. Expected: {expected_tar_name}. Got: {tar_name}" - print(msg) - gitutils.add_output("pr-content-error-message", msg) - exit(1) - - if matches_found == 1: - pattern_match = match - elif pattern_match.groups() != match.groups(): - msg = "[ERROR] A PR must contain only one chart. Current PR includes files for multiple charts." - print(msg) - gitutils.add_output("pr-content-error-message", msg) - exit(1) - - if none_chart_files: - if ( - len(files) > 1 or "OWNERS" not in none_chart_files - ): # OWNERS not present or preset but not the only file - example_file = list(none_chart_files.values())[0] - msg = f"[ERROR] PR includes one or more files not related to charts, e.g., {example_file}" - print(msg) - gitutils.add_output("pr-content-error-message", msg) - - if "OWNERS" in none_chart_files: - file_path = none_chart_files["OWNERS"] - path_parts = file_path.split("/") - category = path_parts[1] # Second after charts - if category == "partners": - msg = "[ERROR] OWNERS file should never be set directly by partners. See certification docs." - print(msg) - gitutils.add_output("owners-error-message", msg) - elif ( - matches_found > 0 - ): # There is a mix of chart and non-chart files including OWNERS - msg = "[ERROR] Send OWNERS file by itself in a separate PR." - print(msg) - gitutils.add_output("owners-error-message", msg) - elif len(files) == 1: # OWNERS file is the only file in PR - msg = "[INFO] OWNERS file changes require manual review by maintainers." - print(msg) - gitutils.add_output("owners-error-message", msg) - - sys.exit(1) - - check_web_catalog_only(report_found, matches_found, pattern_match) - - if matches_found > 0: - category, organization, chart, version = pattern_match.groups() - gitutils.add_output( - "category", f"{'partner' if category == 'partners' else category}" - ) - gitutils.add_output("organization", organization) - gitutils.add_output("chart-name", chart) - - if not semver.VersionInfo.is_valid(version): - msg = ( - f"[ERROR] Helm chart version is not a valid semantic version: {version}" - ) - print(msg) - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - - # Red Hat charts must carry the Red Hat prefix. - if organization == "redhat": - if not chart.startswith("redhat-"): - msg = f"[ERROR] Charts provided by Red Hat must have their name begin with the redhat- prefix. I.e. redhat-{chart}" - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - - # Non Red Hat charts must not carry the Red Hat prefix. - if organization != "redhat" and chart.startswith("redhat-"): - msg = f"[ERROR] The redhat- prefix is reserved for charts provided by Red Hat. Your chart: {chart}" - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - - # Check the index.yaml for the existence of this chart at this version. - print("Downloading index.yaml", category, organization, chart, version) - r = requests.get( - f"https://raw.githubusercontent.com/{repository}/{branch}/index.yaml" - ) - - if r.status_code == 200: - data = yaml.load(r.text, Loader=Loader) - else: - data = {"apiVersion": "v1", "entries": {}} - - entry_name = chart - d = data["entries"].get(entry_name, []) - gitutils.add_output("chart-entry-name", entry_name) - for v in d: - if v["version"] == version: - msg = f"[ERROR] Helm chart release already exists in the index.yaml: {version}" - print(msg) - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - - tag_name = f"{organization}-{chart}-{version}" - gitutils.add_output("release_tag", tag_name) - tag_api = f"https://api.github.com/repos/{repository}/git/ref/tags/{tag_name}" - headers = { - "Accept": "application/vnd.github.v3+json", - "Authorization": f'Bearer {os.environ.get("BOT_TOKEN")}', - } - print(f"[INFO] checking tag: {tag_api}") - r = requests.head(tag_api, headers=headers) - if r.status_code == 200: - msg = f"[ERROR] Helm chart release already exists in the GitHub Release/Tag: {tag_name}" - print(msg) - gitutils.add_output("pr-content-error-message", msg) - sys.exit(1) - try: - if prartifact.xRateLimit in r.headers: - print( - f"[DEBUG] {prartifact.xRateLimit} : {r.headers[prartifact.xRateLimit]}" - ) - if prartifact.xRateRemain in r.headers: - print( - f"[DEBUG] {prartifact.xRateRemain} : {r.headers[prartifact.xRateRemain]}" - ) - - response_content = r.json() - if "message" in response_content: - print( - f'[ERROR] getting index file content: {response_content["message"]}' - ) - sys.exit(1) - except json.decoder.JSONDecodeError: - pass - - -def main(): - parser = argparse.ArgumentParser() - parser.add_argument( - "-b", - "--index-branch", - dest="branch", - type=str, - required=True, - help="index branch", - ) - parser.add_argument( - "-r", - "--repository", - dest="repository", - type=str, - required=True, - help="Git Repository", - ) - parser.add_argument( - "-u", - "--api-url", - dest="api_url", - type=str, - required=True, - help="API URL for the pull request", - ) - args = parser.parse_args() - branch = args.branch.split("/")[-1] - ensure_only_chart_is_modified(args.api_url, args.repository, branch) - - -if __name__ == "__main__": - main() diff --git a/scripts/src/metrics/pushowners.py b/scripts/src/metrics/pushowners.py index 8f49dd8f6..3ba79e0b6 100644 --- a/scripts/src/metrics/pushowners.py +++ b/scripts/src/metrics/pushowners.py @@ -23,7 +23,7 @@ def getFileContent(changed_file): try: owner_data = owners_file.get_owner_data_from_file(changed_file) except owners_file.OwnersFileError as e: - print("Exception loading OWNERS file: {e}") + print(f"Exception loading OWNERS file: {e}") return "", "", "", "", "" users_included = owners_file.get_users_included(owner_data) diff --git a/scripts/src/pullrequest/prartifact.py b/scripts/src/pullrequest/prartifact.py index f70e15760..14c366f03 100644 --- a/scripts/src/pullrequest/prartifact.py +++ b/scripts/src/pullrequest/prartifact.py @@ -1,13 +1,11 @@ import argparse import os -import pathlib -import shutil import sys import requests sys.path.append("../") -from checkprcontent import checkpr +from reporegex import matchers from tools import gitutils pr_files = [] @@ -19,7 +17,7 @@ # TODO(baijum): Move this code under chartsubmission.chart module def get_modified_charts(api_url): files = get_modified_files(api_url) - pattern, _, _ = checkpr.get_file_match_compiled_patterns() + pattern, _, _ = matchers.get_file_match_compiled_patterns() for file in files: match = pattern.match(file) if match: @@ -95,24 +93,6 @@ def get_labels(api_url): return pr_labels -def save_metadata(directory, vendor_label, chart, number): - with open(os.path.join(directory, "vendor"), "w") as fd: - print(f"add {directory}/vendor as {vendor_label}") - fd.write(vendor_label) - - with open(os.path.join(directory, "chart"), "w") as fd: - print(f"add {directory}/chart as {chart}") - fd.write(chart) - - with open(os.path.join(directory, "NR"), "w") as fd: - fd.write(number) - - if os.path.exists("report.yaml"): - shutil.copy("report.yaml", directory) - else: - pathlib.Path(os.path.join(directory, "report.yaml")).touch() - - def main(): parser = argparse.ArgumentParser() parser.add_argument( @@ -148,10 +128,6 @@ def main(): pr_files = get_modified_files(args.api_url) print(f"[INFO] files in pr: {pr_files}") gitutils.add_output("pr_files", pr_files) - else: - os.makedirs(args.directory, exist_ok=True) - category, organization, chart, version = get_modified_charts(args.api_url) - save_metadata(args.directory, organization, chart, args.number) if __name__ == "__main__": diff --git a/scripts/src/pullrequest/prepare_pr_comment.py b/scripts/src/pullrequest/prepare_pr_comment.py index 29f6d4749..84af028e8 100644 --- a/scripts/src/pullrequest/prepare_pr_comment.py +++ b/scripts/src/pullrequest/prepare_pr_comment.py @@ -1,6 +1,7 @@ import os import sys +from submission import validate from tools import gitutils @@ -201,9 +202,12 @@ def main(): pr_content_result = sys.argv[1] run_verifier_result = sys.argv[2] verify_result = sys.argv[3] - issue_number = open("./pr/NR").read().strip() - vendor_label = open("./pr/vendor").read().strip() - chart_name = open("./pr/chart").read().strip() + + submission_path = os.environ.get("SUBMISSION_PATH") + s = validate.read_submission_from_file(articact_path=submission_path) + issue_number = s.get_pr_number() + vendor_label = s.chart.organization + chart_name = s.chart.name community_manual_review = os.environ.get("COMMUNITY_MANUAL_REVIEW", False) oc_install_result = os.environ.get("OC_INSTALL_RESULT") @@ -280,6 +284,7 @@ def main(): print(msg) print("*" * 30) + os.makedirs("pr", exist_ok=True) with open("./pr/comment", "w") as fd: fd.write(msg) gitutils.add_output("message-file", fd.name) diff --git a/scripts/src/reporegex/matchers.py b/scripts/src/reporegex/matchers.py index 2519a720f..dddb7c730 100644 --- a/scripts/src/reporegex/matchers.py +++ b/scripts/src/reporegex/matchers.py @@ -1,3 +1,6 @@ +import re + + def submission_path_matcher( base_dir="charts", strict_categories=True, include_version_matcher=True ): @@ -39,3 +42,24 @@ def submission_path_matcher( matcher += rf"/({versionMatcher})" return matcher + + +def get_file_match_compiled_patterns(): + """Return a tuple of patterns, where the first can be used to match any file in a chart PR + and the second can be used to match a valid report file within a chart PR. The patterns + match based on the relative path of a file to the base repository + + Both patterns capture chart type, chart vendor, chart name and chart version from the file path.. + + Examples of valid file paths are: + + charts/partners/hashicorp/vault/0.20.0/ + charts/partners/hashicorp/vault/0.20.0//report.yaml + """ + + base = submission_path_matcher() + + pattern = re.compile(base + r"/.*") + reportpattern = re.compile(base + r"/report.yaml") + tarballpattern = re.compile(base + r"/(.*\.tgz)") + return pattern, reportpattern, tarballpattern diff --git a/scripts/src/submission/submission.py b/scripts/src/submission/submission.py index d556dceee..45d22cef2 100644 --- a/scripts/src/submission/submission.py +++ b/scripts/src/submission/submission.py @@ -11,7 +11,6 @@ from dataclasses import dataclass, field -from checkprcontent import checkpr from owners import owners_file from tools import gitutils from reporegex import matchers @@ -122,7 +121,7 @@ def register_chart_info( self.version = version - def get_owners_path(self): + def get_owners_path(self) -> str: return f"charts/{self.category}/{self.organization}/{self.name}/OWNERS" def get_vendor_type(self) -> str: @@ -131,10 +130,10 @@ def get_vendor_type(self) -> str: return "partner" return self.category - def get_release_tag(self): + def get_release_tag(self) -> str: return f"{self.organization}-{self.name}-{self.version}" - def check_index(self, index): + def check_index(self, index: dict): """Check if the chart is present in the Helm index Args: @@ -237,7 +236,6 @@ def __post_init__(self): if not self.modified_files: self.modified_files = [] self._get_modified_files() - self._parse_modified_files() def _get_modified_files(self): """Query the GitHub API in order to retrieve the list of files that are added / modified by @@ -276,7 +274,7 @@ def _get_modified_files(self): if "filename" in file: self.modified_files.append(file["filename"]) - def _parse_modified_files(self): + def parse_modified_files(self): """Classify the list of modified files. Modified files are categorized into 5 groups, mapping to 5 class attributes: @@ -309,10 +307,10 @@ def _parse_modified_files(self): self.set_tarball(file_path, match) elif file_category == "owners": self.modified_owners.append(file_path) - elif file_category == "unknwown": + elif file_category == "unknown": self.modified_unknown.append(file_path) - def set_report(self, file_path): + def set_report(self, file_path: str): """Action to take when a file related to the chart-verifier is found. This can either be the report.yaml itself, or the signing key report.yaml.asc @@ -327,7 +325,7 @@ def set_report(self, file_path): else: self.modified_unknown.append(file_path) - def set_source(self, file_path): + def set_source(self, file_path: str): """Action to take when a file related to the chart's source is found. Note that while the source of the Chart can be composed of many files, only the Chart.yaml @@ -338,7 +336,7 @@ def set_source(self, file_path): self.source.found = True self.source.path = file_path - def set_tarball(self, file_path, tarball_match): + def set_tarball(self, file_path: str, tarball_match: re.Match[str]): """Action to take when a file related to the tarball is found. This can either be the .tgz tarball itself, or the .prov provenance key. @@ -363,7 +361,7 @@ def set_tarball(self, file_path, tarball_match): def is_valid_certification_submission( self, ignore_owners: bool = False ) -> tuple[bool, str]: - """Check wether the files in this Submission are valid to attempt to certify a Chart + """Check whether the files in this Submission are valid to attempt to certify a Chart We expect the user to provide either: * Only a report file @@ -373,7 +371,7 @@ def is_valid_certification_submission( Note: While an OWNERS file should never be present in this type of submission, the case of an invalid Submission containing a mix of OWNERS and other files is handled in the is_valid_owners_submission method. The flag "ignore_owners" allows for skipping this - speficic check. + specific check. Returns False if: * The user attempts to create the OWNERS file for its project. @@ -397,8 +395,8 @@ def is_valid_certification_submission( return False, "" - def is_valid_owners_submission(self): - """Check wether the file in this Submission are valid for an OWNERS PR + def is_valid_owners_submission(self) -> tuple[bool, str]: + """Check whether the files in this Submission are valid for an OWNERS PR Returns True if the PR only modified files is an OWNERS file. @@ -535,8 +533,11 @@ def is_valid_web_catalog_only(self, repo_path: str = "") -> tuple[bool, str]: return True, "" + def get_pr_number(self): + return self.api_url.split("/")[-1] -def get_file_type(file_path): + +def get_file_type(file_path: str) -> tuple[str, re.Match[str]]: """Determine the category of a given file As part of a PR, a modified file can relate to one of 5 categories: @@ -547,7 +548,7 @@ def get_file_type(file_path): - or another "unknown" category """ - pattern, reportpattern, tarballpattern = checkpr.get_file_match_compiled_patterns() + pattern, reportpattern, tarballpattern = matchers.get_file_match_compiled_patterns() owners_pattern = re.compile( matchers.submission_path_matcher(include_version_matcher=False) + r"/OWNERS" ) @@ -572,7 +573,7 @@ def get_file_type(file_path): if owners_match: return "owners", owners_match - return "unknwown", None + return "unknown", None def download_index_data( @@ -600,8 +601,9 @@ def download_index_data( r = requests.get(index_url) data = {"apiVersion": "v1", "entries": {}} - if r.status_code != 200 and not ignore_missing: - raise HelmIndexError(f"Error retrieving index file at {index_url}") + if r.status_code != 200: + if not ignore_missing: + raise HelmIndexError(f"Error retrieving index file at {index_url}") else: try: data = yaml.load(r.text, Loader=Loader) diff --git a/scripts/src/submission/submission_test.py b/scripts/src/submission/submission_test.py index 9e7db295b..be303b5a5 100644 --- a/scripts/src/submission/submission_test.py +++ b/scripts/src/submission/submission_test.py @@ -253,6 +253,7 @@ def test_submission_init(test_scenario): with test_scenario.excepted_exception: s = submission.Submission(api_url=test_scenario.api_url) + s.parse_modified_files() assert s == test_scenario.expected_submission diff --git a/scripts/src/submission/validate.py b/scripts/src/submission/validate.py new file mode 100644 index 000000000..ab92ef0ae --- /dev/null +++ b/scripts/src/submission/validate.py @@ -0,0 +1,180 @@ +import argparse +import json +import sys + +from submission import submission, serializer +from tools import gitutils + + +class ParseFilesError(Exception): + pass + + +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: + 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: + s = json.load(f, cls=serializer.SubmissionDecoder) + + return s + + +def craft_pr_content_error_msg( + s: submission.Submission, repository: str, ignore_missing_helm_index: bool +) -> str: + """Generate the pr-content-error-message GitHub output. + + Args: + s (Submission): GitHub Pull Request to validate + repository (str): Name of the git Repository + ignore_missing_helm_index (bool): if set to True, defaults to an empty Helm repository index + when it can't be found, instead of raising an error. + + Returns: + str: The error message to add as GitHub output + + """ + try: + s.parse_modified_files() + except submission.SubmissionError as e: + raise ParseFilesError(str(e)) + + # Checks that this PR is a valid "Chart certification" PR + is_valid, msg = s.is_valid_certification_submission(ignore_owners=True) + if not is_valid: + return msg + + # Parse the modified files and determine if it is a "web_catalog_only" certification + try: + s.parse_web_catalog_only(repo_path="pr-branch") + except submission.SubmissionError as e: + return str(e) + + if s.is_web_catalog_only: + is_valid, msg = s.is_valid_web_catalog_only(repo_path="pr-branch") + if not is_valid: + return msg + + try: + index = submission.download_index_data( + repository, ignore_missing=ignore_missing_helm_index + ) + except submission.HelmIndexError as e: + return str(e) + + try: + s.chart.check_index(index) + except submission.HelmIndexError as e: + return str(e) + + try: + s.chart.check_release_tag(repository) + except submission.ReleaseTagError as e: + return str(e) + + return "" + + +def craft_owners_error_msg(s: submission.Submission) -> str: + """Generate the owners-error-message GitHub Output.""" + is_valid, msg = s.is_valid_owners_submission() + if is_valid: + if s.chart.category == "partners": + msg = "[ERROR] OWNERS file should never be set directly by partners. See certification docs." + else: + msg = "[INFO] OWNERS file changes require manual review by maintainers." + return msg + + +def main(): + """Entry point of the submission module. + + 1. Pull all information related to the PR from GitHub and initialized the Submission object. + 2. Validate the Submission by running a series of checks and created required GitHub outputs + pr-content-error-message and owners-error-message. + 3. Save the Submission object to a file, to be uploaded as an artifact for next jobs. + + """ + 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( + "-r", + "--repository", + dest="repository", + type=str, + required=True, + help="Git Repository", + ) + parser.add_argument( + "-o", + "--output", + dest="output", + type=str, + required=True, + help="Path to artifact file to write Submission json representation", + ) + parser.add_argument( + "--ignore_missing_helm_index", + dest="ignore_missing_helm_index", + default=False, + action="store_true", + help="When checking for the presence of the Helm chart in the Helm repository index, default to an empty index if it can't be found", + ) + + args = parser.parse_args() + s = submission.Submission(args.api_url) + try: + pr_content_error_msg = craft_pr_content_error_msg( + s, args.repository, args.ignore_missing_helm_index + ) + except ParseFilesError as e: + # Save submission and exit early in case of an error during parsing of the modified files + write_submission_to_file(s, args.output) + gitutils.add_output("submission_file_present", "true") + print(str(e)) + gitutils.add_output("pr-content-error-message", str(e)) + sys.exit(10) + + if pr_content_error_msg: + print(pr_content_error_msg) + gitutils.add_output("pr-content-error-message", pr_content_error_msg) + + owners_error_msg = "" + if s.modified_owners: + # If the PR contains an OWNER file, craft a error message to be added as a comment in the PR + owners_error_msg = craft_owners_error_msg(s) + print(owners_error_msg) + gitutils.add_output("owners-error-message", owners_error_msg) + + # Some subsequent steps require those GitHub outputs + gitutils.add_output("chart_entry_name", s.chart.name) + gitutils.add_output("release_tag", s.chart.get_release_tag()) + gitutils.add_output("web_catalog_only", s.is_web_catalog_only) + gitutils.add_output("vendor_type", s.chart.get_vendor_type()) + + write_submission_to_file(s, args.output) + gitutils.add_output("submission_file_present", "true") + + if owners_error_msg or pr_content_error_msg: + print( + f"exit with owners_error_msg={owners_error_msg}; pr_content_error_msg={pr_content_error_msg}" + ) + sys.exit(20) + + +if __name__ == "__main__": + main()