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

Fixes snapshot fetching InvalidParameterValue bug #832

Merged
merged 13 commits into from
Apr 27, 2022

Conversation

Rickerd0613
Copy link
Contributor

It looks like the InvalidParameterValue bug (#825) is still present after #822. This adds an exception for the error.

marco-lancini
marco-lancini previously approved these changes Apr 27, 2022
@marco-lancini
Copy link
Contributor

@ramonpetgrave64 Is it ok to merge this?

make a new paginator instead of reusing
@ramonpetgrave64
Copy link
Contributor

@marco-lancini I still want to figure out why we are getting this error when the parameter should not be empty at this codepath.

I think the issue could be that we are reusing the same paginator object. The boto docs say that paginators are reusable, but @Rickerd0613 can you try my commit?

@Rickerd0613
Copy link
Contributor Author

@ramonpetgrave64 Same error unfortunately. It's a pretty fresh AWS account which likely does not have any snapshots in it.

Let me know if you want me to try running anything else or if you want some print debug lines thrown in anywhere.

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/cartography/sync.py", line 74, in run
    stage_func(neo4j_session, config)
  File "/usr/local/lib/python3.7/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/__init__.py", line 225, in start_aws_ingestion
    requested_syncs,
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/__init__.py", line 163, in _sync_multiple_accounts
    aws_requested_syncs=aws_requested_syncs,  # Could be replaced later with per-account requested syncs
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/__init__.py", line 60, in _sync_one_account
    RESOURCE_FUNCTIONS[func_name](**sync_args)
  File "/usr/local/lib/python3.7/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/ec2/snapshots.py", line 142, in sync_ebs_snapshots
    data = get_snapshots(boto3_session, region, snapshots_in_use)
  File "/usr/local/lib/python3.7/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/cartography/util.py", line 146, in wrapper
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/ec2/snapshots.py", line 42, in get_snapshots
    for page in paginator.paginate(SnapshotIds=list(other_snapshot_ids)):
  File "/usr/local/lib/python3.7/site-packages/botocore/paginate.py", line 253, in __iter__
    response = self._make_request(current_kwargs)
  File "/usr/local/lib/python3.7/site-packages/botocore/paginate.py", line 332, in _make_request
    return self._method(**current_kwargs)
  File "/usr/local/lib/python3.7/site-packages/botocore/client.py", line 415, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/usr/local/lib/python3.7/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
Traceback (most recent call last):
  File "/usr/local/bin/cartography", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.7/site-packages/cartography/cli.py", line 464, in main
    return CLI(default_sync, prog='cartography').main(argv)
  File "/usr/local/lib/python3.7/site-packages/cartography/cli.py", line 444, in main
    return cartography.sync.run_with_config(self.sync, config)
  File "/usr/local/lib/python3.7/site-packages/cartography/sync.py", line 151, in run_with_config
    return sync.run(neo4j_driver, config)
  File "/usr/local/lib/python3.7/site-packages/cartography/sync.py", line 74, in run
    stage_func(neo4j_session, config)
  File "/usr/local/lib/python3.7/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/__init__.py", line 225, in start_aws_ingestion
    requested_syncs,
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/__init__.py", line 163, in _sync_multiple_accounts
    aws_requested_syncs=aws_requested_syncs,  # Could be replaced later with per-account requested syncs
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/__init__.py", line 60, in _sync_one_account
    RESOURCE_FUNCTIONS[func_name](**sync_args)
  File "/usr/local/lib/python3.7/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/ec2/snapshots.py", line 142, in sync_ebs_snapshots
    data = get_snapshots(boto3_session, region, snapshots_in_use)
  File "/usr/local/lib/python3.7/site-packages/cartography/util.py", line 117, in timed
    return method(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/cartography/util.py", line 146, in wrapper
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/cartography/intel/aws/ec2/snapshots.py", line 42, in get_snapshots
    for page in paginator.paginate(SnapshotIds=list(other_snapshot_ids)):
  File "/usr/local/lib/python3.7/site-packages/botocore/paginate.py", line 253, in __iter__
    response = self._make_request(current_kwargs)
  File "/usr/local/lib/python3.7/site-packages/botocore/paginate.py", line 332, in _make_request
    return self._method(**current_kwargs)
  File "/usr/local/lib/python3.7/site-packages/botocore/client.py", line 415, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/usr/local/lib/python3.7/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

@ramonpetgrave64
Copy link
Contributor

ramonpetgrave64 commented Apr 27, 2022

Let me know if you want me to try running anything else or if you want some print debug lines thrown in anywhere.

Thank you, please try the new commit where I add logs. And remember to redact your snapshot ids!

I want to make sure that bool(other_snapshot_ids) truly is False when it should be, and does not contain invalid data, like being a set containing a single item, like {None}.

@Rickerd0613
Copy link
Contributor Author

@ramonpetgrave64 here is the log outputs. Let me know if I missed something or you need anything else.

INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots []
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:other snapshots set()
INFO:cartography.intel.aws.ec2.snapshots:in use snapshots ['snap-redacted-1', '', 'snap-redacted-2', '']
INFO:cartography.intel.aws.ec2.snapshots:owned snapshots {'snap-redacted-2'}
INFO:cartography.intel.aws.ec2.snapshots:other snapshots {'', 'snap-redacted-1'}

@ramonpetgrave64
Copy link
Contributor

ramonpetgrave64 commented Apr 27, 2022

I see now. There are places where the snapshotId of volumes are not being set. And thats why we sometimes get invalid inputs to describe_snapshots. Are you sometimes running different parts of the ec2 sync?

I found one place here

SET vol.lastupdated = {update_tag}, vol.deleteontermination = em.Ebs.DeleteOnTermination

@Rickerd0613 Can you change that line to

SET vol.lastupdated = {update_tag}, vol.deleteontermination = em.Ebs.DeleteOnTermination, vol.snapshotid = volume.SnapshotId

If this works for you, I think we can remove the log statements I added and the extra paginator, as well.

@Rickerd0613
Copy link
Contributor Author

@ramonpetgrave64 Looks like there might be an error with the new SET line:

neobolt.exceptions.CypherSyntaxError: Variable `volume` not defined (line 5, column 120 (offset: 270))
"            SET vol.lastupdated = {update_tag}, vol.deleteontermination = em.Ebs.DeleteOnTermination, vol.snapshotid = volume.SnapshotId"

Should volume be vol?

@Rickerd0613
Copy link
Contributor Author

FWIW changing volume to vol just resulted in the original InvalidParameterValue error.

@ramonpetgrave64
Copy link
Contributor

ramonpetgrave64 commented Apr 27, 2022

I think at this point we might be dealing with a race condition. I added one more place to set the snapshotid. Along with the other change I suggested you add, I think that covers it. So I'm okay with

...
 return [r['snapshot'] for r in results if r['snapshot']]

@Rickerd0613 Please try this new commit, as well.

@Rickerd0613
Copy link
Contributor Author

Seems to be working for me! Thanks for all your help!!

@ramonpetgrave64
Copy link
Contributor

@Rickerd0613 Awesome. And thanks for your patience!

@ramonpetgrave64 ramonpetgrave64 merged commit 75bc122 into lyft:master Apr 27, 2022
@Rickerd0613 Rickerd0613 deleted the patch-1 branch April 27, 2022 21:51
shinya-murakami-paypay pushed a commit to paypay/cartography that referenced this pull request May 23, 2022
* Fixes snapshot fetching InvalidParameterValue bug

It looks like the InvalidParameterValue bug (lyft#825) is still present after lyft#822. This adds an exception for error

* set snapshot id in load_snapshot_volume_relations

* filter for non empty snapshot ids

* setting snapshot id

Co-authored-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>
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.

None yet

3 participants