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

Using Yum's setopt with Salt fails only for dom0 #9509

Open
ben-grande opened this issue Oct 14, 2024 · 4 comments · May be fixed by QubesOS/qubes-mgmt-salt-dom0-update#30 or QubesOS/qubes-core-admin-linux#171
Assignees
Labels
affects-4.2 This issue affects Qubes OS 4.2. C: mgmt C: updates diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. pr submitted A pull request has been submitted for this issue. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.

Comments

@ben-grande
Copy link

How to file a helpful issue

Qubes OS release

R4.2

Brief summary

On qubes-dom0-update, can't pass value of an option separated by space as it is treated differently.

This is especially problematic when the format of the command can't be controlled, such as when using Salt.

Setopt value separated by space fails:

$ sudo qubes-dom0-update --setopt install_weak_deps=False --action=install scrot
Using sys-pihole as UpdateVM for Dom0
Downloading packages. This may take a while...
script --quiet --return --command '/usr/lib/qubes/qubes-download-dom0-updates.sh --doit --nogui '\''--exclude=qubes-template-*'\'' '\''--setopt'\'' '\''install_weak_deps=False'\'' '\''--action=install'\'' '\''scrot'\''' /dev/null

Unable to detect release version (use '--releasever' to specify release version)
fakeroot dnf --best --allowerasing --noplugins -y install --downloadonly --installroot /var/lib/qubes/dom0-updates --config=/var/lib/qubes/dom0-updates/etc/dnf/dnf.conf --setopt=reposdir=/var/lib/qubes/dom0-updates/etc/yum.repos.d --exclude=qubes-template-* --setopt install_weak_deps=False scrot
...
Complete!
The downloaded packages were saved in cache until the next successful transaction.
You can remove cached packages by executing 'dnf clean packages'.
Setopt argument has no value: install
No such command: install_weak_deps=False. Please use /usr/bin/dnf --help
It could be a DNF plugin command, try: "dnf install 'dnf-command(install_weak_deps=False)'"

Setopt separated by = works:

$ sudo qubes-dom0-update --setopt=install_weak_deps=False --action=install scrot
Using sys-pihole as UpdateVM for Dom0
Downloading packages. This may take a while...
script --quiet --return --command '/usr/lib/qubes/qubes-download-dom0-updates.sh --doit --nogui '\''--exclude=qubes-template-*'\'' '\''--setopt=install_weak_deps=False'\'' '\''--action=install'\'' '\''scrot'\''' /dev/null
Unable to detect release version (use '--releasever' to specify release version)
fakeroot dnf --best --allowerasing --noplugins -y install --downloadonly --installroot /var/lib/qubes/dom0-updates --config=/var/lib/qubes/dom0-updates/etc/dnf/dnf.conf --setopt=reposdir=/var/lib/qubes/dom0-updates/etc/yum.repos.d --exclude=qubes-template-* --setopt=install_weak_deps=False scrot
...
Complete!
The downloaded packages were saved in cache until the next successful transaction.
You can remove cached packages by executing 'dnf clean packages'.

Steps to reproduce

With the salt state:

install:
  pkg.installed:
    - install_recommends: False
    - skip_suggestions: True
    - setopt: "install_weak_deps=False"
    - pkgs:
      - scrot

Targetting dom0:

DEBUG Loaded plugins: builddep, changelog, config-manager, copr, debug, debuginfo-install, download, generate_completion_cache, groups-manager, needs-restarting, playground, repoclosure, repodiff, repograph, repomanage, reposync, system-upgrade
DEBUG DNF version: 4.18.0
DDEBUG Command: dnf --exclude=qubes-template-* -y --best --allowerasing --setopt install install_weak_deps=False scrot
DDEBUG Installroot: /
DDEBUG Releasever: 4.2
DEBUG cachedir: /var/cache/dnf
CRITICAL No such command: install_weak_deps=False. Please use /usr/bin/dnf --help
CRITICAL It could be a DNF plugin command, try: "dnf install 'dnf-command(install_weak_deps=False)'"

Targetting a fedora-40 qube:

DEBUG Loaded plugins: builddep, changelog, config-manager, copr, debug, debuginfo-install, download, generate_completion_cache, groups-manager, needs-restarting, playground, qubes-hooks, repoclosure, repodiff, repograph, repomanage, reposync, system-upgrade
DEBUG DNF version: 4.21.1
DDEBUG Command: dnf -y --best --allowerasing --setopt install_weak_deps=False install scrot
DDEBUG Installroot: /
DDEBUG Releasever: 40
DEBUG cachedir: /var/cache/dnf
DDEBUG Base command: install
DDEBUG Extra commands: ['-y', '--best', '--allowerasing', '--setopt', 'install_weak_deps=False', 'install', 'scrot']
DEBUG User-Agent: constructed: 'libdnf (Fedora Linux 40; generic; Linux.x86_64)'

Expected behavior

Qubes Salt module builds command line to qubes-dom0-update that doesn't fail when it is a valid dnf syntax.

Actual behavior

Same state that works on Fedora qubes fails on Dom0 due to how qubes-dom0-update interpret arguments.

@ben-grande ben-grande added P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. labels Oct 14, 2024
ben-grande added a commit to ben-grande/qubes-mgmt-salt-dom0-update that referenced this issue Oct 14, 2024
Avoid qubes-dom0-update treating the setopt value as a command to dnf.

Fix: QubesOS/qubes-issues#9509
@marmarek
Copy link
Member

I know it wouldn't be very consistent, but maybe a better approach is to adjust qubes-dom0-update to understand that --setopt has an argument? I hope it applies to only very few options used by salt (for example --enablerepo= works just fine), maybe just --setopt.

@ben-grande
Copy link
Author

I know it wouldn't be very consistent, but maybe a better approach is to adjust qubes-dom0-update to understand that --setopt has an argument?

I can check if setopt item does not contain = and them replace the space between it and the next item with =, which I guess is less prone to mistakes as maintainer might forget to patch the setopt line when upgrading yumpkg from upstream.

I hope it applies to only very few options used by salt (for example --enablerepo= works just fine), maybe just --setopt.

I don't have the percentage of how many Yum options Salt covers, but not all for sure, which can cause inconsistencies. In this case, as it covers --enablerepo= with the correct separator, it doesn't cause problems.

It is actually an inconsistency on the salt code to not have the = separator, see how setopt differs:

https://github.com/QubesOS/qubes-mgmt-salt-dom0-update/blob/fd81a5a7d2847fb4ed7afa6d573721e21cc49481/_modules/qubes_dom0_update.py#L320-L351

    if fromrepo:
        ...
        ret.extend(["--disablerepo=*", f"--enablerepo={fromrepo}"])
    else:
            ...
            ret.extend([f"--disablerepo={x}" for x in targets])
        if enablerepo:
            ...
            ret.extend([f"--enablerepo={x}" for x in targets])

    if disableexcludes:
        ...
        ret.append(f"--disableexcludes={disableexcludes}")

    if branch:
        ...
        ret.append(f"--branch={branch}")

    for item in setopt:
        ret.extend(["--setopt", str(item)])

    if get_extra_options:
                ...
                ret.append(f"--{key}={value}")

@andrewdavidwong andrewdavidwong added C: mgmt diagnosed Technical diagnosis has been performed (see issue comments). pr submitted A pull request has been submitted for this issue. affects-4.2 This issue affects Qubes OS 4.2. C: updates labels Oct 14, 2024
ben-grande added a commit to ben-grande/qubes-core-admin-linux that referenced this issue Oct 15, 2024
Avoid qubes-dom0-update treating the setopt value as a command to dnf.

Fix: QubesOS/qubes-issues#9509
@ben-grande
Copy link
Author

Well, as setopt was irregular compared to the other options, I believe that give us some leverage to fix it upstream as it would benefit them also to have a standard format:

But in case it is not accepted or delayed to be in a release packaged in Qubes:

@ben-grande
Copy link
Author

PR was merged to upstream's 3006.x branch. Next release 3006.10 might take some time as there are 38 open issues to complete that milestone. Now it is your choice to wait for the 3006.10 release or merging one of these PRs:

This issue can be closed if just waiting for upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.2 This issue affects Qubes OS 4.2. C: mgmt C: updates diagnosed Technical diagnosis has been performed (see issue comments). P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. pr submitted A pull request has been submitted for this issue. T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Projects
None yet
3 participants