redhat-developer / s2i-dotnetcore

.NET Core OpenShift images
Apache License 2.0
112 stars 192 forks source link

Flag changes in packages shipped in containers #442

Closed omajid closed 1 year ago

omajid commented 1 year ago

It's possible for customers to take on a dependency on any package shipped in a container. We shouldn't break that. In other words, any changes in packages installed in our containers should require a manual review.

That review should be useful even between major versions and should get a manual review as well.

omajid commented 1 year ago

Here's a list of packages between our 6.0 and 7.0 build containers. I don't see anything alarming. Is there anything that looks like it should have been included in 7.0 as well?

--- 6.0/build/test/packages.rhel8.list  2023-01-25 15:13:09.580039578 -0500
+++ 7.0/build/test/packages.rhel8.list  2023-01-25 11:15:22.588820390 -0500
@@ -1,6 +1,5 @@
-acl
-aspnetcore-runtime-6.0
-aspnetcore-targeting-pack-6.0
+aspnetcore-runtime-7.0
+aspnetcore-targeting-pack-7.0
 audit-libs
 basesystem
 bash
@@ -10,42 +9,21 @@
 chkconfig
 cmake-filesystem
 coreutils-single
-cracklib
-cracklib-dicts
 crypto-policies
-crypto-policies-scripts
-cryptsetup-libs
 curl
 cyrus-sasl-lib
-dbus
-dbus-common
-dbus-daemon
-dbus-glib
-dbus-libs
-dbus-tools
-device-mapper
-device-mapper-libs
-dmidecode
-dnf
-dnf-data
-dnf-plugin-subscription-manager
-dotnet-apphost-pack-6.0
+dotnet-apphost-pack-7.0
 dotnet-host
-dotnet-hostfxr-6.0
-dotnet-runtime-6.0
-dotnet-sdk-6.0
-dotnet-targeting-pack-6.0
-dotnet-templates-6.0
-elfutils-default-yama-scope
+dotnet-hostfxr-7.0
+dotnet-runtime-7.0
+dotnet-sdk-7.0
+dotnet-targeting-pack-7.0
+dotnet-templates-7.0
 elfutils-libelf
-elfutils-libs
-expat
 file-libs
 filesystem
 findutils
 gawk
-gdb-gdbserver
-gdbm
 gdbm-libs
 glib2
 glibc
@@ -60,12 +38,10 @@
 grep
 groff-base
 gzip
-ima-evm-utils
 info
 json-c
 json-glib
 keyutils-libs
-kmod-libs
 krb5-libs
 langpacks-en
 libacl
@@ -76,12 +52,10 @@
 libcap
 libcap-ng
 libcom_err
-libcomps
 libcurl
 libdb
 libdb-utils
 libdnf
-libfdisk
 libffi
 libgcc
 libgcrypt
@@ -92,15 +66,11 @@
 libmodulemd
 libmount
 libnghttp2
-libnl3
-libnsl2
+libpeas
 libpkgconf
 libpsl
-libpwquality
 librepo
-libreport-filesystem
 librhsm
-libseccomp
 libselinux
 libsemanage
 libsepol
@@ -111,11 +81,8 @@
 libssh-config
 libstdc++
 libtasn1
-libtirpc
 libunistring
 libusbx
-libuser
-libutempter
 libuuid
 libverto
 libxcrypt
@@ -125,6 +92,7 @@
 lttng-ust
 lua-libs
 lz4-libs
+microdnf
 mpfr
 ncurses
 ncurses-base
@@ -140,8 +108,6 @@
 openssl-libs
 p11-kit
 p11-kit-trust
-pam
-passwd
 pcre
 pcre2
 perl-Carp
@@ -179,64 +145,21 @@
 pkgconf
 pkgconf-m4
 pkgconf-pkg-config
-platform-python
-platform-python-setuptools
 popt
 procps-ng
 publicsuffix-list-dafsa
-python3-chardet
-python3-cloud-what
-python3-dateutil
-python3-dbus
-python3-decorator
-python3-dmidecode
-python3-dnf
-python3-dnf-plugins-core
-python3-ethtool
-python3-gobject-base
-python3-gpg
-python3-hawkey
-python3-idna
-python3-iniparse
-python3-inotify
-python3-libcomps
-python3-libdnf
-python3-librepo
-python3-libs
-python3-libxml2
-python3-pip-wheel
-python3-pysocks
-python3-requests
-python3-rpm
-python3-setuptools-wheel
-python3-six
-python3-subscription-manager-rhsm
-python3-syspurpose
-python3-urllib3
 readline
 redhat-release
 rootfiles
 rpm
-rpm-build-libs
 rpm-libs
 sed
 setup
 shadow-utils
 sqlite-libs
-subscription-manager
-subscription-manager-rhsm-certificates
-systemd
 systemd-libs
-systemd-pam
 tar
-tpm2-tss
 tzdata
-usermode
 userspace-rcu
-util-linux
-vim-minimal
-virt-what
-which
 xz-libs
-yum
 zlib
tmds commented 1 year ago

We know we should keep the packages we install as otherwise that would be a breaking change. We defer such breaking changes to a major release, like changing to ubi-minimal.

This test will mainly check if the packages we install don't change their dependencies without us knowing. That would be a breaking change caused by their maintainer. Or the change may not be breaking, like when the maintainer splits up his packages, and then the test will generate a false positive.

omajid commented 1 year ago

I see that we also removed libns2 (provides support NIS) and libnl3 (netlink) between 6.0 and 7.0. Does .NET make use of either of those?

omajid commented 1 year ago

This test will mainly check if the packages we install don't change their dependencies without us knowing.

Yes. That's the primary motivation. We know .NET depends on several libraries. And we know users can run additional commands in their containers, even outside of .NET (eg, curl for HEALTHCHECK). If there's a change, between any major or minor versions, we should catch it and make an explicit call about changing it, instead of possibly ignoring the issue until one of our users hits in production.

tmds commented 1 year ago

I see that we also removed libns2 (provides support NIS) and libnl3 (netlink) between 6.0 and 7.0. Does .NET make use of either of those?

It doesn't.

omajid commented 1 year ago

Is this okay to merge now?

tmds commented 1 year ago

Thanks!

omajid commented 1 year ago

Thanks for the review and valuable feedback!

tmds commented 1 year ago

@omajid fyi, this test doesn't capture the issue fixed by https://github.com/redhat-developer/s2i-dotnetcore/pull/440.

tzdata is present, even when the timezone data is not.

$ podman run ubi8-minimal microdnf install tzdata
(microdnf:1): librhsm-WARNING **: 12:56:05.937: Found 0 entitlement certificates
(microdnf:1): librhsm-WARNING **: 12:56:05.938: Found 0 entitlement certificates
Downloading metadata...
Downloading metadata...
Downloading metadata...
Nothing to do.

The command used in the PR repopulates the data.

$ podman run ubi8-minimal microdnf reinstall tzdata
(microdnf:1): librhsm-WARNING **: 12:58:09.315: Found 0 entitlement certificates
(microdnf:1): librhsm-WARNING **: 12:58:09.315: Found 0 entitlement certificates
Downloading metadata...
Downloading metadata...
Downloading metadata...
Package                                Repository            Size
Reinstalling:                                                    
 tzdata-2022g-1.el8.noarch             ubi-8-baseos-rpms 481.3 kB
   replacing tzdata-2022g-1.el8.noarch                           
Transaction Summary:
 Installing:        0 packages
 Reinstalling:      1 packages
 Upgrading:         0 packages
 Obsoleting:        0 packages
 Removing:          0 packages
 Downgrading:       0 packages
Downloading packages...
Running transaction test...
Reinstalling: tzdata;2022g-1.el8;noarch;ubi-8-baseos-rpms
Complete.
omajid commented 1 year ago

Oh, great catch!