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

Fix EC2 security group ingress/egress deletion for non-existing SGs #7428

Closed

Conversation

dominikschubert
Copy link
Contributor

When trying to delete a security group ingress/egress we've been running into some unexpected issues, since it doesn't return a proper 4xx error, but instead fails with a 500 internal server exception due to an attribute access on None:

2024-03-06T14:06:40.0969862Z            ^^^^^^^^^^^^^^^^^^
2024-03-06T14:06:40.0973388Z   File "/opt/code/localstack/.venv/lib/python3.11/site-packages/moto/core/responses.py", line 575, in call_action
2024-03-06T14:06:40.0974737Z     response = method()
2024-03-06T14:06:40.0975219Z                ^^^^^^^^
2024-03-06T14:06:40.0976704Z   File "/opt/code/localstack/.venv/lib/python3.11/site-packages/moto/ec2/responses/security_groups.py", line 220, in revoke_security_group_ingress
2024-03-06T14:06:40.0978219Z     self.ec2_backend.revoke_security_group_ingress(*args)
2024-03-06T14:06:40.0979850Z   File "/opt/code/localstack/.venv/lib/python3.11/site-packages/moto/ec2/models/security_groups.py", line 759, in revoke_security_group_ingress
2024-03-06T14:06:40.0981473Z     rule for rule in group.ingress_rules if rule.id not in security_rule_ids
2024-03-06T14:06:40.0984656Z                      ^^^^^^^^^^^^^^^^^^^
2024-03-06T14:06:40.0985602Z AttributeError: 'NoneType' object has no attribute 'ingress_rules'

The PR adds a check for this on both revoking ingress and egress rules

TODO

  • write a test

@bblommers
Copy link
Collaborator

Thanks for getting this started @dominikschubert!

@bblommers bblommers closed this Apr 29, 2024
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

2 participants