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

VPCEndpoint: add security group modification #7515

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SoenkeD
Copy link
Contributor

@SoenkeD SoenkeD commented Mar 24, 2024

Currently it is not possible to add / remove security groups for VPCEndpoints (with ModifyVpcEndpoint() ). The changes of this PR allow this and also load the SecurityGroup names when DescribeVpcEndpoint().

Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @SoenkeD , it looks like the linter is currently failing.

I think this change makes sense, but I'll have another look once the build passes.

@SoenkeD SoenkeD force-pushed the feature/VPCEndpoint_security_groups branch 2 times, most recently from 3733040 to 4854902 Compare May 3, 2024 09:02
fix linting

fix none list issue
@SoenkeD SoenkeD force-pushed the feature/VPCEndpoint_security_groups branch from 4854902 to c39e738 Compare May 3, 2024 09:39
@SoenkeD
Copy link
Contributor Author

SoenkeD commented May 3, 2024

Hi @bblommers ,
the main issue was my ruff version. Now it fails in test_ec2_cloudformation which I did not touch. Do you have a guess for the cause?


security_group_ids = []
for service in vpc_end_points:
security_group_ids.extend(service.security_group_ids)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failing error is because service.security_group_ids can be None - so you're effectively running security_group_ids.extend(None), which throws the TypeError: 'NoneType' object is not iterable that you can see in the logs.

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.

2 participants