Skip to content

build(docker): suppress hadolint warnings and add cache flag#491

Open
liam-icheng-lai wants to merge 1 commit intomainfrom
build(docker)-suppress-hadolint-warnings-and-add-cache-flag
Open

build(docker): suppress hadolint warnings and add cache flag#491
liam-icheng-lai wants to merge 1 commit intomainfrom
build(docker)-suppress-hadolint-warnings-and-add-cache-flag

Conversation

@liam-icheng-lai
Copy link
Copy Markdown
Contributor

@liam-icheng-lai liam-icheng-lai commented Mar 30, 2026

  • Add hadolint ignore directives for DL3018 rule in both RUN apk add commands
  • Add --no-cache flag to apk add in run stage for consistency with builder stage
  • Ensures proper package cache handling and suppresses version pinning warnings

This improves Dockerfile linting compliance while maintaining build efficiency by preventing package index caching.

@github-actions
Copy link
Copy Markdown

🔐 Commit Signature Verification

All 1 commit(s) passed verification

Commit Author Signature Key Type Key Check
a451a5f6fdf4 liam.lai sk-ssh-ed25519

Summary

  • Commits verified: 1
  • Signature check: ✅ All passed
  • Key type enforcement: ✅ All sk-ssh-ed25519

Required key type: sk-ssh-ed25519 (FIDO2 hardware key)

Last verified: 2026-03-30 17:54 UTC

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR makes two targeted improvements to the multi-stage Dockerfile: it suppresses hadolint DL3018 (unpinned apk package version) warnings with inline # hadolint ignore comments, and adds the missing --no-cache flag to the RUN apk add in the final (run) stage to avoid storing the package index cache in the image layer. Both changes are directionally correct.

Key findings:

  • Pre-existing P1 bug exposed during review: ARG VERSION="HEAD" is declared in the builder stage only. Docker multi-stage builds scope ARGs to the stage where they are declared, so ${VERSION} in the LABEL of the run stage always expands to an empty string. The version is never embedded in the image label. A simple ARG VERSION="HEAD" re-declaration before the LABEL line in the run stage would fix this.
  • DL3018 suppression (P2): Both apk add commands suppress the version-pinning warning rather than pinning package versions. This is a pragmatic choice but reduces build reproducibility. A comment explaining the rationale would help future maintainers.
  • The --no-cache addition to the final-stage apk add is a good improvement that reduces the image size.

Confidence Score: 4/5

  • Safe to merge once the ARG VERSION scoping bug is addressed — the image label will silently carry an empty version string without the fix.
  • The PR's own changes (--no-cache addition and hadolint suppression) are correct and unambiguous. However, reviewing the file surfaced a pre-existing P1 defect: ARG VERSION is out of scope in the run stage, so the LABEL always carries an empty version. This is a real, observable bug that should be fixed before the image is relied upon for version tracking.
  • contrib/images/staking-api-service/Dockerfile — specifically the missing ARG VERSION re-declaration before the LABEL instruction in the run stage.

Important Files Changed

Filename Overview
contrib/images/staking-api-service/Dockerfile Adds hadolint DL3018 suppression comments and --no-cache to the final-stage apk add; the --no-cache fix is correct, but a pre-existing P1 bug exists where ARG VERSION is scoped to the builder stage and is therefore empty when used in the LABEL of the run stage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["FROM golang:1.24.11-alpine\nAS builder"] --> B["ARG VERSION='HEAD'\n⚠️ scoped to builder only"]
    B --> C["RUN apk add --no-cache ...\n# hadolint ignore=DL3018 ✅ NEW"]
    C --> D["COPY go.mod go.sum"]
    D --> E["COPY ./"]
    E --> F["RUN make build"]
    F --> G["binary: build/staking-api-service"]

    H["FROM alpine:3.16\nAS run"] --> I["RUN addgroup + adduser"]
    I --> J["RUN apk add --no-cache bash curl jq\n# hadolint ignore=DL3018 ✅ FIXED"]
    J --> K["LABEL source=...VERSION\n⚠️ VERSION is empty here"]
    K --> L["COPY --from=builder binary"]
    L --> M["COPY --from=builder entrypoint.sh"]
    M --> N["USER staking-api-service"]
    N --> O["ENTRYPOINT entrypoint.sh"]

    G -.->|"multi-stage copy"| L
Loading

Reviews (1): Last reviewed commit: "build(docker): suppress hadolint warning..." | Re-trigger Greptile

RUN apk add --no-cache bash curl jq

# Label should match your github repo
LABEL org.opencontainers.image.source="https://github.com/babylonlabs-io/staking-api-service:${VERSION}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 ARG VERSION out of scope in final stage

In Docker multi-stage builds, ARG declarations are scoped to the build stage in which they are defined. ARG VERSION="HEAD" is declared in the builder stage (line 3), so ${VERSION} in this LABEL instruction (which is in the run stage) will always expand to an empty string — the version tag will never appear correctly in the image label.

To fix this, ARG VERSION needs to be re-declared in the run stage:

Suggested change
LABEL org.opencontainers.image.source="https://github.com/babylonlabs-io/staking-api-service:${VERSION}"
ARG VERSION="HEAD"
LABEL org.opencontainers.image.source="https://github.com/babylonlabs-io/staking-api-service:${VERSION}"

The re-declaration does not need a default value if it is always passed via --build-arg VERSION=..., but including "HEAD" as the default is a safe fallback consistent with the current intent.

Comment on lines +38 to +39
# hadolint ignore=DL3018
RUN apk add --no-cache bash curl jq
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Suppressing version-pinning warning rather than pinning versions

The # hadolint ignore=DL3018 comment suppresses the hadolint warning about unpinned apk package versions rather than fixing the underlying issue. Without pinned versions (e.g. bash=5.2.21-r0), builds are not fully reproducible — a new package release could silently change the image contents between builds.

This applies to both the builder stage (line 5–6) and this run stage. If reproducibility is not a concern here, the suppression is reasonable, but it's worth documenting the rationale in a comment (e.g. # pinning not required; latest stable is acceptable).


ARG VERSION="HEAD"

# hadolint ignore=DL3018
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what does it mean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/hadolint/hadolint/wiki/DL3018

We need to hardcode the package version

FROM alpine:3.7
RUN apk --no-cache add foo=~1.2.3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

actually, let me get consensus from devops team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants