Small cleanups to pkg/crc/machine/bundle.DownloadDefault() code #3813
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cfergeau
force-pushed
the
bundle-download
branch
from
dd24ab8
to
a06d884
Compare
cfergeau
force-pushed
the
bundle-download
branch
6 times, most recently
from
4ba0c1b
to
f5fb858
Compare
It has a single caller in the bundle package, no need to export it.
Instead of only having getDefaultBundleVerifiedHash() which takes a 'preset' argument and only works for the default bundle, this commit introduces a getVerifiedHash() helper which expects a URL to the signed sha256sum.txt file, and the name of the file we want the sha256sum of. This makes it easier to reuse this function for different purpose, and to unit test this code.
Hopefully makes the code easier to follow.
The call we are reporting an error does not return an `error` value, the
`err` variable was set by the goOpenpgp.ReadArmoredKeyRing call.
We should not include ("%w", err) in the error we return after a
goClearSign.Decode() failure as 'err' is nil anyway if we reach this
call.
cfergeau
force-pushed
the
bundle-download
branch
from
f5fb858
to
40d8827
Compare
|
@cfergeau: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
There should be no visible changes, this PR removes some dead code, and add unit tests for the GPG signature checks for https downloads.
More could be built on top of that, such as trying to get a single code path for Download/downloadDefault, so that there is (optional) GPG signature checks in the non-default bundle case as well.