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

'force' option in copy state deletes target file #25250

Closed
wipfs opened this issue Jul 8, 2015 · 4 comments
Closed

'force' option in copy state deletes target file #25250

wipfs opened this issue Jul 8, 2015 · 4 comments
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE fixed-pls-verify fix is linked, bug author to confirm fix Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases P3 Priority 3 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-high 2nd top severity, seen by most users, causes major problems State-Module
Milestone

Comments

@wipfs
Copy link

wipfs commented Jul 8, 2015

[root@minion]# cat /var/cache/salt/minion/files/base/example/init.sls
/tmp/test:
  file.copy:
    - source: /tmp/test_src
    - force: True

This will behave as expected if the target does not exist or if it is identical to the source
If however the target exists but is different from the source, then this state simply deletes the target, though it claims not to overwrite it:

[root@minion]# ls -l /tmp/test /tmp/test_src
-rw-r--r--. 1 root root 192 Jul  8 12:20 /tmp/test
-rw-r--r--. 1 root root 198 Jul  8 12:19 /tmp/test_src
[root@minion]# salt-call state.sls example
[INFO    ] Loading fresh modules for state activity
[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache 'salt://example/init.sls'
[INFO    ] Running state [/tmp/test] at time 12:20:58.419543
[INFO    ] Executing state file.copy for /tmp/test
[INFO    ] The target file "/tmp/test" exists and will not be overwritten
[INFO    ] Completed state [/tmp/test] at time 12:20:58.421524
local:

          ID: /tmp/test
    Function: file.copy
      Result: True
     Comment: The target file "/tmp/test" exists and will not be overwritten
     Started: 12:20:58.419543
    Duration: 1.981 ms
     Changes:

Summary

Succeeded: 1
Failed:    0

Total states run:     1
[root@minion]# ls -l /tmp/test /tmp/test_src
ls: cannot access /tmp/test: No such file or directory
-rw-r--r--. 1 root root 198 Jul  8 12:19 /tmp/test_src
[root@minion-1]# salt --version
salt 2015.5.2 (Lithium)
@jfindlay jfindlay added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P3 Priority 3 State-Module Platform Relates to OS, containers, platform-based utilities like FS, system based apps labels Jul 9, 2015
@jfindlay
Copy link
Contributor

jfindlay commented Jul 9, 2015

@wipfs, thanks for the report.

@jfindlay jfindlay added this to the Approved milestone Jul 9, 2015
@jahamn jahamn added Confirmed Salt engineer has confirmed bug/feature - often including a MCVE fixed-pls-verify fix is linked, bug author to confirm fix labels Jul 16, 2015
@jahamn jahamn self-assigned this Jul 16, 2015
@thunderk
Copy link
Contributor

This is a rather critical bug as it may cause missing files, and so broken services. Maybe some unit tests would be good to avoid future regressions on this ?

@jfindlay jfindlay added severity-high 2nd top severity, seen by most users, causes major problems Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases and removed severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around labels Jul 23, 2015
@jfindlay
Copy link
Contributor

@thunderk, good ideas, thanks.

jahamn added a commit to jahamn/salt that referenced this issue Jul 24, 2015
@cachedout
Copy link
Contributor

The fix and the test are both merged now. Thanks to all who helped out here! I'll go ahead and close this.

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 fixed-pls-verify fix is linked, bug author to confirm fix Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases P3 Priority 3 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-high 2nd top severity, seen by most users, causes major problems State-Module
Projects
None yet
Development

No branches or pull requests

5 participants