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

feat: add dockerfile linting #382

Merged
merged 155 commits into from
Feb 12, 2024
Merged

feat: add dockerfile linting #382

merged 155 commits into from
Feb 12, 2024

Conversation

SMoraisAnsys
Copy link
Contributor

@SMoraisAnsys SMoraisAnsys commented Jan 3, 2024

This PR addresses #381 by :

  1. adding an extra action ("docker-style") that can be used to evaluate the Dockerfile(s) associated to a repo;
  2. extending action "code-style" by triggering "docker-style" on "docker/Dockerfile" and ".devcontainer/Dockerfile" if the associated directories exist.

It was tested and validated in a sandbox. If you are interested, you can see the CI jobs failing and succeeding when expected (https://github.com/SMoraisAnsys/pyansys-geometry/actions/runs/7628030247/job/21173613810?pr=1).

Here is a summary of the changes:

TLDR

  • docker-style looks for files named "Dockerfile" in the provided directory (default to "docker"). If one wants to propose and evaluate multiple docker files, then the associated repo must follow a directory oriented architecture (e.g. docker/linux/Dockerfile and docker/windows/Dockerfile) and option recursive must be set to true.
  • docker-style fails based on hadolint output and one can define at which ouput level the action fails through option error-level.
  • code-style can be extend to perform docker linting on "docker" and ".devcontainer" directories through option docker-lint which default value is False to not introduce breaking changes.
  • code-style docker linting can be tunned through options docker-recursive and docker-error-level.

docker-style

This action leverages the action jbergstroem/hadolint-gh-action and requires a directory to look for the docker file. This directory can be anything but we emphasize to store docker files in a directory named "docker" at the root of the repository. If no "docker" directory exists, this action will raise a warning through the CI.

If one wants to evaluate multiple docker files nested in the provided directory, option recursive must be set to True and ⚠️ each file expected to be evaluated must be named Dockerfile. Notice that this constraints your repo to follow a directory oriented architecture, i.e. to split the Dockerfiles into multiple directories (e.g. docker/linux/Dockerfile and docker/windows/Dockerfile).

By default the action has a strict error level based on the outputs of hadolint. Any info caught into this ouput will make the action failt. It can be lightened by changing the value of the option error-level. The associated value can be -1, 0, 1 or 2 where:

  • -1: never fail;
  • 0: fail on errors;
  • 1: fail on warnings;
  • 2: fail on infos.

code-style

This action was modified to allow users to integrate docker linting (using previous docker-style action). By default this is not activated but one can activate it with option docker-lint set to True. Directory to be linted are set to "docker" and ".devcontainer", and the options used are recursive=false and error-level=2.
The options passed to the action "docker-style" can be modified through the new options docker-recursive and docker-error-level.

Notes

Github working-directory cannot be used with uses (I tried to use it to specify where to look for docker files).
At first we used hadolint-action but is wouldn't work as wanted when looking for multiple docker files. For example, if we wanted to look for multiple docker files, we would need to use the recursive option. However, if we wanted to look into an empty directory "docker" while having docker files in a directory "some_other_directory", then the action would evaluate the docker files inside of "some_other_directory". Indeed, option recursive searches for specified dockerfile recursively, from the project root.

We also had to fix hadolint-gh-action issue through (jbergstroem/hadolint-gh-action#135)

@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions bot added the enhancement General improvements to existing features label Jan 3, 2024
@SMoraisAnsys SMoraisAnsys changed the title Feature: add dockerfile linting feat: add dockerfile linting Jan 3, 2024
Changes:
- the action is now ingesting only one directory;
- the action is expected to be used in a matrix job;
- the action is now leveraging github working-directory
Note: option 'work-directory' cannot be used with 'uses', it only
works with the key-word 'run'.
@SMoraisAnsys SMoraisAnsys dismissed jorgepiloto’s stale review February 9, 2024 13:53

Review has been taken into account, dismissing due to days off

Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Thanks for adding this, @SMoraisAnsys. Just left some minor suggestions before merging this.

@jorgepiloto jorgepiloto self-requested a review February 12, 2024 09:06
@jorgepiloto
Copy link
Member

The doc build is failing but just because the action is new and declared for the first time in this pull-request.

@SMoraisAnsys SMoraisAnsys merged commit 3029563 into main Feb 12, 2024
14 of 15 checks passed
@SMoraisAnsys SMoraisAnsys deleted the feat/lint_dockerfile branch February 12, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants