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

Added support for AWS API Gateway #539

Merged
merged 28 commits into from
Mar 4, 2021

Conversation

kedarghule
Copy link
Collaborator

This is an initial version of pulling API Gateway REST APIs, Resources, Stages and the Client Certificate details and populating graph db.

mpurusottamc and others added 20 commits December 5, 2020 07:47
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
# Conflicts:
#	cartography/intel/aws/__init__.py
# Conflicts:
#	cartography/intel/aws/__init__.py
#	docs/schema/aws.md
Copy link
Contributor

@achantavy achantavy left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this! Left a few minor minor comments.

return apis


@timeit
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: probably not necessary for a transform.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@achantavy I removed the transform_apigateway_rest_apis() function altogether and added it's body within the load_apigateway_rest_apis() function.



@timeit
def get_rest_api_stages(api, client):
Copy link
Contributor

Choose a reason for hiding this comment

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

[question] Do we need to handle regions here as well? @marco-lancini

return stages['item']


@timeit
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing on regions



@timeit
@aws_handle_regions
Copy link
Contributor

Choose a reason for hiding this comment

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

handle regions is not necessary for a load function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@achantavy Removed it :)

ingest_rest_apis = """
UNWIND {rest_apis_list} AS r
MERGE (rest_api:APIGatewayRestAPI{id:r.id})
ON CREATE SET rest_api.firstseen = timestamp(),
Copy link
Contributor

Choose a reason for hiding this comment

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

The id field you're using here is an arn? If so, also set rest_api.arn in the ON CREATE SET.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@achantavy id field is an identifier for the REST API. Rest apis don't have arns so we don't need to capture them.

"""
ingest_stages = """
UNWIND {stages_list} AS stage
MERGE (s:APIGatewayStage{id: stage.stageName})
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about also setting an arn field so we know what the id was derived from (assuming the id was derived from an arn. otherwise use whatever the original field was.).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@achantavy For stages, I created a new field 'arn' since stage name may be same for multiple rest apis.

ingest_certificates = """
UNWIND {certificates_list} as certificate
MERGE (c:APIGatewayClientCertificate{id: certificate.clientCertificateId})
ON CREATE SET c.firstseen = timestamp(), c.createddate = certificate.createdDate
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing about arns

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@achantavy Client certificates don't have an arn. Just an identifier.

@kedarghule
Copy link
Collaborator Author

@achantavy I've made all the changes as per your review. Please let me know if there are any more changes! :)

achantavy
achantavy previously approved these changes Mar 3, 2021
Copy link
Contributor

@achantavy achantavy left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

# Conflicts:
#	cartography/intel/aws/resources.py
Copy link
Contributor

@achantavy achantavy left a comment

Choose a reason for hiding this comment

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

🍻

@achantavy achantavy merged commit eff645f into lyft:master Mar 4, 2021
This pull request was closed.
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.

4 participants