Bug 1941896

Summary: Review Request: rust-rudo - rudo is a small equivalent of sudo writen in rust
Product: [Fedora] Fedora Reporter: Rémi Lauzier <remilauzier>
Component: Package ReviewAssignee: Robert-André Mauchin 🐧 <zebob.m>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 34CC: package-review, remilauzier, zebob.m
Target Milestone: ---Flags: zebob.m: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2021-07-11 05:07:25 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On: 1901106, 1943989, 1944392, 1944398, 1944403, 1944408, 1944412, 1944422, 1944463    
Bug Blocks:    

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


%files          devel
%doc README.md
%license LICENSE

 - 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.

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:

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

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
I manage to put review for all dependency of rudo and to compile them in my copr.

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
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?

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

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