Skip to content
This repository has been archived by the owner on Jun 5, 2023. It is now read-only.

Purge Database Tables #438

Closed
blueandgold opened this issue Jul 6, 2017 · 7 comments
Closed

Purge Database Tables #438

blueandgold opened this issue Jul 6, 2017 · 7 comments

Comments

@blueandgold
Copy link
Contributor

Currently, the inventory database tables are not purged. So, they stay in place, and grows over time. Which makes it harder to navigate and exceeds the database's capacity. So, we need to provide a way for users to purge these tables. Possibly by a config value that users can specify.

@blueandgold blueandgold added this to the M2 milestone Jul 6, 2017
@blueandgold blueandgold self-assigned this Jul 6, 2017
@blueandgold blueandgold removed their assignment Jul 6, 2017
@gbrindisi
Copy link
Contributor

This is a problem we would like to solve as well :) I'm commenting to remember to get back with ideas later

@blueandgold blueandgold added the priority: p2 Important feature defect, moderate live issue label Aug 2, 2017
@forseti-security-waffle-bot forseti-security-waffle-bot removed this from the M2 milestone Dec 19, 2017
@forseti-security-waffle-bot

Issue review: This is something the team can't get to in the near-term.

@mirons-google mirons-google removed priority: p3 Desirable enhancement or minor bug fix priority: p2 Important feature defect, moderate live issue labels Dec 20, 2017
This was referenced Feb 15, 2018
@victorvess
Copy link

@blueandgold I am interested in picking this up - my thought is to provide a retention_days property in the inventory section of forseti config, that will cause inventory scan to clean up any snapshot tables older than that time period (basically delete tables like '%_cycle_timestamp')

Is there any other dimension than age in days that should be considered?

Also - do we think the corresponding rows within snapshot_cycles should be cleaned up too? I am not sure if there is any kind of audit benefit of leaving those around, I was leaning toward leaving them just in case because it shouldn't take up much space (<9000 rows per year).

This would be my first contribution to Forseti, let me know if you need any additional artifacts

@blueandgold
Copy link
Contributor Author

blueandgold commented Feb 21, 2018

Hi Victor, thank you for your input here! Yes, I think that your approach to use a retention_days is more flexible and comprehensive, and will also cover angelo's use case as well. Please make sure to default it to a state where no data will be purged (perhaps with -1).

Keeping the snapshot data makes sense to me, and preferable as it might be helpful for people to have a sense of when forseti has run previously. As you pointed out, this is very inexpensive to do as well.

Looking forward to your PR, and please ask any questions!

@blueandgold
Copy link
Contributor Author

This also looks good to @angelomellos in PR #1103 .

@victorvess
Copy link

victorvess commented Feb 21, 2018

Already submitted #1129 - per your comment around defaulting, I do this if the directive is missing, but I put a non-zero default in the forseti_conf.yaml.in - would you prefer if I set that to 0 (keep always) instead of 30 days?

blueandgold pushed a commit that referenced this issue Feb 22, 2018
* Add configurable cleanup of inventory tables

* refactor table cleanup into a DAO

* Add configurable cleanup of inventory tables (#438)

* Feedback from product managers:
* change retention_days default from 0 to 1 for keep forever
* retention_days = 0 now means delete all previous inventory before run

Also explictly name forseti schema in cleanup table SQL

* Fix missed merge lines :(

* Remove redundant cleanup call and fix documentation typos
@blueandgold
Copy link
Contributor Author

Fixed by #1129

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants