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

This is an Uber PR bringing together everyone elses fixes and more #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

byt3bl33d3r
Copy link

@byt3bl33d3r byt3bl33d3r commented Jan 23, 2022

  • Ported bloodhoundanalytics.py to Python3 using 2to3
  • DBCreator now works thanks to the fixes from @voutilad
  • Added .devcontainer to quickly setup dev environment in vscode
  • Added Dockerfile
  • exposed DBCreator and bloodhoundanalytics tools as two scripts in the
    package (bh-dbcreator and bh-analytics)
  • Made DBCreator automatically run the do_clear_and_generate functioon
    if the NEO4J_URL / NEO4J_USERNAME / NEO4J_PASSWORD env variables are
    passed

- Ported bloodhoundanalytics.py to Python3 using 2to3
- DBCreator now works thanks to the fixes from @voutilad
- Added .devcontainer to quickly setup dev environment in vscode
- Added Dockerfile
- exposed DBCreator and bloodhoundanalytics tools as two scripts in the
  package (bh-dbcreator and db-analytics)
- Made DBCreator automatically run the do_clear_and_generate functioon
  if the NEO4J_URL / NEO4J_USERNAME / NEO4J_PASSWORD env variables are
passed
@byt3bl33d3r
Copy link
Author

@andyrobbins fixed the Dockefile problem

@@ -0,0 +1,22 @@
# See here for image contents: https://github.com/microsoft/vscode-dev-containers/tree/v0.209.6/containers/python-3/.devcontainer/base.Dockerfile

Copy link

Choose a reason for hiding this comment

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

Please remove all github codespace files so we can focus on the relevant code changes in this change set. You are welcome to submit development environment specific changes in a separate PR. Thanks!

@@ -0,0 +1,22 @@
.PHONY: tests
Copy link

Choose a reason for hiding this comment

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

Please remove build changes and put them in a separate PR for review and discussion.


COPY --from=build-stage /tmp/code/dist .

RUN pip install --no-cache-dir --no-index --find-links . bloodhound-tools
Copy link

@ddlees ddlees Jan 26, 2022

Choose a reason for hiding this comment

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

Since this is an artifact of your pythoncookie template either:

  1. Resubmit this Dockerfile along with the changes made using your template w/ additional stages that have ENTRYPOINTS for each program

  2. Extend Dockerfile to include ENTRYPOINTs for each program

@@ -5,3 +5,7 @@ This is a collection of miscellaneous tools released by the BloodHound team. See
* DBCreator - Tool to generate randomized Neo4j databases for use with BloodHound
* BloodHoundAnalytics.pbix - Proof of concept charting capability
* BloodHoundAnalytics.py - Proof of concept audit script

Copy link

Choose a reason for hiding this comment

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

Should the proposed build changes get merged in a separate PR you are more than welcome to leave yourself and attribution 😄. But let's remove it for this one.

@@ -0,0 +1,38 @@
[tool.poetry]
Copy link

Choose a reason for hiding this comment

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

Submit in separate PR

@@ -0,0 +1,361 @@
[[package]]
Copy link

Choose a reason for hiding this comment

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

Include in separate PR

Copy link

@ddlees ddlees left a comment

Choose a reason for hiding this comment

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

While the build changes look mostly fine please remove those artifacts and resubmit those in a separate PR so they can get properly vetted and we can focus on the code changes in this PR. Thank you!

@byt3bl33d3r
Copy link
Author

@ddlees heya, honestly don't think i'm going to have time to do that for the next few weeks unfortunately. I needed to package and fix these tools up in a relatively short time frame and submitted the PR on a whim. You're more than welcome to separate out the stuff yourself in different PRs if you need that.

Cheers

@ddlees
Copy link

ddlees commented Jan 27, 2022

@byt3bl33d3r No worries, I can totally relate. I'll work w/ @andyrobbins to shepherd in these changes one way or another. Thank you for your contribution!

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