Skip to content

Packit: initial enablement #4

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

Closed
wants to merge 1 commit into from
Closed

Packit: initial enablement #4

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
19 changes: 9 additions & 10 deletions common/.packit.yaml → .packit.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,28 @@
# See the documentation for more information:
# https://packit.dev/docs/configuration/

upstream_tag_template: v{version}
upstream_tag_template: common/v{version}

packages:
containers-common-fedora:
downstream_package_name: containers-common
pkg_tool: fedpkg
specfile_path: rpm/containers-common.spec
specfile_path: common/rpm/containers-common.spec
containers-common-centos:
downstream_package_name: containers-common
pkg_tool: centpkg
specfile_path: rpm/containers-common.spec
specfile_path: common/rpm/containers-common.spec
containers-common-eln:
downstream_package_name: containers-common
specfile_path: rpm/containers-common.spec

actions:
pre-sync: "bash rpm/update-lib-versions.sh"
specfile_path: common/rpm/containers-common.spec

jobs:
- job: copr_build
trigger: pull_request
packages: [containers-common-fedora]
notifications: &ephemeral_build_failure_notification
failure_comment:
message: "Ephemeral COPR build failed. @containers/packit-build please check."
message: "Packit jobs failed. @containers/packit-build please check."
enable_net: true
targets:
- fedora-all
Expand Down Expand Up @@ -55,7 +52,7 @@ jobs:

# Run on commit to main branch
- job: copr_build
trigger: commit
trigger: ignore
packages: [containers-common-fedora]
notifications:
failure_comment:
Expand All @@ -71,11 +68,13 @@ jobs:
dist_git_branches: &fedora_targets
- fedora-all

# Ignore CentOS Stream for now
- job: propose_downstream
trigger: release
trigger: ignore
packages: [containers-common-centos]
dist_git_branches:
- c10s
- c9s

# Fedora Koji build
- job: koji_build
Expand Down
106 changes: 34 additions & 72 deletions common/rpm/containers-common.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,5 @@
# Below definitions are used to deliver config files from a particular branch
# of c/image, c/storage and c/shortnames vendored in all of Buildah, Podman and Skopeo.
# These vendored components must have the same version. If it is not the case,
# pick the oldest version on c/image, c/storage and c/shortnames vendored in
# Buildah/Podman/Skopeo.

# Packit will automatically update the image and storage versions on Fedora and
# CentOS Stream dist-git PRs.
%global image_branch main
%global storage_branch main
%global shortnames_branch main

%global project containers
%global repo common

%global raw_github_url https://raw.githubusercontent.com/%{project}
%global repo container-libs

%if %{defined copr_username}
%define copr_build 1
Expand All @@ -35,7 +21,7 @@ Epoch: 5
%endif
# DO NOT TOUCH the Version string!
# The TRUE source of this specfile is:
# https://github.com/containers/common/blob/main/rpm/containers-common.spec
# https://github.com/containers/container-libs/blob/main/common/rpm/containers-common.spec
# If that's what you're reading, Version must be 0, and will be updated by Packit for
# copr and koji builds.
# If you're reading this on dist-git, the version is automatically filled in by Packit.
Expand All @@ -57,25 +43,9 @@ Requires: (fuse-overlayfs if fedora-release-identity-server)
Suggests: fuse-overlayfs
%endif
URL: https://github.com/%{project}/%{repo}
Source0: %{url}/archive/v%{version_no_tilde}.tar.gz
Source1: %{raw_github_url}/image/%{image_branch}/docs/containers-auth.json.5.md
Source2: %{raw_github_url}/image/%{image_branch}/docs/containers-certs.d.5.md
Source3: %{raw_github_url}/image/%{image_branch}/docs/containers-policy.json.5.md
Source4: %{raw_github_url}/image/%{image_branch}/docs/containers-registries.conf.5.md
Source5: %{raw_github_url}/image/%{image_branch}/docs/containers-registries.conf.d.5.md
Source6: %{raw_github_url}/image/%{image_branch}/docs/containers-registries.d.5.md
Source7: %{raw_github_url}/image/%{image_branch}/docs/containers-signature.5.md
Source8: %{raw_github_url}/image/%{image_branch}/docs/containers-transports.5.md
Source9: %{raw_github_url}/storage/%{storage_branch}/docs/containers-storage.conf.5.md
Source10: %{raw_github_url}/shortnames/%{shortnames_branch}/shortnames.conf
Source11: %{raw_github_url}/image/%{image_branch}/default.yaml
Source12: %{raw_github_url}/image/%{image_branch}/default-policy.json
Source13: %{raw_github_url}/image/%{image_branch}/registries.conf
Source14: %{raw_github_url}/storage/%{storage_branch}/storage.conf
# Fetch RPM-GPG-KEY-redhat-release from the authoritative source instead of storing
# a copy in repo or dist-git. Depending on distribution-gpg-keys rpm is also
# not an option because that package doesn't exist on CentOS Stream.
Source15: https://access.redhat.com/security/data/fd431d51.txt
Source0: %{url}/archive/refs/tags/common/v%{version}.tar.gz
Copy link
Contributor

@jnovy jnovy Aug 13, 2025

Choose a reason for hiding this comment

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

Why it's delivering the whole world and not only bits which are actually needed/installed for the container ecosystem? This is super wasteful in terms of package size, content and overall packaging.

Copy link
Member Author

@lsm5 lsm5 Aug 14, 2025

Choose a reason for hiding this comment

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

where exactly is the package size and content impact felt?

Copy link
Contributor

@jnovy jnovy Aug 14, 2025

Choose a reason for hiding this comment

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

`Source0: %{url}/archive/refs/tags/common/v%{version}.tar.gz'

There is a fundamental difference between a regular package which actually builds from source and this one. The build doesn't happen. The purpose of containers-common is to only deliver small configuration files originating from upstream to container-tools. For this purpose stashing not only config files but also sources - which are unbuilt - into the mothership tarball with 99.99% of stuff uninstalled is very wasteful.

Copy link
Member Author

@lsm5 lsm5 Aug 14, 2025

Choose a reason for hiding this comment

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

Sure, but who's exactly feeling the impact of this?

  • Does it cause unacceptable increase in build time?
  • Does the end user care?
  • Does the distro care?
  • Does the customer, for example CoreOS / OpenShift, care ?

The current PR makes the spec file a lot simpler. So, unless someone is actually feeling the impact of a large tarball, I'd prefer to keep it this way.

Copy link
Contributor

@jnovy jnovy Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

"who cares?" doesn't seem the correct justification :-) Also it is not exactly a simplification for various reasons:

Consider this:

with tarball-only approach:

  1. one needs to patch config files on the fly in the spec file
  2. one needs to be super cautious to not to break container tools if you deploy containers-common which is not vendored into podman/buildah/skopeo yet

With per-version source URLs:

  1. one knows exactly what you package as the required config files are actually stored in dist-git with reference to upstream URL from which is originates from
  2. The update_vendored.sh script actually assures no regression possibility by design - it will set vendored versions of c/storage, c/image and c/common in the spec to pull configuration files from based on actual lowest common denominator of podman/buildah/skopeo vendored content
  3. The configuration files in 2) are comprehensibly updated with per-OS configuration by update.sh script so it is obvious to anyone what is different in e.g. RHEL8, RHEL9 and RHEL10 - the "ensure" bash functions

Copy link
Member Author

@lsm5 lsm5 Aug 18, 2025

Choose a reason for hiding this comment

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

"who cares?" doesn't seem the correct justification :-) Also it is not exactly a simplification for various reasons:

Consider this:

with tarball-only approach:

  1. one needs to patch config files on the fly in the spec file

which is what we've been already doing via the Packit workflow for Fedora for sometime now.

  1. one needs to be super cautious to not to break container tools if you deploy containers-common which is not vendored into podman/buildah/skopeo yet

That's why we have packit copr builds and TMT tests running on podman, buildah and skopeo PRs fetching containers-common from podman-next. So, things are continuously tested upstream.

With per-version source URLs:

  1. one knows exactly what you package as the required config files are actually stored in dist-git with reference to upstream URL from which is originates from

If any auditor cares, they're welcome to fetch and extract files from the rpm. No reason to add unnecessary maintenance burden downstream (Fedora). They're config files so they don't need to be stored in dist-git to be readable. CentOS Stream and RHEL process are your call.

  1. The update_vendored.sh script actually assures no regression possibility by design - it will set vendored versions of c/storage, c/image and c/common in the spec to pull configuration files from based on actual lowest common denominator of podman/buildah/skopeo vendored content

With monorepo, renovate and consistent tests running upstream and on Fedora, I don't see this being an issue. At the very least, nothing worth requiring any manual maintenance on Fedora.

  1. The configuration files in 2) are comprehensibly updated with per-OS configuration by update.sh script so it is obvious to anyone what is different in e.g. RHEL8, RHEL9 and RHEL10 - the "ensure" bash functions

That has been upstreamed already in common/rpm/update-config-files.sh. Again, CentOS Stream and RHEL are your call.

Source1: https://raw.githubusercontent.com/containers/shortnames/refs/heads/main/shortnames.conf
Copy link
Contributor

@jnovy jnovy Aug 13, 2025

Choose a reason for hiding this comment

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

Shouldn't this be a part of container-libs too?

Copy link
Member Author

@lsm5 lsm5 Aug 14, 2025

Choose a reason for hiding this comment

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

that's a separate repo and looks like it's going to remain separate because there are contributors from other projects.

Source2: https://access.redhat.com/security/data/fd431d51.txt

%description
This package contains common configuration files and documentation for container
Expand Down Expand Up @@ -109,34 +79,20 @@ This subpackage will handle dependencies common to Podman and Buildah which are
not required by Skopeo.

%prep
%autosetup -Sgit -n %{repo}-%{version_no_tilde}

# Copy manpages to docs subdir in builddir to build before installing.
cp %{SOURCE1} docs/.
cp %{SOURCE2} docs/.
cp %{SOURCE3} docs/.
cp %{SOURCE4} docs/.
cp %{SOURCE5} docs/.
cp %{SOURCE6} docs/.
cp %{SOURCE7} docs/.
cp %{SOURCE8} docs/.
cp %{SOURCE9} docs/.

# Copy config files to builddir to patch them before installing.
# Currently, only registries.conf and storage.conf files are patched before
# installing.
cp %{SOURCE10} shortnames.conf
cp %{SOURCE13} registries.conf
cp %{SOURCE14} storage.conf
%autosetup -Sgit -n %{repo}-common-v%{version}

# Fine-grain distro- and release-specific tuning of config files,
# e.g., seccomp, composefs, registries on different RHEL/Fedora versions
bash rpm/update-config-files.sh
bash common/rpm/update-config-files.sh

%build
mkdir -p man5
for i in docs/*.5.md; do
go-md2man -in $i -out man5/$(basename $i .md)
mkdir -p man1 man5
for i in common/docs/*.md image/docs/*.md storage/docs/*.md; do
if [[ "$i" == *".5.md" ]]; then
go-md2man -in $i -out man5/$(basename $i .md)
else
go-md2man -in $i -out man1/$(basename $i .md)
fi
done

%install
Expand All @@ -150,32 +106,37 @@ touch %{buildroot}%{_prefix}/lib/containers/storage/overlay-images/images.lock
install -dp -m 700 %{buildroot}%{_prefix}/lib/containers/storage/overlay-layers
touch %{buildroot}%{_prefix}/lib/containers/storage/overlay-layers/layers.lock

install -Dp -m0644 shortnames.conf %{buildroot}%{_sysconfdir}/containers/registries.conf.d/000-shortnames.conf
install -Dp -m0644 %{SOURCE11} %{buildroot}%{_sysconfdir}/containers/registries.d/default.yaml
install -Dp -m0644 %{SOURCE12} %{buildroot}%{_sysconfdir}/containers/policy.json
install -Dp -m0644 registries.conf %{buildroot}%{_sysconfdir}/containers/registries.conf
install -Dp -m0644 storage.conf %{buildroot}%{_datadir}/containers/storage.conf
install -Dp -m0644 %{SOURCE1} %{buildroot}%{_sysconfdir}/containers/registries.conf.d/000-shortnames.conf
install -Dp -m0644 image/default.yaml %{buildroot}%{_sysconfdir}/containers/registries.d/default.yaml
install -Dp -m0644 image/default-policy.json %{buildroot}%{_sysconfdir}/containers/policy.json
install -Dp -m0644 image/registries.conf %{buildroot}%{_sysconfdir}/containers/registries.conf
install -Dp -m0644 storage/storage.conf %{buildroot}%{_datadir}/containers/storage.conf

# RPM-GPG-KEY-redhat-release already exists on rhel envs, install only on
# fedora and centos
%if %{defined fedora} || %{defined centos}
install -Dp -m0644 %{SOURCE15} %{buildroot}%{_sysconfdir}/pki/rpm-gpg/RPM-GPG-KEY-redhat-release
install -Dp -m0644 %{SOURCE2} %{buildroot}%{_sysconfdir}/pki/rpm-gpg/RPM-GPG-KEY-redhat-release
%endif

install -Dp -m0644 contrib/redhat/registry.access.redhat.com.yaml -t %{buildroot}%{_sysconfdir}/containers/registries.d
install -Dp -m0644 contrib/redhat/registry.redhat.io.yaml -t %{buildroot}%{_sysconfdir}/containers/registries.d
install -Dp -m0644 common/contrib/redhat/registry.access.redhat.com.yaml -t %{buildroot}%{_sysconfdir}/containers/registries.d
install -Dp -m0644 common/contrib/redhat/registry.redhat.io.yaml -t %{buildroot}%{_sysconfdir}/containers/registries.d

# install manpages
install -dp %{buildroot}%{_mandir}/man5
install -dp -m 700 %{buildroot}%{_mandir}/man1
pushd man1
for i in *; do
install -p -m0644 $i -T %{buildroot}%{_mandir}/man1/$i.1
done
popd
for i in man5/*.5; do
install -Dp -m0644 $i -t %{buildroot}%{_mandir}/man5
install -Dp -m0644 $i -t %{buildroot}%{_mandir}/man5
done
ln -s containerignore.5 %{buildroot}%{_mandir}/man5/.containerignore.5

# install config files for mounts, containers and seccomp
install -m0644 pkg/subscriptions/mounts.conf %{buildroot}%{_datadir}/containers/mounts.conf
install -m0644 pkg/seccomp/seccomp.json %{buildroot}%{_datadir}/containers/seccomp.json
install -m0644 pkg/config/containers.conf %{buildroot}%{_datadir}/containers/containers.conf
install -m0644 common/pkg/subscriptions/mounts.conf %{buildroot}%{_datadir}/containers/mounts.conf
install -m0644 common/pkg/seccomp/seccomp.json %{buildroot}%{_datadir}/containers/seccomp.json
install -m0644 common/pkg/config/containers.conf %{buildroot}%{_datadir}/containers/containers.conf

# install secrets patch directory
install -d -p -m 755 %{buildroot}/%{_datadir}/rhel/secrets
Expand All @@ -184,7 +145,6 @@ ln -s ../../../..%{_sysconfdir}/pki/entitlement %{buildroot}%{_datadir}/rhel/sec
ln -s ../../../..%{_sysconfdir}/rhsm %{buildroot}%{_datadir}/rhel/secrets/rhsm
ln -s ../../../..%{_sysconfdir}/yum.repos.d/redhat.repo %{buildroot}%{_datadir}/rhel/secrets/redhat.repo

# Placeholder check to silence rpmlint warnings
%check

%files
Expand Down Expand Up @@ -215,6 +175,8 @@ ln -s ../../../..%{_sysconfdir}/yum.repos.d/redhat.repo %{buildroot}%{_datadir}/
%ghost %{_sysconfdir}/containers/storage.conf
%ghost %{_sysconfdir}/containers/containers.conf
%dir %{_sharedstatedir}/containers/sigstore
%{_mandir}/man1/containers-storage*.1.gz
%{_mandir}/man1/signature-protocols.1.gz
%{_mandir}/man5/Containerfile.5.gz
%{_mandir}/man5/containerignore.5.gz
%{_mandir}/man5/.containerignore.5.gz
Expand Down
30 changes: 15 additions & 15 deletions common/rpm/update-config-files.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,45 @@
set -exo pipefail

ensure() {
if [[ ! -f $1 ]]; then
echo "File not found:" $1
if [[ ! -f "$1" ]]; then
echo "File not found:" "$1"
exit 1
fi
if grep ^$2[[:blank:]].*= $1 > /dev/null
if grep "^$2[[:blank:]].*=" "$1" > /dev/null
then
sed -i "s;^$2[[:blank:]]=.*;$2 = $3;" $1
sed -i "s;^$2[[:blank:]]=.*;$2 = $3;" "$1"
else
if grep ^\#.*$2[[:blank:]].*= $1 > /dev/null
if grep "^\#.*$2[[:blank:]].*=" "$1" > /dev/null
then
sed -i "/^#.*$2[[:blank:]].*=/a \
$2 = $3" $1
$2 = $3" "$1"
else
echo "$2 = $3" >> $1
echo "$2 = $3" >> "$1"
fi
fi
}

# Common options enabled across all fedora, centos, rhel
# TBD: Can these be enabled by default upstream?
ensure registries.conf short-name-mode \"enforcing\"
ensure image/registries.conf short-name-mode \"enforcing\"

ensure storage.conf driver \"overlay\"
ensure storage.conf mountopt \"nodev,metacopy=on\"
ensure storage/storage.conf driver \"overlay\"
ensure storage/storage.conf mountopt \"nodev,metacopy=on\"

ensure pkg/config/containers.conf runtime \"crun\"
ensure pkg/config/containers.conf log_driver \"journald\"
ensure common/pkg/config/containers.conf runtime \"crun\"
ensure common/pkg/config/containers.conf log_driver \"journald\"

FEDORA=$(rpm --eval '%{?fedora}')
RHEL=$(rpm --eval '%{?rhel}')

# Set search registries
if [[ -n "$FEDORA" ]]; then
ensure registries.conf unqualified-search-registries [\"registry.fedoraproject.org\",\ \"registry.access.redhat.com\",\ \"docker.io\"]
ensure image/registries.conf unqualified-search-registries [\"registry.fedoraproject.org\",\ \"registry.access.redhat.com\",\ \"docker.io\"]
else
ensure registries.conf unqualified-search-registries [\"registry.access.redhat.com\",\ \"registry.redhat.io\",\ \"docker.io\"]
ensure image/registries.conf unqualified-search-registries [\"registry.access.redhat.com\",\ \"registry.redhat.io\",\ \"docker.io\"]
fi

# Set these on all Fedora and RHEL 10+
if [[ -n "$FEDORA" ]] || [[ "$RHEL" -ge 10 ]]; then
sed -i -e '/^additionalimagestores\ =\ \[/a "\/usr\/lib\/containers\/storage",' storage.conf
sed -i -e '/^additionalimagestores\ =\ \[/a "\/usr\/lib\/containers\/storage",' storage/storage.conf
fi
13 changes: 0 additions & 13 deletions common/rpm/update-lib-versions.sh
View file
Open in desktop

This file was deleted.

Loading