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

[VIRTS-1970] Move files aside on --fresh instead of deleting them #2101

Merged
merged 11 commits into from
Apr 6, 2021

Conversation

bworrell
Copy link
Contributor

@bworrell bworrell commented Apr 1, 2021

Description

Depends on: mitre/fieldmanual#73

When running server.py --fresh, we currently delete the files under the data directory to reset the server state. This changes the behavior to back up the files before deleting them (creating a timestamped, gzipped tarball containing the files that are about to be deleted)

Note

Every time you run the server with --fresh you will generate a new backup tarball. The backup filenames include a timestamp so you can always fine the most recent one.

Examples of directory structure pre/post --fresh (I didn't run tree -a so .gitkeep files don't appear, but they are there!):

Before:

data
├── abilities
│   ├── 7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
│   └── testdir
│       └── 7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
├── adversaries
│   └── 7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
├── backup
├── object_store
├── objectives
│   └── 7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
├── payloads
│   └── 7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
├── results
│   └── 7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
└── sources
    └── 7e422753-ad7a-4401-bc8b-b12a28e69c25.yml

After running w/ --fresh

data
├── abilities
├── adversaries
├── backup
│   └── backup-20210406130339.tar.gz
├── object_store
├── objectives
├── payloads
├── results
└── sources

Contents of the tarball

> tar -tzvf data/backup/backup-20210406125823.tar.gz
-rw-r--r--  0 bworrell staff     603 Apr  6 08:58 data/abilities/7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
drwxr-xr-x  0 bworrell staff       0 Apr  6 08:58 data/abilities/testdir/
-rw-r--r--  0 bworrell staff     603 Apr  6 08:58 data/abilities/testdir/7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
-rw-r--r--  0 bworrell staff     603 Apr  6 08:58 data/adversaries/7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
-rw-r--r--  0 bworrell staff     603 Apr  6 08:58 data/objectives/7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
-rw-r--r--  0 bworrell staff     603 Apr  6 08:58 data/payloads/7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
-rw-r--r--  0 bworrell staff     603 Apr  6 08:58 data/results/7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
-rw-r--r--  0 bworrell staff     603 Apr  6 08:58 data/sources/7e422753-ad7a-4401-bc8b-b12a28e69c25.yml
-rw-r--r--  0 bworrell staff 1137539 Apr  6 08:58 data/object_store

Why?

Sometimes users run the server with --fresh on accident and end up accidentally deleting all their progress/work. This allows users to restore their files in this case. Note that the restore procedure is not actually documented anywhere at this time. We may choose to provide some sort of --restore-previous-state flag that could load the files out of the backup directory.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Still working on this. But I've run the server with --fresh and observed that the tarball is created as expected. Note that this does not remove the .gitkeep file under each data/* subdirectory.

I also ran the following command to restore a backup state:

> cd /path/to/caldera/repo
> tar -zxvf data/backup/backup-20210406125823.tar.gz

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@codecov
Copy link

codecov bot commented Apr 2, 2021

Codecov Report

Merging #2101 (2bd2947) into master (58a1280) will decrease coverage by 0.05%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2101      +/-   ##
==========================================
- Coverage   65.68%   65.63%   -0.06%     
==========================================
  Files          65       65              
  Lines        4890     4906      +16     
==========================================
+ Hits         3212     3220       +8     
- Misses       1678     1686       +8     
Impacted Files Coverage Δ
server.py 0.00% <0.00%> (ø)
app/service/data_svc.py 29.28% <36.00%> (+1.36%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58a1280...2bd2947. Read the comment docs.

@bworrell bworrell marked this pull request as ready for review April 2, 2021 14:18
@bworrell bworrell requested a review from nopfor April 2, 2021 14:21
nopfor
nopfor previously approved these changes Apr 2, 2021
Copy link
Contributor

@nopfor nopfor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Consider using single quotes rather than double.

nopfor
nopfor previously approved these changes Apr 2, 2021
server.py Outdated Show resolved Hide resolved
app/service/data_svc.py Outdated Show resolved Hide resolved
wbooth
wbooth previously approved these changes Apr 5, 2021
Copy link
Contributor

@wbooth wbooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not blockers but I think the updated message and zipping the files would be helpful

@bworrell bworrell dismissed stale reviews from wbooth and nopfor via 2bd2947 April 6, 2021 13:13
@bworrell bworrell requested a review from wbooth April 6, 2021 13:19
@bworrell
Copy link
Contributor Author

bworrell commented Apr 6, 2021

I'll need to make another fieldmanual PR to update the documentation explaining the --fresh behavior and how to restore a backup since the approach is different now.

@wbooth wbooth assigned nopfor and unassigned wbooth Apr 6, 2021
@bworrell bworrell requested a review from nopfor April 6, 2021 14:04
@wbooth wbooth assigned bworrell and unassigned nopfor Apr 6, 2021
@bworrell bworrell merged commit d34fd76 into master Apr 6, 2021
@bworrell bworrell deleted the bworrell/virts-1970/fresh-snapshots branch April 6, 2021 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants