Netflix-Skunkworks / aardvark

Aardvark is a multi-account AWS IAM Access Advisor API
Apache License 2.0
472 stars 77 forks source link

Fix check for truncated response #152

Closed patricksanders closed 1 year ago

patricksanders commented 1 year ago

Follow-up to #149 and #150. We're checking the wrong response attribute (Truncated instead of IsTruncated) according to the docs

cc @rogerfdias -- can you test this change and see if it fixes the problem you're seeing?

rogerfdias commented 1 year ago

HI @patricksanders . I`ll test here in my environment.

Thanks.

rogerfdias commented 1 year ago

I updated my fork here with this update

details.get('IsTruncated', False):

My role has 369 services, but I just received 200 services, as we can confirm with the output from the database:

aardvark=>SELECT COUNT(*) from advisor_data WHERE item_id = 15178;

 count

-------
   200

When I change the configuration to details.get('IsTruncated', True): and run the Aardvark Collector, I receive all the 369 services:

aardvark=> SELECT COUNT(*) from advisor_data WHERE item_id = 15178;
 count

-------

   369

I think is mandatory to keep the configuration as details.get('IsTruncated', True): because even a role has less than 200 services, the parameter IsTruncated will be available, but the value will be false.

I ran the Aardvark Collector for all my 40 accounts with the configuration details.get('IsTruncated', True): and I didn`t see any anormal behavior regarding a infinity loop for missing the Parameter IsTruncated

patricksanders commented 1 year ago

Interesting! That makes me think that something is still wrong here because the call to details.get() shouldn't be falling back to the default value. I'll poke around at it a bit.

patricksanders commented 1 year ago

Alright, I think I figured it out. I refactored the calls to get_service_last_accessed_details into a separate method with a cleaner pagination loop and it appears to behave correctly now. Can you the latest change in your env when you have a chance?

rogerfdias commented 1 year ago

I ran the Aardvark Collector in one Account with the last update and now the collector is able to fetch all the services from the Role

Now, I wil run in my all Organization Accounts and We will check if the Collector Run smoothly in all Accounts.

rogerfdias commented 1 year ago

@patricksanders Aardvark Collector Finished to fetch all data from my accounts Succesfully.

patricksanders commented 1 year ago

Excellent, thank you for testing this @rogerfdias!