Skip to content

Commit c3e4352

Browse files
LikithaVemulapalliLikithaVemulapalli
authored
Deleted default egress rule upon SG creation (#135)
Issue #, if available: aws-controllers-k8s/community#1766 Description of changes: As per the current functionality, default Egress Rule is attached automatically upon creation of Security Group. Modified code to delete the default egress rule that's created even if user doesn't specify any egress rule. Modified the e2e tests to match this requirement. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 615644e commit c3e4352

File tree

8 files changed

+25
-60
lines changed

8 files changed

+25
-60
lines changed

‎apis/v1alpha1/ack-generate-metadata.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
ack_generate_info:
2-
build_date: "2023-02-09T23:51:42Z"
2+
build_date: "2023-04-25T13:24:38Z"
33
build_hash: d0f3d78cbea8061f822cbceac3786128f091efe6
4-
go_version: go1.19
4+
go_version: go1.19.6
55
version: v0.24.2
66
api_directory_checksum: 6e86aa4f26729ce014485b1096286db654459af7
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.44.93
99
generator_config_info:
10-
file_checksum: d9d0156fc1156be66ef8542caa31686764629ad7
10+
file_checksum: cc2c6590c6e77a6125d5eec82ff5f693109d4f99
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation

‎apis/v1alpha1/generator.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ resources:
419419
custom_field:
420420
list_of: IpPermission
421421
EgressRules:
422-
late_initialize: {}
423422
custom_field:
424423
list_of: IpPermission
425424
Rules:

‎generator.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ resources:
419419
custom_field:
420420
list_of: IpPermission
421421
EgressRules:
422-
late_initialize: {}
423422
custom_field:
424423
list_of: IpPermission
425424
Rules:

‎helm/crds/services.k8s.aws_adoptedresources.yaml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,10 @@ spec:
145145
blockOwnerDeletion:
146146
description: If true, AND if the owner has the "foregroundDeletion"
147147
finalizer, then the owner cannot be deleted from the
148-
key-value store until this reference is removed. Defaults
148+
key-value store until this reference is removed. See
149+
https://kubernetes.io/docs/concepts/architecture/garbage-collection/#foreground-deletion
150+
for how the garbage collector interacts with this
151+
field and enforces the foreground deletion. Defaults
149152
to false. To set this field, a user needs "delete"
150153
permission of the owner, otherwise 422 (Unprocessable
151154
Entity) will be returned.

‎pkg/resource/security_group/manager.go‎

Lines changed: 2 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/resource/security_group/sdk.go‎

Lines changed: 3 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎templates/hooks/security_group/sdk_create_post_set_output.go.tpl‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@
33
return nil, ackerr.NotFound
44
}
55

6-
// if user defines any egress rule, then remove the default egress rule
7-
if len(desired.ko.Spec.EgressRules) > 0 {
8-
if err = rm.deleteDefaultSecurityGroupRule(ctx, &resource{ko}); err != nil {
9-
return nil, err
10-
}
6+
// Delete the default egress rule
7+
if err = rm.deleteDefaultSecurityGroupRule(ctx, &resource{ko}); err != nil {
8+
return nil, err
119
}
1210

1311
if err = rm.syncSGRules(ctx, &resource{ko}, nil); err != nil {

‎test/e2e/tests/test_security_group.py‎

Lines changed: 10 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -159,33 +159,18 @@ def test_create_delete(self, ec2_client, simple_security_group):
159159
# Check Security Group no longer exists in AWS
160160
ec2_validator.assert_security_group(resource_id, exists=False)
161161

162-
def test_create_with_vpc_egress_dups_default_delete(self, ec2_client, security_group_with_vpc):
162+
def test_create_with_vpc_add_egress_rule(self, ec2_client, security_group_with_vpc):
163163
(ref, cr) = security_group_with_vpc
164164
resource_id = cr["status"]["id"]
165165

166-
# Check resource is late initialized successfully (sets default egress rule)
166+
# Check resource is synced successfully
167167
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
168168

169169
# Check Security Group exists in AWS
170170
ec2_validator = EC2Validator(ec2_client)
171171
ec2_validator.assert_security_group(resource_id)
172172

173-
# Hook code should update Spec rules using data from ReadOne resp
174-
assert len(cr["spec"]["egressRules"]) == 1
175-
176-
# Check default egress rule present
177-
# default egress rule will be present iff user has NOT specified their own egress rules
178-
assert len(cr["status"]["rules"]) == 1
179-
sg_group = ec2_validator.get_security_group(resource_id)
180-
egress_rules = sg_group["IpPermissionsEgress"]
181-
assert len(egress_rules) == 1
182-
logging.debug(f"Default Egress rule: {str(egress_rules[0])}")
183-
184-
# Check default egress rule data
185-
assert egress_rules[0]["IpProtocol"] == "-1"
186-
assert egress_rules[0]["IpRanges"][0]["CidrIp"] == "0.0.0.0/0"
187-
188-
# Add a new Egress rule that "duplicates" the default via patch
173+
# Add a new Egress rule via patch
189174
new_egress_rule = {
190175
"ipProtocol": "-1",
191176
"ipRanges": [{
@@ -210,7 +195,7 @@ def test_create_with_vpc_egress_dups_default_delete(self, ec2_client, security_g
210195
assert len(sg_group["IpPermissions"]) == 0
211196
assert len(sg_group["IpPermissionsEgress"]) == 1
212197

213-
# Check egress rule data (i.e. ensure default egress rule removed)
198+
# Check egress rule data
214199
assert sg_group["IpPermissionsEgress"][0]["IpProtocol"] == "-1"
215200
assert len(sg_group["IpPermissionsEgress"][0]["IpRanges"]) == 1
216201
ip_range = sg_group["IpPermissionsEgress"][0]["IpRanges"][0]
@@ -239,7 +224,7 @@ def test_rules_create_update_delete(self, ec2_client, simple_security_group):
239224
(ref, cr) = simple_security_group
240225
resource_id = cr["status"]["id"]
241226

242-
# Check resource is late initialized successfully (sets default egress rule)
227+
# Check resource is synced successfully
243228
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
244229

245230
# Check Security Group exists in AWS
@@ -248,18 +233,11 @@ def test_rules_create_update_delete(self, ec2_client, simple_security_group):
248233

249234
# Hook code should update Spec rules using data from ReadOne resp
250235
assert len(cr["spec"]["ingressRules"]) == 1
251-
assert len(cr["spec"]["egressRules"]) == 1
252236

253-
# Check ingress rule added and default egress rule present
254-
# default egress rule will be present iff user has NOT specified their own egress rules
255-
assert len(cr["status"]["rules"]) == 2
237+
# Check ingress rule added
238+
assert len(cr["status"]["rules"]) == 1
256239
sg_group = ec2_validator.get_security_group(resource_id)
257240
assert len(sg_group["IpPermissions"]) == 1
258-
assert len(sg_group["IpPermissionsEgress"]) == 1
259-
260-
# Check default egress rule data
261-
assert sg_group["IpPermissionsEgress"][0]["IpProtocol"] == "-1"
262-
assert sg_group["IpPermissionsEgress"][0]["IpRanges"][0]["CidrIp"] == "0.0.0.0/0"
263241

264242
# Add Egress rule via patch
265243
new_egress_rule = {
@@ -269,7 +247,7 @@ def test_rules_create_update_delete(self, ec2_client, simple_security_group):
269247
"ipRanges": [
270248
{
271249
"cidrIP": "172.31.0.0/16",
272-
"description": "test egress update"
250+
"description": "test egress"
273251
}
274252
]
275253
}
@@ -282,16 +260,15 @@ def test_rules_create_update_delete(self, ec2_client, simple_security_group):
282260
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
283261

284262
# Check ingress and egress rules exist
285-
assert len(cr["status"]["rules"]) == 2
286263
sg_group = ec2_validator.get_security_group(resource_id)
287264
assert len(sg_group["IpPermissions"]) == 1
288265
assert len(sg_group["IpPermissionsEgress"]) == 1
289266

290-
# Check egress rule data (i.e. ensure default egress rule removed)
267+
# Check egress rule data
291268
assert sg_group["IpPermissionsEgress"][0]["IpProtocol"] == "tcp"
292269
assert sg_group["IpPermissionsEgress"][0]["FromPort"] == 25
293270
assert sg_group["IpPermissionsEgress"][0]["ToPort"] == 25
294-
assert sg_group["IpPermissionsEgress"][0]["IpRanges"][0]["Description"] == "test egress update"
271+
assert sg_group["IpPermissionsEgress"][0]["IpRanges"][0]["Description"] == "test egress"
295272

296273
# Remove Ingress rule
297274
patch = {"spec": {"ingressRules":[]}}

0 commit comments

Comments
 (0)