fix(networkacl): Correct update order and entries/associations handlers (#202)

The custom update logic for `NetworkACL` resources was syncing the `Entries` before
the `Tags`. This could lead to issues if the `Entries` sync failed as the Tags
would not be updated..

This change also updates the order to sync `Tags` before `Entries`. It also uses
`DeepCopy` when passing the resource to `createAssociation` and `createEntries
to avoid modifying the original desired state.

Additionally, the exit functions in `hooks.go` were fixed to properly
handle the error return value.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: a-hilaly

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,5 +1,5 @@
 ack_generate_info:
-  build_date: "2024-06-19T06:55:08Z"
+  build_date: "2024-06-19T08:11:53Z"
   build_hash: 14cef51778d471698018b6c38b604181a6948248
   go_version: go1.22.4
   version: v0.34.0

pkg/resource/network_acl/hooks.go

@@ -39,7 +39,7 @@ func (rm *resourceManager) customUpdateNetworkAcl(
 ) (updated *resource, err error) {
 	rlog := ackrtlog.FromContext(ctx)
 	exit := rlog.Trace("rm.customUpdateNetworkAcl")
-	defer exit(err)
+	defer func(err error) { exit(err) }(err)
 
 	// Default `updated` to `desired` because it is likely
 	// EC2 `modify` APIs do NOT return output, only errors.
@@ -48,12 +48,6 @@ func (rm *resourceManager) customUpdateNetworkAcl(
 	// (now updated.Spec) reflects the latest resource state.
 	updated = rm.concreteResource(desired.DeepCopy())
 
-	if delta.DifferentAt("Spec.Entries") {
-		if err := rm.syncEntries(ctx, desired, latest); err != nil {
-			return nil, err
-		}
-	}
-
 	if delta.DifferentAt("Spec.Tags") {
 		if err := tags.Sync(
 			ctx, rm.sdkapi, rm.metrics, *latest.ko.Status.ID,
@@ -63,6 +57,12 @@ func (rm *resourceManager) customUpdateNetworkAcl(
 		}
 	}
 
+	if delta.DifferentAt("Spec.Entries") {
+		if err := rm.syncEntries(ctx, desired, latest); err != nil {
+			return nil, err
+		}
+	}
+
 	if delta.DifferentAt("Spec.Associations") {
 		if err := rm.syncAssociation(ctx, desired, latest); err != nil {
 			return nil, err
@@ -297,7 +297,8 @@ func (rm *resourceManager) syncEntries(
 ) (err error) {
 	rlog := ackrtlog.FromContext(ctx)
 	exit := rlog.Trace("rm.syncEntries")
-	defer exit(err)
+	defer func(err error) { exit(err) }(err)
+
 	toAdd := []*svcapitypes.NetworkACLEntry{}
 	toDelete := []*svcapitypes.NetworkACLEntry{}
 	toUpdate := []*svcapitypes.NetworkACLEntry{}

pkg/resource/network_acl/sdk.go

@@ -4,15 +4,17 @@
 
 	if len(desired.ko.Spec.Associations) > 0 {
 		ko.Spec.Associations = desired.ko.Spec.Associations
-		if err := rm.createAssociation(ctx, &resource{ko}); err != nil {
+		copy := ko.DeepCopy()
+		if err := rm.createAssociation(ctx, &resource{copy}); err != nil {
 			rlog.Debug("Error while syncing Association", err)
 		}
 	}
 
 	if len(desired.ko.Spec.Entries) > 0 {
 		//desired rules are overwritten by NetworkACL's default rules
 		ko.Spec.Entries = append(ko.Spec.Entries, desired.ko.Spec.Entries...)
-		if err := rm.createEntries(ctx, &resource{ko}); err != nil {
+		copy := ko.DeepCopy()
+		if err := rm.createEntries(ctx, &resource{copy}); err != nil {
 			rlog.Debug("Error while syncing routes", err)
 		}
 	}