fedora: Unpack from ISO #121
Merged
+217
−22
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
MusicDin
force-pushed
the
fix/img-builder
branch
from
d39e192 to
a5d94ed
Compare
1 task
simondeziel
reviewed
Jan 23, 2025
| // when the release version is equal or less than 40. | ||
| relNum, err := strconv.Atoi(s.definition.Image.Release) | ||
| if err == nil && relNum <= 40 { | ||
| baseURL := fmt.Sprintf("%s/packages/Fedora-Container-Base", s.definition.Source.URL) |
There was a problem hiding this comment.
I tend to prefer the more performant variant:
Suggested change
| baseURL := fmt.Sprintf("%s/packages/Fedora-Container-Base", s.definition.Source.URL) | |
| baseURL := s.definition.Source.URL + "/packages/Fedora-Container-Base" |
But I understand that might not be relevant for this code base that is probably filled with fmt.Sprintf...
|
|
||
| gpg_keys_official="file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-%[2]s-primary" | ||
|
|
||
| if [ -n "${GPG_KEYS}" ]; then |
There was a problem hiding this comment.
Since we know Fedora publishes package signing keys, would it make sense to require GPG keys to be provided?
There was a problem hiding this comment.
This is for the initial packages we mount on /mnt/cdrom/Packages, and as far as I understand they are not signed which is the reason there is no GPG keys for them. Packages that are downloaded later from the Fedora repositories are checked with GPG keys that are provided within the ISO file.
Here is a snippet what I have tried doing:
# Iterate over all subdirectories in /mnt/cdrom/Packages
for dir in /mnt/cdrom/Packages/*/; do
if [[ -d "$dir" ]]; then
echo "Checking RPMs in: $dir"
rpm --checksig "$dir"*.rpm || true
rpm --query --queryformat '%{NAME}-%{VERSION}-%{RELEASE} -> %{SIGPGP:pgpsig}\n' --package "$dir"*.rpm || true
fi
done
Snippet of the entire output:
+ [[ -d /mnt/cdrom/Packages/j/ ]]
+ echo 'Checking RPMs in: /mnt/cdrom/Packages/j/'
Checking RPMs in: /mnt/cdrom/Packages/j/
+ rpm --checksig /mnt/cdrom/Packages/j/jansson-2.13.1-10.fc41.x86_64.rpm /mnt/cdrom/Packages/j/jbigkit-libs-2.1-30.fc41.i686.rpm /mnt/cdrom/Packages/j/jbigkit-libs-2.1-30.fc41.x86_64.rpm /mnt/cdrom/Packages/j/jemalloc-5.3.0-7.fc41.x86_64.rpm /mnt/cdrom/Packages/j/jomolhari-fonts-0.003-42.fc41.noarch.rpm /mnt/cdrom/Packages/j/jq-1.7.1-8.fc41.x86_64.rpm /mnt/cdrom/Packages/j/json-c-0.17-4.fc41.x86_64.rpm /mnt/cdrom/Packages/j/json-c-devel-0.17-4.fc41.x86_64.rpm /mnt/cdrom/Packages/j/json-glib-1.10.0-1.fc41.i686.rpm /mnt/cdrom/Packages/j/json-glib-1.10.0-1.fc41.x86_64.rpm
/mnt/cdrom/Packages/j/jansson-2.13.1-10.fc41.x86_64.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/jbigkit-libs-2.1-30.fc41.i686.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/jbigkit-libs-2.1-30.fc41.x86_64.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/jemalloc-5.3.0-7.fc41.x86_64.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/jomolhari-fonts-0.003-42.fc41.noarch.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/jq-1.7.1-8.fc41.x86_64.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/json-c-0.17-4.fc41.x86_64.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/json-c-devel-0.17-4.fc41.x86_64.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/json-glib-1.10.0-1.fc41.i686.rpm: digests SIGNATURES NOT OK
/mnt/cdrom/Packages/j/json-glib-1.10.0-1.fc41.x86_64.rpm: digests SIGNATURES NOT OK
+ true
+ echo 'Checking smth in: /mnt/cdrom/Packages/j/'
+ rpm --query --queryformat '%{NAME}-%{VERSION}-%{RELEASE} -> %{SIGPGP:pgpsig}\n' --package /mnt/cdrom/Packages/j/jansson-2.13.1-10.fc41.x86_64.rpm /mnt/cdrom/Packages/j/jbigkit-libs-2.1-30.fc41.i686.rpm /mnt/cdrom/Packages/j/jbigkit-libs-2.1-30.fc41.x86_64.rpm /mnt/cdrom/Packages/j/jemalloc-5.3.0-7.fc41.x86_64.rpm /mnt/cdrom/Packages/j/jomolhari-fonts-0.003-42.fc41.noarch.rpm /mnt/cdrom/Packages/j/jq-1.7.1-8.fc41.x86_64.rpm /mnt/cdrom/Packages/j/json-c-0.17-4.fc41.x86_64.rpm /mnt/cdrom/Packages/j/json-c-devel-0.17-4.fc41.x86_64.rpm /mnt/cdrom/Packages/j/json-glib-1.10.0-1.fc41.i686.rpm /mnt/cdrom/Packages/j/json-glib-1.10.0-1.fc41.x86_64.rpm
warning: /mnt/cdrom/Packages/j/jansson-2.13.1-10.fc41.x86_64.rpm: Header V4 RSA/SHA256 Signature, key ID e99d6ad1: NOKEY
jansson-2.13.1-10.fc41 -> (none)
jbigkit-libs-2.1-30.fc41 -> (none)
jbigkit-libs-2.1-30.fc41 -> (none)
jemalloc-5.3.0-7.fc41 -> (none)
jomolhari-fonts-0.003-42.fc41 -> (none)
jq-1.7.1-8.fc41 -> (none)
json-c-0.17-4.fc41 -> (none)
json-c-devel-0.17-4.fc41 -> (none)
json-glib-1.10.0-1.fc41 -> (none)
json-glib-1.10.0-1.fc41 -> (none)
If anyone has more understanding in this, I recommend myself for some help :)
MusicDin
force-pushed
the
fix/img-builder
branch
from
a5d94ed to
eb0fed3
Compare
simondeziel
previously approved these changes
Feb 7, 2025
simondeziel
added a commit
to canonical/lxd-ci
that referenced
this pull request
Feb 7, 2025
Adds Fedora 41 release. Requires: - [ ] canonical/lxd-imagebuilder#121 Passing build: https://github.com/MusicDin/lxd-ci/actions/runs/12930858195
tomponline
approved these changes
Feb 7, 2025
There was a problem hiding this comment.
@MusicDin just to confirm this is verifying the checksum of the downloaded iso against a separately stored value?
tomponline
reviewed
Feb 7, 2025
| // getDownloadURL fetches JSON representation of current releases and | ||
| // extracts the download URL matching the given release, variant, arch, | ||
| // and filetype (.tar.xz, .iso, etc.). | ||
| func (s *fedora) getDownloadURL(release string, variant string, arch string, fileType string) (url string, checksum string, err error) { |
There was a problem hiding this comment.
@MusicDin when does this ever return a checksum to be verified?
tomponline
reviewed
Feb 7, 2025
sources/fedora-http.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("Failed to unpack %q: %w", filepath.Join(fpath, fname), err) | ||
| // Verify checksum. | ||
| if checksum != "" { |
There was a problem hiding this comment.
Why wouldnt a checksum be returned? This feels like it could introduce accidental issues in case the checksum is ever not returned we would not verify it?
tomponline
requested changes
Feb 7, 2025
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
MusicDin
force-pushed
the
fix/img-builder
branch
3 times, most recently
from
cd5ad8a to
564cf83
Compare
Signed-off-by: Din Music <[email protected]>
MusicDin
force-pushed
the
fix/img-builder
branch
from
564cf83 to
79767f8
Compare
|
Fixed, now the checksum is required in order to proceed with unpacking ISO. Passing build: https://github.com/MusicDin/lxd-ci/actions/runs/13237766213/job/36946163432 |
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.
Adds support to unpack Fedora ISO image, and some small cosmetic fixes.