Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: On vX.Y-rhel branches, ensure that some downstream Jira issue is linked #23622

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,10 @@ test-binaries: test/checkseccomp/checkseccomp test/goecho/goecho install.cataton
tests-included:
contrib/cirrus/pr-should-include-tests

.PHONY: test-jira-links-included
test-jira-links-included:
contrib/cirrus/pr-should-link-jira

.PHONY: tests-expect-exit
tests-expect-exit:
@if grep -E --line-number 'Expect.*ExitCode' test/e2e/*.go | grep -E -v ', ".*"\)'; then \
Expand Down
79 changes: 79 additions & 0 deletions contrib/cirrus/pr-should-link-jira
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#!/bin/bash
#
# Intended for use in CI, for -rhel branches: check git PR, barf if no jira links
#

ME=$(basename $0)

set -e

# Github label which allows overriding this check
OVERRIDE_LABEL="No Jira Link"

# Only -rhel branches need jira links
alexlarsson marked this conversation as resolved.
Show resolved Hide resolved
BRANCH_REGEX="v.*-rhel"
alexlarsson marked this conversation as resolved.
Show resolved Hide resolved

LINK_REGEX="Fixes:?[[:space:]]+https://issues.redhat.com/[[:alnum:]/-]*"

if [[ ! "${DEST_BRANCH}" =~ $BRANCH_REGEX ]]; then
exit 0
fi

if [[ "${CIRRUS_CHANGE_MESSAGE}" =~ $LINK_REGEX ]]; then
exit 0
fi
cevich marked this conversation as resolved.
Show resolved Hide resolved

# Nope. Only allow if the github 'No Jira Link' label is set
if [[ -z "$CIRRUS_PR" ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable will be empty for executions on branches and tags, causing this script to fail and needlessly fire off monitoring alert e-mails. I think maybe an easy fix is to simply set it to exit 0 instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is called from _run_validate-source, which is only run during PRs,, as per:

validate-source_task:
    name: "Validate source code changes"
    alias: validate-source
    # This task is primarily intended to catch human-errors early on, in a
    # PR context.  Skip running it everywhere else.
    only_if: &is_pr "$CIRRUS_PR != ''"

So, this is not a problem.
In fact, this code is an identical copy from the similar pr-should-include-tests that already works for the same reason.

Copy link
Member

@cevich cevich Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, though scripts and tasks do get shuffled around now and again. Would you consider:

if [[ -z "$CIRRUS_PR" ]]; then
    ...
elif [[ ! "$CIRRUS_BRANCH" =~ pull ]] || [[ -n "$CIRRUS_TAG" ]]; then
    echo "Warning: $ME only intended for use on PRs in CI"
    exit 0
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added something like this

if [[ ! "$CIRRUS_BRANCH" =~ pull ]] || [[ -n "$CIRRUS_TAG" ]]; then
echo "Warning: $ME only intended for use on PRs in CI"
exit 0
fi
echo "$ME: cannot query github: \$CIRRUS_PR is undefined" >&2
exit 1
fi

if [[ -z "$CIRRUS_REPO_CLONE_TOKEN" ]]; then
echo "$ME: cannot query github: \$CIRRUS_REPO_CLONE_TOKEN is undefined" >&2
exit 1
fi

query="{
\"query\": \"query {
repository(owner: \\\"containers\\\", name: \\\"podman\\\") {
pullRequest(number: $CIRRUS_PR) {
labels(first: 100) {
nodes {
name
}
}
}
}
}\"
}"

result=$(curl -s -H "Authorization: bearer $CIRRUS_REPO_CLONE_TOKEN" -H "Accept: application/vnd.github.antiope-preview+json" -H "Content-Type: application/json" -X POST --data @- https://api.github.com/graphql <<<"$query")

labels=$(jq -r '.data.repository.pullRequest.labels.nodes[]?.name' <<<"$result")

if grep -F -x -q "$OVERRIDE_LABEL" <<<"$labels"; then
# PR has the label set
exit 0
fi

cat <<EOF
$ME: PR does not include required references to Jira issues

Please add a reference to the related Jira ticket(s) by adding to the
description of the PR something like:

Fixes: https://issues.redhat.com/browse/RHEL-50507

You can use multiple lines like this, but only one issue per line.

If your commit really, truly does not need a jira link, you can proceed
by asking a repo maintainer to set the '$OVERRIDE_LABEL' github label.
This will only be done when there's no reasonable alternative.
alexlarsson marked this conversation as resolved.
Show resolved Hide resolved
EOF

exit 1
146 changes: 146 additions & 0 deletions contrib/cirrus/pr-should-link-jira.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
#!/bin/bash
alexlarsson marked this conversation as resolved.
Show resolved Hide resolved
#
# tests for pr-should-link-jira.t
#

ME=$(basename $0)

# Our test script queries github, for which we need token
if [[ -z "$CIRRUS_REPO_CLONE_TOKEN" ]]; then
if [[ -n "$GITHUB_TOKEN" ]]; then
export CIRRUS_REPO_CLONE_TOKEN="$GITHUB_TOKEN"
else
echo "$ME: Please set \$CIRRUS_REPO_CLONE_TOKEN" >&2
exit 1
fi
fi

###############################################################################
# BEGIN test cases
#

read -d '\n' msg_no_jira << EndOfText
This is some text
without a jira
EndOfText

read -d '\n' msg_invalid << EndOfText
This is some text
without a jira
Fixes #42
More text...
EndOfText

read -d '\n' msg_jira << EndOfText
This is some text
with a jira
Fixes https://issues.redhat.com/browse/RHEL-50507
More text...
EndOfText

read -d '\n' msg_jira2 << EndOfText
This is some text
with a jira
Fixes: https://issues.redhat.com/browse/RHEL-50507
More text...
EndOfText

read -d '\n' msg_multiple << EndOfText
This is some text
with multiple jira lines
Fixes https://issues.redhat.com/browse/RHEL-50507
More text...
Fixes: https://issues.redhat.com/browse/RHEL-50506
More text...
EndOfText

# Feel free to add as needed. Syntax is:
# <exit status> <pr> <commit message> <dest branch> # comments
#
# Where:
# exit status is the expected exit status of the script
# pr pr number (only used to get tag, 0000 if doesn't matter)
# commit message commit message
# dest branch name of branch
#

tests="
0 0000 msg_no_jira main not rhel branch, no link, should pass
0 0000 msg_jira main not rhel branch, link, should pass
0 0000 msg_invalid main not rhel branch, invalid link, should pass
0 0000 msg_no_jira v4.9 not rhel branch, no link, should pass
1 23514 msg_no_jira v4.9-rhel no link, no tag, should fail
0 8890 msg_no_jira v4.9-rhel no link, tag, should work
1 23514 msg_invalid v4.9-rhel invalid link, no tag, should fail
0 0000 msg_jira v4.9-rhel link, should work
0 0000 msg_jira2 v4.9-rhel link with colon, should work
0 0000 msg_multiple v4.9-rhel multiple links, should work
"

# The script we're testing
test_script=$(dirname $0)/$(basename $0 .t)

# END test cases
###############################################################################
# BEGIN test-script runner and status checker

function run_test_script() {
local expected_rc=$1
local testname=$2

testnum=$(( testnum + 1 ))

# DO NOT COMBINE 'local output=...' INTO ONE LINE. If you do, you lose $?
local output
output=$( $test_script )
local actual_rc=$?

if [[ $actual_rc != $expected_rc ]]; then
echo "not ok $testnum $testname"
echo "# expected rc $expected_rc"
echo "# actual rc $actual_rc"
if [[ -n "$output" ]]; then
echo "# script output: $output"
fi
rc=1
else
if [[ $expected_rc == 1 ]]; then
# Confirm we get an error message
if [[ ! "$output" =~ "Please add a reference" ]]; then
echo "not ok $testnum $testname"
echo "# Expected: ~ 'Please add a reference'"
echo "# Actual: $output"
rc=1
else
echo "ok $testnum $testname - rc=$expected_rc"
fi
else
echo "ok $testnum $testname - rc=$expected_rc"
fi
fi
}

# END test-script runner and status checker
###############################################################################
# BEGIN test-case parsing

rc=0
testnum=0
tested_override=

while read expected_rc pr msg branch rest; do
# Skip blank lines
test -z "$expected_rc" && continue

export DEST_BRANCH=$branch
export CIRRUS_CHANGE_MESSAGE="${!msg}"
export CIRRUS_PR=$pr

run_test_script $expected_rc "PR $pr $msg $branch - $rest"
done <<<"$tests"

echo "1..$testnum"
exit $rc

# END Test-case parsing
###############################################################################
3 changes: 3 additions & 0 deletions contrib/cirrus/prebuild.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ if [[ "${DISTRO_NV}" == "$PRIOR_FEDORA_NAME" ]]; then
# Tests for lib.sh
showrun ${SCRIPT_BASE}/lib.sh.t

# Tests for pr-should-link-jira
showrun ${SCRIPT_BASE}/pr-should-link-jira.t

msg "Checking renovate config."
showrun podman run -it \
-v ./.github/renovate.json5:/usr/src/app/renovate.json5:z \
Expand Down
3 changes: 3 additions & 0 deletions contrib/cirrus/runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ function _run_validate-source() {

# make sure PRs have tests
showrun make tests-included

# make sure PRs have jira links (if needed for branch)
showrun make test-jira-links-included
}

function _run_unit() {
Expand Down