Skip to content
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.

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

[3006.x][BUG] Log file rotation sets ownership to salt, overwriting ownership of log file #65288

Closed
1 of 9 tasks
dmurphy18 opened this issue Sep 25, 2023 · 6 comments
Closed
1 of 9 tasks
Assignees
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Packaging Related to packaging of Salt, not Salt's support for package management.

Comments

@dmurphy18
Copy link
Contributor

Description
If a user a has set user to something other than salt, the log rotation sets the new log ownership to salt, causing issue with salt-master to halt since the key log file has its ownership changed. From salt-common.logrotate:

 18 /var/log/salt/key {
 19     weekly
 20     missingok
 21     rotate 7
 22     compress
 23     notifempty
 24     create 0640 salt salt
 25 }

Setup
Setup master with configuration with something other than salt

Please be as specific as possible and give set-up details.

  • on-prem machine
  • VM (Virtualbox, KVM, etc. please specify)
  • VM running on a cloud service, please be explicit and add details
  • container (Kubernetes, Docker, containerd, etc. please specify)
  • or a combination, please be explicit
  • jails if it is FreeBSD
  • classic packaging
  • onedir packaging
  • used bootstrap to install

Steps to Reproduce the behavior
Setup master with configuration with something other than salt, for example mymaster, then force log rotation with '-f' option

Expected behavior
The ownership of the log files to remain owned by mymaster

Screenshots
If applicable, add screenshots to help explain your problem.

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
[root@Unknown david]# salt-run --versions-report
Salt Version:
          Salt: 3006.3
 
Python Version:
        Python: 3.10.13 (main, Sep  6 2023, 02:11:27) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: 18.6.1
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.2
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 23.2.0
        relenv: 0.13.10
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: centos 7.9.2009 Core
        locale: utf-8
       machine: x86_64
       release: 3.10.0-1160.99.1.el7.x86_64
        system: Linux
       version: CentOS Linux 7.9.2009 Core
 
[root@Unknown david]#

Additional context
Add any other context about the problem here.

@dmurphy18 dmurphy18 added Bug broken, incorrect, or confusing behavior needs-triage Confirmed Salt engineer has confirmed bug/feature - often including a MCVE and removed needs-triage labels Sep 25, 2023
@dmurphy18 dmurphy18 added this to the Sulfur v3006.4 milestone Sep 25, 2023
@dmurphy18
Copy link
Contributor Author

Problem is commit 5d3ffc9 from @barneysowood forcing salt:salt, assuming salt is correct, over-writing the user specified in the salt-master's configuration file.

@dmurphy18
Copy link
Contributor Author

using forced logrotation

logrotate -f -v /etc/logrotate.d/salt
.
.
.
[root@Unknown david]# l /var/log/salt/*
-rw-r----- 1 david david 7.7M Sep 17 03:42 /var/log/salt/minion-20230917.gz
-rw-r----- 1 david david 3.2M Sep 21 10:01 /var/log/salt/minion-20230925.gz
-rw-r----- 1 david david    0 Sep 25 09:31 /var/log/salt/key
-rw-r----- 1 david david    0 Sep 25 10:37 /var/log/salt/minion
-rw-r----- 1 david david  527 Sep 25 11:25 /var/log/salt/master.1.gz
-rw-r----- 1 salt  salt     0 Sep 25 11:26 /var/log/salt/master
[root@Unknown david]# 

the /varlog/salt/master has been rotated but it's ownership changed

@dmurphy18
Copy link
Contributor Author

@OrangeDog
Copy link
Contributor

OrangeDog commented Sep 26, 2023

Related: #65231

@OrangeDog OrangeDog marked this as a duplicate of #65231 Sep 26, 2023
@OrangeDog OrangeDog added Packaging Related to packaging of Salt, not Salt's support for package management. Duplicate Duplicate of another issue or PR - will be closed and removed Duplicate Duplicate of another issue or PR - will be closed labels Sep 26, 2023
@OrangeDog OrangeDog marked this as not a duplicate of #65231 Sep 26, 2023
@barneysowood
Copy link
Contributor

See #65231 (comment)

@dmurphy18
Copy link
Contributor Author

Closing since PR #65290 is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Packaging Related to packaging of Salt, not Salt's support for package management.
Projects
None yet
Development

No branches or pull requests

4 participants