Lint yaml workflow update #2547

Checksumz · 2024-07-02T12:22:25Z

What type of PR is this:

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

…lpha.1' into npolshak/lint-yaml-workflow

k8s-ci-robot · 2024-07-02T12:22:30Z

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

cpanato

thanks for working on this
a few comments

.github/workflows/yaml-lint-checker.yaml

cpanato · 2024-07-02T12:55:05Z

.github/workflows/yaml-lint-checker.yaml

+      - name: Check out code
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: '0'


do you need to fetch the entire history?

As we edit the release notes, we usually keep the history: #2542

But then before we merge into the sig-release repo, we clean up everything into a single commit.

I thought fetch-depth: 1 only fetches the latest commit? So unless we get the entire history, the workflow will only run on a single commit (which might be fine since the last commit will have the entire diff, but makes it a little harder to identify the invalid yaml during the release notes review).

cpanato · 2024-07-02T12:57:21Z

.github/scripts/validate_yaml.go

+		fmt.Println("YAML Validation Failed:\n")
+		for _, errorMsg := range invalidFiles {
+			fmt.Println(errorMsg)
+			fmt.Println("\n" + strings.Repeat("-", 40) + "\n")


here should exit with error

os.Exit(1)

I think we want to print all the invalid files, and then exit after the loop.

As Nina mentioned, this is to gather all the output into the variable on failure

cpanato · 2024-07-02T12:59:02Z

.github/workflows/yaml-lint-checker.yaml

+          INVALID_FILES=""
+          # check diff for invalid yaml files
+          git diff --name-only ${{ github.event.pull_request.base.sha }} ${{ github.sha }} -- releases/ | grep -E '\.ya?ml$'
+          CHANGED_FILES=$(git diff --name-only $(git merge-base origin/master HEAD) -- releases/ | grep -E '\.ya?ml$' || true)


for this part you maybe can use this action that get all you need: https://github.com/tj-actions/changed-files

checking this

cpanato · 2024-07-02T12:59:34Z

.github/workflows/yaml-lint-checker.yaml

+              # allows you to set env variables for subsequent steps in the same job
+              echo "invalid_files<<EOF" >> $GITHUB_ENV
+              echo "$OUTPUT" >> $GITHUB_ENV
+              echo "EOF" >> $GITHUB_ENV


this works? not 100% sure that this works

Verified on my local branch that this allows to create a multiline variable for use in the next step. Will send a screenshot with echo later

Release action from today's release meeting for reference : https://github.com/kubernetes-sigs/release-actions

cpanato

i think this should go to the release-actions repo we have and then re-use here, to avoid having code in this repo.

wdyt @saschagrunert @puerco @xmudrii ?

k8s-ci-robot · 2024-07-02T13:00:59Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Checksumz
Once this PR has been reviewed and has the lgtm label, please ask for approval from cpanato. For more information see the Kubernetes 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

cpanato · 2024-07-02T13:01:05Z

/hold

saschagrunert · 2024-07-02T13:14:36Z

i think this should go to the release-actions repo we have and then re-use here, to avoid having code in this repo.

wdyt @saschagrunert @puerco @xmudrii ?

Yep I agree, thank you for the effort into this @Checksumz 🙏

npolshakova · 2024-07-02T16:41:36Z

i think this should go to the release-actions repo we have and then re-use here, to avoid having code in this repo.

wdyt @saschagrunert @puerco @xmudrii ?

Are these the release actions workflows? We'd want to be able to run this on release notes PRs going into sig-release and block if there's any invalid yaml in the diff. That would prevent having to create a separate PR to unblock the release notes like this one. If this workflow lives in the release-actions repo, can we still require it for PRs going into the sig-release repo?

Co-authored-by: Carlos Tadeu Panato Junior <ctadeu@gmail.com>

…umz/sig-release into lint-yaml-workflow-update

Checksumz · 2024-07-03T11:52:30Z

@cpanato i have implemented most of your suggestions apart from exit(0).

@npolshakova @cpanato @saschagrunert Just want to confirm if i need to close this PR and reopen it with https://github.com/kubernetes-sigs/release-actions and update it to be a reusable workflow ?

npolshakova and others added 3 commits June 12, 2024 12:19

yaml lint workflow

b7bb4ca

Merge remote-tracking branch 'checksumz/release-notes-draft-v1.31.0-a…

6f564be

…lpha.1' into npolshak/lint-yaml-workflow

Update verification of valid yaml to go k8s

5704e85

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 2, 2024

k8s-ci-robot added do-not-merge/contains-merge-commits cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 2, 2024

k8s-ci-robot requested a review from cici37 July 2, 2024 12:22

k8s-ci-robot added the sig/release Categorizes an issue or PR as relevant to SIG Release. label Jul 2, 2024

k8s-ci-robot requested a review from puerco July 2, 2024 12:22

k8s-ci-robot added needs-priority size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 2, 2024

Update go libs

f1c7199

cpanato requested changes Jul 2, 2024

View reviewed changes

k8s-ci-robot assigned cpanato Jul 2, 2024

cpanato reviewed Jul 2, 2024

View reviewed changes

cpanato requested changes Jul 2, 2024

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 2, 2024

Checksumz and others added 6 commits July 3, 2024 19:38

Update .github/workflows/yaml-lint-checker.yaml

634970b

Co-authored-by: Carlos Tadeu Panato Junior <ctadeu@gmail.com>

remove eye emoji

d72bb9c

Update .github/workflows/yaml-lint-checker.yaml

3b3105e

Co-authored-by: Carlos Tadeu Panato Junior <ctadeu@gmail.com>

Update all action versions to specific hash

72a2b19

Update .github/workflows/yaml-lint-checker.yaml

79d0111

Co-authored-by: Carlos Tadeu Panato Junior <ctadeu@gmail.com>

pull suggestions for go version

2e0645c

Merge branch 'lint-yaml-workflow-update' of https://github.com/Checks…

6e4e5da

…umz/sig-release into lint-yaml-workflow-update

Checksumz closed this by deleting the head repository Jul 16, 2024

npolshakova mentioned this pull request Sep 17, 2024

Add krel validate cmd kubernetes/release#3765

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lint yaml workflow update #2547

Lint yaml workflow update #2547

Checksumz commented Jul 2, 2024

k8s-ci-robot commented Jul 2, 2024

cpanato left a comment

cpanato Jul 2, 2024

npolshakova Jul 2, 2024

cpanato Jul 2, 2024

npolshakova Jul 2, 2024

Checksumz Jul 2, 2024

cpanato Jul 2, 2024

Checksumz Jul 3, 2024

cpanato Jul 2, 2024

Checksumz Jul 2, 2024

cloudmelon Jul 2, 2024

cpanato left a comment

k8s-ci-robot commented Jul 2, 2024

cpanato commented Jul 2, 2024

saschagrunert commented Jul 2, 2024

npolshakova commented Jul 2, 2024 •

edited

Loading

Checksumz commented Jul 3, 2024

Lint yaml workflow update #2547

Lint yaml workflow update #2547

Conversation

Checksumz commented Jul 2, 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:

k8s-ci-robot commented Jul 2, 2024

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

cpanato left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 2, 2024

cpanato commented Jul 2, 2024

saschagrunert commented Jul 2, 2024

npolshakova commented Jul 2, 2024 • edited Loading

Checksumz commented Jul 3, 2024

npolshakova commented Jul 2, 2024 •

edited

Loading