Add krel release notes validation workflow #2698

npolshakova · 2024-12-17T22:41:14Z

What type of PR is this:

/kind feature

What this PR does / why we need it:

Adds a GitHub Action to setup krel.

This is required to setup a presubmit job for PRs like: #2659. The current requirement is to add validation using:

$ krel release-notes validate --help
krel release-notes validate

The 'validate' subcommand of krel has been developed to:

Check release notes maps for valid yaml.
Check release notes maps for valid punctuation.

Usage:
  krel release-notes validate [flags]

Flags:
  -h, --help                           help for validate
      --path-to-release-notes string   The path to the release notes to validate. Can be a top level directory or a specific file.

The release notes team is blocked whenever invalid yaml is merged into sig-release:

#2589
#2541

Invalid yaml can be introduced during the review process when people apply changes or manually edit the maps (not through krel, but in an editor). This workflow automates preventing invalid yaml from merging into sig-release.

Which issue(s) this PR fixes:

Fixes kubernetes/release#2753

Special notes for your reviewer:

Based on @Vyom-Yadav's GitHub workflow: kubernetes-sigs/release-actions#101

Vyom-Yadav · 2024-12-18T12:38:30Z

/assign

.github/workflows/krel-release-notes-validate.yaml

Vyom-Yadav · 2025-01-05T08:54:31Z

.github/workflows/krel-release-notes-validate.yaml

+
+          KREL_VERSION=v0.17.12


I've pinned this to v0.17.12, if and when we do a new release of krel, we'd need to update this. (I think there may be some automated job to do that, or we can add this to the handbook (for the lead to update it).

Alternatively, do we want to fetch the latest release?

Hm, currently the handbook suggests the docs team uses the lastest krel, so maybe it's fine just fetching the latest release? https://github.com/kubernetes/sig-release/blob/ff03da9043cdd4cfd63f4c06c901909a744f2daa/release-team/role-handbooks/docs/README.md#tools

Done, I changed to latest.

Vyom-Yadav · 2025-01-11T10:45:47Z

Test run: https://github.com/npolshakova/sig-release/actions/runs/12723351941/job/35468384786?pr=4
Test comment: npolshakova#4 (comment)

Vyom-Yadav · 2025-01-11T10:46:27Z

/remove hold

Vyom-Yadav · 2025-01-11T10:52:34Z

/assign @npolshakova

npolshakova · 2025-01-12T02:58:03Z

Changes look good @Vyom-Yadav!

Is there a way to make this workflow required when merging release notes changes in? @kubernetes/sig-release-leads

It should only trigger on release notes changed conditions, but I think we might need admin permissions in GitHub to make it required?

Vyom-Yadav · 2025-01-13T16:25:18Z

I think required workflows might not be supported anymore (for new workflows - https://docs.github.com/en/enterprise-server@3.11/actions/sharing-automations/required-workflows), but a github admin can check that for us. Anyways, that can only be enabled after merging this action.

stmcginnis · 2025-01-14T19:51:01Z

/lgtm

cpanato

some comments

thanks for doing this

cpanato · 2025-01-15T09:53:15Z

.github/workflows/krel-release-notes-validate.yaml

+      - releases/**/release-notes/**.yml
+  # Allow manual triggering
+  workflow_dispatch: { }
+


add

permissions: {}

this will drop all permissions for the workflow, as we dont need

cpanato · 2025-01-15T09:53:52Z

.github/workflows/krel-release-notes-validate.yaml

+  krel_release_notes_validate_action:
+    name: Validate release notes with krel
+    runs-on: ubuntu-latest
+    if: ${{ !github.event.pull_request.draft }}


add

permissions: contents: read

we might just need the contents read to be able to clone

Since, it is a public repo, I don't think this is required. actions/checkout directly uses git, so I don't think we need to add any permission to the github_token

cpanato · 2025-01-15T09:54:50Z

.github/workflows/krel-release-notes-validate.yaml

+      - uses: sigstore/cosign-installer@dc72c7d5c4d10cd6bcb8cf6e3fd625a9e5e537da # v3.7.0
+        with:
+          use-sudo: false
+      - id: install-krel


i think this is fine for now, we can add install krel action in the release-actions repo.

It was discussed that krel install should not be exportable. Ref: kubernetes-sigs/release-actions#101 (comment)

cpanato · 2025-01-15T09:55:06Z

.github/workflows/krel-release-notes-validate.yaml

+                issue_number: context.issue.number,
+                body: `❌ YAML validation failed for the following files:\n\n${invalidFiles}`
+              });
+            }


nit: new line

Added a new line at the end.

cpanato · 2025-01-15T09:57:49Z

.github/workflows/krel-release-notes-validate.yaml

+          else
+            echo "No YAML files changed under /releases/*"
+          fi
+      - name: Comment on PR if invalid yaml detected


i think this will not work as expected because in fork PRs we dont have the write permission for the github token

i would add this as job summary and then we can see this in the checks tab

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#adding-a-job-summary

I agree. It only needs pull-requests but I'd prefer to keep the token with less permissions. Just fail it and we can check the results on the workflow run.

Migrated to github job summary. Test run: https://github.com/npolshakova/sig-release/actions/runs/12806142763?pr=4

puerco · 2025-01-15T17:55:07Z

.github/workflows/krel-release-notes-validate.yaml

+      - name: Check out code
+        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+        with:
+          # we need to fetch the full history in order to check changes across all commits on the branch


I think this comment is from another workflow :)

Suggested change

# we need to fetch the full history in order to check changes across all commits on the branch

We do CHANGED_FILES=$(git diff --name-only ${{ github.event.pull_request.base.sha }} ${{ github.sha }} -- releases/ | grep -E '\.ya?ml$' || true), so need fetch-depth 0. I removed the comment though (I think that is just extra)

puerco · 2025-01-15T17:57:15Z

.github/workflows/krel-release-notes-validate.yaml

+  push:
+    branches:
+      - master
+    paths:
+      - releases/**/release-notes/**.yaml
+      - releases/**/release-notes/**.yml


Since we're gating on the PR I think we can drop the run on push as we know the YAMLs are already valid.

Suggested change

push:

branches:

- master

paths:

- releases/**/release-notes/**.yaml

- releases/**/release-notes/**.yml

puerco · 2025-01-15T18:02:03Z

.github/workflows/krel-release-notes-validate.yaml

+          else
+            echo "No YAML files changed under /releases/*"
+          fi
+      - name: Comment on PR if invalid yaml detected


I agree. It only needs pull-requests but I'd prefer to keep the token with less permissions. Just fail it and we can check the results on the workflow run.

k8s-ci-robot · 2025-01-16T09:36:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: npolshakova
Once this PR has been reviewed and has the lgtm label, please ask for approval from cpanato. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Co-authored-by: npolshakova <nina.polshakova@solo.io> Signed-off-by: Vyom-Yadav <jackhammervyom@gmail.com>

Vyom-Yadav · 2025-01-16T09:48:57Z

New test run: https://github.com/npolshakova/sig-release/actions/runs/12806142763?pr=4

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 17, 2024

k8s-ci-robot requested review from gracenng and puerco December 17, 2024 22:41

k8s-ci-robot added needs-priority sig/release Categorizes an issue or PR as relevant to SIG Release. labels Dec 17, 2024

npolshakova added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/release Categorizes an issue or PR as relevant to SIG Release. needs-priority labels Dec 17, 2024

puerco mentioned this pull request Dec 18, 2024

Release Notes linting improvements kubernetes/release#3767

Open

k8s-ci-robot assigned Vyom-Yadav Dec 18, 2024

Vyom-Yadav force-pushed the npolshak/release-notes-validation-github-workflow branch from c469b5e to cc3bf07 Compare January 5, 2025 08:47

Vyom-Yadav reviewed Jan 5, 2025

View reviewed changes

.github/workflows/krel-release-notes-validate.yaml Outdated Show resolved Hide resolved

Vyom-Yadav reviewed Jan 5, 2025

View reviewed changes

.github/workflows/krel-release-notes-validate.yaml Outdated Show resolved Hide resolved

Vyom-Yadav reviewed Jan 5, 2025

View reviewed changes

.github/workflows/krel-release-notes-validate.yaml Outdated Show resolved Hide resolved

Vyom-Yadav reviewed Jan 5, 2025

View reviewed changes

npolshakova mentioned this pull request Jan 8, 2025

[1.33] Release Team Lead Cycle Progress #2705

Open

Vyom-Yadav force-pushed the npolshak/release-notes-validation-github-workflow branch 2 times, most recently from dea0462 to 83d5f96 Compare January 11, 2025 10:43

Vyom-Yadav removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 11, 2025

k8s-ci-robot assigned npolshakova Jan 11, 2025

k8s-ci-robot assigned stmcginnis Jan 14, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 14, 2025

cpanato requested changes Jan 15, 2025

View reviewed changes

k8s-ci-robot assigned cpanato Jan 15, 2025

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 15, 2025

puerco reviewed Jan 15, 2025

View reviewed changes

Vyom-Yadav force-pushed the npolshak/release-notes-validation-github-workflow branch 2 times, most recently from f9e3293 to 032ca70 Compare January 16, 2025 09:42

Add krel release notes validation workflow

1e65e4f

Co-authored-by: npolshakova <nina.polshakova@solo.io> Signed-off-by: Vyom-Yadav <jackhammervyom@gmail.com>

Vyom-Yadav force-pushed the npolshak/release-notes-validation-github-workflow branch from 032ca70 to 1e65e4f Compare January 16, 2025 09:48

Vyom-Yadav requested review from puerco and cpanato January 16, 2025 10:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add krel release notes validation workflow #2698

Add krel release notes validation workflow #2698

npolshakova commented Dec 17, 2024

Vyom-Yadav commented Dec 18, 2024

Vyom-Yadav Jan 5, 2025

npolshakova Jan 8, 2025

Vyom-Yadav Jan 11, 2025

Vyom-Yadav commented Jan 11, 2025

Vyom-Yadav commented Jan 11, 2025

Vyom-Yadav commented Jan 11, 2025

npolshakova commented Jan 12, 2025 •

edited

Loading

Vyom-Yadav commented Jan 13, 2025

stmcginnis commented Jan 14, 2025

cpanato left a comment

cpanato Jan 15, 2025

Vyom-Yadav Jan 16, 2025

cpanato Jan 15, 2025

Vyom-Yadav Jan 16, 2025

cpanato Jan 15, 2025

Vyom-Yadav Jan 16, 2025

cpanato Jan 15, 2025

Vyom-Yadav Jan 16, 2025

cpanato Jan 15, 2025

puerco Jan 15, 2025

Vyom-Yadav Jan 16, 2025

puerco Jan 15, 2025

Vyom-Yadav Jan 16, 2025

puerco Jan 15, 2025

Vyom-Yadav Jan 16, 2025

puerco Jan 15, 2025

k8s-ci-robot commented Jan 16, 2025

Vyom-Yadav commented Jan 16, 2025

Add krel release notes validation workflow #2698

Are you sure you want to change the base?

Add krel release notes validation workflow #2698

Conversation

npolshakova commented Dec 17, 2024

What type of PR is this:

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Vyom-Yadav commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vyom-Yadav commented Jan 11, 2025

Vyom-Yadav commented Jan 11, 2025

Vyom-Yadav commented Jan 11, 2025

npolshakova commented Jan 12, 2025 • edited Loading

Vyom-Yadav commented Jan 13, 2025

stmcginnis commented Jan 14, 2025

cpanato 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

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

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

k8s-ci-robot commented Jan 16, 2025

Vyom-Yadav commented Jan 16, 2025

npolshakova commented Jan 12, 2025 •

edited

Loading