Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: improve mirrorlist mechanism #55

Merged
merged 1 commit into from
Jul 14, 2022

Conversation

ejose19
Copy link
Contributor

@ejose19 ejose19 commented Jan 28, 2021

This PR addresses 2 things:

  • Set defaults in Makefile.archlinux only (previously, ARCHLINUX_MIRROR on Makefile.archlinux was replacing defaults set in prepare-chroot-base)
  • Use a file instead of mirror string envs separated by comma and sed'ing into mirrorlist.dist, this method is way more clean and direct, also it's more ergonomic for users in case they want to specify a long list of servers.

@ejose19 ejose19 force-pushed the ej/improveMirrorlistMechanism branch 2 times, most recently from f80ca15 to 5695850 Compare January 28, 2021 02:05
@ejose19
Copy link
Contributor Author

ejose19 commented Jan 28, 2021

Also, I've noticed builds errors out a lot with 404 on servers, so maybe we should increase the default servers even if they're not worldwide (users will prefer some packages to be downloaded slowly than failing the entire build). If agree I will update the mirrorlist with 10-20 mirrors.

Copy link
Member

@marmarek marmarek left a comment

Choose a reason for hiding this comment

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

Besides technical comments below, I'm not sure if using a variable pointing at a separate file is really an improvement. This means if you copy your builder.conf elsewhere, it won't work (unless you left the default, in which case the setting isn't really useful). This is the case for building the template in DisposableVM for example.

The code for sed-ing mirrorlist file indeed wasn't pretty, but I'd prefer to keep the actual setting directly in builder.conf. Maybe the variable should contain the full path (possibly without $repo/os/$arch to avoid the need to escape the dollar sign) and have mirrorlist constructed from it directly, instead of using mirrorlist.dist as a source? Something like:

for MIRROR_ENTRY in "${ARCHLINUX_MIRROR[@]}"; do
    echo "Server = $MIRROR_ENTRY/\$repo/os/\$arch"
done > "${BOOTSTRAP_DIR}/etc/pacman.d/mirrorlist"

prepare-chroot-base Outdated Show resolved Hide resolved
scripts/mirrorlist Outdated Show resolved Hide resolved
@ejose19 ejose19 force-pushed the ej/improveMirrorlistMechanism branch from a6ff047 to 9fb225d Compare February 16, 2021 17:25
@ejose19
Copy link
Contributor Author

ejose19 commented Feb 16, 2021

I applied the review with some comments:

  • Removed ARCHLINUX_MIRROR default from Makefile, as readability is way better in shell syntax when having many mirrors.
  • The output of "Setting pacman mirrorlist as:" now print each mirror in a separate line, improving readability as well.
  • Use https only for default mirrors, since there was a previous vulnerability (Privilege escalation vulnerability in Arch templates qubes-issues#6242), there shouldn't be any http mirror by default making any mitm attack easier.

Originally I wanted to use the output of cat mirrorlist into the variable directly, as making users strip Server = ... /$repo/os/$arch with a command like export ARCHLINUX_MIRROR=$(sed -E 's|Server = (.*)/\$repo/os/\$arch|\1|' mirrorlist | tr '\n' ',') only to be placed again by the script is a bit "wasteful", however seems like this
https://github.com/QubesOS/qubes-linux-template-builder/blob/4d8c03b9a88693fb74435d8d3dde09ea02641bb7/Makefile#L65-L68
doesn't support variables with newlines, leaving with current implementation or either make users:

  1. replace newlines with another character
  2. encoding the variable with base64 and decode in prepare-chroot-base

These solutions would be easier but the variable will surely have an ambiguous name.

@ejose19
Copy link
Contributor Author

ejose19 commented Mar 23, 2021

@marmarek is there something left to be done or it can be merged?

@ejose19
Copy link
Contributor Author

ejose19 commented Aug 26, 2021

Friendly ping @marmarek

@marmarek
Copy link
Member

marmarek commented Jan 6, 2022

PipelineRetry

@marmarek
Copy link
Member

marmarek commented Jan 6, 2022

@ejose19 we (finally!) have CI for Arch working again, recent run on the master branch succeeded, but with this PR it fails. It gets 404 on accessing ftp.halifax.rwth-aachen.de (qubes repo mirror), but I don't see why your changes would cause that. Maybe some mirrorlist file formatting issue?

@ejose19
Copy link
Contributor Author

ejose19 commented Jan 6, 2022

Upon checking the logs , I see that only this mirror is set (https://ftp.halifax.rwth-aachen.de), and that mirror only expose qubes repo, no core, extra, ect, so that's why it gives 404 when querying core.db

 --> Setting pacman mirrorlist as:
https://ftp.halifax.rwth-aachen.de
error: failed retrieving file 'core.db' from ftp.halifax.rwth-aachen.de : The requested URL returned error: 404

the solution I see to make this simpler, is to add another env APPEND_ARCHLINUX_MIRROR, and then merge both variables before creating the mirrorlist, that way, when specifying only a subset of all available repositories through ARCHLINUX_MIRROR it doesn't unintentionally overwrite the defaults (pipeline would need to use this env, but I don't see where it's being set)

@marmarek
Copy link
Member

marmarek commented Jan 6, 2022

Do you see why this mirror is set? While it is indeed one of the mirrors for qubes repo, I can't find where ARCHLINUX_MIRROR is pointed at it.

@ejose19
Copy link
Contributor Author

ejose19 commented Jan 6, 2022

I've checked all repositories under github QubesOS org and couldn't find anything relevant:

$ rg --no-ignore --hidden -i -F "halifax"

qubesos.github.io/_data/downloads-mirrors.yml
52:  org_url: https://www.halifax.rwth-aachen.de
55:  - url: http://ftp.halifax.rwth-aachen.de/qubes/iso/

qubesos.github.io/_data/downloads.yml
47:      - name: halifax.rwth-aachen.de
53:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.1.0-rc3-x86_64.iso
55:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.1.0-rc3-x86_64.iso.DIGESTS
56:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.1.0-rc3-x86_64.iso.asc
69:      - name: halifax.rwth-aachen.de
75:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.1.0-rc3-x86_64.torrent
77:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.1.0-rc3-x86_64.iso.DIGESTS
78:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.1.0-rc3-x86_64.iso.asc
132:      - name: halifax.rwth-aachen.de
138:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.0.4-x86_64.iso
140:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.0.4-x86_64.iso.DIGESTS
141:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.0.4-x86_64.iso.asc
154:      - name: halifax.rwth-aachen.de
160:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.0.4-x86_64.torrent
162:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.0.4-x86_64.iso.DIGESTS
163:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R4.0.4-x86_64.iso.asc
195:      - name: halifax.rwth-aachen.de
201:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.2.1-x86_64.iso
203:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.2.1-x86_64.iso.DIGESTS
204:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.2.1-x86_64.iso.asc
228:      - name: halifax.rwth-aachen.de
234:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.2.1-x86_64.torrent
236:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.2.1-x86_64.iso.DIGESTS
237:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.2.1-x86_64.iso.asc
281:      - name: halifax.rwth-aachen.de
287:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-x86_64.iso
289:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-x86_64.iso.DIGESTS
290:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-x86_64.iso.asc
314:      - name: halifax.rwth-aachen.de
320:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-x86_64.torrent
322:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-x86_64.iso.DIGESTS
323:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-x86_64.iso.asc
372:      - name: halifax.rwth-aachen.de
378:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-alpha1.1-x86_64.iso
380:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-alpha1.1-x86_64.iso.DIGESTS
381:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.1-alpha1.1-x86_64.iso.asc
414:      - name: halifax.rwth-aachen.de
420:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.0-x86_64-DVD.iso
422:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.0-x86_64-DVD.iso.DIGESTS
423:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R3.0-x86_64-DVD.iso.asc
468:      - name: halifax.rwth-aachen.de
474:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R2-x86_64-DVD.iso
476:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R2-x86_64-DVD.iso.DIGESTS
477:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R2-x86_64-DVD.iso.asc
515:      - name: halifax.rwth-aachen.de
521:        url: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R1-x86_64-DVD.iso
523:          hash: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R1-x86_64-DVD.iso.DIGESTS
524:          sig: http://ftp.halifax.rwth-aachen.de/qubes/iso/Qubes-R1-x86_64-DVD.iso.asc

build-logs-secondary/build-archlinux/log_2022-01-05_23-02-49
426:2022-01-05 23:02:49.608456 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
427:2022-01-05 23:02:49.608467 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1095:2022-01-05 23:02:49.615565 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1096:2022-01-05 23:02:49.615576 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1762:2022-01-05 23:02:49.622715 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1763:2022-01-05 23:02:49.622726 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch

build-logs-secondary/build-archlinux/log_2022-01-05_23-02-13
426:2022-01-05 23:02:13.505189 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
427:2022-01-05 23:02:13.505200 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1095:2022-01-05 23:02:13.511890 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1096:2022-01-05 23:02:13.511901 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1762:2022-01-05 23:02:13.518785 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1763:2022-01-05 23:02:13.518796 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch

build-logs-secondary/build-archlinux/log_2022-01-05_18-22-59
426:2022-01-05 18:22:59.866978 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
427:2022-01-05 18:22:59.866989 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1095:2022-01-05 18:22:59.884852 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1096:2022-01-05 18:22:59.884863 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1762:2022-01-05 18:22:59.894105 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1763:2022-01-05 18:22:59.894116 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch

build-logs-secondary/build-archlinux/log_2022-01-06_00-02-46
426:2022-01-06 00:02:46.649816 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
427:2022-01-06 00:02:46.649828 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1095:2022-01-06 00:02:46.657548 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1096:2022-01-06 00:02:46.657559 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1762:2022-01-06 00:02:46.664442 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
1763:2022-01-06 00:02:46.664453 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch

build-logs-secondary/build-archlinux/log_2022-01-05_23-07-04
3936:2022-01-05 23:18:31.743279 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
3937:2022-01-05 23:18:31.743290 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
4738:2022-01-05 23:18:31.751749 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
4739:2022-01-05 23:18:31.751761 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
5538:2022-01-05 23:18:31.759914 +0000 build-archlinux: #Server = http://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch
5539:2022-01-05 23:18:31.759925 +0000 build-archlinux: #Server = https://ftp.halifax.rwth-aachen.de/archlinux/$repo/os/$arch

qubes-linux-yum/mirrors.list
4:https://ftp.halifax.rwth-aachen.de/qubes/repo/yum
rg --no-ignore --hidden -i -F "ARCHLINUX_MIRROR"

qubes-builder-archlinux/Makefile.archlinux
21:ARCHLINUX_MIRROR ?= http://mirror.rackspace.com
59:  $(info ║ ARCHLINUX_MIRROR:      $(ARCHLINUX_MIRROR))       # mirror.rackspace.com

qubes-builder-archlinux/prepare-chroot-base
11:DEFAULT_ARCHLINUX_MIRROR="\
16:ARCHLINUX_MIRROR="${ARCHLINUX_MIRROR:-$DEFAULT_ARCHLINUX_MIRROR}"
17:IFS="," read -r -a ARCHLINUX_MIRROR \
18:  <<< $ARCHLINUX_MIRROR
53:echo "  --> Setting pacman mirrorlist as '${ARCHLINUX_MIRROR[@]}'..."
54:ARCHLINUX_MIRRORLIST=$(cat ${CACHEDIR}/bootstrap/etc/pacman.d/mirrorlist.dist)
55:for MIRROR_ENTRY in "${ARCHLINUX_MIRROR[@]}"
57:  ARCHLINUX_MIRRORLIST=$(sed "s|#Server = ${MIRROR_ENTRY}/|Server = ${MIRROR_ENTRY}/|" <<< $ARCHLINUX_MIRRORLIST)
59:echo "$ARCHLINUX_MIRRORLIST" > "${CACHEDIR}/bootstrap/etc/pacman.d/mirrorlist"

qubes-builder-archlinux/scripts/00_prepare.sh
40:    # value of $ARCHLINUX_MIRROR each run (by the Makefile)

build-logs-secondary/build-archlinux/log_2022-01-05_23-07-04
3662:2022-01-05 23:18:31.740304 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##
4462:2022-01-05 23:18:31.748595 +0000 build-archlinux: + for MIRROR_ENTRY in "${ARCHLINUX_MIRROR[@]}"
4464:2022-01-05 23:18:31.748718 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##

build-logs-secondary/build-archlinux/log_2022-01-05_23-02-49
194:2022-01-05 23:02:49.605954 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##
861:2022-01-05 23:02:49.613091 +0000 build-archlinux: + for MIRROR_ENTRY in "${ARCHLINUX_MIRROR[@]}"
863:2022-01-05 23:02:49.613112 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##

build-logs-secondary/build-archlinux/log_2022-01-05_23-02-13
194:2022-01-05 23:02:13.502773 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##
861:2022-01-05 23:02:13.509562 +0000 build-archlinux: + for MIRROR_ENTRY in "${ARCHLINUX_MIRROR[@]}"
863:2022-01-05 23:02:13.509583 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##

build-logs-secondary/build-archlinux/log_2022-01-06_00-02-46
194:2022-01-06 00:02:46.647243 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##
861:2022-01-06 00:02:46.654960 +0000 build-archlinux: + for MIRROR_ENTRY in "${ARCHLINUX_MIRROR[@]}"
863:2022-01-06 00:02:46.654981 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##

build-logs-secondary/build-archlinux/log_2022-01-05_18-22-59
194:2022-01-05 18:22:59.862704 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##
861:2022-01-05 18:22:59.882327 +0000 build-archlinux: + for MIRROR_ENTRY in "${ARCHLINUX_MIRROR[@]}"
863:2022-01-05 18:22:59.882355 +0000 build-archlinux: + ARCHLINUX_MIRRORLIST='##

qubes-linux-template-builder/Makefile
24:	GENTOO_MIRROR ARCHLINUX_MIRROR \

Maybe @fepitre knows more about the issue?

BTW: Just checked now and that mirror https://ftp.halifax.rwth-aachen.de is no longer giving 404 errors, maybe it was a temporal issue? Anyways we'd want to fix the underlying problem to make tests more robust

@ejose19
Copy link
Contributor Author

ejose19 commented Jan 10, 2022

I'm pretty sure that variable is set at gitlab pipeline config, but since the display of such env is not implemented yet (https://gitlab.com/gitlab-org/gitlab/-/issues/22204), it's only an assumption. @marmarek can you trigger another pipeline run?

@marmarek
Copy link
Member

I'm pretty sure that variable is set at gitlab pipeline config

You are absolutely right! It is indeed set there. Cleared it now.

@marmarek
Copy link
Member

PipelineRetry

@ejose19
Copy link
Contributor Author

ejose19 commented Jan 11, 2022

Seems CI is all green now

@marmarek marmarek merged commit 352f440 into QubesOS:master Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants