-
Notifications
You must be signed in to change notification settings - Fork 340
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
Adding support for determining the last used date of an access keys #1101
base: master
Are you sure you want to change the base?
Conversation
"iterative": true, | ||
"iterationsize": 100 | ||
} | ||
], |
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.
cleanup the relationship as well
], | |
{ | |
"query": "MATCH (UserAccessKey)<-[r:AWS_ACCESS_KEY]-(:AWSUser)<-[:RESOURCE]-(:AWSAccount{id: $AWS_ID}) WHERE r.lastupdated <> $UPDATE_TAG WITH n LIMIT $LIMIT_SIZE DELETE (r)", | |
"iterative": true, | |
"iterationsize": 100 | |
} | |
], |
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.
not really sure what the changes being outlined here are
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.
This is just the standard for cartography.
- cleanup stale nodes
- cleanup stale relationships that are still attached to fresh nodes
This shouldn't really be needed for access keys, since a given access key should only ever relate to one user in its lifetime. But you never know.
@@ -65,8 +65,8 @@ CREATE INDEX IF NOT EXISTS FOR (n:AWSUser) ON (n.name); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AWSUser) ON (n.lastupdated); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AWSVpc) ON (n.id); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AWSVpc) ON (n.lastupdated); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AccountAccessKey) ON (n.accesskeyid); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AccountAccessKey) ON (n.lastupdated); | |||
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid); |
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.
Add an additional property id
, which is will be equivalent to accesskeyid
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.
I think this might be on a previous version of the code - as it currently stands the following is the code
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid);
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.lastupdated);
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.
this is what I mean, along with another suggestion below upon MERGE
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid); | |
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.id); | |
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid); |
Co-authored-by: Ramon Petgrave <[email protected]>
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.
Looks alright. Just a few comments. Also try to add an integration test.
"iterative": true, | ||
"iterationsize": 100 | ||
} | ||
], |
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.
This is just the standard for cartography.
- cleanup stale nodes
- cleanup stale relationships that are still attached to fresh nodes
This shouldn't really be needed for access keys, since a given access key should only ever relate to one user in its lifetime. But you never know.
MATCH (user:AWSUser{arn: $UserARN}) | ||
WITH user | ||
MERGE (key:AccountAccessKey{accesskeyid: $AccessKeyId}) | ||
MERGE (key:UserAccessKey{accesskeyid: $AccessKeyId}) |
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.
MERGE (key:UserAccessKey{accesskeyid: $AccessKeyId}) | |
MERGE (key:UserAccessKey{accesskeyid: $AccessKeyId, id:$AccessKeyId}) |
@@ -65,8 +65,8 @@ CREATE INDEX IF NOT EXISTS FOR (n:AWSUser) ON (n.name); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AWSUser) ON (n.lastupdated); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AWSVpc) ON (n.id); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AWSVpc) ON (n.lastupdated); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AccountAccessKey) ON (n.accesskeyid); | |||
CREATE INDEX IF NOT EXISTS FOR (n:AccountAccessKey) ON (n.lastupdated); | |||
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid); |
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.
this is what I mean, along with another suggestion below upon MERGE
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid); | |
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.id); | |
CREATE INDEX IF NOT EXISTS FOR (n:UserAccessKey) ON (n.accesskeyid); |
Also, about the CLA check. It looks like you need to go back and edit the first two commits' authors to be explicitly set to @csanders-git. And then force-push. |
…artography-cncf#1169) Redo of cartography-cncf#1101 1. Add access keys last used data 2. cleanup unattached AccessKey nodes. This happens when the associated User no longer exists. ### Testing No unit tests, but I did test manually.
No description provided.