Bug 1941896 - Review Request: rust-rudo - rudo is a small equivalent of sudo writen in rust
Summary: Review Request: rust-rudo - rudo is a small equivalent of sudo writen in rust
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: 34
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1901106 1943989 1944392 1944398 1944403 1944408 1944412 1944422 1944463
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-03-23 03:59 UTC by Rémi Lauzier
Modified: 2021-07-11 05:07 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-07-11 05:07:25 UTC
Type: ---
Embargoed:
zebob.m: fedora-review+


Attachments (Terms of Use)

Description Rémi Lauzier 2021-03-23 03:59:06 UTC
Spec URL: https://github.com/remilauzier/rudo/blob/main/rust-rudo.spec
SRPM URL: https://github.com/remilauzier/rudo/releases/download/0.4.0/rust-rudo-0.4.0-1.fc34.src.rpm
Description: Rudo use pam to give privilege access to a authorized user and send log to journald to keep trace of it
Fedora Account System Username:

Comment 1 Rémi Lauzier 2021-03-23 04:03:04 UTC
If you need any modification of the .spec or something else you can contact me.

Comment 2 Robert-André Mauchin 🐧 2021-03-28 20:33:02 UTC
 - The LICENSE file must be installed with %license in %files

%files       -n %{crate}
%doc README.md
%license LICENSE
%{_bindir}/rudo
%endif

[…]

%files          devel
%doc README.md
%license LICENSE
%{cargo_registry}/%{crate}-%{version_no_tilde}/

 - The configuration files are not installed?
You need to BuildRequires:  pam-devel and install the pam file conf/rudo to %{buildroot}%{_sysconfdir}/pam.d/rudo
	
%config(noreplace) %{_sysconfdir}/pam.d/rudo

A default configuration file should probably be installed with wheel permission enabled. If this is auto generated at startup, I'd still recommend to add %ghost directive so the file is taken care of when uninstalling.

%ghost %{_sysconfdir}/rudo.conf

 - You should set suid in %files:

%attr(4755,root,root) %{_bindir}/rudo

 - The documentation for the config file is not very clear. Consider writing a man page.

Comment 3 Rémi Lauzier 2021-03-28 22:41:28 UTC
Thanks for the help!
I am not sure i have fully understand where i should have put the lines after the BuildRequires: pam.
I hope i put them in the right place.

I have made a default configuration for rudo but will have to change it soon to have multiple option per user.
I will try to make a man page after that and put comment in the file if yaml permit it.

For now i have try to make a copr repository but failed.
https://copr.fedorainfracloud.org/coprs/remilauzier/rudo

Pam-sys has update but i have to wait for pam-client to follow since it doesn't compile for now.

Thanks again for your time!

Comment 4 Robert-André Mauchin 🐧 2021-03-28 23:07:29 UTC
 - You can do:

%generate_buildrequires
%cargo_generate_buildrequires
echo "pam-devel"


 - You need to open review requests for you dependencies too. Also you may need to patch some Cargo.toml to build the packages. For example, in your pam-sys build you have:
No matching package to install: 'crate(bindgen/default) = 0.53.2'
But Fedora is at rust-bindgen 0.56.0. So you need to use 'rust2rpm -p' to patch the Cargo.toml.

Comment 5 Rémi Lauzier 2021-03-28 23:26:24 UTC
Thanks!

Pam-sys has been update today but for now i must wait for pam-client to adapt to it. I will update pam-sys in my copr and open a review for it and the others dependency.

Comment 6 Rémi Lauzier 2021-03-30 22:25:11 UTC
Hi!
I manage to put review for all dependency of rudo and to compile them in my copr.
https://copr.fedorainfracloud.org/coprs/remilauzier/rudo/

For now i manage to compile rudo but the config file appear to install as directory even if it is a file in the final rpm.
Thanks for your time.

Comment 7 Rémi Lauzier 2021-03-31 20:55:02 UTC
Hi!
i manage to get the config file to install as file.
i just have a problem with the man page.
i must convert markdown to man page but i don't know how to do it.
Thanks for your time!

Comment 8 Robert-André Mauchin 🐧 2021-04-01 00:19:17 UTC
Sent you a PR. Use go-md2man as indicated in the commit log.

Comment 9 Rémi Lauzier 2021-04-01 03:57:05 UTC
Thanks for the help!
i got the package to compile and the man page to install.
Can confirm that everything work on my end.

Comment 10 Robert-André Mauchin 🐧 2021-04-06 20:18:18 UTC
 - /etc → %{_sysconfdir}

 - Use install -p to keep timestamps

 - Why are you doing this:

cp conf/rudo ~/
cp conf/rudo.conf ~/
cp man/rudo.1 ~/
cp man/rudo.conf.5 ~/

instead of just installing them from their current location?

mkdir -p %{buildroot}%{_sysconfdir}
mkdir -p %{buildroot}%{_sysconfdir}/pam.d/
mkdir -p %{buildroot}%{_mandir}/man1
mkdir -p %{buildroot}%{_mandir}/man5
install -pm 0640 cp conf/rudo.conf %{buildroot}%{_sysconfdir}/rudo.conf
install -pm 0644 conf/rudo %{buildroot}%{_sysconfdir}/pam.d/rudo
install -pm 0644 man/rudo.1 %{buildroot}%{_mandir}/man1/rudo.1
install -pm 0644 man/rudo.conf.5 %{buildroot}%{_mandir}/man5/rudo.conf.5

 - This is not needed anymore:

echo "pam-devel"
echo "systemd-devel"

pam-devel is implied by the rust-pam-sys dep and systemd-devel by the rust-libsystemd-sys dep


 - License ok
 - Latest version packaged
 - Builds in mock
 - No rpmlint errors
 - Conforms to Packaging Guidelines

Comment 11 Rémi Lauzier 2021-04-07 22:52:28 UTC
Done! Not sure if i should put some requires to systemd-libs for journald support?
https://download.copr.fedorainfracloud.org/results/remilauzier/rudo/fedora-34-x86_64/02121437-rust-rudo/rust-rudo.spec

Comment 12 Robert-André Mauchin 🐧 2021-04-08 15:44:29 UTC
I don't think this is necessary.

Package approved. You still need to find a sponsor.

Comment 13 Robert-André Mauchin 🐧 2021-04-08 21:27:56 UTC
Sponsored.

Comment 14 Tomas Hrcka 2021-04-27 09:13:53 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-rudo


Note You need to log in before you can comment on or make changes to this bug.