Skip to content

added pinned checksum for mc#56

Open
noahgildersleeve wants to merge 2 commits intoharvester:mainfrom
noahgildersleeve:checksum-fix
Open

added pinned checksum for mc#56
noahgildersleeve wants to merge 2 commits intoharvester:mainfrom
noahgildersleeve:checksum-fix

Conversation

@noahgildersleeve
Copy link
Copy Markdown
Collaborator

@noahgildersleeve noahgildersleeve commented Mar 27, 2026 •
edited
Loading

fixed syntax

Which issue(s) this PR fixes:

What this PR does / why we need it:

This will pin the checksum for mc

Test Result - fixed the following test cases:

@noahgildersleeve noahgildersleeve self-assigned this Mar 27, 2026
Copilot AI review requested due to automatic review settings March 27, 2026 18:22
Copilot AI reviewed Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a pinned SHA-256 checksum validation step for the downloaded mc (MinIO client) binary in the Docker image to improve supply-chain integrity/reproducibility.

Changes:

  • Introduce MC_SUM build argument with expected SHA-256 for mc
  • Add sha256sum -c validation before marking mc executable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

irishgordo
irishgordo suggested changes Mar 27, 2026
View reviewed changes
Dockerfile Outdated
@@ -1,9 +1,14 @@
FROM cypress/base:16
Copy link
Copy Markdown

@irishgordo irishgordo Mar 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

based on:

we additionally need to be validating base package.
additionally link out to a "final" form of utilizing dp.apps.rancher.io of our goal, but this as an intermediate step

Suggested change
FROM cypress/base:16
# TODO: https://github.com/harvester/tests/issues/2540
# this is cypress/base:16
FROM cypress/base@sha256:f4d5f616e83ee6f37913ea18bc1bc4f483bd49b3d7353d04a555af034087b9ff

Copy link
Copy Markdown
Collaborator Author

@noahgildersleeve noahgildersleeve Mar 27, 2026

Choose a reason for hiding this comment

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

Good catch.

Dockerfile
@@ -1,9 +1,14 @@
FROM cypress/base:16

ARG MC_SUM=01f866e9c5f9b87c2b09116fa5d7c06695b106242d829a8bb32990c00312e891
Copy link
Copy Markdown

@irishgordo irishgordo Mar 27, 2026

Choose a reason for hiding this comment

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

can we add a comment to reference the minio version?
additionally I believe double quotes need to wrap the arg

additionally we should be making sure it's not "latest".

Suggested change
ARG MC_SUM=01f866e9c5f9b87c2b09116fa5d7c06695b106242d829a8bb32990c00312e891
# checksum from https://dl.min.io/aistor/mc/release/linux-amd64/mc.RELEASE.2026-03-12T04-18-55Z.sha256sum
# for minio client version: mc.RELEASE.2026-03-12T04-18-55Z
ARG MC_SUM="c9500a16f76884f6a1ac60cda91afd7efdf8b5acda43d6ae8b07579ff91500cf"

Copy link
Copy Markdown
Collaborator Author

@noahgildersleeve noahgildersleeve Mar 27, 2026

Choose a reason for hiding this comment

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

Good catch. I'll do that.

Copy link
Copy Markdown
Collaborator Author

@noahgildersleeve noahgildersleeve Mar 27, 2026

Choose a reason for hiding this comment

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

Although I don't think we need to quote the checksum according to this. We'll check the main repo with a jenkins test run first to verify that it's working in general before merging.

Copy link
Copy Markdown

@irishgordo irishgordo Mar 28, 2026

Choose a reason for hiding this comment

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

yeah sounds good if we don't have to wrap double quotes around it & it parses it correctly that's awesome 😄

Dockerfile Outdated
RUN apt-get update
RUN apt-get install -y git

ADD https://dl.min.io/client/mc/release/linux-amd64/mc /usr/local/bin/mc
Copy link
Copy Markdown

@irishgordo irishgordo Mar 27, 2026

Choose a reason for hiding this comment

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

not latest, but we need a specific version, pinning versions for "everything"

Suggested change
ADD https://dl.min.io/client/mc/release/linux-amd64/mc /usr/local/bin/mc
# this is for version minio mc: mc.RELEASE.2026-03-12T04-18-55Z
# this is for version mino mc from: https://dl.min.io/aistor/mc/release/linux-amd64/mc.RELEASE.2026-03-12T04-18-55Z
ADD https://dl.min.io/aistor/mc/release/linux-amd64/mc.RELEASE.2026-03-12T04-18-55Z /usr/local/bin/mc

irishgordo
irishgordo suggested changes Mar 27, 2026
View reviewed changes
Dockerfile Outdated

ADD https://dl.min.io/client/mc/release/linux-amd64/mc /usr/local/bin/mc
# This runs the checksum and won't add the executable if the checksum doesn't match
RUN echo "${!MC_SUM} /usr/local/bin/mc" | sha256sum -c \
Copy link
Copy Markdown

@irishgordo irishgordo Mar 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
RUN echo "${!MC_SUM} /usr/local/bin/mc" | sha256sum -c \
RUN echo "${MC_SUM} /usr/local/bin/mc" | sha256sum -c \

removing space

@noahgildersleeve noahgildersleeve force-pushed the checksum-fix branch 2 times, most recently from 14c0dc4 to 0472c26 Compare March 27, 2026 18:43
@noahgildersleeve
added pinned checksum for mc
14903ce 
Signed-off-by: Noah Gildersleeve <noah.gildersleeve@suse.com>

fixed syntax

Signed-off-by: Noah Gildersleeve <noah.gildersleeve@suse.com>

fixed a bad bash substitution

Signed-off-by: Noah Gildersleeve <noah.gildersleeve@suse.com>

removed dupe chmod command

Signed-off-by: Noah Gildersleeve <noah.gildersleeve@suse.com>

fixed typo

Signed-off-by: Noah Gildersleeve <noah.gildersleeve@suse.com>
irishgordo
irishgordo suggested changes Mar 27, 2026
View reviewed changes
Dockerfile
ADD https://dl.min.io/client/mc/release/linux-amd64/mc /usr/local/bin/mc
RUN chmod +x /usr/local/bin/mc
# Validate the downloaded binary and abort the build if the checksum doesn't match
RUN echo "${MC_SUM} /usr/local/bin/mc" | sha256sum -c \
Copy link
Copy Markdown

@irishgordo irishgordo Mar 27, 2026

Choose a reason for hiding this comment

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

same as, #56 (comment)

removing that space

@noahgildersleeve
changed some versions for the client
4e4f242 
Signed-off-by: Noah Gildersleeve <noah.gildersleeve@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@irishgordo irishgordo irishgordo requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@noahgildersleeve noahgildersleeve

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@noahgildersleeve @irishgordo