dbelyaev / action-checkstyle

reviewdog based GitHub action to run Checkstyle on your Java code.
MIT License
21 stars 18 forks source link

Consider replacing included BusyBox 'wget' with Alpine's own #230

Closed usrme closed 2 months ago

usrme commented 2 months ago

Currently when using a rootless Docker setup to run this action, the image fails to build with the following:

#7 [3/4] RUN wget -O - -q https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b /usr/local/bin/ v0.18.0 &&     mkdir -p /opt/lib &&     wget -q -O /opt/lib/checkstyle.jar https://github.com/checkstyle/checkstyle/releases/download/checkstyle-10.17.0/checkstyle-10.17.0-all.jar
#7 0.297 reviewdog/reviewdog info checking GitHub for tag 'v0.18.0'
#7 0.311 wget: bad address 'github.com'
#7 0.312 reviewdog/reviewdog crit unable to find 'v0.18.0' - use 'latest' or see https://github.com/reviewdog/reviewdog/releases for details
#7 ERROR: process "/bin/ash -eo pipefail -c wget -O - -q https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b /usr/local/bin/ ${REVIEWDOG_VERSION} &&     mkdir -p /opt/lib &&     wget -q -O /opt/lib/checkstyle.jar [https://github.com/checkstyle/checkstyle/releases/download/checkstyle-${CHECKSTYLE_VERSION}/checkstyle-${CHECKSTYLE_VERSION}-all.jar](https://github.com/checkstyle/checkstyle/releases/download/checkstyle-$%7BCHECKSTYLE_VERSION%7D/checkstyle-$%7BCHECKSTYLE_VERSION%7D-all.jar)" did not complete successfully: exit code: 1

This was not an issue when using a non-rootless setup. However, this can easily be remedied within the Dockerfile by adding the wget package to be installed so that the BusyBox version won't be used. The BusyBox version could be used if it would support the -4 flag to force name resolution to just IPv4 addresses.

I've tried disabling IPv6 on the underlying host, but that didn't help, so I'm turning to you in the hopes you might make something akin to the following change:

@@ -6,12 +6,12 @@
 SHELL ["/bin/ash", "-eo", "pipefail", "-c"]

 # hadolint ignore=DL3018
-RUN apk --no-cache add git
+RUN apk --no-cache add git wget

 # pre-install reviewdog and checkstyle
-RUN wget -O - -q https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b /usr/local/bin/ ${REVIEWDOG_VERSION} && \
+RUN wget -4 -O - -q https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b /usr/local/bin/ ${REVIEWDOG_VERSION} && \
     mkdir -p /opt/lib && \
-    wget -q -O /opt/lib/checkstyle.jar https://github.com/checkstyle/checkstyle/releases/download/checkstyle-${CHECKSTYLE_VERSION}/checkstyle-${CHECKSTYLE_VERSION}-all.jar
+    wget -4 -q -O /opt/lib/checkstyle.jar https://github.com/checkstyle/checkstyle/releases/download/checkstyle-${CHECKSTYLE_VERSION}/checkstyle-${CHECKSTYLE_VERSION}-all.jar

 COPY entrypoint.sh /entrypoint.sh

I'd be more than willing to submit a PR myself. Here is more information regarding this: https://github.com/alpinelinux/docker-alpine/issues/155.

usrme commented 2 months ago

Scratch the above. Since the downloaded install.sh script still uses wget without the -4 flag, then things still do not work properly. I'll try to find some other way to get this working.

usrme commented 2 months ago

The following change set would result in a functioning image as the downloaded install.sh first checks for the existence of curl and uses that instead of wget:

@@ -6,12 +6,12 @@
 SHELL ["/bin/ash", "-eo", "pipefail", "-c"]

 # hadolint ignore=DL3018
-RUN apk --no-cache add git
+RUN apk --no-cache add curl git wget

 # pre-install reviewdog and checkstyle
-RUN wget -O - -q https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b /usr/local/bin/ ${REVIEWDOG_VERSION} && \
+RUN wget -4 -O - -q https://raw.githubusercontent.com/reviewdog/reviewdog/master/install.sh | sh -s -- -b /usr/local/bin/ ${REVIEWDOG_VERSION} && \
     mkdir -p /opt/lib && \
-    wget -q -O /opt/lib/checkstyle.jar https://github.com/checkstyle/checkstyle/releases/download/checkstyle-${CHECKSTYLE_VERSION}/checkstyle-${CHECKSTYLE_VERSION}-all.jar
+    wget -4 -q -O /opt/lib/checkstyle.jar https://github.com/checkstyle/checkstyle/releases/download/checkstyle-${CHECKSTYLE_VERSION}/checkstyle-${CHECKSTYLE_VERSION}-all.jar

 COPY entrypoint.sh /entrypoint.sh
dbelyaev commented 2 months ago

@usrme can you submit an PR on this?

usrme commented 2 months ago

Would you also be okay with me updating the ENV declarations as well?

 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 3)
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 4)
usrme commented 2 months ago

https://github.com/dbelyaev/action-checkstyle/pull/231.

dbelyaev commented 2 months ago

Would you also be okay with me updating the ENV declarations as well?

 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 3)
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 4)

I'm sorry I didn't see your comment before the new version was released, but you're welcome to submit the change.

dbelyaev commented 2 months ago

@usrme Your fix is available in the latest release: https://github.com/dbelyaev/action-checkstyle/releases/tag/v1.16.2.

usrme commented 2 months ago

You, sir, are a gentleman and a scholar!