-
Notifications
You must be signed in to change notification settings - Fork 329
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
Kandji device #1295
Kandji device #1295
Conversation
@achantavy Requesting review of this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR! Appreciate you taking the time and effort to adopt cartography patterns used in other modules.
For this PR, let's focus on introducing a Kandji device node and a Kandji account/tenant node. Having a tenant-like node is needed for cartography automatic cleanup jobs to work so that the data in the graph is always up to date. You can copy this PR as a good example: https://github.com/lyft/cartography/pull/1083/files
Let's also defer adding the crowdstrike relationship and the analysis jobs to separate PRs.
Basically just copy that example PR and tag me again when this is ready. @ me on Slack if you need faster answers to fast questions; this way we can be better at the back and forth needed sometimes in these reviews.
Thanks again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there, left a note about relationship direction convention. Once that is done, we can merge!
This is looking super clean btw
cartography/models/kandji/device.py
Outdated
|
||
|
||
@dataclass(frozen=True) | ||
# (:KandjiTenant)<-[:DEVICE]-(:KandjiDevice) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the relationship label is called DEVICE
, the convention is to point to the device. Like this:
(:KandjiTenant)-[:DEVICE]->(:KandjiDevice)
So you would do LinkDirection.INWARD
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the confusion Alex!
The correct relationship is
(:KandjiDevice)-[:ENROLLED_TO]->(:KandjiTenant)
I had updated the doc but forgot to update the comment (fixed now).
Does the relationship makes sense now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your PR! Appreciate you taking the time to use the patterns and adjust
Thank you @achantavy for reviewing the changes and walking me through the steps! |
Add support for Kandji device. lyft#1293
Add support for Kandji device.
#1293