lycheeverse / lychee

⚡ Fast, async, stream-based link checker written in Rust. Finds broken URLs and mail addresses inside Markdown, HTML, reStructuredText, websites and more!
https://lychee.cli.rs
Apache License 2.0
2.23k stars 136 forks source link

Docker images contain incorrect Lychee binary versions #1496

Closed oponomarov-tu closed 2 months ago

oponomarov-tu commented 2 months ago

Downloading older Docker images for testing purposes results in images containing incorrect Lychee binary versions. Specifically, the Docker tags (named after specific Lychee versions, and sometimes even shas) sometimes contain a different binary version than expected, e.g.:

Correct:

$ docker run --init -it --rm lycheeverse/lychee:0.14.3 --version
lychee 0.14.3

Incorrect (should be 0.15.0 in both scenarios, see sha: 8451f88):

$ docker run --init -it --rm lycheeverse/lychee:0.15.0 --version
lychee 0.15.1

$ docker run --init -it --rm lycheeverse/lychee:sha-8451f88 --version
lychee 0.15.1

This inconsistency makes it difficult to test older versions of Lychee accurately.

mre commented 2 months ago

Ouch. Thanks for the heads-up. For future builds, this should be fixed. We made some changes to the pipeline. However, I don't think we'll have time to tackle the old images.

It's unfortunate, but if you need the images for testing, you can build them by checking out the commits from source and rebuilding the correct images. I'm aware that this is quite tedious, but it could serve as a workaround.

Hope that works. 😉

oponomarov-tu commented 2 months ago

Ouch. Thanks for the heads-up. For future builds, this should be fixed. We made some changes to the pipeline. However, I don't think we'll have time to tackle the old images.

It's unfortunate, but if you need the images for testing, you can build them by checking out the commits from source and rebuilding the correct images. I'm aware that this is quite tedious, but it could serve as a workaround.

Hope that works. 😉

I don't think rebuilding the images locally would be work out, as the current Dockerfiles are configured to always fetch the latest binary, regardless of the checked-out SHA. For example:

https://github.com/lycheeverse/lychee/blob/53d234d18e1eec6ec932e9d4d15d9d9862dba6a2/Dockerfile-CI.Dockerfile#L12

I'm not fully familiar with Rust development practices, but perhaps a more reproducible approach could be:

  1. Copying the build artifact into the image after the cargo build process finishes
  2. Compiling the binary directly inside the container
  3. Fetching specific version of the binary from GitHub rather than latest

What do you think?

mre commented 2 months ago

We have two different types of Dockerfiles

If the goal is to make some tests for yourself, then I'd suggest using the first Dockerfile. I think it gets used automatically. If it's about fixing the upstream builds, then we could make the binary configurable. Maybe this would work?

# First stage: builder
FROM debian:bullseye-slim as builder
WORKDIR /builder

# Add build argument for Lychee version
ARG LYCHEE_VERSION=latest

RUN apt-get update \
    && DEBIAN_FRONTEND=noninteractive apt-get install -y \
    --no-install-recommends \
    ca-certificates \
    wget \
    && case $(dpkg --print-architecture) in \
      "amd64") \
        ARCH="x86_64-unknown-linux-gnu" \
      ;; \
      "arm64") \
        ARCH="aarch64-unknown-linux-gnu" \
      ;; \
    esac \
    && DOWNLOAD_URL="https://github.com/lycheeverse/lychee/releases/${LYCHEE_VERSION}/download/lychee-${ARCH}.tar.gz" \
    && wget -q -O - $DOWNLOAD_URL | tar -xz lychee \
    && chmod +x lychee

# Second stage: final image
FROM debian:bullseye-slim

RUN apt-get update \
    && DEBIAN_FRONTEND=noninteractive apt-get install -y \
    --no-install-recommends \
    ca-certificates \
    tzdata \
    && rm -rf /var/cache/debconf/* \
    && apt-get clean \
    && rm -rf /var/lib/apt/lists/*

COPY --from=builder /builder/lychee /usr/local/bin/lychee
ENTRYPOINT [ "/usr/local/bin/lychee" ]
oponomarov-tu commented 2 months ago

I apologize, didn't notice regular master/Dockerfile. I think we can close this issue then :)