Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1911565 - Review Request: snebu-1.1.1-1 - Simple Network Encrypting Backup Utility
Summary: Review Request: snebu-1.1.1-1 - Simple Network Encrypting Backup Utility
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: dan.cermak
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2020-12-30 04:52 UTC by Derek Pressnall
Modified: 2021-01-14 12:28 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
ppisar: fedora-review?


Attachments (Terms of Use)

Description Derek Pressnall 2020-12-30 04:52:17 UTC
Spec URL: https://github.com/derekp7/snebu/releases/download/v1.1.1/snebu.spec
SRPM URL: https://github.com/derekp7/snebu/releases/download/v1.1.1/snebu-1.1.1-1.fc33.src.rpm
Description: Snebu is a backup system for Linux which supports single-host installs or server-based installs.  It supports arbitrarily named retention schedules (typically "daily", "weekly", "monthly", etc), compression, encryption, file-level de-duplication (across multiple clients), granular user access permissions, client-initiated push or server-initiated pull backups, and agent-less client backups.

See https://www.snebu.com.

Fedora Account System Username: derekp

Other info: I am a first-time Fedora package contributor, and require sponsorship.  I am also the upstream package author / maintainer.

Koji test scratch build URL: https://koji.fedoraproject.org/koji/taskinfo?taskID=58580246

Comment 1 dan.cermak 2020-12-30 15:35:46 UTC
Overall this looks good to me, here are a few suggestions:
- please BuildRequire systemd to get the correct directory ownership of /usr/lib/sysusers.d
- use the %{version} macro everywhere, so that you don't have to change the hardcoded version in all places every time there is a new release, so e.g.
Source0:	https://github.com/derekp7/snebu/releases/download/v1.1.1/snebu-1.1.1.tar.gz
becomes:
Source0:	https://github.com/derekp7/%{name}/releases/download/v%{version}/%{name}-%{version}.tar.gz
- use more macros & modern macros, e.g. instead of %setup -q use %autosetup, you can also replace the make install with %make_install (you'll have to add your prefix override to that though), replace all paths for which there are macros with those (/etc -> %_sysconfdir, /usr/bin/ -> %_bindir)
- please add newlines between the changelog entries
- Consider marking the config file with noreplace or add a comment to the spec why it is not marked as such. Also, since you're upstream, you might want to consider shipping a default config in /usr/ and allow the admin/user to supply their own overrides via /etc/snebu.conf leveraging libeconf (https://github.com/openSUSE/libeconf)
- you can replace this:
%license /usr/share/doc/snebu/COPYING.txt
with
%license COPYING.txt

Comment 2 dan.cermak 2020-12-30 15:39:48 UTC
And another thing, in case you have tests in your repository that can be run during rpmbuild, then consider adding them to %check.

Comment 3 Derek Pressnall 2020-12-30 22:01:22 UTC
I have made the requested adjustments, and some additional cleanup in the spec file.

Will look into the libeconf suggestion for the config file as part of the upstream project, however the config is minimal (containing a path to the catalog database and file repositories).  In the upstream project, future configuration items may be planned to be handled out of the SQLite DB used for the backup catalog in something like a sysparams table (at that point, even the "vault=" specification can be migrated to the DB).  Still weighing the pros and cons of that.

Currently there isn't publicly available unit tests, all testing at the moment requires manual intervention.  However that will be automated in an upcoming release.  In that case, what type of resources does the Fedora build infrastructure have available for testing?  For example, simple tests could be made to create sample directories to backup up and verify, with verification of the expiration/purge processes.  However, some tests require creation of various sparse files with varying amount of "holes" (this test the ability to ingest GNU tar format sparse entries -- there are edge cases in the format at 4 holes, 24, and multiples of 20 afterwards, so testing should include the boundary points).  Also GNU tar switches file size representation at 2GB (switches from octal to network byte order binary format).  So that would require large files to test against.  For now, will concentrate on the minimal test cases to ensure basic functionality though.

I do have one question in the Source specification.  The Sourc0 line has the full URL for the upstream source.  Is a full URL also required for Source1, which points to the ".sysusers" file consumed by %sysusers_create_compat?  It appears to not have a URL specified in other spec files I've looked at.

Thanks.

Comment 4 Derek Pressnall 2021-01-01 03:27:23 UTC
A couple more packaging questions to bring the package more in alignment with the packaging guidelines.

1) The package consists of a front-end (client) shell script (snebu-client), a client-side encryption filter (tarcrypt), and a backend (server side) binary (snebu).  Should the client side components be broken out into a sub package snebu-client, and server side as snebu-server?  On a single host installation, they would both be installed.  Server side should have both (push-based backup can work without snebu-client on the server, but pull-based backups require it).  But a client that pushes backups to a server only needs the snebu-client package (containing snebu-client shell script and tarcrypt).  So I was thinking that the -server package should depend on the -client package, but since it can be used in some modes of operations without it, then maybe it makes sense for both to be installable independently of each other.

2) The client script can call user-supplied plugin scripts that do things like put a database server in hot backup mode, or invoke and mount an LVM snapshot.  A template script for DB backups is provided in a man5 page describing how to create a plugin.  This template doesn't get executed directly on its own, but needs to be finished off based on a user's requirements.  So should a copy of template scripts like this go in the doc directory?  The upstream project will eventually provide plugin parametrized scripts for various situations (PostgrSQL and Oracle DB backups, LVM snapshots, etc).  At that point these will probably belong in libexec (since they are called by the app, not invoked directly).

3) The documentation is provided in manpages, and more detailed docs is in an Asciidoc format file.  This is mostly the contents of the project web page (minus download and installation instructions).  Is this content big enough to be placed in a separate -doc subpackage?  It will eventually grow a bit bigger, by about 20% - 50%.  Also should a pre-rendered HTML version of the .adoc file be included?  This would of course add "asciidoctor" to the build requirements.

4) As revisions are made to the package during the review process, does the -release tag get bumped up also (along with associated entries in the changelog), or is the -release tag only frozen once / if the package is accepted?

Thanks.

Comment 5 Derek Pressnall 2021-01-06 05:20:01 UTC
Updated spec file, and updated upstream:

Spec URL: https://github.com/derekp7/snebu/releases/download/v1.1.2/snebu.spec
SRPM URL: https://github.com/derekp7/snebu/releases/download/v1.1.2/snebu-1.1.2-1.fc33.src.rpm

The spec file has been brought in line with the packaging guidelines, and areas where there were clarifications needed were resolved by comparing against other Fedora packages that have similar features to the items in question.  Also compared against recently accepted packages to try to identify common errors.

Added '%global _hardened_build 1' due to snebu binary being capable of running as a server type process (not a daemon, but executable via ssh from front-end client)

Split out client script and encryption utility into snebu-client package.

Unit tests have been added to %check, these tests will eventually be put upstream.  Current tests cover:
1) Creating a new backup (using the build directory)
2) Verifying backup exist in repository
3) Testing restore (piping through tar with the "-d" flag)
4) Performing a second backup with new file contents
5) Expiring the first backup
6) Validating that unique file from second backup still restores correctly
7) Executing "purge" process, and verifying that unique file from first backup is no longer in storage repository.

This covers all base use cases of Snebu.

I ran fedora-review, and all automated tests passed
The rpmlint section shows errors due to snebu being installed suid to user/group snebu (to support the permissions model of the application).

I think that about covers it.


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