From d86a0df4aeaf18423b3f1ecd41ee373ec416bd86 Mon Sep 17 00:00:00 2001 From: Matthias Goerens Date: Tue, 9 Jul 2024 12:43:50 +0200 Subject: [PATCH] Add redhat prefix check Ported from: https://github.com/openshift-helm-charts/development/blob/919146bc199e6366c40cf10d533d4669bd3def70/scripts/src/checkprcontent/checkpr.py#L211-L222 Signed-off-by: Matthias Goerens stash Signed-off-by: Matthias Goerens Add owners-error-message GH output Signed-off-by: Matthias Goerens Better craft pr-content-error-message Signed-off-by: Matthias Goerens rework build Fix build.yaml add precheck to venv fix import fix api_url arg Add bot_token env first attempt fix submission env var; add owners_message debug fix craft pr content error msg && ensure comments are added fix condition fix condition pr artifcat should run always fix submission env_var bool env vars python output vendor labels partners to partner debug remove debug and fix vendor output fix condition for release move github output fix check auto merge condition remove komish ping --- .github/workflows/build.yml | 180 ++++++++++++------ scripts/setup.cfg | 1 + scripts/src/chartprreview/chartprreview.py | 7 +- .../src/chartrepomanager/chartrepomanager.py | 7 +- scripts/src/checkprcontent/checkpr.py | 93 --------- scripts/src/precheck/precheck.py | 116 +++++++++++ scripts/src/precheck/submission.py | 75 ++++++-- scripts/src/precheck/submission_test.py | 74 +++++-- scripts/src/updateindex/updateindex.py | 7 +- 9 files changed, 379 insertions(+), 181 deletions(-) create mode 100644 scripts/src/precheck/precheck.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d9cda1ae..4e7e16c1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,9 +1,15 @@ name: CI - + on: pull_request_target: types: [opened, synchronize, reopened, edited, ready_for_review, labeled] +env: + SUBMISSION_PATH: "submission.json" + # Temporary workaround. See + # https://github.com/redhat-actions/openshift-tools-installer/issues/105 + ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true + jobs: setup: name: Setup CI @@ -96,20 +102,19 @@ jobs: fi echo "insecure_skip_tls_verify=true" >> $GITHUB_OUTPUT - - chart-verifier: - name: Run chart-verifier + pre-check: + 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.pre-check.outputs.pr-content-error-message }} + owners-error-message: ${{ steps.pre-check.outputs.owners-error-message }} + pre-check-outcome: ${{ steps.pre-check.outcome }} + chart_entry_name: ${{ steps.pre-check.outputs.chart_entry_name }} + release_tag: ${{ steps.pre-check.outputs.release_tag }} + web_catalog_only: ${{ steps.pre-check.outputs.web_catalog_only }} + category: ${{ steps.pre-check.outputs.category }} steps: - name: Checkout @@ -117,7 +122,7 @@ jobs: - name: Checkout PR Branch if: ${{ needs.setup.outputs.run_build == 'true' }} - uses: actions/checkout@v4 + uses: actions/checkout@v3 with: ref: ${{ github.event.pull_request.head.ref }} repository: ${{ github.event.pull_request.head.repo.full_name }} @@ -137,21 +142,24 @@ 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: pre-check 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 }} + ./ve1/bin/pre-check \ + --api_url "${{ github.event.pull_request._links.self.href }}" \ + --output "$SUBMISSION_PATH" \ + --chart_submission + + - name: Upload PR information + uses: actions/upload-artifact@v4 + 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: | @@ -164,8 +172,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: | @@ -176,15 +183,60 @@ 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, pre-check] + + if: ${{ always() }} + + 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 }} + # 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 + if: ${{ ! contains(join(needs.*.result, ','), 'failure') }} + with: + name: submission - name: Remove 'authorized-request' label from PR uses: actions/github-script@v7 - if: ${{ needs.setup.outputs.run_build == 'true' && contains( github.event.pull_request.labels.*.name, 'authorized-request') }} + if: ${{ needs.setup.outputs.run_build == 'true' && contains( github.event.pull_request.labels.*.name, 'authorized-request') && ! contains(join(needs.*.result, ','), 'failure') }} continue-on-error: true with: github-token: ${{ secrets.GITHUB_TOKEN }} @@ -199,13 +251,15 @@ jobs: - name: install chart verifier for action uses: redhat-actions/openshift-tools-installer@v1 + if: ${{ ! contains(join(needs.*.result, ','), 'failure') }} with: source: github skip_cache: true chart-verifier: "${{ needs.setup.outputs.verifier-action-image }}" + # TODO: check what needs to stay here vs what could go to pre-check - name: determine verify requirements - if: ${{ needs.setup.outputs.run_build == 'true' }} + if: ${{ needs.setup.outputs.run_build == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} id: verify_requires env: BOT_TOKEN: ${{ secrets.BOT_TOKEN }} @@ -215,14 +269,14 @@ jobs: - name: Install oc id: install-oc - if: ${{ steps.verify_requires.outputs.cluster_needed == 'true' }} + if: ${{ steps.verify_requires.outputs.cluster_needed == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} uses: redhat-actions/openshift-tools-installer@v1 with: oc: latest - name: Set cluster login params id: login-params - if: ${{ steps.verify_requires.outputs.cluster_needed == 'true' }} + if: ${{ steps.verify_requires.outputs.cluster_needed == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} run: | #calculate cluster params API_SERVER=$( echo -n ${{ secrets.API_SERVER }} | base64 -d) @@ -230,7 +284,7 @@ jobs: - uses: redhat-actions/oc-login@v1 id: oc_login - if: ${{ steps.verify_requires.outputs.cluster_needed == 'true' }} + if: ${{ steps.verify_requires.outputs.cluster_needed == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} with: openshift_server_url: ${{ steps.login-params.outputs.API_SERVER }} openshift_token: ${{ secrets.CLUSTER_TOKEN }} @@ -238,7 +292,7 @@ jobs: - name: create service account id: create_service_account - if: ${{ steps.verify_requires.outputs.cluster_needed == 'true' }} + if: ${{ steps.verify_requires.outputs.cluster_needed == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} env: API_SERVER: ${{ steps.login-params.outputs.API_SERVER }} BOT_TOKEN: ${{ secrets.BOT_TOKEN }} @@ -249,7 +303,7 @@ jobs: - uses: redhat-actions/chart-verifier@v1 id: run-verifier - if: ${{ steps.verify_requires.outputs.report_needed == 'true' }} + if: ${{ steps.verify_requires.outputs.report_needed == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} with: chart_uri: ${{ steps.verify_requires.outputs.verify_uri }} verify_args: ${{ steps.verify_requires.outputs.verify_args }} @@ -258,21 +312,21 @@ jobs: - name: check-verifier-result id: check-verifier-result - if: ${{ always() && steps.run-verifier.outcome == 'failure' }} + if: ${{ always() && steps.run-verifier.outcome == 'failure' && ! contains(join(needs.*.result, ','), 'failure') }} run: | error_message="The chart verifier returned an error when trying to obtain a verification report for the chart." echo "verifier_error_message=$error_message" >> $GITHUB_OUTPUT - name: Get profile version set in report provided by the user id: get-profile-version - if: ${{ needs.setup.outputs.run_build == 'true' && steps.verify_requires.outputs.report_provided == 'true' }} + if: ${{ needs.setup.outputs.run_build == 'true' && steps.verify_requires.outputs.report_provided == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} uses: mikefarah/yq@master with: cmd: yq '.metadata.tool.profile.version' ${{ format('./pr-branch/{0}', steps.verify_requires.outputs.provided_report_relative_path) }} - name: Get the range of Kubernetes versions set in the report provided by the user id: get-kube-range - if: ${{ needs.setup.outputs.run_build == 'true' && steps.verify_requires.outputs.report_provided == 'true' }} + if: ${{ needs.setup.outputs.run_build == 'true' && steps.verify_requires.outputs.report_provided == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} continue-on-error: true uses: mikefarah/yq@master with: @@ -280,31 +334,33 @@ jobs: - name: Get the corresponding range of OCP versions id: get-ocp-range - if: ${{ needs.setup.outputs.run_build == 'true' && steps.verify_requires.outputs.report_provided == 'true' }} + if: ${{ needs.setup.outputs.run_build == 'true' && steps.verify_requires.outputs.report_provided == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} continue-on-error: true uses: ./.github/actions/get-ocp-range with: kube-version-range: ${{ steps.get-kube-range.outputs.result }} - name: Only ignore errors in get-ocp-range for profile in version v1.0 - if: ${{ (steps.get-kube-range.outcome == 'failure' || steps.get-ocp-range.outcome == 'failure') && steps.get-profile-version.outputs.result != 'v1.0' }} + if: ${{ (steps.get-kube-range.outcome == 'failure' || steps.get-ocp-range.outcome == 'failure') && steps.get-profile-version.outputs.result != 'v1.0' && ! contains(join(needs.*.result, ','), 'failure') }} run: | echo "::error file=.github/workflows/build.yaml::Failure in get-ocp-range, mandatory for profile version ${{ steps.get-profile-version.outputs.result }}" exit 1 - name: Check Report id: check_report - if: ${{ needs.setup.outputs.run_build == 'true' }} + if: ${{ needs.setup.outputs.run_build == 'true' && ! contains(join(needs.*.result, ','), 'failure') }} 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.pre-check.outputs.category }} + WEB_CATALOG_ONLY: ${{ needs.pre-check.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 }} WORKFLOW_WORKING_DIRECTORY: "../pr" OCP_VERSION_RANGE: ${{ steps.get-ocp-range.outputs.ocp-version-range }} run: | + # export VENDOR_TYPE=`jq .chart.category ${{ github.env.SUBMISSION_PATH }}` + # export WEB_CATALOG_ONLY=`jq .is_web_catalog_only ${{ github.env.SUBMISSION_PATH }}` cd pr-branch ../ve1/bin/chart-pr-review \ --directory=../pr \ @@ -313,7 +369,7 @@ jobs: cd .. - name: Delete Namespace - if: ${{ always() && steps.oc_login.conclusion == 'success' }} + if: ${{ always() && steps.oc_login.conclusion == 'success' && ! contains(join(needs.*.result, ','), 'failure') }} env: KUBECONFIG: /tmp/ci-kubeconfig run: | @@ -321,6 +377,7 @@ 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 }} + # TODO - name: Save PR artifact env: BOT_TOKEN: ${{ secrets.BOT_TOKEN }} @@ -328,18 +385,27 @@ jobs: run: | ve1/bin/pr-artifact --directory=./pr --pr-number=${{ github.event.number }} --api-url=${{ github.event.pull_request._links.self.href }} + # manage-gh-pr: + # name: Comment and merge PR + # runs-on: ubuntu-22.04 + # needs: [setup, pre-check, chart-verifier] + + # # outputs: + # # if: ${{ always() && contains(join(needs.*.result, ','), 'success') }} + + # steps: - name: Prepare PR comment id: pr_comment if: ${{ always() && 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 }} + PR_CONTENT_ERROR_MESSAGE: ${{ needs.pre-check.outputs.pr-content-error-message }} + OWNERS_ERROR_MESSAGE: ${{ needs.pre-check.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 }} run: | - ve1/bin/pr-comment ${{ steps.check_pr_content.outcome }} ${{ steps.run-verifier.outcome }} ${{ steps.check_report.conclusion }} + ve1/bin/pr-comment ${{ needs.pre-check.outputs.pre-check-outcome }} ${{ steps.run-verifier.outcome }} ${{ steps.check_report.conclusion }} # 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. @@ -367,7 +433,7 @@ jobs: }); - 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: ${{ always() && needs.pre-check.outputs.pre-check-outcome == 'success' && steps.run-verifier.outcome != 'failure' && needs.setup.outputs.run_build == 'true' }} uses: actions/github-script@v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} @@ -404,7 +470,7 @@ jobs: MERGE_LABELS: "" - name: Check for PR merge - if: ${{ needs.setup.outputs.run_build == 'true' }} + if: ${{ steps.merge_pr.conclusion == 'success' }} env: BOT_TOKEN: ${{ secrets.BOT_TOKEN }} run: | @@ -414,7 +480,7 @@ jobs: release: name: Release Chart runs-on: ubuntu-22.04 - needs: [setup, chart-verifier] + needs: [setup, pre-check, chart-verifier] steps: - name: Checkout @@ -467,10 +533,12 @@ 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 }} + WEB_CATALOG_ONLY: ${{ needs.pre-check.outputs.web_catalog_only }} OCP_VERSION_RANGE: ${{ steps.get-ocp-range.outputs.ocp-version-range }} id: prepare-chart-release run: | + # export WEB_CATALOG_ONLY=`jq .is_web_catalog_only ${{ github.env.SUBMISSION_PATH }}` + cd pr-branch ../ve1/bin/chart-repo-manager \ --repository=${{ github.repository }} \ @@ -480,10 +548,10 @@ 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.pre-check.outputs.web_catalog_only == 'False' }} uses: softprops/action-gh-release@v1 with: - tag_name: ${{ needs.chart-verifier.outputs.release_tag }} + tag_name: ${{ needs.pre-check.outputs.release_tag }} files: | ${{ steps.prepare-chart-release.outputs.report_file }} ${{ steps.prepare-chart-release.outputs.public_key_file }} @@ -496,9 +564,11 @@ 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 }} + CHART_ENTRY_NAME: ${{ needs.pre-check.outputs.chart_entry_name }} + WEB_CATALOG_ONLY: ${{ needs.pre-check.outputs.web_catalog_only }} run: | + # export WEB_CATALOG_ONLY=`jq .is_web_catalog_only ${{ github.env.SUBMISSION_PATH }}` + INDEX_BRANCH=$(if [ "${GITHUB_REF}" = "refs/heads/main" ]; then echo "gh-pages"; else echo "${GITHUB_REF##*/}-gh-pages"; fi) echo "[INFO] Creating Git worktree for index branch" @@ -547,7 +617,7 @@ jobs: An error occured while updating the Helm repository index. - cc @komish @mgoerens` + cc @mgoerens` }); # Note(komish): This step is a temporary workaround. Metrics requires the PR comment diff --git a/scripts/setup.cfg b/scripts/setup.cfg index 0fc80aef..4424fcbd 100644 --- a/scripts/setup.cfg +++ b/scripts/setup.cfg @@ -34,6 +34,7 @@ console_scripts = chart-repo-manager = chartrepomanager.chartrepomanager:main chart-pr-review = chartprreview.chartprreview:main check-pr-content = checkprcontent.checkpr:main + pre-check = precheck.precheck: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/chartprreview/chartprreview.py b/scripts/src/chartprreview/chartprreview.py index 33e00991..87047436 100644 --- a/scripts/src/chartprreview/chartprreview.py +++ b/scripts/src/chartprreview/chartprreview.py @@ -10,7 +10,7 @@ import semantic_version import semver import yaml -from environs import Env +from environs import Env, EnvValidationError try: from yaml import CLoader as Loader @@ -516,7 +516,10 @@ def main(): generated_report_path = os.environ.get("GENERATED_REPORT_PATH") generated_report_info_path = os.environ.get("REPORT_SUMMARY_PATH") env = Env() - web_catalog_only = env.bool("WEB_CATALOG_ONLY", False) + try: + web_catalog_only = env.bool("WEB_CATALOG_ONLY", False) + except EnvValidationError: + web_catalog_only = False submitted_report_path = os.path.join( "charts", category, organization, chart, version, "report.yaml" diff --git a/scripts/src/chartrepomanager/chartrepomanager.py b/scripts/src/chartrepomanager/chartrepomanager.py index 65af0fbe..e5328d0c 100644 --- a/scripts/src/chartrepomanager/chartrepomanager.py +++ b/scripts/src/chartrepomanager/chartrepomanager.py @@ -32,7 +32,7 @@ import urllib.parse import yaml -from environs import Env +from environs import Env, EnvValidationError try: from yaml import CDumper as Dumper @@ -448,7 +448,10 @@ def main(): ) env = Env() - web_catalog_only = env.bool("WEB_CATALOG_ONLY", False) + try: + web_catalog_only = env.bool("WEB_CATALOG_ONLY", False) + except EnvValidationError: + web_catalog_only = False ocp_version_range = os.environ.get("OCP_VERSION_RANGE", "N/A") print(f"[INFO] webCatalogOnly/providerDelivery is {web_catalog_only}") diff --git a/scripts/src/checkprcontent/checkpr.py b/scripts/src/checkprcontent/checkpr.py index 8190a15a..6a977225 100644 --- a/scripts/src/checkprcontent/checkpr.py +++ b/scripts/src/checkprcontent/checkpr.py @@ -1,100 +1,7 @@ -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 diff --git a/scripts/src/precheck/precheck.py b/scripts/src/precheck/precheck.py new file mode 100644 index 00000000..da171211 --- /dev/null +++ b/scripts/src/precheck/precheck.py @@ -0,0 +1,116 @@ +import argparse +import json +import sys + +from precheck import submission, serializer +from tools import gitutils + + +def write_submission_to_file(s: submission.Submission, artifact_path: str): + data = serializer.SubmissionEncoder().encode(s) + + with open(artifact_path, "w") as f: + f.write(data) + + +def read_submission_from_file(articact_path: str): + 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): + # 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() + except submission.SubmissionError as e: + return str(e) + + if s.is_web_catalog_only: + if not s.is_valid_web_catalog_only(): + msg = "nope" + return msg + + return "" + + +def craft_owners_error_msg(s): + is_valid, msg = s.is_valid_owners_submission() + if is_valid: + msg = "[INFO] OWNERS file changes require manual review by maintainers." + return msg + + +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( + "-o", + "--output", + dest="output", + type=str, + required=True, + help="Path to artifact file to write Submission json representation", + ) + parser.add_argument( + "-c", + "--chart_submission", + dest="check_chart_submission", + default=False, + action="store_true", + help="Signify that the PR referenced by api_url is expected to be a certification submission", + ) + + args = parser.parse_args() + try: + s = submission.Submission(args.api_url) + except submission.SubmissionError as e: + print(str(e)) + gitutils.add_output("pr-content-error-message", str(e)) + sys.exit(10) + + ### TODO: add check index + + 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) + + pr_content_error_msg = "" + if args.check_chart_submission: + pr_content_error_msg = craft_pr_content_error_msg(s) + if pr_content_error_msg: + print(pr_content_error_msg) + gitutils.add_output("pr-content-error-message", pr_content_error_msg) + + 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("category", s.chart.get_vendor_label()) + + 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) + + write_submission_to_file(s, args.output) + + +if __name__ == "__main__": + main() diff --git a/scripts/src/precheck/submission.py b/scripts/src/precheck/submission.py index 0a5d989a..1e3be558 100644 --- a/scripts/src/precheck/submission.py +++ b/scripts/src/precheck/submission.py @@ -68,7 +68,29 @@ class Chart: name: str = None version: str = None - def register_chart_info(self, category, organization, name, version): + def register_chart_info( + self, category: str, organization: str, name: str, version: str = None + ): + """Initialize the chart's category, organization, name and version + + Providing a version is not mandatory. In case of a Submission that only contains an OWNERS + file, a version is not present. + + This function ensures that once set, the chart's information are not modified, as a PR must + only relate to a unique chart. + + Args: + category (str): Type of profile (community, partners, or redhat) + organization (str): Name of the organization (ex: hashicorp) + chart (str): Name of the chart (ex: vault) + version (str): The version of the chart (ex: 1.4.0) + + Raises: + DuplicateChartError if the caller attempts to modify the chart's information + ChartError if the redhat prefix is incorrectly set + VersionError if the provided version is not semver compatible + + """ if ( (self.category and self.category != category) or (self.organization and self.organization != organization) @@ -78,11 +100,16 @@ def register_chart_info(self, category, organization, name, version): msg = "[ERROR] A PR must contain only one chart. Current PR includes files for multiple charts." raise DuplicateChartError(msg) - if not semver.VersionInfo.is_valid(version): - msg = ( - f"[ERROR] Helm chart version is not a valid semantic version: {version}" - ) - raise VersionError(msg) + # Red Hat charts must carry the Red Hat prefix. + if organization == "redhat": + if not name.startswith("redhat-"): + msg = f"[ERROR] Charts provided by Red Hat must have their name begin with the redhat- prefix. I.e. redhat-{name}" + raise ChartError(msg) + + # Non Red Hat charts must not carry the Red Hat prefix. + if organization != "redhat" and name.startswith("redhat-"): + msg = f"[ERROR] The redhat- prefix is reserved for charts provided by Red Hat. Your chart: {name}" + raise ChartError(msg) # Red Hat charts must carry the Red Hat prefix. if organization == "redhat": @@ -98,11 +125,22 @@ def register_chart_info(self, category, organization, name, version): self.category = category self.organization = organization self.name = name - self.version = version + + if version: + if not semver.VersionInfo.is_valid(version): + msg = f"[ERROR] Helm chart version is not a valid semantic version: {version}" + raise VersionError(msg) + + self.version = version def get_owners_path(self): return f"charts/{self.category}/{self.organization}/{self.name}/OWNERS" + def get_vendor_label(self): + if self.category == "partners": + return "partner" + return self.category + def get_release_tag(self): return f"{self.organization}-{self.name}-{self.version}" @@ -332,7 +370,7 @@ def set_tarball(self, file_path, tarball_match): else: self.modified_unknown.append(file_path) - def is_valid_certification_submission(self): + def is_valid_certification_submission(self, ignore_owners: bool = False): """Check wether the files in this Submission are valid to attempt to certify a Chart We expect the user to provide either: @@ -347,7 +385,7 @@ def is_valid_certification_submission(self): Returns True in all other cases """ - if self.modified_owners: + if self.modified_owners and not ignore_owners: return False, "[ERROR] Send OWNERS file by itself in a separate PR." if self.modified_unknown: @@ -363,22 +401,27 @@ def is_valid_certification_submission(self): return False, "" def is_valid_owners_submission(self): - """Check wether the file in this Submission are valid for an OWNERS PR + """Check wether the files in this Submission are valid for an OWNERS PR - Returns True if the PR only modified files is an OWNERS file. + A valid OWNERS PR contains only the OWNERS file, and is not submitted by a partner - Returns False in all other cases. """ + if (self.chart.category == "partners") and self.modified_owners: + # The PR contains an OWNERS file for a parnter + msg = "[ERROR] OWNERS file should never be set directly by partners. See certification docs." + return False, msg + if len(self.modified_owners) == 1 and len(self.modified_files) == 1: + # Happy path: PR contains a single modified files that is an OWNERS, and is not for a partner return True, "" - msg = "" if self.modified_owners: + # At least one OWNERS file, with other files (modified_files > 1) msg = "[ERROR] Send OWNERS file by itself in a separate PR." - else: - msg = "No OWNERS file provided" + return False, msg - return False, msg + # No OWNERS have been provided + return False, "No OWNERS file provided" def parse_web_catalog_only(self, repo_path=""): """Set the web_catalog_only attribute diff --git a/scripts/src/precheck/submission_test.py b/scripts/src/precheck/submission_test.py index 7446024f..c4b81d6c 100644 --- a/scripts/src/precheck/submission_test.py +++ b/scripts/src/precheck/submission_test.py @@ -281,6 +281,7 @@ class CertificationScenario: input_submission: submission.Submission expected_is_valid_certification: bool expected_reason: str = "" + ignore_owners: bool = False scenarios_certification_submission = [ @@ -303,6 +304,26 @@ 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 + CertificationScenario( + input_submission=submission.Submission( + api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/1", + modified_files=[ + f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml" + f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS" + ], + modified_owners=[ + f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS" + ], + report=submission.Report( + found=True, + signed=False, + path=f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml", + ), + ), + expected_is_valid_certification=True, + ignore_owners=True, + ), # Invalid certification Submission contains unknown files CertificationScenario( input_submission=submission.Submission( @@ -319,7 +340,9 @@ class CertificationScenario: @pytest.mark.parametrize("test_scenario", scenarios_certification_submission) def test_is_valid_certification(test_scenario): is_valid_certification, reason = ( - test_scenario.input_submission.is_valid_certification_submission() + test_scenario.input_submission.is_valid_certification_submission( + test_scenario.ignore_owners + ) ) assert test_scenario.expected_is_valid_certification == is_valid_certification assert test_scenario.expected_reason in reason @@ -333,10 +356,29 @@ class OwnersScenario: scenarios_owners_submission = [ - # Valid submission contains only one OWNERS file + # Valid submission contains only one OWNERS file, not from a partner + OwnersScenario( + input_submission=submission.Submission( + api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/1", + chart=submission.Chart( + category="community", + organization=expected_organization, + name=expected_name, + ), + modified_files=[ + f"charts/community/{expected_organization}/{expected_name}/OWNERS" + ], + modified_owners=[ + f"charts/community/{expected_organization}/{expected_name}/OWNERS" + ], + ), + expected_is_valid_owners=True, + ), + # Invalid submission contains an OWNERS file from a partner OwnersScenario( input_submission=submission.Submission( api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/1", + chart=expected_chart, modified_files=[ f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS" ], @@ -344,31 +386,41 @@ class OwnersScenario: f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS" ], ), - expected_is_valid_owners=True, + expected_is_valid_owners=False, + expected_reason="[ERROR] OWNERS file should never be set directly by partners. See certification docs.", ), - # Invalid submission contains multiple OWNERS file + # Invalid submission an OWNERS file and other files OwnersScenario( input_submission=submission.Submission( api_url="https://api.github.com/repos/openshift-helm-charts/charts/pulls/1", + chart=submission.Chart( + category="community", + organization=expected_organization, + name=expected_name, + version=expected_version, + ), modified_files=[ - f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS", - f"charts/{expected_category}/{expected_organization}/another_chart/OWNERS", + f"charts/community/{expected_organization}/{expected_name}/OWNERS", + f"charts/community/{expected_organization}/{expected_name}/{expected_version}/report.yaml", ], modified_owners=[ - f"charts/{expected_category}/{expected_organization}/{expected_name}/OWNERS", - f"charts/{expected_category}/{expected_organization}/another_chart/OWNERS", + f"charts/community/{expected_organization}/{expected_name}/OWNERS", ], + report=submission.Report( + found=True, + signed=False, + path=f"charts/{expected_category}/{expected_organization}/{expected_name}/{expected_version}/report.yaml", + ), ), expected_is_valid_owners=False, expected_reason="[ERROR] Send OWNERS file by itself in a separate PR.", ), - # Invalid submission contains unknown files + # Invalid submission doesn't contain an OWNERS file OwnersScenario( input_submission=make_new_report_only_submission(), expected_is_valid_owners=False, expected_reason="No OWNERS file provided", ), - # Invalid submission doesn't contain an OWNER file ] @@ -377,8 +429,8 @@ def test_is_valid_owners(test_scenario): is_valid_owners, reason = ( test_scenario.input_submission.is_valid_owners_submission() ) - assert test_scenario.expected_is_valid_owners == is_valid_owners assert test_scenario.expected_reason in reason + assert test_scenario.expected_is_valid_owners == is_valid_owners @dataclass diff --git a/scripts/src/updateindex/updateindex.py b/scripts/src/updateindex/updateindex.py index e0f89ff2..6e248de6 100644 --- a/scripts/src/updateindex/updateindex.py +++ b/scripts/src/updateindex/updateindex.py @@ -11,7 +11,7 @@ import requests import yaml -from environs import Env +from environs import Env, EnvValidationError try: from yaml import CDumper as Dumper @@ -217,7 +217,10 @@ def main(): chart_entry = _decode_chart_entry(args.chart_entry_encoded) env = Env() - web_catalog_only = env.bool("WEB_CATALOG_ONLY", False) + try: + web_catalog_only = env.bool("WEB_CATALOG_ONLY", False) + except EnvValidationError: + web_catalog_only = False index_data = download_index(args.index_file, args.repository, args.index_branch) update_index(