Skip to content

rework spec files, use rpm macros to handle systemd #5235

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.

Sign up for GitHub

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

Open
mirostauder wants to merge 1 commit into
base: v3.0
Choose a base branch
from
Open

rework spec files, use rpm macros to handle systemd #5235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 37 additions & 45 deletions docker/images/proxysql/rhel-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# we don't want separate debuginfo packages
%global _enable_debug_package 0
%define debug_package %{nil}
# do not strip binaries
%global __strip /bin/true
Comment on lines +1 to +5
Copy link

@gemini-code-assist gemini-code-assist bot Dec 1, 2025

Choose a reason for hiding this comment

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

medium

The explicit disabling of debug packages and binary stripping is a good practice for container images. This helps in reducing the final image size and potentially improves security by removing unnecessary debugging information. This aligns well with the goal of addressing systemd-related issues in container environments, where minimal images are often preferred.

%define __spec_install_post %{nil}
%define debug_package %{nil}
%define __os_install_post %{_dbpath}/brp-compress
%define __os_install_post %{_dbpath}/brp-compress %{nil}

Summary: A high-performance MySQL and PostgreSQL proxy
Name: proxysql
Expand All @@ -9,8 +13,12 @@ Release: 1
License: GPL-3.0-only
Source: %{name}-%{version}.tar.gz
URL: https://proxysql.com/
Requires: gnutls, (openssl >= 3.0.0 or openssl3 >= 3.0.0)
Requires: gnutls
Requires: (openssl >= 3.0.0 or openssl3 >= 3.0.0)
#BuildRequires: systemd-rpm-macros
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
Provides: user(%{name})
Provides: group(%{name})
Comment on lines +20 to +21
Copy link

@gemini-code-assist gemini-code-assist bot Dec 1, 2025

Choose a reason for hiding this comment

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

medium

Adding Provides: user(%{name}) and Provides: group(%{name}) is a good practice for RPM packages that create system users and groups. This explicitly declares these provisions, which can be helpful for other packages that might depend on the existence of this user or group, improving the overall package ecosystem's clarity.


%description
%{summary}
Expand All @@ -19,72 +27,56 @@ BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
%setup -q

%pre
# Cleanup artifacts
if [ -f /var/lib/%{name}/PROXYSQL_UPGRADE ]; then
rm -fr /var/lib/%{name}/PROXYSQL_UPGRADE
fi
# setup user, group
getent passwd %{name} &>/dev/null || useradd -r -U -s /bin/false -d /var/lib/%{name} -c "ProxySQL Server" %{name}

%build
# Packages are pre-built, nothing to do

%install
export DONT_STRIP=1
# Clean buildroot and install files
/bin/rm -rf %{buildroot}
/bin/mkdir -p %{buildroot}
/bin/cp -a * %{buildroot}
rm -rf %{buildroot}
mkdir -p %{buildroot}
cp -a * %{buildroot}
Comment on lines +39 to +41
Copy link

@gemini-code-assist gemini-code-assist bot Dec 1, 2025

Choose a reason for hiding this comment

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

medium

While rm, mkdir, and cp are usually in the system's PATH, it's generally a more robust practice in RPM spec files to use their absolute paths (e.g., /bin/rm, /bin/mkdir, /bin/cp). This prevents potential issues if the build environment's PATH is unexpectedly modified or minimal, ensuring the correct utilities are always invoked.

/bin/rm -rf %{buildroot}
/bin/mkdir -p %{buildroot}
/bin/cp -a * %{buildroot}

mkdir -p %{buildroot}/var/run/%{name}
mkdir -p %{buildroot}/var/lib/%{name}

%clean
/bin/rm -rf %{buildroot}
rm -rf %{buildroot}

%post
# Create relevant user, directories and configuration files
if [ ! -d /var/run/%{name} ]; then /bin/mkdir /var/run/%{name} ; fi
if [ ! -d /var/lib/%{name} ]; then /bin/mkdir /var/lib/%{name} ; fi
if ! id -u %{name} > /dev/null 2>&1; then useradd -r -U -s /bin/false -d /var/lib/%{name} -c "ProxySQL Server" %{name}; fi
/bin/chown -R %{name}: /var/lib/%{name} /var/run/%{name}
/bin/chown root:%{name} /etc/%{name}.cnf
/bin/chmod 640 /etc/%{name}.cnf
# Configure systemd appropriately.
/bin/systemctl daemon-reload
/bin/systemctl enable %{name}.service
# Notify that a package update is in progress in order to start service.
if [ $1 -eq 2 ]; then /bin/touch /var/lib/%{name}/PROXYSQL_UPGRADE ; fi
# install service
%systemd_post %{name}.service
#%systemd_post_with_reload %{name}.service

%preun
# When uninstalling always try stop the service, ignore failures
/bin/systemctl stop %{name} || true
# remove service
%systemd_preun %{name}.service

%postun
if [ $1 -eq 0 ]; then
# This is a pure uninstall, systemd unit file removed
# only daemon-reload is needed.
/bin/systemctl daemon-reload
else
# This is an upgrade, ProxySQL should be started. This
# logic works for packages newer than 2.0.7 and ensures
# a faster restart time.
/bin/systemctl start %{name}.service
/bin/rm -fr /var/lib/%{name}/PROXYSQL_UPGRADE
fi
# remove user, group on uninstall
# dont, its against the recommended practice
#if [ "$1" == "0" ]; then
# groupdel %{name}
# userdel %{name}
#fi

%posttrans
if [ -f /var/lib/%{name}/PROXYSQL_UPGRADE ]; then
# This is a safeguard to start the service after an update
# which supports legacy "preun" / "postun" logic and will
# only execute for packages before 2.0.7.
/bin/systemctl start %{name}.service
/bin/rm -fr /var/lib/%{name}/PROXYSQL_UPGRADE
fi
# reload, restart service
#%systemd_posttrans_with_reload %{name}.service
#%systemd_posttrans_with_restart %{name}.service

%files
%defattr(-,root,root,-)
%config(noreplace) %{_sysconfdir}/%{name}.cnf
%attr(640,root,%{name}) %{_sysconfdir}/%{name}.cnf
%config(noreplace) %attr(640,root,%{name}) %{_sysconfdir}/%{name}.cnf
%config(noreplace) %attr(640,root,%{name}) %{_sysconfdir}/logrotate.d/%{name}
%{_bindir}/*
%{_sysconfdir}/systemd/system/%{name}.service
%{_sysconfdir}/systemd/system/%{name}-initial.service
/usr/share/proxysql/tools/proxysql_galera_checker.sh
/usr/share/proxysql/tools/proxysql_galera_writer.pl
%config(noreplace) %attr(750,%{name},%{name}) /var/run/%{name}/
%config(noreplace) %attr(750,%{name},%{name}) /var/lib/%{name}/
Comment on lines +79 to +80
Copy link

@gemini-code-assist gemini-code-assist bot Dec 1, 2025

Choose a reason for hiding this comment

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

medium

Explicitly defining the permissions and ownership for /var/run/%{name}/ and /var/lib/%{name}/ in the %files section is a strong improvement. This ensures that these directories are created with the correct security context, preventing potential privilege escalation or access issues for the application.


%changelog
81 changes: 36 additions & 45 deletions docker/images/proxysql/suse-compliant/rpmmacros/rpmbuild/SPECS/proxysql.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# we don't want separate debuginfo packages
%global _enable_debug_package 0
%define debug_package %{nil}
# do not strip binaries
%global __strip /bin/true
Comment on lines +1 to +5
Copy link

@gemini-code-assist gemini-code-assist bot Dec 1, 2025

Choose a reason for hiding this comment

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

medium

The explicit disabling of debug packages and binary stripping is a good practice for container images. This helps in reducing the final image size and potentially improves security by removing unnecessary debugging information. This aligns well with the goal of addressing systemd-related issues in container environments, where minimal images are often preferred.

%define __spec_install_post %{nil}
%define debug_package %{nil}
%define __os_install_post %{_dbpath}/brp-compress
%define __os_install_post %{_dbpath}/brp-compress %{nil}

Summary: A high-performance MySQL and PostgreSQL proxy
Name: proxysql
Expand All @@ -9,8 +13,11 @@ Release: 1
License: GPL-3.0-only
Source: %{name}-%{version}.tar.gz
URL: https://proxysql.com/
Requires: gnutls, (openssl >= 3.0.0 or openssl3 >= 3.0.0)
Requires: gnutls
Requires: (openssl >= 3.0.0 or openssl3 >= 3.0.0)
#BuildRequires: systemd-rpm-macros
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
Provides: user(%{name})
Provides: group(%{name})
Comment on lines +20 to 21
Copy link

@gemini-code-assist gemini-code-assist bot Dec 1, 2025

Choose a reason for hiding this comment

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

medium

Adding Provides: user(%{name}) and Provides: group(%{name}) is a good practice for RPM packages that create system users and groups. This explicitly declares these provisions, which can be helpful for other packages that might depend on the existence of this user or group, improving the overall package ecosystem's clarity.


%description
Expand All @@ -20,72 +27,56 @@ Provides: group(%{name})
%setup -q

%pre
# Cleanup artifacts
if [ -f /var/lib/%{name}/PROXYSQL_UPGRADE ]; then
rm -fr /var/lib/%{name}/PROXYSQL_UPGRADE
fi
if ! id -u %{name} > /dev/null 2>&1; then useradd -r -U -s /bin/false -d /var/lib/%{name} -c "ProxySQL Server" %{name}; fi
# setup user, group
getent passwd %{name} &>/dev/null || useradd -r -U -s /bin/false -d /var/lib/%{name} -c "ProxySQL Server" %{name}

%build
# Packages are pre-built, nothing to do

%install
export DONT_STRIP=1
# Clean buildroot and install files
/bin/rm -rf %{buildroot}
/bin/mkdir -p %{buildroot}
/bin/cp -a * %{buildroot}
rm -rf %{buildroot}
mkdir -p %{buildroot}
cp -a * %{buildroot}
Comment on lines +39 to +41
Copy link

@gemini-code-assist gemini-code-assist bot Dec 1, 2025

Choose a reason for hiding this comment

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

medium

While rm, mkdir, and cp are usually in the system's PATH, it's generally a more robust practice in RPM spec files to use their absolute paths (e.g., /bin/rm, /bin/mkdir, /bin/cp). This prevents potential issues if the build environment's PATH is unexpectedly modified or minimal, ensuring the correct utilities are always invoked.

/bin/rm -rf %{buildroot}
/bin/mkdir -p %{buildroot}
/bin/cp -a * %{buildroot}

mkdir -p %{buildroot}/var/run/%{name}
mkdir -p %{buildroot}/var/lib/%{name}

%clean
/bin/rm -rf %{buildroot}
rm -rf %{buildroot}

%post
# Create relevant user, directories and configuration files
if [ ! -d /var/run/%{name} ]; then /bin/mkdir /var/run/%{name} ; fi
if [ ! -d /var/lib/%{name} ]; then /bin/mkdir /var/lib/%{name} ; fi
/bin/chown -R %{name}: /var/lib/%{name} /var/run/%{name}
/bin/chown root:%{name} /etc/%{name}.cnf
/bin/chmod 640 /etc/%{name}.cnf
# Configure systemd appropriately.
/bin/systemctl daemon-reload
/bin/systemctl enable %{name}.service
# Notify that a package update is in progress in order to start service.
if [ $1 -eq 2 ]; then /bin/touch /var/lib/%{name}/PROXYSQL_UPGRADE ; fi
# install service
%systemd_post %{name}.service
#%systemd_post_with_reload %{name}.service

%preun
# When uninstalling always try stop the service, ignore failures
/bin/systemctl stop %{name} || true
# remove service
%systemd_preun %{name}.service

%postun
if [ $1 -eq 0 ]; then
# This is a pure uninstall, systemd unit file removed
# only daemon-reload is needed.
/bin/systemctl daemon-reload
else
# This is an upgrade, ProxySQL should be started. This
# logic works for packages newer than 2.0.7 and ensures
# a faster restart time.
/bin/systemctl start %{name}.service
/bin/rm -fr /var/lib/%{name}/PROXYSQL_UPGRADE
fi
# remove user, group on uninstall
# dont, its against the recommended practice
#if [ "$1" == "0" ]; then
# groupdel %{name}
# userdel %{name}
#fi

%posttrans
if [ -f /var/lib/%{name}/PROXYSQL_UPGRADE ]; then
# This is a safeguard to start the service after an update
# which supports legacy "preun" / "postun" logic and will
# only execute for packages before 2.0.7.
/bin/systemctl start %{name}.service
/bin/rm -fr /var/lib/%{name}/PROXYSQL_UPGRADE
fi
# reload, restart service
#%systemd_posttrans_with_reload %{name}.service
#%systemd_posttrans_with_restart %{name}.service

%files
%defattr(-,root,root,-)
%config(noreplace) %{_sysconfdir}/%{name}.cnf
%attr(640,root,%{name}) %{_sysconfdir}/%{name}.cnf
%config(noreplace) %attr(640,root,%{name}) %{_sysconfdir}/%{name}.cnf
%config(noreplace) %attr(640,root,%{name}) %{_sysconfdir}/logrotate.d/%{name}
%{_bindir}/*
%{_sysconfdir}/systemd/system/%{name}.service
%{_sysconfdir}/systemd/system/%{name}-initial.service
/usr/share/proxysql/tools/proxysql_galera_checker.sh
/usr/share/proxysql/tools/proxysql_galera_writer.pl
%config(noreplace) %attr(750,%{name},%{name}) /var/run/%{name}/
%config(noreplace) %attr(750,%{name},%{name}) /var/lib/%{name}/
Comment on lines +79 to +80
Copy link

@gemini-code-assist gemini-code-assist bot Dec 1, 2025

Choose a reason for hiding this comment

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

medium

Explicitly defining the permissions and ownership for /var/run/%{name}/ and /var/lib/%{name}/ in the %files section is a strong improvement. This ensures that these directories are created with the correct security context, preventing potential privilege escalation or access issues for the application.


%changelog
Loading