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

fix(package): set rpm user and group to create files with correct owner #2186

Merged
merged 1 commit into from
Apr 9, 2019

Conversation

affo
Copy link
Contributor

@affo affo commented Apr 4, 2019

This PR makes rpm create files and directories with the right owner (user and group).
This makes it possible to avoid changing ownership after install and so breaking verification (rpm -V ...).

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated
  • Sign CLA (if not already signed)
Required only if applicable

You can erase any checkboxes below this note if they are not applicable to your Pull Request.

@affo affo requested a review from nathanielc April 4, 2019 10:12
@affo
Copy link
Contributor Author

affo commented Apr 4, 2019

@nathanielc That option actually makes rpm create every file with that ownership, so, for example:

$ ls -al /usr/bin/kapacitor
-rwxr-xr-x 1 kapacitor kapacitor 8215494 Apr  4 09:56 /usr/bin/kapacitor

I can set finer grained permissions by using

--rpm-attr 750,kapacitor,kapacitor:/var/lib/kapacitor
--rpm-attr 750,kapacitor,kapacitor:/var/log/kapacitor

And change only lib and log dirs as it was done in post-install.sh.
Once this PR passes, I can apply the same logic to every other packaged project, because this bug affects them.

Tested on CentOS 7.

@affo
Copy link
Contributor Author

affo commented Apr 4, 2019

I also realised, that no post install script is run when dpkg -i ... on Ubuntu.
Is this the expected behavior?

@nathanielc
Copy link
Contributor

@affo Let's use the finer grained permissions.

@affo
Copy link
Contributor Author

affo commented Apr 8, 2019

Tested on CentOS:

Installed:
  kapacitor.x86_64 0:1.5.3~rc0~6d9228f-0                                                                                                           

Complete!
[root@ddd77b7d6e55 /]# rpm -V kapacitor
[root@ddd77b7d6e55 /]# ls -al /var/lib/
total 60
drwxr-xr-x 1 root       root       4096 Apr  8 16:26 .
drwxr-xr-x 1 root       root       4096 Mar  5 17:34 ..
drwxr-xr-x 2 root       root       4096 Mar  5 17:35 alternatives
drwxr-xr-x 2 chronograf chronograf 4096 Mar 20 22:55 chronograf
drwxr-xr-x 2 root       root       4096 Nov  2 16:19 dbus
drwxr-xr-x 2 root       root       4096 Apr 11  2018 games
drwxr-xr-x 5 influxdb   influxdb   4096 Apr  3 07:13 influxdb
drwxr-xr-x 2 root       root       4096 Nov  2 17:40 initramfs
drwxr-xr-x 2 kapacitor  kapacitor  4096 Apr  8 16:24 kapacitor
drwx------ 2 root       root       4096 Mar  5 17:35 machines
drwxr-xr-x 2 root       root       4096 Apr 11  2018 misc
drwxr-xr-x 1 root       root       4096 Mar  5 17:36 rpm
drwxr-xr-x 2 root       root       4096 Apr 11  2018 rpm-state
drwxr-xr-x 4 root       root       4096 Mar  5 17:35 systemd
drwxr-xr-x 1 root       root       4096 Apr  8 16:26 yum
[root@ddd77b7d6e55 /]# rpm -qlv kapacitor
drwxr-xr-x    2 root    root                        0 Apr  8 16:24 /etc/kapacitor
-rw-r--r--    1 root    root                    24022 Apr  8 16:24 /etc/kapacitor/kapacitor.conf
-rw-r--r--    1 root    root                      151 Apr  8 16:24 /etc/logrotate.d/kapacitor
-rwxr-xr-x    1 root    root                  8215494 Apr  8 16:24 /usr/bin/kapacitor
-rwxr-xr-x    1 root    root                 78130638 Apr  8 16:24 /usr/bin/kapacitord
-rwxr-xr-x    1 root    root                  4560489 Apr  8 16:24 /usr/bin/tickfmt
drwxr-xr-x    2 root    root                        0 Apr  8 16:24 /usr/lib/kapacitor
drwxr-xr-x    2 root    root                        0 Apr  8 16:24 /usr/lib/kapacitor/scripts
-rwxr-xr-x    1 root    root                     5609 Apr  8 16:24 /usr/lib/kapacitor/scripts/init.sh
-rw-r--r--    1 root    root                      447 Apr  8 16:24 /usr/lib/kapacitor/scripts/kapacitor.service
-rw-r--r--    1 root    root                     9006 Apr  8 16:24 /usr/share/bash-completion/completions/kapacitor
drwxr-xr-x    2 kapacitokapacito                    0 Apr  8 16:24 /var/lib/kapacitor
drwxr-xr-x    2 kapacitokapacito                    0 Apr  8 16:24 /var/log/kapacitor

Also tested the deb package, and ownership looks correct:

root@bb3310ca8459:/# ls -al /var/lib/
total 32
drwxr-xr-x 1 root      root      4096 Apr  8 16:26 .
drwxr-xr-x 1 root      root      4096 Mar  7 21:01 ..
drwxr-xr-x 1 root      root      4096 Mar  7 21:01 apt
drwxr-xr-x 1 root      root      4096 Apr  8 16:26 dpkg
drwxr-xr-x 2 kapacitor kapacitor 4096 Apr  8 16:24 kapacitor
drwxr-xr-x 2 root      root      4096 Apr 24  2018 misc
drwxr-xr-x 2 root      root      4096 Mar  7 21:01 pam
drwxr-xr-x 3 root      root      4096 Mar  7 21:00 systemd

mkdir -p $LOG_DIR
chown -R -L kapacitor:kapacitor $LOG_DIR
mkdir -p $DATA_DIR
chown -R -L kapacitor:kapacitor $DATA_DIR
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved lines here to preserve previous behavior in non REHL distros.

Copy link
Contributor

@nathanielc nathanielc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Can you add an entry to the CHANGELOG for the fix?

@affo
Copy link
Contributor Author

affo commented Apr 9, 2019

@nathanielc done

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 this pull request may close these issues.

2 participants