Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Add more validations to Security Groups.
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff-3153027-12.txt | 1.45 KB | baldwinlouie |
#12 | 3153027-12.patch | 125.19 KB | baldwinlouie |
|
Comments
Comment #2
baldwinlouie CreditAttribution: baldwinlouie commentedAttaching the first pass of the patch. It includes a bunch of things
- Validates whether the Security Group name is "default". This is a reserved term and errors out if sent to AWS
- Validates duplicate Security Group names.
- Validates duplicate permissions. It takes the to_port/from_port and ip_address, group_name, prefix_id, and checks the rule being added is a duplicate.
- Properly supporting prefix list id. Prefix_id is similar to Group, where you can have a IP permission against a prefix list id.
- Validates when adding a group_id based permission, that the group_id is in the same VPC as the Security Group.
- Validates when adding a group_id based permission, that the group_id actually exists.
- Validates ICMP ports
- Validates ICMPv6 ports
- Fixes a bug in IpPermission.php
return $this->get('peering_connection_id')->value();
, which should be < return $this->get('peering_connection_id')->getValue();- Slightly refactor
Ec2Service.php
- Refactor the code in
SecurityGroupEditForm.php
- Added unit tests for Security Group Name
- Added unit tests for Port validation
There's a few things to note in the patch.
- I re-abled the following for Security
group_name
. This way, I can add a constraint to the field. Because of this, you'll see references togroup_name
as$add[$i]['group_name[0][value]']
in the Unit Tests.- In the tests, I had to remove "group" from any permission unit test. That is because the validation requires a valid security group. I'd have to rewrite all those tests to accommodate that.
- I can't add unit tests for duplicate permissions. That is because adding a second IP permission means using the "Add Rule" button, which is Ajax based.
Comment #4
baldwinlouie CreditAttribution: baldwinlouie commentedUpdating with Coder fixes.
Comment #6
baldwinlouie CreditAttribution: baldwinlouie commentedAttaching patch that fixes the tests.
Comment #7
baldwinlouie CreditAttribution: baldwinlouie commentedNew patch that fixes the openstack tests.
Comment #8
baldwinlouie CreditAttribution: baldwinlouie commentedOne more try. Coder complained about K8sCloudConfigTest.php
Comment #9
yas@baldwinlouie
The coder complaint was my mistake. I fixed and merged the patch at #3153103-10 before I came here.
Comment #10
baldwinlouie CreditAttribution: baldwinlouie commented@yas, no problem!
Comment #11
baldwinlouie CreditAttribution: baldwinlouie commentedUpdating the patch which fixes the issue with the
public function isEmpty()
inIpPermission.php
Comment #12
baldwinlouie CreditAttribution: baldwinlouie commentedupdating the
public function isEmpty()
to use a switch statement.Comment #15
yas