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

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

Closed
phlax opened this issue Jun 22, 2021 · 15 comments · Fixed by #70
Closed

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

phlax opened this issue Jun 22, 2021 · 15 comments · Fixed by #70

Comments

@phlax
Copy link
Contributor

phlax commented Jun 22, 2021

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
Copy link
Collaborator

jarondl commented Jun 25, 2021

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
Copy link
Contributor Author

phlax commented Jun 25, 2021

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
Copy link
Collaborator

jarondl commented Jun 27, 2021

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
Copy link

notnarb commented Sep 24, 2021

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
Copy link
Contributor Author

phlax commented Sep 24, 2021

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

@jarondl
Copy link
Collaborator

jarondl commented Nov 15, 2021

@phlax , can you share your workaround?

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

@phlax
Copy link
Contributor Author

phlax commented Nov 15, 2021

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
Copy link

notnarb commented Nov 16, 2021

@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 added a commit to jarondl/rpmpack that referenced this issue Nov 20, 2021
…ories to inlclude. Content is always included.

fixes google#62
@jarondl
Copy link
Collaborator

jarondl commented Nov 20, 2021

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

@jarondl
Copy link
Collaborator

jarondl commented Nov 20, 2021

@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
Copy link
Contributor Author

phlax commented Nov 20, 2021

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

jarondl added a commit to jarondl/rpmpack that referenced this issue Nov 20, 2021
…ories to inlclude. Content is always included.

fixes google#62
@jarondl
Copy link
Collaborator

jarondl commented Nov 22, 2021

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
Copy link

notnarb commented Nov 22, 2021

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)
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
Copy link

notnarb commented Nov 22, 2021

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
Copy link

notnarb commented Nov 22, 2021

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

Workspace 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:

  • centos:7
  • registry.access.redhat.com/ubi8/ubi-minimal:8.5-204
  • the centos-based VM where I originally encountered the error

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 a pull request may close this issue.

3 participants