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(bundle-manager): blacklisted bundle directory names #357

Merged
merged 5 commits into from
Jul 7, 2018

Conversation

haganbmj
Copy link
Contributor

@haganbmj haganbmj commented Dec 25, 2017

Allows irrelevant folders (like .git) and invalid bundles to exist in the bundles directory without crashing.
It's likely that the bundle-parser should be logging these errors and returning an undefined bundle instead to avoid using this blind try/catch. That'd mean passing in and constructing a new logger instance each time with the current implementation, though.

@haganbmj
Copy link
Contributor Author

Not sure there's a way to get this to rerun the security checks.

@haganbmj
Copy link
Contributor Author

Yeah, maybe I'm just trying to solve a problem I created for myself when I decided that my bundles directory should be one big git repo of submodules.

Without this commit nodecg exits because .git is an invalid bundle.

@haganbmj
Copy link
Contributor Author

There might be a better way though, I don't doubt that at all. Maybe just a couple blacklisted directory names and the ability to add a few exclusions in your cfg/node.json

@haganbmj
Copy link
Contributor Author

haganbmj commented Mar 20, 2018

Went with blacklisting node_modules, bower_components, and anything that starts with a dot. Figured that'd take care of .git, .idea, .vscode, etc.
Any custom exclusions can be added in the bundles.disabled config to supplement that, which I'm realizing is just what I could've done from the start.

@haganbmj
Copy link
Contributor Author

haganbmj commented May 2, 2018

Whoops, was able to get some new issues related to chokidar because the watchers don't adhere to the ignored directories.

Unsure what the symlink limitations are, wasn't even sure nodecg resolved symlinks, but it's probably alright to just move this up into the bundle discovery loop. Whether that's before or after the semver compatibility check or not.

watcher.add([
path.join(bundlePath, '.git'), // Watch .git folders
path.join(bundlePath, 'dashboard'), // Watch dashboard folders
path.join(bundlePath, 'package.json') // Watch bundle package.json files
]);

2018-05-02 10:38 NodeCG[43514] (FSEvents.framework) process_dir_events: creation: watch_path() failed for '~/git/nodecg/bundles/.git/dashboard'
2018-05-02 10:38 NodeCG[43514] (FSEvents.framework) process_dir_events: creation: watch_path() failed for '~/git/nodecg/bundles/.git/.git'
2018-05-02 10:38 NodeCG[43514] (FSEvents.framework) process_dir_events: creation: watch_path() failed for '~/git/nodecg/bundles/.git/.git'
2018-05-02 10:38 NodeCG[43514] (FSEvents.framework) process_dir_events: creation: watch_path() failed for '~/git/nodecg/bundles/.git/dashboard'
2018-05-02 10:39 NodeCG[43514] (FSEvents.framework) process_dir_events: creation: watch_path() failed for '~/git/nodecg/bundles/.git/dashboard'
2018-05-02 10:39 NodeCG[43514] (FSEvents.framework) process_dir_events: creation: watch_path() failed for '~/git/nodecg/bundles/.git/.git'

@haganbmj
Copy link
Contributor Author

haganbmj commented May 2, 2018

Change:

Add the concept of blacklisted bundle directories, directories that can safely exist in your bundles directory without nodecg attempting to load them.

My use case for this was having my bundles directory managed as a git repository with submodules. Due to this the .git directory was being parsed as a bundle and nodecg would exception out.

The currently ignored directories are the common things like node_modules and bower_components, but also any dot-prefixed directories (like .vscode, .idea).

As for a status, the ignoring works fine. There's some changes that need to be made to the chokidar logic though because it bypasses the concept of enabled/disabled bundles entirely and watches every folder in the bundles directory.

@haganbmj haganbmj changed the title fix(bundle-manager): catch exceptions loading bundles feat(bundle-manager): blacklisted bundle directory names May 31, 2018
There's already a check for whether the target is a directory and this ensures that we don't watch disabled/invalid bundles. Some of this logic is already sort of handled in that un-loaded bundles are declared invalid if they don't exist in a map of loaded bundles.
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.

None yet

1 participant