google / rpmpack

rpmpack (tar2rpm) - package rpms in pure golang or cli
Apache License 2.0
116 stars 32 forks source link

Unable to install the built rpm (without force) #62

Closed phlax closed 2 years ago

phlax commented 3 years ago

Ive been trying to build an rpm using pkg_tar2rpm

when i go to install the rpm im seeing the following errors

Error: Transaction check error:
  file /opt from install of envoy-0:1.19.0-pre.noarch conflicts with file from package filesystem-3.8-2.el8.x86_64
  file /usr from install of envoy-0:1.19.0-pre.noarch conflicts with file from package filesystem-3.8-2.el8.x86_64
  file /usr/bin from install of envoy-0:1.19.0-pre.noarch conflicts with file from package filesystem-3.8-2.el8.x86_64

if i use rpm -i --force my.rpm it works - im wondering what im doing wrong (or not doing right) that is fooing the paths in the package

jarondl commented 3 years ago

That's interesting. It seems to want to own common directories such as "/opt". Did you do anything special to include the directories in the package? IIRC, we only create dirs if they are explicitly mentioned, but may be I'm wrong here.

Can you share the exact thing you are trying to run?

phlax commented 3 years ago

Can you share the exact thing you are trying to run?

yep - its currently in a pr im working on here https://github.com/envoyproxy/envoy/pull/16899/files#diff-200c06371b99690e7a6152eefa0e7c8883e27b0a17016bb0a062a4acdf91fbc5R108

jarondl commented 3 years ago

OK. The directories get added to the tar file, and tar2rpm just includes them.

Annoyingly the rpm ecosystem assumes that when you build rpm's you also have all of the build deps installed and the basic filesystem directories. (/etc, /usr,...) You are not supposed to include dirs in your rpm if a dep already has that.

That is insane, and we are not going to do that. I start to think that tar2rpm should by default omit directories, and only include them explicitly. I'll try to think of something.

notnarb commented 3 years ago

Dropping in here because I also have this issue with rpmpack and did not have this issue with FPM

FPM seems to address the issue by maintaining its own allowlist of directories to ignore: https://github.com/jordansissel/fpm/blob/4f51caf8fc4de914e3dc261706a5dfabd2d94778/templates/rpm/filesystem_list

but also allows the user to use CLI flags to both add to that list or disable directory ignoring altogether.


IMO the most straightforward approach would be for rpmpack to take a similar approach: default to ignoring common directories and allowing the user to configure these with CLI flags.

To throw out an idea: maybe rather than include the list of 14 thousand directories specifically from EL6 that FPM uses perhaps the list could be filtered to just the 60 directories that are one to two levels deep?

cat filesystem_list | awk -F '/' '{print "/"$2"/"$3}' | uniq

/bin/
/boot/
/dev/
/etc/
/etc/opt
/etc/pki
/etc/pm
/etc/skel
/etc/sysconfig
/etc/X11
/etc/xdg
/etc/xinetd.d
/home/
/lib/
/lib64/
/lib64/tls
/lib/modules
/media/
/mnt/
/mnt/cdrom
/mnt/floppy
/opt/
/proc/
/root/
/sbin/
/selinux/
/srv/
/sys/
/tmp/
/usr/
/usr/bin
/usr/etc
/usr/games
/usr/include
/usr/lib
/usr/lib64
/usr/libexec
/usr/lib
/usr/local
/usr/sbin
/usr/share
/usr/src
/usr/tmp
/var/
/var/cache
/var/db
/var/empty
/var/games
/var/lib
/var/local
/var/lock
/var/log
/var/mail
/var/nis
/var/opt
/var/preserve
/var/run
/var/spool
/var/tmp
/var/yp

and anything more complex than that (e.g. if I had a package that installed something in /usr/local/games) could be provided as a CLI argument to rpmpack if it conflicts with their desired deployment target.

phlax commented 3 years ago

as a ~simple workaround, i added something to strip all of the directories from the tarball

jarondl commented 2 years ago

@phlax , can you share your workaround?

@notnarb, would flipping the default to not include directories help?

phlax commented 2 years ago

it didnt land yet, but the workaround is something like:

    native.genrule(
        name = "tar-rpm-data",
        tools = ["//tools/distribution:strip_dirs"],
        srcs = [":base-tar-rpm-data"],
        cmd = """
        $(location //tools/distribution:strip_dirs) \
            $(location :base-tar-rpm-data) \
            $@
        """,
        outs = ["tar-rpm-data.tar"],
    )

and

        with tempfile.TemporaryDirectory() as tempdir:
            with tarfile.open(self.args.infile) as f:
                f.extractall(path=tempdir)

            with tarfile.open(self.args.outfile, "w") as f:
                for root, dirs, files in os.walk(tempdir):
                    for filename in files:
                        fpath = os.path.join(root, filename)
                        f.add(
                            fpath,
                            filter=self.reset,
                            arcname=fpath.replace(tempdir, "").lstrip("/"))

im wondering if it could be done in bash, or better still not required at all

notnarb commented 2 years ago

@notnarb, would flipping the default to not include directories help?

As in not specifying the directories in the RPM's spec?

If this means the RPM will create the files in the specified location but not create the parent directories, I think that fixes the problem for me.

I would be curious what that the behavior would be if the parent direct doesn't exist (e.g. /opt/<namespace>/<binary>) and how through rpmpack I might specify an empty folder.

As long as I am not picky about directory ownership I imagine I could worth through those two problems with lifecycle scripts (e.g. %pre %post)

jarondl commented 2 years ago

I've suggested a solution in #70, please take a look.

jarondl commented 2 years ago

@notnarb Our test cases (in example_bazel) include the case of missing parents, and rpm seems to install fine. OTOH, none of my tests need --force to overwrite existing directories, so perhaps this changes between distros. Can you tell me in which distro (preferably a docker image) you can reproduce the original issue? I'd like to add it as a regression test.

phlax commented 2 years ago

i originally posted the bug testing against the universal base image 7

jarondl commented 2 years ago

Is there any chance you can build an rpm with the changes from #70, and see if it works? I couldn't reproduce the issue even with the universal base image 7.

notnarb commented 2 years ago

Can you tell me in which distro (preferably a docker image) you can reproduce the original issue? I'd like to add it as a regression test.

I can reproduce this with the centos:7 image

Build file config (some details truncated/renamed) ```python pkg_tar( name = "watcher_script", srcs = [...], package_dir = "opt/test/watcher", ) pkg_tar( name = "watcher_service", srcs = ["watcher.service"], package_dir = "etc/systemd/system", ) pkg_tar( name = "watcher_tar", ownername = "root.root", deps = [ ":watcher_script", ":watcher_service", ], ) pkg_tar2rpm( name = "watcher_rpm", data = ":watcher_tar", pkg_name = "watcher", ... ) ```

then an rpm -i watcher_rpm.rpm results in:

]$ sudo docker run -it --rm -v ~/tmp:/share centos:7
...
[root@c224e7410d6a share]# rpm -i watcher_rpm.rpm
        file /etc from install of watcher-0:0.0.1-0.noarch conflicts with file from package filesystem-3.2-25.el7.x86_64
        file /opt from install of watcher-0:0.0.1-0.noarch conflicts with file from package filesystem-3.2-25.el7.x86_64
        file /etc/systemd from install of watcher-0:0.0.1-0.noarch conflicts with file from package systemd-219-78.el7.x86_64
        file /etc/systemd/system from install of watcher-0:0.0.1-0.noarch conflicts with file from package systemd-219-78.el7.x86_64

I'll try your PR soon to see if it resolves the issue

notnarb commented 2 years ago

FWIW I could reproduce with ubi-minimal:8.5 as well.

]$ sudo docker run -it --rm -v ~/tmp:/share registry.access.redhat.com/ubi8/ubi-minimal:8.5-204
...
[root@c4af2806b011 /]# rpm -qf /etc
filesystem-3.8-6.el8.x86_64
[root@c4af2806b011 /]# cd /share
[root@c4af2806b011 share]# rpm -i watcher_rpm.rpm --nodeps
        file /etc from install of watcher-0:0.0.1-0.noarch conflicts with file from package filesystem-3.8-6.el8.x86_64
        file /opt from install of watcher-0:0.0.1-0.noarch conflicts with file from package filesystem-3.8-6.el8.x86_64
notnarb commented 2 years ago

I can confirm that when using 3d6252945290ac5a55ebe5bc675ff9d49a4e6226 and setting use_dir_allowlist = True, that the RPMs now install without issue 🥳

Workspace Diff ```diff diff --git a/WORKSPACE b/WORKSPACE index fbcd8799..285cc140 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -365,8 +365,8 @@ http_file( git_repository( name = "com_github_google_rpmpack", # Lastest commit in main branch as of 2021-09-22 - commit = "dc539ef4f2ea1c53ab359280cadc7249f2fb3e61", - remote = "https://github.com/google/rpmpack.git", + commit = "3d6252945290ac5a55ebe5bc675ff9d49a4e6226", + remote = "https://github.com/jarondl/rpmpack.git", ) load("@com_github_google_rpmpack//:deps.bzl", "rpmpack_dependencies") diff --git a/-redacted-/-redacted-/-redacted-/BUILD b/-redacted-/-redacted-/-redacted-/BUILD index f6cb5d9e..33f5f393 100644 --- a/-redacted-/-redacted-/-redacted-/BUILD +++ b/-redacted-/-redacted-/-redacted-/BUILD @@ -56,5 +56,6 @@ pkg_tar2rpm( requires = [ "PyYAML", ], + use_dir_allowlist = True, version = "0.0.1", ) ```

I've verified the new RPM works in: