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

Add GitHub Actions workflow for linting YAML files #11493

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

KenEkanem
Copy link

Add GitHub Actions workflow for linting YAML files

Description

Adding a new workflow to lint GitHub Actions workflow files. This workflow ensures that any YAML files in the .github/workflows directory are checked for syntax and formatting issues using yamllint, and GitHub Actions-specific errors are caught using actionlint. This will help prevent issues related to incorrect workflows during pull requests.
Link to tracking issue

Fixes

#9676

Testing

Tested by opening a pull request that modifies a workflow file to ensure that the linting workflow triggers and validates the changes correctly.

Documentation

No new documentation added, as this change is self-explanatory within the context of CI.

@KenEkanem KenEkanem requested a review from a team as a code owner October 21, 2024 11:25
Copy link

linux-foundation-easycla bot commented Oct 21, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

- This workflow ensures that any YAML files in the .github/workflows directory are checked for syntax and formatting issues using yamllint, and GitHub Actions-specific errors are caught using actionlint.
- This will help prevent issues related to incorrect workflows during pull requests.
- Added a reminder step to ensure contributors see a message to address all linting errors before merging the pull request.
Copy link

codecov bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.45%. Comparing base (f2b31d1) to head (6089eb7).
Report is 42 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11493      +/-   ##
==========================================
- Coverage   91.48%   91.45%   -0.04%     
==========================================
  Files         433      438       +5     
  Lines       23617    23827     +210     
==========================================
+ Hits        21607    21791     +184     
- Misses       1642     1658      +16     
- Partials      368      378      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

run: yamllint .github/workflows/*.yml

- name: Run Actions-specific checks
uses: reviewdog/action-actionlint@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you had a chance to test this w/ the original issue https://github.com/open-telemetry/opentelemetry-collector/pull/9675/files?

Copy link
Author

Choose a reason for hiding this comment

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

Updated and tested
image

- Updated the PR to replace yamllint with actionlint, as actionlint is better suited for catching mistakes in GitHub Actions workflow files specifically.
- For general YAML code style checks, we can use a general YAML checker like yamllint.
Copy link
Author

@KenEkanem KenEkanem left a comment

Choose a reason for hiding this comment

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

image

@KenEkanem
Copy link
Author

Action lint also detected a couple more errors

  • SC2086: "Double quote to prevent globbing and word splitting"
  • SC2236: "Use -n instead of ! -z"
  • SC2129: "Consider using { cmd1; cmd2; } >> file instead of individual redirects"
  • SC2034: "Variable appears unused. Verify use (or export if used externally)"

with your permissions I'd like to work on this as well when this is merged


- name: Lint GitHub Workflow YAML Files with Docker
run: |
docker run --rm -v "${{ github.workspace }}:/repo" --workdir /repo rhysd/actionlint:latest -color .github/workflows/*.yml .github/workflows/*.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Should this use a specific version rather than latest to prevent the build from suddenly breaking with an unexpected change in a new release?

(ideally, the version should also be auto-upgraded with a renovate PR)

Copy link
Author

Choose a reason for hiding this comment

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

You're right using a specific version will ensure stability. I will update that. thanks

Copy link
Author

@KenEkanem KenEkanem Oct 29, 2024

Choose a reason for hiding this comment

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

Changes made here 420b4a8 and 115372d

Comment on lines +36 to +41
{
"matchManagers": ["dockerfile"],
"matchPackageNames": ["rhysd/actionlint"],
"groupName": "Actionlint docker image",
"versioning": "docker"
},
Copy link
Author

Choose a reason for hiding this comment

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

Actionlint Docker image for stable updates

…lity

- Updated Docker command in GitHub workflow to use `rhysd/actionlint:1.7.3`
  instead of `latest` to maintain workflow stability.
- Specifying a version tag to avoid potential unexpected breaks due to changes
  in the latest version of actionlint.
… versioning

- Added Renovate package rule to monitor `rhysd/actionlint` Docker image
  in workflows to avoid using `latest` tag and instead update to a specific
  version when available.
- Set versioning to `docker` to ensure Renovate fetches the latest tag
  from the Docker registry.
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants