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

Advisory --advisory-severities and type filters produce union instead of intersection #1401

Open
pkratoch opened this issue Apr 11, 2024 · 10 comments · May be fixed by #1433 or #1467
Open

Advisory --advisory-severities and type filters produce union instead of intersection #1401

pkratoch opened this issue Apr 11, 2024 · 10 comments · May be fixed by #1433 or #1467
Labels
Priority: MEDIUM Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take

Comments

@pkratoch
Copy link
Contributor

For example, dnf5 advisory list --security --advisory-severities=critical shows all advisories that are either or type security or have critical severity (so even e.g. moderate securities or critical bugfixes are shown).

I believe that other filter combinations produce intersection (except when specifying multiple type filters, but that makes sense if viewed as if it was just one list option), so this is inconsistent. If there is a reason for this behavior, it should be at least documented.

@ppisar ppisar added Priority: MEDIUM Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take labels Apr 11, 2024
@ppisar
Copy link
Contributor

ppisar commented Apr 11, 2024

I confirm the bug. --advisory-severities option is document as "limit…", but it does not do an intersection:

# dnf5 advisory list --security --advisory-severities=critical
Updating and loading repositories:
Repositories loaded.
Name                   Type     Severity                           Package              Issued
FEDORA-2024-43a0920f12 security Low      perl-Clipboard-0.29-1.fc39.noarch 2024-04-10 14:44:52

@kontura
Copy link
Contributor

kontura commented Apr 11, 2024

I think the reason is compatibility with old dnf which is unfortunately also doing an union.

I agree that the docs should definitely be fixed (both man pages and --help).
I have also noticed that the --help of advisory commands refers to packages which is confusing because its filtering advisories. This is probably caused by the fact that the options are shared with other commands.

@j-mracek j-mracek self-assigned this Apr 22, 2024
j-mracek added a commit to j-mracek/dnf5 that referenced this issue Apr 22, 2024
This is a difference in comparison to DNF4 and it also intorduce
a different behavior than DNF5 upgrade command.

Closes: rpm-software-management#1401
j-mracek added a commit to j-mracek/dnf5 that referenced this issue Apr 22, 2024
This is a difference in comparison to DNF4 and it also intorduce
a different behavior than DNF5 upgrade command.

Closes: rpm-software-management#1401
j-mracek added a commit to j-mracek/dnf5 that referenced this issue Apr 22, 2024
This is a difference in comparison to DNF4 and it also intorduce
a different behavior than DNF5 upgrade command.

Closes: rpm-software-management#1401
@j-mracek j-mracek linked a pull request Apr 22, 2024 that will close this issue
@j-mracek
Copy link
Contributor

j-mracek commented Apr 22, 2024

I have created a patch, but I am not sure whether we should modify the behavior or modify DNF5 in more complex way.

I think that both following command should target the same set of advisories (The PR change behavior for all commands), but is it OK?

dnf5 advisory list --security --advisory-severities=critical

dnf5 upgrade --security --advisory-severities=critical

What do you think?

@ppisar
Copy link
Contributor

ppisar commented Apr 22, 2024

Both commands should behave the same.

I'm for changing it to an intersection because users who want an union can use the options in separate calls. Contrary with the union behavior, users cannot not achieve the intersection.

@kontura
Copy link
Contributor

kontura commented Apr 25, 2024

I agree that all the commands should behave the same.

I also prefer intersection but changing it now could be quite unexpected for users.

@j-mracek
Copy link
Contributor

I agree that all the commands should behave the same.

I also prefer intersection but changing it now could be quite unexpected for users.

@kontura May I ask you when you would prefer that the change happen?

@kontura
Copy link
Contributor

kontura commented Apr 30, 2024

@kontura May I ask you when you would prefer that the change happen?

I think the best time was when we introduced the advisory options to dnf5 but since the change was rejected at that time due to compatibility concerns I wanted to mention it.

@j-mracek
Copy link
Contributor

j-mracek commented May 2, 2024

I am still not sure whether the change make sense from upgrade command usages.

I think that command dnf5 upgrade --security --advisory-severities=critical with a result that both filters are applied with and operator os OK and expected.

But other combination might be difficult dnf5 upgrade --advisory FEDORA-2024-1706127797 --security

  • It returns updates for advisory FEDORA-2024-a7a8f8f01b and all other security updates
  • The new behavior results in an empty set because the advisory has the Bugfix type.
  • It means that after the change several combination of filters would be allowed but not usable.

Other combination might be difficult dnf5 upgrade --advisory FEDORA-2024-1706127797 --bzs 2271998

  • It returns updates for advisory FEDORA-2024-a7a8f8f01b and bugzilla 2271998
  • The new behavior results in an empty set because the advisory does not fix the Bugzilla.

The same problem is related to CVEs.

What about to only modify applying and operator between type and severity? (#1467)
Or we can add a restrictions that only some combination will be not allowed.

j-mracek added a commit to j-mracek/dnf5 that referenced this issue May 2, 2024
It allows to better specify user request when user wants to update
only advisory with certain type and severity.

Closes: rpm-software-management#1401
@ppisar
Copy link
Contributor

ppisar commented May 2, 2024

I will repeat myself, I like the new behavior more.

What about to only modify applying and operator between type and severity?

Does severity exists in non-security advisories?

@j-mracek
Copy link
Contributor

j-mracek commented May 2, 2024

Yes, severity is used with other types of advisories, but not so often

===============================================================================
  Update ID: FEDORA-2024-39f1aba503
       Type: bugfix
    Updated: 2024-05-01 03:36:32
       Bugs: 2276226 - python-identify-2.5.36 is available
Description: 2.5.36
   Severity: Low

@j-mracek j-mracek removed their assignment Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: MEDIUM Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take
Projects
Status: Backlog
4 participants