Skip to content

Commit

Permalink
Drop cockpit-tests package
Browse files Browse the repository at this point in the history
This is only required for running our integration tests. It really
shouldn't be in the distributions. While the playground may only hurt
mildly (being clutter), the mock PAM module is touching a security
critical piece of the OS.

Instead, install the playground into /usr/local/ in `image-prepare`.
Fish the mock PAM module out of the build tree. As that isn't visible in
pbuilder (on Debian/Ubuntu), use a pbuilder hook to run `make install-tests`
into a host bind-mounted directory.

Remove the package on upgrade.

Skip TestLogin.testSELinuxRestrictedUser in tmt. It's the only test that
requires the playground, and it mostly just tests our own SELinux policy
which we can do upstream.
  • Loading branch information
martinpitt committed Dec 5, 2024
1 parent 4862de2 commit 2a16cde
Show file tree
Hide file tree
Showing 15 changed files with 41 additions and 56 deletions.
2 changes: 1 addition & 1 deletion pkg/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ EXTRA_DIST += build.js files.js package.json package-lock.json
# don't install empty LEGAL.txt files, see https://github.com/evanw/esbuild/issues/3670
INSTALL_DATA_LOCAL_TARGETS += install-bundles
install-bundles:
cd $(srcdir)/dist; find */* -type f ! -name '*.LEGAL.txt' -exec install -D -m 644 '{}' '$(abspath $(DESTDIR)$(datadir))/cockpit/{}' \;
cd $(srcdir)/dist; find */* -type f ! -name '*.LEGAL.txt' -a ! -path 'playground/*' -exec install -D -m 644 '{}' '$(abspath $(DESTDIR)$(datadir))/cockpit/{}' \;
find $(srcdir)/dist -name '*.LEGAL.txt' ! -empty -exec install --target-directory '$(abspath $(DESTDIR)$(docdir))/legal' '{}' -D -m 644 '{}' \;

UNINSTALL_LOCAL_TARGETS += uninstall-bundles
Expand Down
1 change: 0 additions & 1 deletion test/browser/main.fmf
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
# - cockpit-networkmanager
# - cockpit-sosreport
- cockpit-packagekit
- cockpit-tests
# build/test infra dependencies
- podman
# required by tests
Expand Down
1 change: 1 addition & 0 deletions test/browser/run-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ if [ "$PLAN" = "main" ]; then
TestLogin.testSSH
TestLogin.testRaw
TestLogin.testServer
TestLogin.testSELinuxRestrictedUser
TestLogin.testUnsupportedBrowser
TestNetworkingBasic.testIpHelper
Expand Down
30 changes: 27 additions & 3 deletions test/image-prepare
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,16 @@ def build_install_package(dist_tar, image):
# our images have distro cockpit packages pre-installed, remove them
args = ["--run-command"]
if 'debian' in image or 'ubuntu' in image:
args.append("dpkg --purge cockpit cockpit-ws cockpit-bridge cockpit-system")
# set up pbuilder hook to make install-tests
cmd = """
set -ex
dpkg --purge cockpit cockpit-ws cockpit-bridge cockpit-system
mkdir -p /var/tmp/tests
echo 'HOOKDIR="/etc/pbuilder-hooks"' >> /etc/pbuilderrc
echo 'BINDMOUNTS="/var/tmp/tests"' >> /etc/pbuilderrc
"""
args.append(cmd)
args += ["--upload", os.path.join(TEST_DIR, "pbuilder-hooks") + ":/etc/"]
else:
# subscription-manager-cockpit needs these, thus --nodeps
args.append(r"""
Expand All @@ -105,6 +114,18 @@ def build_install_package(dist_tar, image):

args += ["--build", dist_tar]

# install the playground
args += ["--upload", os.path.join(BASE_DIR, "dist/playground") + ":/usr/local/share/cockpit/"]

# install mock pam module; we don't want to ship it as package, so fish it out of the build root
args.append("--run-command")
if 'debian' in image or 'ubuntu' in image:
args.append(r"find /var/tmp/tests -name mock-pam-conv-mod.so -exec cp {} /lib/x86_64-linux-gnu/security \;")
elif 'arch' in image:
args.append(r"find /var/lib/archbuild -name mock-pam-conv-mod.so -exec cp {} /usr/lib/security/ \;")
else:
args.append(r"find /var/lib/mock -name mock-pam-conv-mod.so -exec cp {} /usr/lib64/security/ \;")

if 'debian' in image or 'ubuntu' in image:
args.append("--run-command")
suppress = {'initial-upload-closes-no-bugs', 'newer-standards-version'}
Expand Down Expand Up @@ -139,7 +160,8 @@ def build_install_ostree(dist_tar, image, verbose, quick):
args += [
"--upload", os.path.join(BASE_DIR, "containers") + ":/var/tmp/",
"--script", os.path.join(TEST_DIR, "ws-container.install"),
"--script", os.path.join(TEST_DIR, "ostree.install")]
"--script", os.path.join(TEST_DIR, "ostree.install"),
"--upload", os.path.join(BASE_DIR, "dist/playground") + ":/usr/local/share/cockpit/"]
return args


Expand All @@ -161,8 +183,10 @@ def build_install_container(dist_tar, image, verbose, quick):

with create_machine(f"fedora-{fedora_version}") as m:
build_rpms(dist_tar, m, verbose, quick)
# copy them where ws-container.install expects them
# copy the rpms and playground where ws-container.install expects them
m.execute(r"find /var/tmp/build -name '*.rpm' -not -name '*.src.rpm' -exec mv {} /var/tmp/ \;")
m.execute("mkdir -p /var/tmp/install/usr/share/cockpit/")
m.upload([os.path.join(BASE_DIR, "dist/playground")], "/var/tmp/install/usr/share/cockpit/")
# create container
m.upload([os.path.join(BASE_DIR, "containers")], "/var/tmp/")
with open(os.path.join(TEST_DIR, "ws-container.install")) as f:
Expand Down
2 changes: 1 addition & 1 deletion test/ostree.install
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ cd /var/tmp/

# Note: cockpit-selinux would be desirable, but needs setroubleshoot-server which isn't installed
rpm-ostree install --cache-only cockpit-bridge-*.rpm \
cockpit-networkmanager-*.rpm cockpit-system-*.rpm cockpit-tests-*.rpm
cockpit-networkmanager-*.rpm cockpit-system-*.rpm

# run cockpit/ws once to generate certificate; avoids slow down on every start
podman container runlabel INSTALL cockpit/ws
4 changes: 4 additions & 0 deletions test/pbuilder-hooks/I01install-tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/sh
set -eux
cd "$(find $BUILDDIR -type d -maxdepth 1 -name 'cockpit-*')"
make DESTDIR=/var/tmp/tests install-tests
2 changes: 1 addition & 1 deletion test/verify/check-static-login
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ account required pam_succeed_if.so user ingroup %s""" % m.get_admin_group
'.*Sorry, passwords do not match.')
self.allow_restart_journal_messages()

@testlib.skipWsContainer("mock-pam-conv from cockpit-tests not installed")
@testlib.skipWsContainer("mock-pam-conv not installed")
def testConversation(self):
m = self.machine
b = self.browser
Expand Down
1 change: 1 addition & 0 deletions test/verify/check-superuser
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ class TestSuperuser(testlib.MachineCase):
b.become_superuser(passwordless=True)

@testlib.skipBeiboot("no local PAM config in beiboot mode")
@testlib.skipOstree("mock-pam-conv not installed")
def testTwoPasswds(self):
b = self.browser
m = self.machine
Expand Down
2 changes: 1 addition & 1 deletion test/ws-container.install
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ storaged
system
"

for rpm in ws bridge tests $PACKAGES; do
for rpm in ws bridge $PACKAGES; do
rpm2cpio /var/tmp/cockpit-$rpm-*.rpm | cpio -i --make-directories --directory=/var/tmp/install
done

Expand Down
14 changes: 1 addition & 13 deletions tools/arch/PKGBUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# Contributor: Anatol Pomozov <[email protected]>

pkgbase=cockpit
pkgname=(cockpit cockpit-packagekit cockpit-storaged cockpit-test)
pkgname=(cockpit cockpit-packagekit cockpit-storaged)
pkgver=0
pkgrel=1
pkgdesc='A systemd web based user interface for Linux servers'
Expand Down Expand Up @@ -75,18 +75,6 @@ package_cockpit() {
chmod 644 "$pkgdir"/etc/cockpit/disallowed-users
}

package_cockpit-test() {
pkgdesc='Cockpit test playground'
depends=(cockpit)

# Install test
cd cockpit-$pkgver
make DESTDIR="$pkgdir" install-tests
cd ..

_do_package_component playground
}

_do_package_component() {
_component="${1:-${pkgname#cockpit-}}"

Expand Down
19 changes: 1 addition & 18 deletions tools/cockpit.spec
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ export NO_QUNIT=1

%install
%make_install
make install-tests DESTDIR=%{buildroot}
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/pam.d
install -p -m 644 tools/cockpit.pam $RPM_BUILD_ROOT%{_sysconfdir}/pam.d/cockpit
rm -f %{buildroot}/%{_libdir}/cockpit/*.so
Expand Down Expand Up @@ -209,9 +208,6 @@ find %{buildroot}%{_datadir}/cockpit/apps -type f >> packagekit.list
echo '%dir %{_datadir}/cockpit/selinux' > selinux.list
find %{buildroot}%{_datadir}/cockpit/selinux -type f >> selinux.list

echo '%dir %{_datadir}/cockpit/playground' > tests.list
find %{buildroot}%{_datadir}/cockpit/playground -type f >> tests.list

echo '%dir %{_datadir}/cockpit/static' > static.list
echo '%dir %{_datadir}/cockpit/static/fonts' >> static.list
find %{buildroot}%{_datadir}/cockpit/static -type f >> static.list
Expand Down Expand Up @@ -313,6 +309,7 @@ Provides: cockpit-kdump = %{version}-%{release}
Provides: cockpit-networkmanager = %{version}-%{release}
Provides: cockpit-selinux = %{version}-%{release}
Provides: cockpit-sosreport = %{version}-%{release}
Obsoletes: cockpit-tests < 331
%endif
%if 0%{?fedora}
Recommends: (reportd if abrt)
Expand Down Expand Up @@ -539,20 +536,6 @@ The Cockpit component for managing storage. This package uses udisks.
%files -n cockpit-storaged -f storaged.list
%{_datadir}/metainfo/org.cockpit_project.cockpit_storaged.metainfo.xml

%package -n cockpit-tests
Summary: Tests for Cockpit
Requires: cockpit-bridge >= %{required_base}
Requires: cockpit-system >= %{required_base}
Requires: openssh-clients
Provides: cockpit-test-assets = %{version}-%{release}

%description -n cockpit-tests
This package contains tests and files used while testing Cockpit.
These files are not required for running Cockpit.

%files -n cockpit-tests -f tests.list
%{pamdir}/mock-pam-conv-mod.so

%package -n cockpit-packagekit
Summary: Cockpit user interface for packages
BuildArch: noarch
Expand Down
1 change: 0 additions & 1 deletion tools/debian/cockpit-tests.install

This file was deleted.

3 changes: 0 additions & 3 deletions tools/debian/cockpit-tests.lintian-overrides

This file was deleted.

13 changes: 2 additions & 11 deletions tools/debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -131,21 +131,12 @@ Provides: cockpit-shell,
cockpit-tuned,
cockpit-users
Suggests: lastlog2
Conflicts: cockpit-tests (<< 331)
Replaces: cockpit-tests (<< 331)
Description: Cockpit admin interface for a system
Cockpit admin interface package for configuring and
troubleshooting a system.

Package: cockpit-tests
Architecture: any
Multi-Arch: foreign
Depends: ${misc:Depends},
${shlibs:Depends},
cockpit-system (>= ${source:Version}),
openssh-client
Description: Tests for Cockpit
This package contains tests and files used while testing Cockpit.
These files are not required for running Cockpit.

Package: cockpit-ws
Architecture: any
Depends: ${misc:Depends},
Expand Down
2 changes: 0 additions & 2 deletions tools/debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ ifeq ($(PRE4AA),)
install -p -m 644 tools/apparmor.d/cockpit-desktop debian/cockpit-ws/etc/apparmor.d/
endif

make install-tests DESTDIR=debian/cockpit-tests

execute_after_dh_install-indep:
# avoid dh_missing failure
rm -r debian/tmp/usr/lib/python*
Expand Down

0 comments on commit 2a16cde

Please sign in to comment.