Skip to content

Declare scriptlet-only OS dependencies in salt.spec - #70149

Open
jcp-avgo wants to merge 4 commits into
saltstack:3008.xfrom
jcp-avgo:fix/vcops-106330-scriptlet-requires-3008x
Open

Declare scriptlet-only OS dependencies in salt.spec#70149
jcp-avgo wants to merge 4 commits into
saltstack:3008.xfrom
jcp-avgo:fix/vcops-106330-scriptlet-requires-3008x

Conversation

@jcp-avgo

@jcp-avgo jcp-avgo commented Aug 26, 2026

Copy link
Copy Markdown

Summary

The %pre/%post/%preun/%posttrans scriptlets for the salt base
package, and %pre/%post/%preun/%postun/%posttrans for the
minion subpackage, invoke coreutils, grep, findutils, glibc
(getent), sed, and systemd tools without declaring them as RPM
dependencies. This was surfaced by an internal packaging dependency
audit that diffed each scriptlet's actual tool usage against the
spec's declared Requires.

Undeclared scriptlet dependencies let a resolver like tdnf install
salt/salt-minion in an order where those tools aren't yet present,
so the scriptlet can silently fail on minimal or air-gapped installs
instead of the transaction failing loudly up front.

  • Add Requires(pre)/Requires(post)/Requires(preun)/Requires(posttrans)
    to the base salt package for the tools its scriptlets call.
  • Add Requires(pre)/Requires(post)/Requires(preun)/Requires(postun)/
    Requires(posttrans) to the minion subpackage for the same reason.
    openssl is declared explicitly on minion even though salt (which
    minion requires) already declares it unconditionally — per-package
    dependency audits inspect each subpackage's own Requires header and
    won't follow it transitively through %{name}.
  • No scriptlet logic changed — this only adds missing dependency
    metadata.

Test plan

  • Added test_rpm_spec_scriptlet_requires_present to
    tests/pytests/unit/test_pkg_scripts.py, following the existing
    test_rpm_spec_extras_dir_override_present pattern, asserting each
    Requires(...) line is present in the correct package's preamble.

@jcp-avgo
jcp-avgo requested a review from a team as a code owner August 26, 2026 13:21

@twangboy twangboy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please make this against the 3008.x branch. This also needs a changelog.

@twangboy twangboy added the test:full Run the full test suite label Aug 26, 2026
@jcp-avgo
jcp-avgo changed the base branch from 3008.1-patch to 3008.x August 26, 2026 18:53
The %pre/%post/%preun/%posttrans scriptlets for the salt base package
and %pre/%post/%preun/%postun/%posttrans for the minion subpackage
invoke coreutils, grep, findutils, glibc (getent), awk, and
systemd/sed/openssl tools without declaring them as RPM dependencies.
This lets tdnf install salt/salt-minion in an order where those tools
aren't yet present, causing scriptlets to silently fail on minimal or
air-gapped installs.

Add scriptlet-scoped Requires(pre)/Requires(post)/Requires(preun)/
Requires(postun)/Requires(posttrans) so package managers can
correctly resolve install ordering.
Asserts the base package's Requires(pre)/Requires(post)/Requires(preun)/
Requires(posttrans) and the minion subpackage's Requires(pre)/Requires(post)/
Requires(preun)/Requires(postun)/Requires(posttrans) lines stay declared,
following the existing test_rpm_spec_extras_dir_override_present pattern.
@jcp-avgo
jcp-avgo force-pushed the fix/vcops-106330-scriptlet-requires-3008x branch from 4a77c91 to 77ea3e2 Compare August 26, 2026 19:05
@jcp-avgo
jcp-avgo requested a review from twangboy August 26, 2026 19:06
Comment thread pkg/rpm/salt.spec Outdated
getent is split into a separate subpackage (e.g. glibc-common) on
some distros/minimal images, so pinning the glibc package name by
itself isn't portable across every RPM distro this spec targets.
Requiring the file path lets the package manager resolve whichever
package actually provides it, matching the file-based Requires
already used above for shadow-utils's usermod/groupadd/useradd.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants