Page MenuHomePhabricator

Wikibase Release Pipeline: Update hadolint github action to the official `hadolint/hadolint-action`
Open, MediumPublic

Description

Note: Not to be tackled until at least July 1st 2021

During work on T283174: Optimize MediaWiki docker image in wikibase release pipeline, @ItamarWMDE created a fork of the hadolint GitHub action in order to enable linting for multiple of Docker files in a repository (to ensure all Dockerfiles in the release pipeline are indeed linted). However, relying on a 3rd party fork is not ideal, and it was decided to give the upstream PR a month to be merged.

Therefore, the lint_dockerfiles job in the lint.yml workflow (link) must be updated according to the current state of the official hadolint GitHub Action.

  • If the PR was merged and a new version was released, then the dependency must be updated accordingly
  • Otherwise, we need to decide whether WMDE want to maintain a fork of the hadolint action, or forgo the functionality offered by the fork for some other solution.

ACs

  • lint_dockerfiles job in the lint.yml workflow no longer relies on Itamar's personal fork

Hints:

  • One suggested course of action, if we don't want to maintain our own fork and the PR wasn't merged, is to split the lint_dockerfiles job into multiple steps that lint each subdirectory individually

Event Timeline

During work on T283174: Optimize MediaWiki docker image in wikibase release pipeline, @ItamarWMDE created a fork of the hadolint GitHub action in order to enable linting for multiple of Docker files in a repository (to ensure all Dockerfiles in the release pipeline are indeed linted)

Is there an upstream PR for adding this to the upstream github action?

One suggested course of action, if we don't want to maintain our own fork and the PR wasn't merged, is to split the lint_dockerfiles job into multiple steps that lint each subdirectory individually

This could be done with a 2 step workflow:

  1. Collect the name of all Dockerfiles that need linting
  2. Create a matrix for these and run the linting.

Is there an upstream PR for adding this to the upstream github action?

Yes, it's also mentioned in the PR for the original ticket: https://github.com/hadolint/hadolint-action/pull/34

This could be done with a 2 step workflow...

Potentially, I thought something like that might be doable at first as well, but I couldn't find any documentation or examples of it being done as such elsewhere. I think this probably might not be as simple as that, though, since the matrix is declared on the job level rather than the step level. Any common information between jobs I've seen so far is only accessible at the step level.

Is there an upstream PR for adding this to the upstream github action?

Yes, it's also mentioned in the PR for the original ticket: https://github.com/hadolint/hadolint-action/pull/34

Great thanks! (I missed this).
Thanks for adding it to the description here.

This could be done with a 2 step workflow...

Potentially, I thought something like that might be doable at first as well, but I couldn't find any documentation or examples of it being done as such elsewhere. I think this probably might not be as simple as that, though, since the matrix is declared on the job level rather than the step level. Any common information between jobs I've seen so far is only accessible at the step level.

I have an example for how this could work here https://github.com/addshore/mwcli/blob/dev/.github/workflows/go-ci.yml#L113-L122
Another slightly different (and possibly harder to read example) would be https://github.com/addwiki/addwiki/blob/main/.github/workflows/split_composer_test.yaml#L12-L32

If the upstream PR doesn't get accepted for whatever reason then something like this could be a good approach.

oh, alright yeah, then I guess we could grab all the paths to check with find -type f -name Dockerfile, and the run the official action on that matrix.

Looks like https://github.com/hadolint/hadolint-action/pull/34 didn't see much progress (other than upstream asking for a rebase/merge conflict fix) and it's not clear if the upstream maintainers are likely to merge this patch.

I guess we return to the previous two possible solutions:

  • Run the action multiple times (either manually or with a matrix perhaps built from searching the repo for all files named Dockerfile
  • Move @ItamarWMDE 's fork to a wmde place and take ownership of it

We could also set a timer for another month and see if this patch eventually gets merged upstream.

I don't feel strongly opinionated about this though. I think probably for now I very lightly favour the "run multiple times with some GitHub actions matrixing option"

Addshore triaged this task as Medium priority.Jul 14 2021, 11:11 AM