Skip to content

Gem updates for rmt3 - #1538

Open
mssola wants to merge 7 commits into
rmt_3from
gem-updates-for-rmt3
Open

Gem updates for rmt3#1538
mssola wants to merge 7 commits into
rmt_3from
gem-updates-for-rmt3

Conversation

@mssola

@mssola mssola commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Reacting to #1520 and #1535, I've done a general cleanup of gems for the RMT3 branch. I've separated things into commits, so for a better review go commit by commit instead of checking out the general diff. To highlight:

  1. As suggested by the aforementioned depfu PRs, I've upgraded the gems suggested there and tested that nothing broke.
  2. I've upgraded Puma from 7.x to 8.x and tested that nothing utterly broke. This has been previously discussed by @rtamalin and @digitaltom. I've also pinned down the Puma version so depfu doesn't automatically suggest major upgrades without us being sure about it.
  3. I've pinned down mysql2 and nokogiri which are either crucial (mysql) or hard to upgrade (nokogiri) gems.
  4. Upgraded the rest of gems to minor/patch level releases. This included an upgrade to rubocop, so I've added another commit which fixes issues brought up by the newly upgraded rubocop.

How to test

bundle exec rails and stuff. Everything should work as before.

Related PRs

Closes #1477
Closes #1520
Closes #1535

mssola added 5 commits August 12, 2026 09:38
As reported by depfu, these two gems had candidates for security
updates. Let's use them as they are safe and they bring security
updates (even if we are not particularly affected by them).

Closes #1535
Closes #1520

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>
As raised by Fergal in #1477, this is
not a trivial update, but testing this locally it seems fine. We can
always revert this commit if things go south.

Moreover, the puma version has been pinned to use the 8.x releases, so
to ensure that no future major changes are suggested by tools like depfu
without human consideration.

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>
The mysql2 gem is fundamental to the proper behavior of RMT, so it's a
good idea to pin it down. Note that sqlite3 has not been pinned down as
it's still considered an experimental feature for RMT.

Also pin down nokogiri, as it's prone to difficult upgrades.

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>
A bunch of minor gems have been upgraded to their latest
patch-level/minor candidates. No regressions have been discovered
because of this.

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>
While upgrading rubocop in commit a2061f7 ("General upgrade of
minor gems"), some minor style issues were detected. This was mainly
about the new Rails/ResponseParsedBody rule, but there was also an issue
on strong parameters.

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>
@mssola
mssola requested a review from rtamalin August 12, 2026 09:45
@mssola

mssola commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

There's an error on feature tests when validating the resulting RPM, but I don't think it's related to this PR. @felixsch @rtamalin, is this a known issue?

@ngetahun

Copy link
Copy Markdown
Contributor

There's an error on feature tests when validating the resulting RPM, but I don't think it's related to this PR. @felixsch @rtamalin, is this a known issue?

This is related to building transactional filesystem (like sl-micro and sle 16.1). Seems like one of the gems tries to change ownership in /etc

@rtamalin

Copy link
Copy Markdown
Collaborator

I think that feature test failure is a false positive; the regexp check being used to test for the restricted file system paths will erroneously match any path ending in /etc or /var/lib, not just the /etc or /var/lib paths.

To demonstrate, using a downloaded rmt-server-3.0.0 package from the 16.1 stream and checking for a path /rmt the old way yields the following:

30068d380cee:/ # rpm -qplv /var/cache/zypp/packages/SLE_BCI/x86_64/rmt-server-3.0.0-160099.1.10.x86_64.rpm | grep -E "^d.*/rmt$"
drwxr-xr-x    2 _rmt     nginx                       0 May 18 16:07 /etc/rmt
drwxr-xr-x    2 root     root                        0 May 18 16:07 /usr/lib64/rmt
drwxr-xr-x    2 root     root                        0 May 18 16:07 /usr/libexec/rmt
drwxr-xr-x    2 root     root                        0 May 18 16:07 /usr/share/rmt
drwxr-xr-x    2 root     root                        0 May 18 16:07 /usr/share/rmt/lib/rmt
drwxr-xr-x    2 _rmt     nginx                       0 May 18 16:07 /var/lib/rmt
30068d380cee:/ #

Adding a requirement to the regexp to match a whitespace before the target path yields the following:

30068d380cee:/ # rpm -qplv /var/cache/zypp/packages/SLE_BCI/x86_64/rmt-server-3.0.0-160099.1.10.x86_64.rpm | grep -e "^d.*[[:space:]]/rmt$"
30068d380cee:/ #

We also aren't using any extended regexp features, so we don't need the -E. ;-)

A change like the following should fix it:

diff --git a/ci/rmt-validate-packaging b/ci/rmt-validate-packaging
index 6efcdb2e..db9f9524 100755
--- a/ci/rmt-validate-packaging
+++ b/ci/rmt-validate-packaging
@@ -46,7 +46,7 @@ while IFS= read -r RPM; do

   # Check for forbidden directory ownership
   for dir_pattern in "${FORBIDDEN_DIRS[@]}"; do
-    if rpm -qplv "$RPM" 2>/dev/null | grep -E "^d.*${dir_pattern}"; then
+    if rpm -qplv "$RPM" 2>/dev/null | grep -e "^d.*[[:space:]]${dir_pattern}"; then
       echo "  ✗ ERROR: Package claims ownership of ${dir_pattern}"
       echo "    Top-level directories should be owned by filesystem package"
       ((ERRORS++))

@rtamalin

Copy link
Copy Markdown
Collaborator

I've proposed #1543 with the suggested CI fix.

@rtamalin

Copy link
Copy Markdown
Collaborator

#1543 has resolved the feature test RPM validation issue.

@rtamalin rtamalin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good

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.

3 participants