diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d9cda1ae..4f3eac6b 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 @@ -137,21 +142,25 @@ 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" \ + --repository ${{ github.repository }} \ + --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 +173,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 +184,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 +252,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 +270,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 +285,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 +293,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 +304,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 +313,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 +335,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 +370,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 +378,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 +386,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 +434,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 +471,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 +481,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 +534,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 +549,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 +565,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 +618,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..471e17bf --- /dev/null +++ b/scripts/src/precheck/precheck.py @@ -0,0 +1,133 @@ +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, repository: str): + # 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 + + index = submission.download_index_data(repository) + 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): + 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( + "-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( + "-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) + + 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: #TODO: why ? + pr_content_error_msg = craft_pr_content_error_msg(s, args.repository) + 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 43066153..fb3cfaa9 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 @@ -447,9 +490,10 @@ def parse_web_catalog_only(self, repo_path=""): ) if not owners_web_catalog_only == report_web_catalog_only: - raise WebCatalogOnlyError( - f"Value of web_catalog_only in OWNERS ({owners_web_catalog_only}) doesn't match the value in report ({report_web_catalog_only})" - ) + if owners_web_catalog_only: + raise WebCatalogOnlyError("[ERROR] The web catalog distribution method is set for the chart but is not set in the report.") + if report_web_catalog_only: + raise WebCatalogOnlyError("[ERROR] Report indicates web catalog only but the distribution method set for the chart is not web catalog only.") self.is_web_catalog_only = owners_web_catalog_only @@ -471,17 +515,21 @@ def is_valid_web_catalog_only(self, repo_path=""): """ if not self.report.found: - return False + return False, "nope" if len(self.modified_files) > 1: - return False + msg = "[ERROR] The web catalog distribution method requires the pull request to be report only." + return False, msg report_path = os.path.join(repo_path, self.report.path) found, report_data = verifier_report.get_report_data(report_path) if not found: raise WebCatalogOnlyError(f"Failed to get report data at {report_path}") - return verifier_report.get_package_digest(report_data) is not None + if verifier_report.get_package_digest(report_data) is None: + return False, "[ERROR] The web catalog distribution method requires a package digest in the report." + + return True, "" def get_file_type(file_path): @@ -523,7 +571,7 @@ def get_file_type(file_path): return "unknwown", None -def download_index_data(repository, branch="gh_pages"): +def download_index_data(repository, branch="gh-pages"): """Download the helm repository index""" r = requests.get( f"https://raw.githubusercontent.com/{repository}/{branch}/index.yaml" 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(