Fix updateTags support for Subnet (#92)

Description of changes:

- Removed unused `compareTag` function and changed key-value names to be `TAG_KEY` and `TAG_VALUE` in e2e tests.
- Removed `createTags`, `deleteTags` functions and added `sdkTags` function to reduce the number of API calls.

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

Author: LikithaVemulapalli

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,5 +1,5 @@
 ack_generate_info:
-  build_date: "2022-09-08T18:33:19Z"
+  build_date: "2022-09-13T16:18:40Z"
   build_hash: 585f06bbd6d4cc1b738acb85901e7a009bf452c7
   go_version: go1.18.3
   version: v0.20.0

pkg/resource/subnet/hooks.go

@@ -262,73 +262,57 @@ func (rm *resourceManager) syncTags(
 		exit(err)
 	}(err)
 
+	resourceId := []*string{latest.ko.Status.SubnetID}
+
 	toAdd, toDelete := computeTagsDelta(
 		desired.ko.Spec.Tags, latest.ko.Spec.Tags,
 	)
 
-	if err = rm.deleteTags(ctx, latest, toDelete); err != nil {
-		return err
-	}
+	if len(toDelete) > 0 {
+		rlog.Debug("removing tags from subnet resource", "tags", toDelete)
+		_, err = rm.sdkapi.DeleteTagsWithContext(
+			ctx,
+			&svcsdk.DeleteTagsInput{
+				Resources: resourceId,
+				Tags:      rm.sdkTags(toDelete),
+			},
+		)
+		rm.metrics.RecordAPICall("UPDATE", "DeleteTags", err)
+		if err != nil {
+			return err
+		}
 
-	if err = rm.createTags(ctx, desired, toAdd); err != nil {
-		return err
 	}
 
-	return nil
-}
-
-// createTags function creates tags for subnet resource using CreateTags API calls
-func (rm *resourceManager) createTags(
-	ctx context.Context,
-	r *resource,
-	tags []*svcapitypes.Tag,
-) (err error) {
-	rlog := ackrtlog.FromContext(ctx)
-	exit := rlog.Trace("rm.createTags")
-	defer exit(err)
-
-	resourceId := []*string{r.ko.Status.SubnetID}
-
-	for _, i := range tags {
-		toAdd := rm.newTag(*i)
-		req := &svcsdk.CreateTagsInput{
-			Resources: resourceId,
-			Tags:      []*svcsdk.Tag{toAdd},
-		}
-		_, err := rm.sdkapi.CreateTagsWithContext(ctx, req)
-		rm.metrics.RecordAPICall("CREATE", "CreateTags", err)
+	if len(toAdd) > 0 {
+		rlog.Debug("adding tags to subnet resource", "tags", toAdd)
+		_, err = rm.sdkapi.CreateTagsWithContext(
+			ctx,
+			&svcsdk.CreateTagsInput{
+				Resources: resourceId,
+				Tags:      rm.sdkTags(toAdd),
+			},
+		)
+		rm.metrics.RecordAPICall("UPDATE", "CreateTags", err)
 		if err != nil {
 			return err
 		}
 	}
-	return err
+
+	return nil
 }
 
-// deleteTags function deletes tags from subnet resource using DeleteTags API calls
-func (rm *resourceManager) deleteTags(
-	ctx context.Context,
-	r *resource,
+// sdkTags converts *svcapitypes.Tag array to a *svcsdk.Tag array
+func (rm *resourceManager) sdkTags(
 	tags []*svcapitypes.Tag,
-) (err error) {
-	rlog := ackrtlog.FromContext(ctx)
-	exit := rlog.Trace("rm.deleteTags")
-	defer exit(err)
-
-	resourceId := []*string{r.ko.Status.SubnetID}
+) (sdktags []*svcsdk.Tag) {
 
 	for _, i := range tags {
-		toDelete := rm.newTag(*i)
-		req := &svcsdk.DeleteTagsInput{
-			Resources: resourceId,
-			Tags:      []*svcsdk.Tag{toDelete},
-		}
-		_, err = rm.sdkapi.DeleteTagsWithContext(ctx, req)
-		rm.metrics.RecordAPICall("DELETE", "DeleteTags", err)
-		if err != nil {
-			return err
-		}
+		sdktag := rm.newTag(*i)
+		sdktags = append(sdktags, sdktag)
 	}
-	return err
+
+	return sdktags
 }
 
 // computeTagsDelta returns tags to be added and removed from the resource

pkg/resource/subnet/sdk.go

@@ -306,7 +306,7 @@ func (rm *resourceManager) syncTags(
 	)
 
 	if len(toDelete) > 0 {
-		rlog.Debug("removing tags from parameter group", "tags", toDelete)
+		rlog.Debug("removing tags from vpc resource", "tags", toDelete)
 		_, err = rm.sdkapi.DeleteTagsWithContext(
 			ctx,
 			&svcsdk.DeleteTagsInput{
@@ -322,7 +322,7 @@ func (rm *resourceManager) syncTags(
 	}
 
 	if len(toAdd) > 0 {
-		rlog.Debug("adding tags to parameter group", "tags", toAdd)
+		rlog.Debug("adding tags to vpc resource", "tags", toAdd)
 		_, err = rm.sdkapi.CreateTagsWithContext(
 			ctx,
 			&svcsdk.CreateTagsInput{

pkg/resource/vpc/hook.go

@@ -10,15 +10,6 @@
 {{- if eq $subnetRefName "Tags" }}
 {{- $subnetRef := $subnetMemberRefs.Shape.MemberRef }}
 {{- $subnetRefName = "Tag" }}
-func compare{{ $subnetRefName }}(
-	    a *svcapitypes.{{ $subnetRefName }},
-	    b *svcapitypes.{{ $subnetRefName }},
-) *ackcompare.Delta {
-	delta := ackcompare.NewDelta()
-{{ GoCodeCompareStruct $CRD $subnetRef.Shape "delta" "a" "b" $subnetRefName 1 }}
-	return delta
-}
-
 func (rm *resourceManager) new{{ $subnetRefName }}(
 	    c svcapitypes.{{ $subnetRefName }},
 ) *svcsdk.{{ $subnetRefName }} {

templates/hooks/subnet/sdk_file_end.go.tpl

@@ -6,5 +6,5 @@ spec:
   cidrBlock: $CIDR_BLOCK
   vpcID: $VPC_ID
   tags:
-    - key: $KEY
-      value: $VALUE
+    - key: $TAG_KEY
+      value: $TAG_VALUE

test/e2e/resources/subnet.yaml

@@ -147,8 +147,8 @@ def test_crud_tags(self, ec2_client):
         test_resource_values["VPC_ID"] = vpc_id
         # CIDR needs to be within SharedTestVPC range and not overlap other subnets
         test_resource_values["CIDR_BLOCK"] = "10.0.255.0/24"
-        test_resource_values["KEY"] = "initialtagkey"
-        test_resource_values["VALUE"] = "initialtagvalue"
+        test_resource_values["TAG_KEY"] = "initialtagkey"
+        test_resource_values["TAG_VALUE"] = "initialtagvalue"
 
         # Load Subnet CR
         resource_data = load_ec2_resource(