reviewdog / action-remark-lint

Run remark-lint with Reviewdog :dog:
https://github.com/reviewdog/reviewdog
MIT License
5 stars 9 forks source link

Automatic update of packages in the Docker image #47

Open asbjornu opened 3 years ago

asbjornu commented 3 years ago

In the Dockerfile, both APK and NPM packages are pinned to specific versions as per best practice. We need to devise a way to keep these packages up to date.

https://github.com/reviewdog/action-remark-lint/blob/e16cd952fa1e8d24f95e3aee266fbd55e1470b0a/Dockerfile#L9

For NPM, perhaps we can use package.json and package-lock.json to move the packages out of the Dockerfile and then use Renovate to keep package-lock.json up to date.

https://github.com/reviewdog/action-remark-lint/blob/e16cd952fa1e8d24f95e3aee266fbd55e1470b0a/Dockerfile#L3

For APK packages, a possible solution is described in https://github.com/renovatebot/renovate/issues/5422#issuecomment-645947041.

asbjornu commented 3 years ago

With all the files this change seems to spur, I think it would be best if we moved the Dockerfile and all related files into a docker subdirectory of the repository.

rickstaa commented 3 years ago

@asbjornu Both solutions look good to me.

I think especially the APK package solution is trivial since judging from https://github.com/renovatebot/renovate/issues/5422#issuecomment-645947041 the Dockerfile might break when the base image is updated. Good catch!

I have no problem with moving the Dockerfile into a docker subdirectory. I quickly checked the other Reviewdog actions and saw that it has not been done before. However, most other actions don't use pinned versions or pin the docker base image. I don't think there is a special reason for the flat folder structure used in the https://github.com/reviewdog/action-template, but if you want, we can quickly check this with @haya14busa.

asbjornu commented 3 years ago

I've started digging into this and think I've hit a couple of snags. If we start using package.json and package-lock.json to install packages during docker build, we can't install packages globally. That makes the following line fail for obvious reasons:

https://github.com/reviewdog/action-remark-lint/blob/e16cd952fa1e8d24f95e3aee266fbd55e1470b0a/entrypoint.sh#L10

I can replace remark with npx remark-cli, but npx remark-lint doesn't work and I can't figure out how to execute remark-lint when it's installed locally. Ideas?

The other snag that may be a problem is that the following lines may conflict with the package.json file from the user?

https://github.com/reviewdog/action-remark-lint/blob/e16cd952fa1e8d24f95e3aee266fbd55e1470b0a/entrypoint.sh#L13-L16

Thoughts?

asbjornu commented 3 years ago

Actually, running npm remark-lint --version locally only returns the version number of npm and not that of remark-lint. It seems like passing --version to npm will ignore everything in between and just return the version of npm. Example:

$ npm non-existent-package-being-ignored --version
7.20.3
rickstaa commented 3 years ago

@asbjornu Good point, I also overlooked that. Some solutions can be found here (https://stackoverflow.com/questions/14657170/installing-global-npm-dependencies-via-package-json). I liked the PATH solution the most, but I could not get it to work on my local environment (i.e., I didn't spend enough time trying to get it to work).

If we however are planning to use renovate, I think it already provides us with the tools to upgrade npm packages inside docker files without using a package.json file (see https://github.com/renovatebot/renovate/issues/3717#issuecomment-792652444).

rickstaa commented 3 years ago

Thanks again for working on this! If implemented, I think it is a good feature to also use in the other actions. Let me know if you run into any problems. I haven't worked with these more advanced features of renovate features, but I’m happy to read up.