boto / boto3

AWS SDK for Python
https://aws.amazon.com/sdk-for-python/
Apache License 2.0
9.05k stars 1.87k forks source link

boto3.client("autoscaling").describe_auto_scaling_groups should raise a ValidationError exception when AutoScalingGroupNames is an empty list #3740

Closed seandockery closed 1 year ago

seandockery commented 1 year ago

Describe the bug

When passed an empty list for the argument AutoScalingGroupNames, the describe_auto_scaling_groups method in the autoscaling client unexpectedly returned all auto scaling groups in the account region rather than returning an empty list of auto scaling groups or raising an ValidationError exception that the list must contain at least one element. The list of auto scaling groups returned by describe_auto_scaling_groups should never be a superset of the AutoScalingGroupNames list when that argument is provided to the autoscaling client.

Expected Behavior

~$ aws autoscaling describe-auto-scaling-groups --auto-scaling-group-names ""

Parameter validation failed:
Invalid length for parameter AutoScalingGroupNames[0], value: 0, valid min length: 1

Current Behavior

(.venv) ~$ pip freeze | grep boto3
boto3==1.26.146
(.venv) ~$ python
Python 3.8.10 (default, Mar 13 2023, 10:26:41)
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import boto3
>>> resp = boto3.client("autoscaling").describe_auto_scaling_groups(AutoScalingGroupNames=[])
>>> print(len(resp["AutoScalingGroups"]))
50
>>> quit()

Reproduction Steps

(.venv) ~$ pip freeze | grep boto3
boto3==1.26.146
(.venv) ~$ python
Python 3.8.10 (default, Mar 13 2023, 10:26:41)
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import boto3
>>> resp = boto3.client("autoscaling").describe_auto_scaling_groups(AutoScalingGroupNames=[])
>>> print(len(resp["AutoScalingGroups"]))
50
>>> quit()

Possible Solution

Update the handling of AutoScalingGroupNames parameter in describe_auto_scaling_groups to raise an exception when the list is empty with the complaint that at least one item is required when the argument is provided.

Additional Information/Context

No response

SDK version used

1.26.146

Environment details (OS name and version, etc.)

WSL Ubuntu 20.04.6 LTS

seandockery commented 1 year ago

I think that this line should be changed: https://github.com/boto/boto/blob/70c65b4f67af41ccfd40d21e49880be568997ba6/boto/ec2/autoscale/__init__.py#L359

...from...

if names:
    self.build_list_params(params, names, 'AutoScalingGroupNames')

...to something like...

if names is not None:
    self.build_list_params(params, names, 'AutoScalingGroupNames')

I don't know if the lower levels will throw an exception that this point. If not, then, is this where additional validation would be done, too?

if names is not None:
    if len(names) == 0:
        raise ValidationError("AutoScalingGroupNames must contain at least 1 element.")
    self.build_list_params(params, names, 'AutoScalingGroupNames')

...and I'm not familiar with how to add a regression test.

tim-finnigan commented 1 year ago

Hi @seandockery thanks for reaching out. Both the AWS CLI and boto3 share the same service API models from botocore. The script you referenced is from boto which is no longer maintained and has been replaced by boto3.

If you were to add quotes in that list then it would trigger the same parameter validation error:

client.describe_auto_scaling_groups(AutoScalingGroupNames=[''])

Results in: Invalid length for parameter AutoScalingGroupNames[0], value: 0, valid min length: 1

Because of the AWS CLI's shorthand syntax the brackets aren't necessary when running that command.

seandockery commented 1 year ago

Hi @tim-finnigan, Thanks for following up.

You're right. I misinterpreted the error from awscli in the description of this issue. I thought that it was indicating that AutoScalingGroupNames needed to contain at least 1 item, but it was actually telling me that the 1 item that I did provide needed to be at least 1 character in length at a minimum.

Furthermore, I am beside myself in astonishment: Without shorthand syntax, awscli behaves the same as boto3 by returning all auto scaling groups in a region.

~$ aws autoscaling describe-auto-scaling-groups --auto-scaling-group-names '[]' --query 'AutoScalingGroups[*].AutoScalingGroupName' --output text | wc -w
85

The fact that there is parity in behaviour probably isn't a surprise to you by the fact that they share everything under the covers. I just can't believe that it behaves this way as, surely, no sane person would rely on this behaviour. If a user had a list of auto scaling group names and was filtering them down based on some criteria that it happened to reduced to an empty list, it is not really intuitive when asking AWS for a list of zero names to get all auto scaling groups in the response. No user would excitedly exclaim, "Yes, that's exactly what I expected and wanted!" More likely, the user would be like me and hammer on CTRL+C with panic when they realized their program was acting upon all auto scaling groups in a region.

Anyway, I appreciate that this isn't an issue that will be resolved in this project. It is more fundamental in scope. Feel free to close this issue.

Kind regards,

Sean

tim-finnigan commented 1 year ago

Thanks for following up, I see what you're saying about how this behavior is not intuitive. We can forward this kind of feedback to the Auto Scaling team as they own the DescribeAutoScalingGroups API but as you noted it is beyond the scope of the the CLI or boto3 to directly address.