Skip to content

Commit 550df47

Browse files
brycahtabrycahta
authored
Refactor customUpdate hooks (#105)
* refactor: rename hook.go to hooks.go * refactor: make customUpdate hook code consistent * refactor: align customUpdateSecurityGroup code with other resources * refactor: sdkFindRules into getRules
1 parent 0015eb0 commit 550df47

File tree

18 files changed

+149
-84
lines changed

18 files changed

+149
-84
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
ack_generate_info:
2-
build_date: "2022-09-27T22:29:46Z"
2+
build_date: "2022-09-30T18:33:33Z"
33
build_hash: 6e2ffbc3b16a30ac59be6719918c601c2c864064
44
go_version: go1.18.1
55
version: v0.20.1-3-g6e2ffbc
66
api_directory_checksum: 127aa0f51fbcdde596b8f42f93bcdf3b6dee8488
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.44.93
99
generator_config_info:
10-
file_checksum: a31177552ce6ea5ce3b624520711382f113cf05b
10+
file_checksum: 100888fa9c8d854c46c386442c6e7e06e34940cf
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation

‎apis/v1alpha1/generator.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ resources:
453453
custom_method_name: customUpdateTransitGateway
454454
Vpc:
455455
update_operation:
456-
custom_method_name: customUpdate
456+
custom_method_name: customUpdateVPC
457457
exceptions:
458458
errors:
459459
404:

‎generator.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ resources:
453453
custom_method_name: customUpdateTransitGateway
454454
Vpc:
455455
update_operation:
456-
custom_method_name: customUpdate
456+
custom_method_name: customUpdateVPC
457457
exceptions:
458458
errors:
459459
404:

‎pkg/resource/dhcp_options/hook.go renamed to ‎pkg/resource/dhcp_options/hooks.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,20 @@ func (rm *resourceManager) customUpdateDHCPOptions(
3232
exit := rlog.Trace("rm.customUpdateDHCPOptions")
3333
defer exit(err)
3434

35-
// Merge in the information we read from the API call above to the copy of
36-
// the original Kubernetes object we passed to the function
37-
ko := desired.ko.DeepCopy()
35+
// Default `updated` to `desired` because it is likely
36+
// EC2 `modify` APIs do NOT return output, only errors.
37+
// If the `modify` calls (i.e. `sync`) do NOT return
38+
// an error, then the update was successful and desired.Spec
39+
// (now updated.Spec) reflects the latest resource state.
40+
updated = desired
3841

3942
if delta.DifferentAt("Spec.Tags") {
4043
if err := rm.syncTags(ctx, desired, latest); err != nil {
4144
return nil, err
4245
}
4346
}
4447

45-
rm.setStatusDefaults(ko)
46-
return &resource{ko}, nil
48+
return updated, nil
4749
}
4850

4951
// syncTags used to keep tags in sync by calling Create and Delete API's

‎pkg/resource/elastic_ip_address/hook.go renamed to ‎pkg/resource/elastic_ip_address/hooks.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,20 @@ func (rm *resourceManager) customUpdateElasticIP(
3232
exit := rlog.Trace("rm.customUpdateElasticIP")
3333
defer exit(err)
3434

35-
// Merge in the information we read from the API call above to the copy of
36-
// the original Kubernetes object we passed to the function
37-
ko := desired.ko.DeepCopy()
35+
// Default `updated` to `desired` because it is likely
36+
// EC2 `modify` APIs do NOT return output, only errors.
37+
// If the `modify` calls (i.e. `sync`) do NOT return
38+
// an error, then the update was successful and desired.Spec
39+
// (now updated.Spec) reflects the latest resource state.
40+
updated = desired
3841

3942
if delta.DifferentAt("Spec.Tags") {
4043
if err := rm.syncTags(ctx, desired, latest); err != nil {
4144
return nil, err
4245
}
4346
}
4447

45-
rm.setStatusDefaults(ko)
46-
return &resource{ko}, nil
48+
return updated, nil
4749
}
4850

4951
// syncTags used to keep tags in sync by calling Create and Delete API's

‎pkg/resource/instance/hooks.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,20 @@ func (rm *resourceManager) customUpdateInstance(
4545
exit := rlog.Trace("rm.customUpdateInstance")
4646
defer exit(err)
4747

48-
// Merge in the information we read from the API call above to the copy of
49-
// the original Kubernetes object we passed to the function
50-
ko := desired.ko.DeepCopy()
48+
// Default `updated` to `desired` because it is likely
49+
// EC2 `modify` APIs do NOT return output, only errors.
50+
// If the `modify` calls (i.e. `sync`) do NOT return
51+
// an error, then the update was successful and desired.Spec
52+
// (now updated.Spec) reflects the latest resource state.
53+
updated = desired
5154

5255
if delta.DifferentAt("Spec.Tags") {
5356
if err := rm.syncTags(ctx, desired, latest); err != nil {
5457
return nil, err
5558
}
5659
}
5760

58-
rm.setStatusDefaults(ko)
59-
return &resource{ko}, nil
61+
return updated, nil
6062
}
6163

6264
// syncTags used to keep tags in sync by calling Create and Delete API's

‎pkg/resource/internet_gateway/hook.go renamed to ‎pkg/resource/internet_gateway/hooks.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,12 @@ func (rm *resourceManager) customUpdateInternetGateway(
3434
exit(err)
3535
}(err)
3636

37-
ko := desired.ko.DeepCopy()
38-
rm.setStatusDefaults(ko)
37+
// Default `updated` to `desired` because it is likely
38+
// EC2 `modify` APIs do NOT return output, only errors.
39+
// If the `modify` calls (i.e. `sync`) do NOT return
40+
// an error, then the update was successful and desired.Spec
41+
// (now updated.Spec) reflects the latest resource state.
42+
updated = desired
3943

4044
if delta.DifferentAt("Spec.VPC") {
4145
if latest.ko.Spec.VPC != nil {
@@ -56,7 +60,7 @@ func (rm *resourceManager) customUpdateInternetGateway(
5660
}
5761
}
5862

59-
return &resource{ko}, nil
63+
return updated, nil
6064
}
6165

6266
// getAttachedVPC will attempt to find the VPCID for any VPC that the

‎pkg/resource/nat_gateway/hook.go renamed to ‎pkg/resource/nat_gateway/hooks.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,20 @@ func (rm *resourceManager) customUpdateNATGateway(
3232
exit := rlog.Trace("rm.customUpdateNATGateway")
3333
defer exit(err)
3434

35-
// Merge in the information we read from the API call above to the copy of
36-
// the original Kubernetes object we passed to the function
37-
ko := desired.ko.DeepCopy()
35+
// Default `updated` to `desired` because it is likely
36+
// EC2 `modify` APIs do NOT return output, only errors.
37+
// If the `modify` calls (i.e. `sync`) do NOT return
38+
// an error, then the update was successful and desired.Spec
39+
// (now updated.Spec) reflects the latest resource state.
40+
updated = desired
3841

3942
if delta.DifferentAt("Spec.Tags") {
4043
if err := rm.syncTags(ctx, desired, latest); err != nil {
4144
return nil, err
4245
}
4346
}
4447

45-
rm.setStatusDefaults(ko)
46-
return &resource{ko}, nil
48+
return updated, nil
4749
}
4850

4951
// syncTags used to keep tags in sync by calling Create and Delete API's

‎pkg/resource/route_table/hooks.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,23 @@ func (rm *resourceManager) customUpdateRouteTable(
191191
exit := rlog.Trace("rm.customUpdateRouteTable")
192192
defer exit(err)
193193

194-
ko := desired.ko.DeepCopy()
195-
rm.setStatusDefaults(ko)
194+
// Default `updated` to `desired` because it is likely
195+
// EC2 `modify` APIs do NOT return output, only errors.
196+
// If the `modify` calls (i.e. `sync`) do NOT return
197+
// an error, then the update was successful and desired.Spec
198+
// (now updated.Spec) reflects the latest resource state.
199+
updated = desired
196200

197201
if delta.DifferentAt("Spec.Routes") {
198202
if err := rm.syncRoutes(ctx, desired, latest); err != nil {
199203
return nil, err
200204
}
205+
// A ReadOne call is made to refresh Status.RouteStatuses
206+
// with the recently-updated data from the above `sync` call
207+
updated, err = rm.sdkFind(ctx, desired)
208+
if err != nil {
209+
return nil, err
210+
}
201211
}
202212

203213
if delta.DifferentAt("Spec.Tags") {
@@ -206,12 +216,7 @@ func (rm *resourceManager) customUpdateRouteTable(
206216
}
207217
}
208218

209-
latest, err = rm.sdkFind(ctx, latest)
210-
if err != nil {
211-
return nil, err
212-
}
213-
214-
return latest, nil
219+
return updated, nil
215220
}
216221

217222
func (rm *resourceManager) requiredFieldsMissingForCreateRoute(

‎pkg/resource/security_group/hook.go renamed to ‎pkg/resource/security_group/hooks.go

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -52,47 +52,46 @@ func (rm *resourceManager) addRulesToSpec(
5252
}
5353
}
5454

55-
// addRulesToStatus updates a resource's Status Rules
56-
// by calling DescribeSecurityGroupRules and using data
57-
// from the response
58-
func (rm *resourceManager) addRulesToStatus(
59-
ko *svcapitypes.SecurityGroup,
55+
// getRules calls DescribeSecurityGroupRules
56+
// and returns the response data to populate a Security Group's
57+
// Status.Rules
58+
func (rm *resourceManager) getRules(
6059
ctx context.Context,
61-
) (err error) {
60+
res *resource,
61+
) (rules []*svcapitypes.SecurityGroupRule, err error) {
6262
rlog := ackrtlog.FromContext(ctx)
63-
exit := rlog.Trace("rm.addRulesToStatus")
63+
exit := rlog.Trace("rm.getRules")
6464
defer exit(err)
6565

6666
groupIDFilter := "group-id"
6767
input := &svcsdk.DescribeSecurityGroupRulesInput{
6868
Filters: []*svcsdk.Filter{
6969
{
7070
Name: &groupIDFilter,
71-
Values: []*string{ko.Status.ID},
71+
Values: []*string{res.ko.Status.ID},
7272
},
7373
},
7474
}
75-
rulesForResource := []*svcapitypes.SecurityGroupRule{}
75+
7676
for {
7777
resp, err := rm.sdkapi.DescribeSecurityGroupRulesWithContext(ctx, input)
7878
rm.metrics.RecordAPICall("READ_MANY", "DescribeSecurityGroupRules", err)
7979
if err != nil || resp == nil {
8080
break
8181
}
8282
for _, sgRule := range resp.SecurityGroupRules {
83-
rulesForResource = append(rulesForResource, rm.setResourceSecurityGroupRule(sgRule))
83+
rules = append(rules, rm.setResourceSecurityGroupRule(sgRule))
8484
}
8585
if resp.NextToken == nil || *resp.NextToken == "" {
8686
break
8787
}
8888
input.SetNextToken(*resp.NextToken)
8989
}
9090
if err != nil {
91-
return err
91+
return nil, err
9292
}
9393

94-
ko.Status.Rules = rulesForResource
95-
return nil
94+
return rules, nil
9695
}
9796

9897
func (rm *resourceManager) requiredFieldsMissingForSGRule(
@@ -153,9 +152,6 @@ func (rm *resourceManager) syncSGRules(
153152
if err = rm.createSecurityGroupRules(ctx, desired, toAddIngress, toAddEgress); err != nil {
154153
return err
155154
}
156-
if err = rm.addRulesToStatus(desired.ko, ctx); err != nil {
157-
return err
158-
}
159155

160156
return nil
161157
}
@@ -418,13 +414,26 @@ func (rm *resourceManager) customUpdateSecurityGroup(
418414
rlog := ackrtlog.FromContext(ctx)
419415
exit := rlog.Trace("rm.customUpdateSecurityGroup")
420416
defer exit(err)
421-
ko := desired.ko.DeepCopy()
422-
rm.setStatusDefaults(ko)
417+
418+
// Default `updated` to `desired` because it is likely
419+
// EC2 `modify` APIs do NOT return output, only errors.
420+
// If the `modify` calls (i.e. `sync`) do NOT return
421+
// an error, then the update was successful and desired.Spec
422+
// (now updated.Spec) reflects the latest resource state.
423+
updated = desired
423424

424425
if delta.DifferentAt("Spec.IngressRules") || delta.DifferentAt("Spec.EgressRules") {
425426
if err := rm.syncSGRules(ctx, desired, latest); err != nil {
426427
return nil, err
427428
}
429+
// A ReadOne call for SecurityGroup Rules (NOT SecurityGroups)
430+
// is made to refresh Status.Rules with the recently-updated
431+
// data from the above `sync` call
432+
if rules, err := rm.getRules(ctx, desired); err != nil {
433+
return nil, err
434+
} else {
435+
updated.ko.Status.Rules = rules
436+
}
428437
}
429438

430439
if delta.DifferentAt("Spec.Tags") {
@@ -433,7 +442,7 @@ func (rm *resourceManager) customUpdateSecurityGroup(
433442
}
434443
}
435444

436-
return desired, nil
445+
return updated, nil
437446
}
438447

439448
// defaultEgressRule returns the egress rule that

0 commit comments

Comments
 (0)