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

state.file.retention_schedule does not ignore relative directory when using getmtime() #48637

Closed
slaws opened this Issue Jul 17, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@slaws
Contributor

slaws commented Jul 17, 2018

Description of Issue/Question

When using salt.state.file.retention_schedule without using strptime, relative directory . and .. are processed like regular file.
As such, they can be marked for deletion or counted as file to be retained.

Steps to Reproduce Issue

With this state:

# cat /srv/salt/test.sls 
test_retention:
  file.retention_schedule:
    - name: /tmp/test
    - retain:
        most_recent: 2

Having these files

# ls -al /tmp/test/
total 0
drwxr-xr-x 2 root root 120 juil. 18 00:28 .
drwxrwxrwt 8 root root 160 juil. 18 00:28 ..
-rw-r--r-- 1 root root   0 avril 17 01:00 foo-1.0.0.jar
-rw-r--r-- 1 root root   0 mai   17 01:00 foo-1.0.1.jar
-rw-r--r-- 1 root root   0 juin  17 01:00 foo-1.1.0.jar
-rw-r--r-- 1 root root   0 juil. 17 01:00 foo-1.1.1.jar

It results to

# salt host1 state.apply test 
host1:
----------
          ID: test_retention
    Function: file.retention_schedule
        Name: /tmp/test
      Result: True
     Comment: 4 backups were removed from /tmp/test.
     Started: 00:31:55.600693
    Duration: 16.172 ms
     Changes:   
              ----------
              deleted:
                  - foo-1.1.1.jar
                  - foo-1.1.0.jar
                  - foo-1.0.1.jar
                  - foo-1.0.0.jar
              ignored:
              retained:
                  - ..
                  - .

Summary for host1
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  16.172 ms

However using strptime_format this problem does not happen because . and .. do not match the provided format.

A straightforward fix would be to add a test in get_file_time_from_mtime (https://github.com/saltstack/salt/blob/v2018.3.2/salt/states/file.py#L3675)

def get_file_time_from_mtime(f):
    if f == '.' or f == '..':
       return (None, None)
   lstat = __salt__['file.lstat'](os.path.join(name, f))
   if lstat:
       mtime = lstat['st_mtime']
       return (datetime.fromtimestamp(mtime, timezone), mtime)
   else:   # maybe it was deleted since we did the readdir?
       return (None, None)

Probably not the most elegant though..

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

# salt --versions-report
Salt Version:
           Salt: 2018.3.2
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.4.2
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.9.4
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.9 (default, Jun 29 2016, 13:08:31)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.4.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5
 
System Versions:
           dist: debian 8.11 
         locale: UTF-8
        machine: x86_64
        release: 4.9.0-0.bpo.6-amd64
         system: Linux
        version: debian 8.11 
@Ch3LL

This comment has been minimized.

Contributor

Ch3LL commented Jul 18, 2018

seems you have a grasp of the fix. want to push as a PR?

@Ch3LL Ch3LL added this to the Approved milestone Jul 18, 2018

slaws added a commit to slaws/salt that referenced this issue Jul 18, 2018

@slaws

This comment has been minimized.

Contributor

slaws commented Jul 18, 2018

I've made the PR to 2017.7. I hope it's OK.

Let me know if I need to do it to another branch.

@Ch3LL

This comment has been minimized.

Contributor

Ch3LL commented Jul 19, 2018

2017.7 is perfect. It will get merged forward to the other branches. Thanks :)

@rallytime rallytime closed this in 5539eff Aug 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment