Skip to content

Commit

Permalink
Implement bugfix for chart dependencies and gitops.
Browse files Browse the repository at this point in the history
This PR contains all of the impactful changes to how the certification
pipeline works in order to resolve issues when depending on charts we
provide.

In effect, this PR enables:

- Locked helm chart name tracking and generation
- Checks on OWNERS file submission to ensure locks are respected
- Fixes indexing strategy to index on chart names only

In addition, this PR includes E2E testing extensions for some of the new
logic.

Signed-off-by: Jose R. Gonzalez <komish@flutes.dev>
  • Loading branch information
komish committed Mar 4, 2024
1 parent af6cbb5 commit 801c53e
Show file tree
Hide file tree
Showing 8 changed files with 548 additions and 11 deletions.
176 changes: 176 additions & 0 deletions .github/workflows/check-locks-on-owners-submission.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
name: Check Chart Lock Status

# Checks Red Hat and Community OWNERS file additions against our chart lockfile.
#
# Red Hat and Community OWNERS files are not auto-merged, and require maintainer
# review and action. This is intended to automate checking lock files.
#
# Technically maintainers should re-run this workflow just before merging. This is
# noted in the comment sent to the PR.
#
# This does not fail even if a chart is locked. A modification to an existing chart
# lock is considered valid, and therefore maintainers will use their discretion
# to merge modifications.

on:
pull_request_target:
types: [opened, synchronize, reopened, edited, ready_for_review, labeled]
paths:
- charts/community/**/OWNERS
- charts/redhat/**/OWNERS

jobs:
owners-file-check:
name: OWNERS file PR checker
runs-on: ubuntu-20.04
if: |
github.event.pull_request.draft == false
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Set up Python 3.x Part 1
uses: actions/setup-python@v4
with:
python-version: "3.10"

- name: Set up Python 3.x Part 2
run: |
# set up python
python3 -m venv ve1
cd scripts
../ve1/bin/pip3 install -r requirements.txt
../ve1/bin/pip3 install .
cd ..
- name: get files changed
id: get_files_changed
env:
BOT_TOKEN: ${{ secrets.BOT_TOKEN }}
run: |
# get files in PR
./ve1/bin/pr-artifact --api-url=${{ github.event.pull_request._links.self.href }} \
--get-files
- name: check if only an OWNERS file is pushed
id: check_for_owners
env:
pr_files_py: "${{ steps.get_files_changed.outputs.pr_files }}"
run: |
# check if PR contains just one redhat/community OWNERS file
pr_files=($(echo "$pr_files_py" | tr -d '[],'))
echo "Files in PR: ${pr_files[@]}"
eval first_file=${pr_files[0]}
if [ ${#pr_files[@]} == 1 ]; then
eval first_file=${pr_files[0]}
if [[ $first_file == "charts/redhat/"*/*"/OWNERS" ]] || [[ $first_file == "charts/community/"*/*"/OWNERS" ]] ; then
echo "An OWNERS file has been modified or added"
echo "merge_pr=true" | tee -a $GITHUB_OUTPUT
else
echo "The file in the PR is not a Red Hat or Community OWNERS file"
echo "merge_pr=false" | tee -a $GITHUB_OUTPUT
echo "msg=ERROR: PR does not include a redhat/community OWNERS file." >> $GITHUB_OUTPUT
fi
else
echo "The PR contains multiple files."
echo "msg=ERROR: PR contains multiple files." >> $GITHUB_OUTPUT
echo "merge_pr=false" | tee -a $GITHUB_OUTPUT
fi
# We know that only one file was modified at this point, and it seems
# mergeable. Determine if that file was created or modified here.
#
# This step only checks the first file for its modification type!
- name: Determine if net-new OWNERS file
id: populate-file-mod-type
if: ${{ steps.check_for_owners.outputs.merge_pr == 'true' }}
uses: actions/github-script@v6
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const resp = await github.rest.pulls.listFiles({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: context.issue.number,
});
const ownersFile = resp.data[0];
console.log(`Modified file "${ownersFile.filename} has a status of ${ownersFile.status}`);
console.log(`setting output: net-new-owners-file=${ownersFile.status == 'added'}`);
core.setOutput('net-new-owners-file', ownersFile.status == 'added');
- name: Add category/organization/chart-name from modified file to GITHUB_OUTPUT
id: gather-metadata
env:
API_URL: ${{ github.event.pull_request._links.self.href }}
BOT_TOKEN: ${{ secrets.BOT_TOKEN }}
run: |
./ve1/bin/extract-metadata-from-pr \
--emit-to-github-output \
${{ env.API_URL }}
# Only used to assert content of the OWNERS file.
- name: Checkout Pull Request
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
token: ${{ secrets.BOT_TOKEN }}
fetch-depth: 0
path: "pr-branch"

- name: Ensure OWNERS content and path content align
working-directory: "pr-branch"
id: fact-check
run: |
file=$(echo ${{ steps.get_files_changed.outputs.pr_files }} | yq .0)
owner_contents_chart_name=$(yq '.chart.name' ${file})
owner_contents_vendor_label=$(yq '.vendor.label' ${file})
echo "Chart Name Comparison: ${owner_contents_chart_name} = ${{ steps.gather-metadata.outputs.chart-name }}"
echo "Vendor Label Comparison: ${owner_contents_vendor_label} = ${{ steps.gather-metadata.outputs.organization }}"
test "${owner_contents_chart_name}" = "${{ steps.gather-metadata.outputs.chart-name }}"
test "${owner_contents_vendor_label}" = "${{ steps.gather-metadata.outputs.organization }}"
- name: Check if chart name is locked
id: determine-lock-status
uses: ./.github/actions/check-chart-locks
with:
chart-name: ${{ steps.gather-metadata.outputs.chart-name }}
fail-workflow-if-locked: 'false'

- name: Comment on PR if chart is locked
id: comment-if-locked
if: steps.determine-lock-status.outputs.chart-is-locked == 'true'
run: |
gh pr comment ${{ github.event.number }} --body "${{ env.COMMENT_BODY }}"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
COMMENT_BODY: |
### :lock: Chart Lock Check
The OWNERS file contributed here has a chart name that is **LOCKED**!
| chart name | lock path |
| - | - |
| ${{ steps.gather-metadata.outputs.chart-name }} | ${{ steps.determine-lock-status.outputs.locked-to-path }} |
This OWNERS file is being ${{ steps.populate-file-mod-type.outputs.net-new-owners-file && '**created**' || '**modified**'}} in this pull request.
_This comment was auto-generated by [GitHub Actions](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})._
- name: Comment on PR if chart is not locked
id: comment-if-available
if: steps.determine-lock-status.outputs.chart-is-locked == 'false'
run: |
gh pr comment ${{ github.event.number }} --body "${{ env.COMMENT_BODY }}"
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
COMMENT_BODY: |
### :unlock: Maintainers:
The OWNERS file contributed here has a chart name that is **AVAILABLE**!
The chart name '${{ steps.gather-metadata.outputs.chart-name }}' does not appear in our lockfile.
After reviewing this pull request, please re-run this workflow once more before merging.
_This comment was auto-generated by [GitHub Actions](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})._
59 changes: 59 additions & 0 deletions .github/workflows/lock-sanity-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
name: Chart Lock Generation Sanity Check

# Runs chart generation on every push to main to make
# sure things are healthy. Alerts maintainers if there
# are issues.
#
# Main should *always* be in a healthy state

on:
push:
branches:
- main
workflow_dispatch:

jobs:
ensure-health:
runs-on: ubuntu-latest
steps:
- name: checkout
uses: actions/checkout@v4
- 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: |
# set up python
python3 -m venv ve1
cd scripts
../ve1/bin/pip3 install -r requirements.txt
../ve1/bin/pip3 install .
cd ..
- name: generate chart locks
id: generate
uses: ./.github/actions/generate-chart-locks
- name: notify maintainers on failure
id: notify
if: failure() \
&& steps.generate.outcome == 'failure' \
&& github.repository == 'openshift-helm-charts/charts'
uses: archive/github-actions-slack@v2.7.0
with:
slack-bot-user-oauth-access-token: ${{ secrets.SLACK_BOT_USER_OAUTH_ACCESS_TOKEN }}
slack-channel: C02979BDUPL
slack-text: |
Failing to generate chart locks! '${{github.server_url}}/${{github.repository}}/actions/runs/${{github.run_id}}'
- name: create issue if notification failure
id: allback-notify
if: failure() && steps.notify.outcome == 'failure'
run: |
gh issue create --title "Chart lock generation sanity check is failing" --body "${{ env.COMMENT_BODY }}"
env:
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
COMMENT_BODY: |
While performing a sanity check of the chart generation logic on the main branch, a failure occurred!
See workflow: ${{github.server_url}}/${{github.repository}}/actions/runs/${{github.run_id}}
cc @komish
103 changes: 94 additions & 9 deletions .github/workflows/mercury_bot.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
name: OWNERS PR Check

# Checks OWNERS file additions and modifications submitted by mercury-bot on a
# partner's behalf, typically in response to a partner updating their project
# metadata.
#
# Automatically approves modifications mercury-bot makes. Sanity checks
# net-new OWNERS files to ensure their addition does not conflict with
# existing locks.

on:
pull_request_target:
types: [opened, synchronize, reopened, edited, ready_for_review, labeled]
paths:
- charts/partners/**/OWNERS


jobs:
owners-file-check:
Expand Down Expand Up @@ -49,18 +60,91 @@ jobs:
eval first_file=${pr_files[0]}
if [[ $first_file == "charts/partners/"*/*"/OWNERS" ]] ; then
echo "An OWNERS file has been modified or added"
echo "merge_pr=true" >> $GITHUB_OUTPUT
echo "merge_pr=true" | tee -a $GITHUB_OUTPUT
else
echo "The file in the PR is not a partner OWNERS file"
echo "merge_pr=false" >> $GITHUB_OUTPUT
echo "merge_pr=false" | tee -a $GITHUB_OUTPUT
echo "msg=ERROR: PR does not include a partner OWNERS file." >> $GITHUB_OUTPUT
fi
else
echo "The PR contains multiple files."
echo "msg=ERROR: PR contains multiple files." >> $GITHUB_OUTPUT
echo "merge_pr=false" >> $GITHUB_OUTPUT
echo "merge_pr=false" | tee -a $GITHUB_OUTPUT
fi
# We know that only one file was modified at this point, and it seems
# mergeable. Determine if that file was created or modified here.
#
# This step only checks the first file for its modification type!
- name: Determine if net-new OWNERS file
id: populate-file-mod-type
if: ${{ steps.check_for_owners.outputs.merge_pr == 'true' }}
uses: actions/github-script@v6
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const resp = await github.rest.pulls.listFiles({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: context.issue.number,
});
const ownersFile = resp.data[0];
console.log(`Modified file "${ownersFile.filename} has a status of ${ownersFile.status}`);
console.log(`setting output: net-new-owners-file=${ownersFile.status == 'added'}`);
core.setOutput('net-new-owners-file', ownersFile.status == 'added');
- name: Add category/organization/chart-name from modified file to GITHUB_OUTPUT
id: gather-metadata
env:
API_URL: ${{ github.event.pull_request._links.self.href }}
BOT_TOKEN: ${{ secrets.BOT_TOKEN }}
run: |
./ve1/bin/extract-metadata-from-pr \
--emit-to-github-output \
${{ env.API_URL }}
# Only used to assert content of the OWNERS file.
- name: Checkout Pull Request
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
token: ${{ secrets.BOT_TOKEN }}
fetch-depth: 0
path: "pr-branch"

# TODO: Implemented as a shell script temporarily. Future enhancement
# would re-use some existing logic that we have for the Red Hat OWNERS
# check, and build something similar for general use.
- name: Ensure OWNERS content and path content align
working-directory: "pr-branch"
id: fact-check
run: |
file=$(echo ${{ steps.get_files_changed.outputs.pr_files }} | yq .0)
owner_contents_chart_name=$(yq '.chart.name' ${file})
owner_contents_vendor_label=$(yq '.vendor.label' ${file})
echo "Chart Name Comparison: ${owner_contents_chart_name} = ${{ steps.gather-metadata.outputs.chart-name }}"
echo "Vendor Label Comparison: ${owner_contents_vendor_label} = ${{ steps.gather-metadata.outputs.organization }}"
test "${owner_contents_chart_name}" = "${{ steps.gather-metadata.outputs.chart-name }}"
test "${owner_contents_vendor_label}" = "${{ steps.gather-metadata.outputs.organization }}"
- name: Check if chart name is locked
id: determine-lock-status
uses: ./.github/actions/check-chart-locks
with:
chart-name: ${{ steps.gather-metadata.outputs.chart-name }}
fail-workflow-if-locked: 'false'

# Do not merge net-new OWNERS files for locked chart names. Allow a
# modification to an existing file. The mercury-bot periodically updates
# this file as users make changes to their project settings.
- name: Determine if should merge and is safe to merge
id: safe-to-merge
run: |
echo "OWNERS file is net new: ${{ steps.populate-file-mod-type.outputs.net-new-owners-file }}"
echo "Chart name is already locked: ${{ steps.determine-lock-status.outputs.chart-is-locked }}"
echo "merge_pr=${{ steps.populate-file-mod-type.outputs.net-new-owners-file == 'false' || (steps.populate-file-mod-type.outputs.net-new-owners-file == 'true' && steps.determine-lock-status.outputs.chart-is-locked == 'false') }}" | tee -a $GITHUB_OUTPUT
- name: Comment on PR
if: ${{ steps.check_for_owners.outputs.merge_pr == 'false' }}
uses: actions/github-script@v7
Expand All @@ -77,15 +161,18 @@ jobs:
});
- name: Reflect on check for OWNERS file result
if: ${{ steps.check_for_owners.outputs.merge_pr == 'false' }}
if: |
steps.check_for_owners.outputs.merge_pr == 'false'
|| steps.safe-to-merge.outputs.merge_pr == 'false'
run: |
# exit with failure if PR is not for a single partner OWNERS file
echo "The PR was not for a single partner OWNERS file."
echo "::error: The PR was not for a single partner OWNERS file, or was for a chart name locked to another contributor"
exit 1
- name: Approve PR
id: approve_pr
if: ${{ steps.check_for_owners.outputs.merge_pr == 'true' }}
if: |
steps.check_for_owners.outputs.merge_pr == 'true'
&& steps.safe-to-merge.outputs.merge_pr == 'true'
uses: hmarr/auto-approve-action@v3
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
Expand All @@ -98,5 +185,3 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
MERGE_METHOD: squash
MERGE_LABELS: ""


Loading

0 comments on commit 801c53e

Please sign in to comment.