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

feat: Delete local .iac-data directory after scan is finished [CC-765] #1762

Merged
merged 1 commit into from
Mar 26, 2021

Conversation

ipapast
Copy link
Contributor

@ipapast ipapast commented Mar 24, 2021

What does this PR do?

In a previous task, we introduced the download and extraction of files in a local cache directory, called '.iac-data'.

We do not need the files anymore after the scan/test, so we decided to clean up that local directory after the tests finishes.

We also move the dev dependency rimraf from dev deps to prod deps. When we support Node >= v12.10.0 we can switch to the supported rmdirSync with recursive options.
https://snyk.slack.com/archives/C0127HWU0E7/p1616688051070900

How should this be manually tested?

  • Run npm run build && node iac test file.tf --experimental
  • Run ls .iac-data and see that there is no such directory in the filesystem anymore.

Any background context you want to provide?

The package rimraf was used for the deletion of the directory. This was already in our dev dependencies.

Option 2:
In our version of node fs.rmdirSync is not supported, so in order to delete a non empty directory we would need to make our own recursive function, to check for files in the directory, unlink them and then delete them one by one. Rimraf does this in one line. That function would look like this:

export const cleanLocalCache = (directoryPath) => {
  if (fs.existsSync(directoryPath)) {
    fs.readdirSync(directoryPath).forEach((file) => {
      const curPath = path.join(directoryPath, file);
      fs.unlinkSync(curPath);
    });
    fs.rmdirSync(directoryPath);
  } else {
    console.log('Directory path not found');
  }
};

What are the relevant tickets?

CC-765

@ipapast ipapast changed the title feat: delete iac cache directory after test feat: Delete local .iac-data directory after scan is finished [CC-765] Mar 24, 2021
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch from dd4aeed to 7a37a90 Compare March 24, 2021 17:12
@ipapast ipapast self-assigned this Mar 24, 2021
@ipapast ipapast marked this pull request as ready for review March 24, 2021 17:13
@ipapast ipapast requested a review from a team March 24, 2021 17:13
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch from 0e37d86 to e3fa251 Compare March 24, 2021 18:21
@ipapast ipapast requested review from a team as code owners March 24, 2021 18:21
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch from e3fa251 to 9390456 Compare March 24, 2021 18:29
@ipapast ipapast marked this pull request as draft March 24, 2021 18:37
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch from 9390456 to 7a37a90 Compare March 25, 2021 08:46
@ipapast ipapast removed request for a team March 25, 2021 08:47
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch 4 times, most recently from 90d6dfc to e8d37ed Compare March 25, 2021 12:44
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch from e8d37ed to 1848bf4 Compare March 25, 2021 12:52
@ipapast ipapast marked this pull request as ready for review March 25, 2021 13:03
@ipapast ipapast requested a review from a team March 25, 2021 14:58
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch 2 times, most recently from d7b3d0d to 6de51e9 Compare March 25, 2021 16:18
@ipapast ipapast changed the title feat: Delete local .iac-data directory after scan is finished [CC-765] feat: Delete local .iac-data directory after scan is finished, using rimraf [CC-765] Mar 25, 2021
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch 2 times, most recently from bae5a78 to 8044917 Compare March 26, 2021 12:10
Copy link
Contributor

@aron aron left a comment

Choose a reason for hiding this comment

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

👍 Very nice, shall we add a note above the rimraf import/call site that says it can be removed when minimum node version is above 12.x?

@ipapast
Copy link
Contributor Author

ipapast commented Mar 26, 2021

👍 Very nice, shall we add a note above the rimraf import/call site that says it can be removed when minimum node version is above 12.x?

Good call, I'll add that now

@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch 2 times, most recently from 75e1732 to d49c829 Compare March 26, 2021 12:47
[CC-765]

refactor: Move rimraf to prod deps
@ipapast ipapast force-pushed the refactor/cleanup-local-iac-cache branch from d49c829 to b6803e8 Compare March 26, 2021 12:50
@ipapast ipapast changed the title feat: Delete local .iac-data directory after scan is finished, using rimraf [CC-765] feat: Delete local .iac-data directory after scan is finished [CC-765] Mar 26, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Mar 26, 2021

Expected release notes (by @ipapast)

features:
delete iac cache directory after test (b6803e8)

others (will not be included in Semantic-Release notes):
Remove SNYK_IAC_SKIP_BUNDLE_DOWNLOAD flag for IaC (29db293)
target node14.4.0 instead of node14 to fix build (1ce0d11)
iac smoke tests to be less explicit (2d6cd7e)
fix broken IaC smoke test (259c263)
remove now-redundant and excessively complex acceptance tests (20d560b)
refactor and improve testing (afdf3b3)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

export function cleanLocalCache() {
// path to delete is hardcoded for now
const iacPath: fs.PathLike = path.join(`${process.cwd()}`, '.iac-data');
if (fs.existsSync(iacPath) && fs.lstatSync(iacPath).isDirectory()) {
Copy link

Choose a reason for hiding this comment

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

Not blocking, but this if guard shouldn't be needed as rimraf already checks and succeeds if the directory doesn't exist.

If the file doesn't exist, rimraf will return successfully, since your desired outcome is already the case.

Source: https://github.com/isaacs/rimraf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I'll merge it right after this. thank you!

@ipapast ipapast merged commit ef0bcbb into master Mar 26, 2021
@ipapast ipapast deleted the refactor/cleanup-local-iac-cache branch March 26, 2021 14:19
@ipapast ipapast removed the request for review from a team March 26, 2021 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants