Fix #821: Error while ingesting snapshots

[marco-lancini]

Fix for #821

[ramonpetgrave64]

Looks good, but to be conservative, do you think we can skip snapshots on the specific exception you had InvalidSnapshot.NotFound?

[marco-lancini]

Looks good, but to be conservative, do you think we can skip snapshots on the specific exception you had InvalidSnapshot.NotFound?

@ramonpetgrave64 I've updated this MR.
Does this address what you were referring to?

[ramonpetgrave64]

@marco-lancini Yes. Are you finding that at least some of the other_snapshot_ids can be fetched? I want to avoid a situation where we skip a whole batch only because a single id can't be fetched.

[marco-lancini]

In the meantime I've faced another error while running from master branch:

Traceback (most recent call last):
  File "/src/custom_sync.py", line 56, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/src/custom_sync.py", line 51, in main
    return cli.CLI(build_custom_sync(), prog='cartography').main(argv)
  File "/usr/local/lib/python3.9/site-packages/cartography/cli.py", line 444, in main
    return cartography.sync.run_with_config(self.sync, config)
  File "/usr/local/lib/python3.9/site-packages/cartography/sync.py", line 151, in run_with_config
    return sync.run(neo4j_driver, config)
  File "/usr/local/lib/python3.9/site-packages/cartography/sync.py", line 74, in run
    stage_func(neo4j_session, config)
  File "/usr/local/lib/python3.9/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/cartography/intel/aws/__init__.py", line 220, in start_aws_ingestion
    _sync_multiple_accounts(
  File "/usr/local/lib/python3.9/site-packages/cartography/intel/aws/__init__.py", line 157, in _sync_multiple_accounts
    _sync_one_account(
  File "/usr/local/lib/python3.9/site-packages/cartography/intel/aws/__init__.py", line 60, in _sync_one_account
    RESOURCE_FUNCTIONS[func_name](**sync_args)
  File "/usr/local/lib/python3.9/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/cartography/intel/aws/ec2/snapshots.py", line 132, in sync_ebs_snapshots
    data = get_snapshots(boto3_session, region, snapshots_in_use)
  File "/usr/local/lib/python3.9/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/cartography/util.py", line 146, in wrapper
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/cartography/intel/aws/ec2/snapshots.py", line 39, in get_snapshots
    for page in paginator.paginate(SnapshotIds=list(other_snapshot_ids)):
  File "/usr/local/lib/python3.9/site-packages/botocore/paginate.py", line 253, in __iter__
    response = self._make_request(current_kwargs)
  File "/usr/local/lib/python3.9/site-packages/botocore/paginate.py", line 332, in _make_request
    return self._method(**current_kwargs)
  File "/usr/local/lib/python3.9/site-packages/botocore/client.py", line 415, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/usr/local/lib/python3.9/site-packages/botocore/client.py", line 745, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (InvalidParameterValue) when calling the DescribeSnapshots operation: Value () for parameter snapshotId is invalid. Parameter may not be null or empty

This PR should address it as well

[marco-lancini]

@marco-lancini Yes. Are you finding that at least some of the other_snapshot_ids can be fetched? I want to avoid a situation where we skip a whole batch only because a single id can't be fetched.

For my use case it seemed to return a decent amount of snapshots any way (I haven't dug into them to be fair)

[ramonpetgrave64]

@marco-lancini Maybe the safer way would be to fetch each of the other_snapshot_ids individually. Not optimal, but it's simple. What do you think?

[marco-lancini]

@marco-lancini Maybe the safer way would be to fetch each of the other_snapshot_ids individually. Not optimal, but it's simple. What do you think?

Hi @ramonpetgrave64,
I'm not super familiar with this part of the codebase, and here I was trying to get a patch in since this is causing master to systematically fail for multiple users (e.g., #825).

Any other implementations are welcome! But I'm afraid I won't have time personally to provide/test one

[achantavy]

Left a nitpicky wording suggestion but otherwise looks good to me!

[ramonpetgrave64]

@marco-lancini It's supposed to load snapshots that you own, and then also load snapshots are attached to any volumes you own. I don't understand why you get this error, saying the parameter is empty.

botocore.exceptions.ClientError: An error occurred (InvalidParameterValue) when calling the DescribeSnapshots operation: Value () for parameter snapshotId is invalid. Parameter may not be null or empty

But we can fix that one if it comes up again.