project-copacetic / copacetic

🧵 CLI tool for directly patching container images using reports from vulnerability scanners
https://project-copacetic.github.io/copacetic/
Apache License 2.0
843 stars 57 forks source link

feat: update all - check for upgradable packages #644

Closed ashnamehrotra closed 3 days ago

ashnamehrotra commented 1 month ago

Adds a check for upgradable packages before updating all when patching without scanner input.

Wanted feedback for microdnf check - discussed with @cpuguy83 to install dnf to call check-update option since microdnf does not support it, but the only way to remove it after is with rpm -e dnf.

Describe the changes in this pull request using active verbs such as Add, Remove, Replace ...

Closes #594

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 2.94118% with 33 lines in your changes missing coverage. Please review.

Project coverage is 33.01%. Comparing base (2602d59) to head (60649af). Report is 94 commits behind head on main.

Files Patch % Lines
pkg/pkgmgr/rpm.go 0.00% 24 Missing :warning:
pkg/pkgmgr/dpkg.go 0.00% 6 Missing :warning:
pkg/pkgmgr/apk.go 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #644 +/- ## ========================================== + Coverage 32.51% 33.01% +0.50% ========================================== Files 17 18 +1 Lines 1621 1578 -43 ========================================== - Hits 527 521 -6 + Misses 1062 1024 -38 - Partials 32 33 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cpuguy83 commented 1 month ago

For dnf based distros, have you tried using the tooling image and mounting the rootfs of the image you want to check?

Rough example:

llb.Image(myToolingImage).Run(
  llb.AddMount(rootfsToCheck, "/tmp/rootfs"),
  llb.Args([]string{"sh", "-c", "dnf --installroot=/tmp/rootfs check-update"})
cpuguy83 commented 1 month ago

I say dnf, any distro that uses rpm+yum/dnf style repos and configs.

ashnamehrotra commented 3 weeks ago

For dnf based distros, have you tried using the tooling image and mounting the rootfs of the image you want to check?

Rough example:

llb.Image(myToolingImage).Run(
  llb.AddMount(rootfsToCheck, "/tmp/rootfs"),
  llb.Args([]string{"sh", "-c", "dnf --installroot=/tmp/rootfs check-update"})

I made the changes to get this to work and was able to test it with yum based images. However, for microdnf based images (like quay.io/calico/cni:v3.15.1) it will always detect no upgrade when using dnf, even if the image is unpatched and would detect upgrades when running microdnf install dnf; dnf check-update in the container.

cpuguy83 commented 3 weeks ago

That is interesting and I wonder if that is related to this:

  • configuration file and reposdir are searched inside the
         installroot first. If they are not present, they are taken from
         the host system.  Note:  When a path is specified within a
         command line argument (--config=<config file> in case of
         configuration file and --setopt=reposdir=<reposdir> for
         reposdir) then this path is always relative to the host with no
         exceptions.

ref: https://www.man7.org/linux/man-pages/man8/dnf.8.html

It could be falling back to the tooling image's repo config and/or dnf config. We should probably make sure to set those properly so they don't fallback that way.

cpuguy83 commented 2 weeks ago

We investigated this a bit yesterday and it looks like --installroot is just broken with this case. It loads the correct repos but does do the right thing at all and just always says that there are no updates. I'm sure this could probably be fixed upstream in dnf/tdnf but that will probably take a long time.

The work-around we discussed is to solve an intermediate LLB state with dnf installed in the target image and run it like normal and write some file to the rootfs that we'll extract to determine if there are updates.

ashnamehrotra commented 1 week ago

@sozercan in order to end patching workflow, we need to return an error when there are no upgradable packages (similar to with scanner https://github.com/project-copacetic/copacetic/blob/main/pkg/pkgmgr/pkgmgr.go#L51-L54). Do we want to change this in all places?