From f3dfcd0d46bc30167efe81cd0c81df9fea26a82d Mon Sep 17 00:00:00 2001 From: Barney Sowood Date: Fri, 29 Sep 2023 17:48:22 +0100 Subject: [PATCH 1/7] Fix install of logrotate config for debian pkg Fixes the install of logrotate config for the debian pkg by moving file to pkg/common/logrotate/salt-common. File is installed via salt-common.install file, which can't rename files, only copy them to a directory, so we need to rename the file and put it in a subdir of pkg/common. Adds /etc/logrotate.d/salt-common to salt.common.conffiles to ensure that dpkg will not overwrite configs modified by users. Also updates RPM spec file for new location of logrotate config. Config needs to be /etc/logrotate.d/salt-common as that is what is used by 3005.x packages. --- pkg/common/{salt-common.logrotate => logrotate/salt-common} | 0 pkg/debian/salt-common.conffiles | 1 + pkg/debian/salt-common.dirs | 1 + pkg/debian/salt-common.install | 2 +- pkg/debian/salt-common.preinst | 4 ++++ pkg/rpm/salt.spec | 2 +- 6 files changed, 8 insertions(+), 2 deletions(-) rename pkg/common/{salt-common.logrotate => logrotate/salt-common} (100%) create mode 100644 pkg/debian/salt-common.conffiles diff --git a/pkg/common/salt-common.logrotate b/pkg/common/logrotate/salt-common similarity index 100% rename from pkg/common/salt-common.logrotate rename to pkg/common/logrotate/salt-common diff --git a/pkg/debian/salt-common.conffiles b/pkg/debian/salt-common.conffiles new file mode 100644 index 000000000000..595731d1d020 --- /dev/null +++ b/pkg/debian/salt-common.conffiles @@ -0,0 +1 @@ +/etc/logrotate.d/salt-common diff --git a/pkg/debian/salt-common.dirs b/pkg/debian/salt-common.dirs index 5c6dd29e224e..381ec1f48ce0 100644 --- a/pkg/debian/salt-common.dirs +++ b/pkg/debian/salt-common.dirs @@ -4,3 +4,4 @@ /usr/share/fish/vendor_completions.d /opt/saltstack/salt /etc/salt +/etc/logrotate.d diff --git a/pkg/debian/salt-common.install b/pkg/debian/salt-common.install index 09bb9e28099a..4f8dac552ece 100644 --- a/pkg/debian/salt-common.install +++ b/pkg/debian/salt-common.install @@ -1,4 +1,4 @@ -pkg/common/salt-common.logrotate /etc/logrotate.d/salt +pkg/common/logrotate/salt-common /etc/logrotate.d pkg/common/fish-completions/salt-cp.fish /usr/share/fish/vendor_completions.d pkg/common/fish-completions/salt-call.fish /usr/share/fish/vendor_completions.d pkg/common/fish-completions/salt-syndic.fish /usr/share/fish/vendor_completions.d diff --git a/pkg/debian/salt-common.preinst b/pkg/debian/salt-common.preinst index 0e4ce9c59f7f..0e45d2399f68 100644 --- a/pkg/debian/salt-common.preinst +++ b/pkg/debian/salt-common.preinst @@ -31,5 +31,9 @@ case "$1" in -s $SALT_SHELL \ -g $SALT_GROUP \ $SALT_USER + + # Remove incorrectly installed logrotate config - issue 65231 + test -d /etc/logrotate.d/salt && rm -r /etc/logrotate.d/salt || /bin/true + ;; esac diff --git a/pkg/rpm/salt.spec b/pkg/rpm/salt.spec index 6cc4e85c23c3..4659c9fd3433 100644 --- a/pkg/rpm/salt.spec +++ b/pkg/rpm/salt.spec @@ -266,7 +266,7 @@ install -p -m 0644 %{_salt_src}/pkg/common/salt-proxy@.service %{buildroot}%{_un # Logrotate #install -p %{SOURCE10} . mkdir -p %{buildroot}%{_sysconfdir}/logrotate.d/ -install -p -m 0644 %{_salt_src}/pkg/common/salt-common.logrotate %{buildroot}%{_sysconfdir}/logrotate.d/salt +install -p -m 0644 %{_salt_src}/pkg/common/logrotate/salt-common %{buildroot}%{_sysconfdir}/logrotate.d/salt # Bash completion mkdir -p %{buildroot}%{_sysconfdir}/bash_completion.d/ From f5827a0be27ba04c8554ef90622aac1fad5c0930 Mon Sep 17 00:00:00 2001 From: Barney Sowood Date: Thu, 12 Oct 2023 19:04:51 +0100 Subject: [PATCH 2/7] Add initial tests for logrotate configs Adds initial tests for logrotate configs to ensure they are installed in the correct location. --- .../integration/test_logrotate_config.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 pkg/tests/integration/test_logrotate_config.py diff --git a/pkg/tests/integration/test_logrotate_config.py b/pkg/tests/integration/test_logrotate_config.py new file mode 100644 index 000000000000..9179ae631d69 --- /dev/null +++ b/pkg/tests/integration/test_logrotate_config.py @@ -0,0 +1,34 @@ +""" +Tests for logrotate config +""" + +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.skip_on_windows, pytest.mark.skip_on_darwin] + + +@pytest.fixture +def logrotate_config_file(salt_call_cli): + """ + Fixture for logrotate config file path + """ + logrotate_dir = Path("/etc/logrotate.d") + + os_family = salt_call_cli.run("grains.get", "os_family") + assert os_family.returncode == 0 + + if os_family.data == "RedHat": + return logrotate_dir / "salt" + elif os_family.data == "Debian": + return logrotate_dir / "salt-common" + + +def test_logrotate_config(logrotate_config_file): + """ + Test that logrotate config has been installed in correctly + """ + assert logrotate_config_file.is_file() + assert logrotate_config_file.owner() == "root" + assert logrotate_config_file.group() == "root" From 3b3953242d951a7ff180ab8e81104bd412081f25 Mon Sep 17 00:00:00 2001 From: Barney Sowood Date: Thu, 12 Oct 2023 19:52:43 +0100 Subject: [PATCH 3/7] Add test for issue 65231 --- pkg/tests/integration/test_logrotate_config.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/tests/integration/test_logrotate_config.py b/pkg/tests/integration/test_logrotate_config.py index 9179ae631d69..5581105f11f0 100644 --- a/pkg/tests/integration/test_logrotate_config.py +++ b/pkg/tests/integration/test_logrotate_config.py @@ -32,3 +32,12 @@ def test_logrotate_config(logrotate_config_file): assert logrotate_config_file.is_file() assert logrotate_config_file.owner() == "root" assert logrotate_config_file.group() == "root" + + +def test_issue_65231_etc_logrotate_salt_dir_removed(): + """ + Test that /etc/logrotate.d/salt is not a directory + """ + path = Path("/etc/logrotate.d/salt") + if path.exists(): + assert path.is_dir() is False From 3fe1fbab2c9628c6de73edc019a376414d46c39f Mon Sep 17 00:00:00 2001 From: Barney Sowood Date: Thu, 12 Oct 2023 20:21:19 +0100 Subject: [PATCH 4/7] Add changelog --- changelog/65231.fixed.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog/65231.fixed.md diff --git a/changelog/65231.fixed.md b/changelog/65231.fixed.md new file mode 100644 index 000000000000..50d225e7452f --- /dev/null +++ b/changelog/65231.fixed.md @@ -0,0 +1,2 @@ +Install logrotate config as /etc/logrotate.d/salt-common for Debian packages +Remove broken /etc/logrotate.d/salt directory from 3006.3 if it exists. From ba84b72845a5138b6a6ef9edb3aa65b169fa89d5 Mon Sep 17 00:00:00 2001 From: Barney Sowood Date: Thu, 19 Oct 2023 21:50:41 +0100 Subject: [PATCH 5/7] Disable test for 65231 on downgrade to 3006.3 Disables the running of the test for 65231 when downgrading to 3006.3 as the issue that is fixed is in 3006.3 so test will fail when 3006.3 is installed. --- pkg/tests/integration/test_logrotate_config.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/tests/integration/test_logrotate_config.py b/pkg/tests/integration/test_logrotate_config.py index 5581105f11f0..0ce9552546c9 100644 --- a/pkg/tests/integration/test_logrotate_config.py +++ b/pkg/tests/integration/test_logrotate_config.py @@ -4,6 +4,7 @@ from pathlib import Path +import packaging.version import pytest pytestmark = [pytest.mark.skip_on_windows, pytest.mark.skip_on_darwin] @@ -34,10 +35,15 @@ def test_logrotate_config(logrotate_config_file): assert logrotate_config_file.group() == "root" -def test_issue_65231_etc_logrotate_salt_dir_removed(): +def test_issue_65231_etc_logrotate_salt_dir_removed(install_salt): """ Test that /etc/logrotate.d/salt is not a directory """ + if install_salt.prev_version and packaging.version.parse( + install_salt.prev_version + ) == packaging.version.parse("3006.3"): + pytest.skip("Testing a downgrade to 3006.3, do not run") + path = Path("/etc/logrotate.d/salt") if path.exists(): assert path.is_dir() is False From d6dccc1cda56553685cbae0c0b5e2c4cb02380c5 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Thu, 2 Nov 2023 12:54:02 +0000 Subject: [PATCH 6/7] Address my own review comments Signed-off-by: Pedro Algarvio --- .../integration/test_logrotate_config.py | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/pkg/tests/integration/test_logrotate_config.py b/pkg/tests/integration/test_logrotate_config.py index 0ce9552546c9..2a1cb7cd334f 100644 --- a/pkg/tests/integration/test_logrotate_config.py +++ b/pkg/tests/integration/test_logrotate_config.py @@ -2,28 +2,25 @@ Tests for logrotate config """ -from pathlib import Path +import pathlib import packaging.version import pytest -pytestmark = [pytest.mark.skip_on_windows, pytest.mark.skip_on_darwin] +pytestmark = [ + pytest.mark.skip_unless_on_linux, +] @pytest.fixture -def logrotate_config_file(salt_call_cli): +def logrotate_config_file(grains): """ Fixture for logrotate config file path """ - logrotate_dir = Path("/etc/logrotate.d") - - os_family = salt_call_cli.run("grains.get", "os_family") - assert os_family.returncode == 0 - - if os_family.data == "RedHat": - return logrotate_dir / "salt" - elif os_family.data == "Debian": - return logrotate_dir / "salt-common" + if grains["os_family"] == "RedHat": + return pathlib.Path("/etc/logrotate.d", "salt") + elif grains["os_family"] == "Debian": + return pathlib.Path("/etc/logrotate.d", "salt-common") def test_logrotate_config(logrotate_config_file): @@ -44,6 +41,6 @@ def test_issue_65231_etc_logrotate_salt_dir_removed(install_salt): ) == packaging.version.parse("3006.3"): pytest.skip("Testing a downgrade to 3006.3, do not run") - path = Path("/etc/logrotate.d/salt") + path = pathlib.Path("/etc/logrotate.d/salt") if path.exists(): assert path.is_dir() is False From 6bff5cbe752f57a89e0268d316220d9636e4e82d Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Thu, 2 Nov 2023 16:22:14 +0000 Subject: [PATCH 7/7] The problem is still present in 3006.4, can't test downgrade Signed-off-by: Pedro Algarvio --- pkg/tests/integration/test_logrotate_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/tests/integration/test_logrotate_config.py b/pkg/tests/integration/test_logrotate_config.py index 2a1cb7cd334f..fea0123b6eb6 100644 --- a/pkg/tests/integration/test_logrotate_config.py +++ b/pkg/tests/integration/test_logrotate_config.py @@ -38,8 +38,8 @@ def test_issue_65231_etc_logrotate_salt_dir_removed(install_salt): """ if install_salt.prev_version and packaging.version.parse( install_salt.prev_version - ) == packaging.version.parse("3006.3"): - pytest.skip("Testing a downgrade to 3006.3, do not run") + ) <= packaging.version.parse("3006.4"): + pytest.skip("Testing a downgrade to 3006.4, do not run") path = pathlib.Path("/etc/logrotate.d/salt") if path.exists():