scripts: refactor scripts/download_hash.sh #10713
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
kind/feature
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
|
Not including this in the PR to try to be conflict free here but FYI here is the current diff on the checksum file if you were to run this script: |
MrFreezeex
force-pushed
the
download-hash-improvements
branch
2 times, most recently
from
02f4d8f to
a8e17a1
Compare
yankay
reviewed
Dec 13, 2023
| get_containerd_archive_checksums $(get_versions github_tags containerd/containerd 30) | ||
| get_checksums skopeo_binary $(get_versions github_tags lework/skopeo-binary) | ||
| get_checksums yq $(get_versions github_tags mikefarah/yq) | ||
| if echo "${urls[$binary]}" | grep -qi sha256sum; then |
There was a problem hiding this comment.
HI @MrFreezeex
Based on my original understanding, the goal of computing the hash is to avoid security risks like middle-man attacks.
For example, a firewall between the download machine and github.com changes the download file. The kubespray can compare the sha256sum between the downloaded file and code and avoid security risks. The risks cannot be avoided if the kubespray downloads the sha256sum file instead of computing it.
Thanks :-) 🤝🤝🤝
There was a problem hiding this comment.
Hi!
For example, a firewall between the download machine and github.com changes the download file. The kubespray can compare the sha256sum between the downloaded file and code and avoid security risks. The risks cannot be avoided if the kubespray downloads the sha256sum file instead of computing it.
I don't see how it is any different. If the file you download with this script is compromised whether it's a hash, a hashsum or the file itself that you are computing the hash, it will put a bad hash in the kubespray checksum file. I don't believe there is any difference in term of security but it does have some performance benefits for sure.
There was a problem hiding this comment.
@yankay the goal would be to record the hash in kubespray so that users can check the hash when getting binaries. But when we get the hash, we're getting them from the same source than the binaries, so if one is compromised, so is the other.
If we want stronger guarantees about the file provenance we could instead look at signed Kubernetes artifacts but that's another subject
|
I was about to provide a PR with some new hashes and the fix for crictl as I encountered it 😉 |
The new version brings the following improvements: - remove having to resort to python python to limit tags (it it slower than the sh equivalent as python has a somewhat significant startup time). - Introduce a concept of min version so that it can only get Kubernetes version supported by Kubespray. - Fix an issue with kata changing their file scheme (the arch specifically) - Now download sha256/sha256sum files if provided rather than downloading the full file and computing the hash - A few minor style tweaks Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr.fr>
MrFreezeex
force-pushed
the
download-hash-improvements
branch
from
a8e17a1 to
537f11e
Compare
I'm not sure the python startup time is really significant (or even the interpretor overhead, really) when downloading once in a while stuff from the Net : $ time python -c 'print (4)'
4
real 0m0.016s
user 0m0.015s
sys 0m0.000s |
You are right it's not significant indeed, I was assuming that from some previous experience but I guess that with a very small "script" it's not significant indeed. The speedup is probably mainly about downloading the hash directly then |
|
Hello @MrFreezeex , Anything pending to merge this? Is the above conversation resolved? |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MrFreezeex, mzaian The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
pedro-peter
pushed a commit
to pedro-peter/kubespray
that referenced
this pull request
Mar 23, 2024
The new version brings the following improvements: - remove having to resort to python python to limit tags (it it slower than the sh equivalent as python has a somewhat significant startup time). - Introduce a concept of min version so that it can only get Kubernetes version supported by Kubespray. - Fix an issue with kata changing their file scheme (the arch specifically) - Now download sha256/sha256sum files if provided rather than downloading the full file and computing the hash - A few minor style tweaks Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr.fr>
dibi-codes
pushed a commit
to fino-digital/kubespray
that referenced
this pull request
May 7, 2024
The new version brings the following improvements: - remove having to resort to python python to limit tags (it it slower than the sh equivalent as python has a somewhat significant startup time). - Introduce a concept of min version so that it can only get Kubernetes version supported by Kubespray. - Fix an issue with kata changing their file scheme (the arch specifically) - Now download sha256/sha256sum files if provided rather than downloading the full file and computing the hash - A few minor style tweaks Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr.fr>
pedro-peter
pushed a commit
to pedro-peter/kubespray
that referenced
this pull request
May 8, 2024
The new version brings the following improvements: - remove having to resort to python python to limit tags (it it slower than the sh equivalent as python has a somewhat significant startup time). - Introduce a concept of min version so that it can only get Kubernetes version supported by Kubespray. - Fix an issue with kata changing their file scheme (the arch specifically) - Now download sha256/sha256sum files if provided rather than downloading the full file and computing the hash - A few minor style tweaks Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr.fr>
unai-ttxu
pushed a commit
to Stratio/kubespray
that referenced
this pull request
Jun 10, 2024
The new version brings the following improvements: - remove having to resort to python python to limit tags (it it slower than the sh equivalent as python has a somewhat significant startup time). - Introduce a concept of min version so that it can only get Kubernetes version supported by Kubespray. - Fix an issue with kata changing their file scheme (the arch specifically) - Now download sha256/sha256sum files if provided rather than downloading the full file and computing the hash - A few minor style tweaks Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr.fr>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind feature
What this PR does / why we need it:
The new version brings the following improvements:
remove having to resort to python to limit tags
Introduce a concept of min version so that it can only get Kubernetes version supported by Kubespray.
Fix an issue with kata changing their file scheme (the arch specifically)
Fix crictl not supporting arm in 1.29.0 anymore
Now download sha256/sha256sum files if provided rather than downloading the full file and computing the hash
A few minor style tweaks
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: