Enhanced changelog validation #7942

cbbayburt · 2023-11-22T20:52:52Z

A complete overhaul of the changelog validation workflow, introducing many enhancements over the existing GitHub action.

Features

Validation is done by a standalone script (.github/workflows/changelogs/changelogs.py)
Checks if the changelog is added to the correct package
Performs basic checks against typos, spacing, capitalization, whitespace, and line length issues
Ensures correct spelling of tracker IDs (bsc#, jira-#, etc.) mentioned in the changelogs
Matches mentioned tracker IDs against the PR title and commit messages
Verifies that a bug exists and belongs to "SUSE Manager" in bugzilla.suse.com
Pinpoints the exact location of an issue in a changelog file
Recognizes every type of tracker as defined by OBS/IBS (see: .github/workflows/changelogs/trackers.xml)
Inline reporting of issues in the GitHub UI (see GUI diff)
Can also be executed locally by executing changelogs.py directly (See "How to run locally" below)

Bugzilla access and tracker information

To enable tracker validation through Bugzilla, we need a Bugzilla API key as a repository secret in uyuni-project/uyuni. The code uses python-bugzilla client and requests a bug via its ID to retrieve its product name.

If the API key is not set up in Github, the Bugzilla checks are skipped.

Tracker information is provided with the trackers.xml file included in the PR. This file is retrieved from the OBS API endpoint /issue_trackers.

GUI Diff

Console output

Inline reporting in GUI

How to run locally

Note: To run the script, you need to have the gh (GitHub CLI tool) and the python-bugzilla-3.2.0 library installed.

Basic functionality

Once you finish working on your feature and add the necessary changelog entries, execute the following command from your Uyuni repository root, providing all your modified files including the changelog files as positional arguments:

python .github/workflows/changelogs/changelogs.py $(git diff --name-only <pr_base_rev>..<pr_head_ref>)

Since no PR or tracker information is provided, PR and tracker validation will be skipped.

Full validation (PR, trackers, Bugzilla, etc.)

If you have any trackers (bsc#xxx, etc.) mentioned in your changelogs, PR title, and/or commit messages, you should enable the additional checks as well. To do that, add a PR number and a tracker file to the command using the corresponding arguments:

# Get XML tracker definition data from OBS and save it to a file
osc api /issue_trackers > trackers.xml

# Set the required environment variables (for PR and Bugzilla validation)
export GH_TOKEN=<your GitHub access token>
export BZ_TOKEN=<your Bugzilla API key>

python .github/workflows/changelogs/changelogs.py --tracker-file ./trackers.xml --pr-number <your_pr_no> \
    $(git diff --name-only <pr_base_rev>..<pr_head_ref>)

If you want to validate a PR in a different fork, you can specify any Uyuni fork using the --git-repo argument.

Below is a full list of available arguments, their usages, and default values:

python .github/workflows/changelogs/changelogs.py --help
usage: changelogs.py [-h] [-v] [-l LINE_LENGTH] [-t TRACKER_FILE] [-d SPACEWALK_DIR] [-p PR_NUMBER]
                     [-r GIT_REPO] [-b BUGZILLA_URI]
                     [FILE ...]

Validate changelog entries for Uyuni PRs

positional arguments:
  FILE                  the list of modified files in the pull request

options:
  -h, --help            show this help message and exit
  -v, --verbose         enable verbose output
  -l LINE_LENGTH, --line-length LINE_LENGTH
                        maximum line length allowed in changelog files (default: 67)
  -t TRACKER_FILE, --tracker-file TRACKER_FILE
                        tracker definitions XML document retrieved from the OBS/IBS API. Bypass tracker
                        validation if not provided.
  -d SPACEWALK_DIR, --spacewalk-dir SPACEWALK_DIR
                        path to the local git repository root (default: current directory)
  -p PR_NUMBER, --pr-number PR_NUMBER
                        the ID of the pull request to be validated. Bypass PR validation if not provided.
  -r GIT_REPO, --git-repo GIT_REPO
                        the Uyuni repository to validate the PR against (default: 'uyuni-project/uyuni')
  -b BUGZILLA_URI, --bugzilla-uri BUGZILLA_URI
                        the URI to the Bugzilla host to verify bug trackers (default: 'bugzilla.suse.com')

Uyuni project: <https://github.com/uyuni-project/uyuni>

More information

Check out the Python docstrings for more information.

How to review

Unit tests cover every validation rule extensively and provide a good understanding of what rules exist and how each one is enforced (test_validate_chlog_file_rules test method is a good start). Also, feel free to checkout the branch and run the script in your local copy of the Uyuni repository.

Documentation

Documentation included (docstrings and --help command)

Test coverage

Unit tests were added (pytest)

Links

Fixes https://github.com/SUSE/spacewalk/issues/2734

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

No changelog needed (how ironic 🤔)

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

Re-run test "changelog_test"
Re-run test "backend_unittests_pgsql"
Re-run test "java_pgsql_tests"
Re-run test "schema_migration_test_pgsql"
Re-run test "susemanager_unittests"
Re-run test "javascript_lint"
Re-run test "spacecmd_unittests"

agraul

Thanks a lot for the work, this looks nice already! You've done a great job with the test suite to show the different validations!

I left a few comments in line, most of them are just informational (about Python). Other than that, I have a general remark:

Can we move the user documentation from the PR description to its own file? It'll be easier to reference it in the future (e.g. when running the code locally). BTW, python3 .github/workflows/changelogs/changelogs.py $(git diff --name-only origin/master...HEAD) can be used to pass all files to the script (assuming a. the remote is called origin and b. you want to merge into master).

.github/workflows/changelogs/changelogs.py

agraul · 2023-11-24T13:36:36Z

.github/workflows/changelogs/changelogs.py

+            if not files["files"]:
+                # Changelog added but no file is modified
+                issues.append(Issue(IssueType.WRONG_CHLOG, package=pkg))


Could this happen intentionally? E.g. when we forgot a (second) tracker and want to add it afterwards

Sure, but I think it's better fixed with an explicit approval from rel-eng. The test itself should fail anyway.

I agree. in general we should not edit old changelogs. if this happen it should be only for good reasons. and if we add additional bsc we (releng) need to also be sure that the related patchinfo will also include that reference in a different way from a standard fixed bsc.
@juliogonzalez / @raulillo82 what's your take?

Yes, makes sense.

.github/workflows/changelogs/test_changelogs.py

cbosdo · 2023-11-27T10:12:07Z

Note that mkchglog has been extracted to: https://github.com/uyuni-project/uyuni-releng-tools

deneb-alpha

@cbbayburt I left a comment about the available trackers.

deneb-alpha · 2023-11-27T10:18:27Z

.github/workflows/changelogs/changelogs.py

+                    bug = self.bzapi.getbug(bug_id)
+                    logging.debug(f"Bug #{bug_id} belongs to product '{bug.product}'")
+
+                    if not bug.product.startswith("SUSE Manager"):


yes, for security the product is called
SUSE Security Incidents

.github/workflows/changelogs/changelogs.py

deneb-alpha · 2023-11-27T10:21:59Z

.github/workflows/changelogs/changelogs.py

+    parser.add_argument("-t", "--tracker-file",
+                        help="tracker definitions XML document retrieved from the OBS/IBS API. Bypass tracker validation if not provided.")


Please, keep in mind that we have some trackers in IBS not available in OBS.

I compared them, and it seems there are only minor differences in between. Which one should we use? OBS for Uyuni, and IBS for SUSE Manager? or something else?

I compared them, and it seems there are only minor differences in between. Which one should we use? OBS for Uyuni, and IBS for SUSE Manager? or something else?

the easy way it could also be to switch between Uyuni / OBS and SUMA / IBS

...but it's true that for using IBS we will need authentication. 🤔

in any case, IBS has all the trackers available in OBS, if the parsed tracker is valid in OBS it'svalid in IBS too. let's start easy

OBS also requires authentication 🤷🏻‍♂️

So we either create an account and store credentials in GitHub, or we update the file manually whenever needed.

before uploading that list we need to check why also this endpoint requires authentication :(

There's nothing risky in the data so my guess is it's just the default way the API is designed. But let's ask and make sure of course.

@deneb-alpha thanks for opening the OBS issue. IIUC from this comment, it's ok to upload it to our public repo.

cbbayburt · 2023-11-30T14:48:18Z

Thanks, @agraul for the review and all the tips. I'll address them all separately.

Can we move the user documentation from the PR description to its own file? It'll be easier to reference it in the future (e.g. when running the code locally). BTW, python3 .github/workflows/changelogs/changelogs.py $(git diff --name-only origin/master...HEAD) can be used to pass all files to the script (assuming a. the remote is called origin and b. you want to merge into master).

Good idea, I'll add it in a README in the directory.

deneb-alpha

left few more comments

deneb-alpha · 2023-11-30T17:51:20Z

.github/workflows/changelogs/changelogs.py

+                    bug = self.bzapi.getbug(bug_id)
+                    logging.debug(f"Bug #{bug_id} belongs to product '{bug.product}'")
+
+                    if not bug.product.startswith("SUSE Manager"):


it could also be that a bsc under SUSE Security Incidents could also be restricted. I think we could start checking only for SUMA and enhance this later

deneb-alpha · 2023-11-30T17:54:03Z

.github/workflows/changelogs/changelogs.py

+            if not files["files"]:
+                # Changelog added but no file is modified
+                issues.append(Issue(IssueType.WRONG_CHLOG, package=pkg))


I agree. in general we should not edit old changelogs. if this happen it should be only for good reasons. and if we add additional bsc we (releng) need to also be sure that the related patchinfo will also include that reference in a different way from a standard fixed bsc.
@juliogonzalez / @raulillo82 what's your take?

deneb-alpha · 2023-11-30T17:55:20Z

.github/workflows/changelogs/trackers.xml

how often we will refresh this xml file?

In the current state, it needs to be manually uploaded. I don't think this data gets updated often so it shouldn't be much of a problem.

I was also thinking about requesting it from the action itself and caching it, but it seems that the /issue_trackers endpoint needs authentication, so for that, we need to set up an OBS account and store the credentials as additional secrets in Github. All this seems like more work than just manually uploading it. Also, since IBS is not reachable from here, we can only do this for OBS.

deneb-alpha

@cbbayburt from my side +1.
but let's also check with the team if they have issues with what will be marked as failure and what only as warning.

deneb-alpha · 2023-12-04T17:34:28Z

.github/workflows/changelogs/changelogs.py

+    parser.add_argument("-t", "--tracker-file",
+                        help="tracker definitions XML document retrieved from the OBS/IBS API. Bypass tracker validation if not provided.")


...but it's true that for using IBS we will need authentication. 🤔

in any case, IBS has all the trackers available in OBS, if the parsed tracker is valid in OBS it'svalid in IBS too. let's start easy

juliogonzalez · 2024-01-25T09:05:52Z

.github/workflows/changelogs.yml

    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v4
      with:
-        fetch-depth: 1
+        fetch-depth: 2


Just out of curiousity, why 2?

juliogonzalez

One comment about the credentials.

juliogonzalez · 2024-01-25T09:06:42Z

.github/workflows/changelogs.yml

-      if: steps.changelogs.conclusion == 'success' && steps.changelogs.outputs.added_modified == ''
+    - name: Test changelog entries
+      env:
+        GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}


I think this won't work on forks, as the credentials won't be present there, so we'll have the same problem we had for SonarQube? @cbosdo, am I right about this one?

github-actions · 2024-03-26T01:39:46Z

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2024-05-26T01:56:38Z

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2024-07-27T01:55:52Z

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.

deneb-alpha

@cbbayburt for now I think it LGTM

* Use 'dataclasses' * Use python3 shebang * Use re.match to match beginning * Remove 'type' keyword usages * Use 'setdefault' * Remove 'wrong capitalization after colon' test * Rename 'spacewalk_dir' to 'uyuni_dir' * Use 'enumerate' in main loop * Fix line numbers off-by-one issue * Simplify runner code

cbbayburt requested review from deneb-alpha, juliogonzalez, raulillo82, agraul and a team November 22, 2023 20:52

cbbayburt requested a review from a team as a code owner November 22, 2023 20:52

cbbayburt force-pushed the changelog-action branch from 672bf7c to b1ffb4b Compare November 22, 2023 23:17

agraul reviewed Nov 24, 2023

View reviewed changes

deneb-alpha reviewed Nov 27, 2023

View reviewed changes

deneb-alpha reviewed Nov 30, 2023

View reviewed changes

deneb-alpha previously approved these changes Dec 4, 2023

View reviewed changes

juliogonzalez reviewed Jan 25, 2024

View reviewed changes

github-actions bot added the Stale label Mar 26, 2024

cbbayburt removed the Stale label Mar 26, 2024

github-actions bot added the Stale label May 26, 2024

cbbayburt removed the Stale label May 27, 2024

github-actions bot added the Stale label Jul 27, 2024

cbbayburt removed the Stale label Jul 27, 2024

raulillo82 previously approved these changes Nov 8, 2024

View reviewed changes

cbbayburt dismissed stale reviews from raulillo82 and deneb-alpha via ef24e5e November 24, 2024 17:45

cbbayburt force-pushed the changelog-action branch from 8b4660c to ef24e5e Compare November 24, 2024 17:45

cbbayburt requested a review from a team as a code owner November 24, 2024 17:45

cbbayburt force-pushed the changelog-action branch from cb76be1 to 921c0e4 Compare November 24, 2024 18:24

cbbayburt force-pushed the changelog-action branch 19 times, most recently from a9b7b94 to ab0d518 Compare November 25, 2024 14:21

cbbayburt requested a review from deneb-alpha November 25, 2024 14:26

deneb-alpha previously approved these changes Nov 25, 2024

View reviewed changes

cbbayburt dismissed deneb-alpha’s stale review via 0ef543f November 25, 2024 17:22

cbbayburt added 6 commits November 25, 2024 18:40

Add GH action for enhanced changelog validation

874f387

Skip Bugzilla validation if API key is not set

4a4263d

Update trackers file

eb29e66

Update failure comment message

d5affce

Add README.md

bd133a3

cbbayburt force-pushed the changelog-action branch from 0ef543f to bd133a3 Compare November 25, 2024 17:40

cbbayburt merged commit 762eca4 into uyuni-project:master Nov 25, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced changelog validation #7942

Enhanced changelog validation #7942

cbbayburt commented Nov 22, 2023 •

edited

Loading

agraul left a comment

agraul Nov 24, 2023

cbbayburt Nov 30, 2023

deneb-alpha Nov 30, 2023

juliogonzalez Jan 25, 2024

cbosdo commented Nov 27, 2023

deneb-alpha left a comment

deneb-alpha Nov 27, 2023

deneb-alpha Nov 27, 2023

cbbayburt Dec 1, 2023

deneb-alpha Dec 4, 2023

deneb-alpha Dec 4, 2023

cbbayburt Dec 4, 2023 •

edited

Loading

deneb-alpha Dec 4, 2023

cbbayburt Dec 4, 2023

cbbayburt Dec 5, 2023

cbbayburt commented Nov 30, 2023

deneb-alpha left a comment

deneb-alpha Nov 30, 2023

deneb-alpha Nov 30, 2023

deneb-alpha Nov 30, 2023

cbbayburt Dec 1, 2023

deneb-alpha left a comment

deneb-alpha Dec 4, 2023

juliogonzalez Jan 25, 2024

juliogonzalez left a comment

juliogonzalez Jan 25, 2024

github-actions bot commented Mar 26, 2024

github-actions bot commented May 26, 2024

github-actions bot commented Jul 27, 2024

deneb-alpha left a comment

		parser.add_argument("-t", "--tracker-file",
		help="tracker definitions XML document retrieved from the OBS/IBS API. Bypass tracker validation if not provided.")

Enhanced changelog validation #7942

Enhanced changelog validation #7942

Conversation

cbbayburt commented Nov 22, 2023 • edited Loading

Features

Bugzilla access and tracker information

GUI Diff

Console output

Inline reporting in GUI

How to run locally

Basic functionality

Full validation (PR, trackers, Bugzilla, etc.)

More information

How to review

Documentation

Test coverage

Links

Changelogs

Re-run a test

agraul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbosdo commented Nov 27, 2023

deneb-alpha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbbayburt Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbbayburt commented Nov 30, 2023

deneb-alpha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deneb-alpha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliogonzalez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 26, 2024

github-actions bot commented May 26, 2024

github-actions bot commented Jul 27, 2024

deneb-alpha left a comment

Choose a reason for hiding this comment

cbbayburt commented Nov 22, 2023 •

edited

Loading

cbbayburt Dec 4, 2023 •

edited

Loading