CloudWanderer-io / CloudWanderer

A Python package which wanders across your AWS account and records your resources in a variety of Storage Connectors
http://cloudwanderer.io
MIT License
24 stars 1 forks source link

CW generates invalid URNs for some resource types #260

Closed fwojciec closed 2 years ago

fwojciec commented 2 years ago

Happy New Year Sam!

We've found two edge cases where incorrect URNs are generated for specific (dependent) resource types:

I can look into making a PR later, but would have to read the code to figure out where in CW exactly the ID generation is taking place... For now we're handling it in our app when parsing the CW URN, but I doubt this is intended behavior so probably needs fixing.

Sam-Martin commented 2 years ago

Interesting! I can't reproduce the autoscaling group one...

from cloudwanderer.aws_interface import CloudWandererAWSInterface

def test_autoscaling_auto_scaling_group():
    interface = CloudWandererAWSInterface()

    result = next(
        interface.get_resources(service_name="autoscaling", resource_type="auto_scaling_group", region="eu-west-1")
    )

    assert result.urn == []

outputs:

 AssertionError: assert URN(cloud_name='aws', account_id='0123456789012', region='eu-west-1', service='autoscaling', resource_type='auto_scaling_group', resource_id_parts=['test']) == []

Which doesn't show the nested list? I'm confused as to how auto scaling group is getting multiple id parts in the first place for you. Can you provide some example (anonymised) DescribeAutoScalingGroups output?

I can however replicate the issue you're talking about with layers.

from cloudwanderer.aws_interface import CloudWandererAWSInterface

def test_autoscaling_auto_scaling_group():
    interface = CloudWandererAWSInterface()

    result = next(interface.get_resources(service_name="lambda", resource_type="layer", region="eu-west-1"))

    assert result.urn == []

output:

URN(cloud_name='aws', account_id='0123456789012', region='eu-west-1', service='lambda', resource_type='layer_version', resource_id_parts=['example-layer-name', 2])

which shows the version as an integer.

I think this behaviour was preserved originally due to boto3's get_layer_version argument requiring an integer but I agree it needs fixing somehow as it is problematic as URNs should be wholly stringy!

I'll see what I can do with the lambda layers integer problem, but let me know if you find any more info about the ASG issue as without being able to reproduce it I'm at a bit of a loss!

Sam-Martin commented 2 years ago

Integer and list resource_id_parts should no longer be possible as CloudWanderer URN now autoconverts int to str and throws a ValueError if anything else which is not a string is passed in.

Try updating to 0.29.1 and raise a new issue if you still find nested lists occurring.

fwojciec commented 2 years ago

Thank you!

0bart commented 2 years ago

Hi @Sam-Martin, regarding issue with nested list. It is caused by incorrectly defined relationship. For now it is visible for autoscaling's relationship with elb - https://github.com/CloudWanderer-io/CloudWanderer/blob/97ac3d0c69402cbf5f043a97e8a670d001719017/cloudwanderer/aws_interface/resource_definitions/autoscaling/2011-01-01/resources-cw-1.json#L24 LoadBalancerNames is a list os str rather than a str, so we should define it as:

...
          "basePath": "LoadBalancerNames[]",
          "idParts": [
            {
              "path": "@"
            }
          ],
...

PS: I've created fix for that, but can't push to the repo :(

Sam-Martin commented 2 years ago

Thanks @0bart ! Fixed in 0.29.2 with https://github.com/CloudWanderer-io/CloudWanderer/pull/265