Support tag updates for Certificate resource (#32)

Adding tag update support for `Certificate` resource + e2e tests for the new feature.

> Most of this code is copy-pasted and refined to fit the ACM API, the end goal is
generate all this code after detecting most of the patterns if not all of them.

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,13 +1,13 @@
 ack_generate_info:
-  build_date: "2024-02-14T04:06:57Z"
-  build_hash: 947081ffebdeefcf2c61c4ca6d7e68810bdf9d08
+  build_date: "2024-02-19T08:00:53Z"
+  build_hash: e8313de46f391b8c8044963a1becf781ffd7a326
   go_version: go1.22.0
-  version: v0.30.0
-api_directory_checksum: eabe0fe64d57edf571ba0eb0217fc376f1185cc0
+  version: v0.30.0-6-ge8313de
+api_directory_checksum: e337526dd1438ddb861e06bd92b5f640ba9ed537
 api_version: v1alpha1
 aws_sdk_go_version: v1.49.0
 generator_config_info:
-  file_checksum: 229489e50bc34730f31e2e0578bec6f9ea7d7215
+  file_checksum: 910047b7946c5968bbc58b6334099deb005e95cc
   original_file_name: generator.yaml
 last_modification:
   reason: API generation

apis/v1alpha1/generator.yaml

@@ -17,6 +17,8 @@ operations:
 resources:
   Certificate:
     hooks:
+      sdk_update_pre_build_request:
+        template_path: hooks/certificate/sdk_update_pre_build_request.go.tpl
       sdk_create_pre_build_request:
         template_path: hooks/certificate/sdk_create_pre_build_request.go.tpl
       sdk_create_post_build_request:

generator.yaml

@@ -17,6 +17,8 @@ operations:
 resources:
   Certificate:
     hooks:
+      sdk_update_pre_build_request:
+        template_path: hooks/certificate/sdk_update_pre_build_request.go.tpl
       sdk_create_pre_build_request:
         template_path: hooks/certificate/sdk_create_pre_build_request.go.tpl
       sdk_create_post_build_request:

pkg/resource/certificate/hooks.go

@@ -16,16 +16,11 @@ package certificate
 import (
 	"errors"
 	"fmt"
-	"time"
+
+	"github.com/aws-controllers-k8s/acm-controller/pkg/tags"
 )
 
 const (
-	// See note on
-	// https://docs.aws.amazon.com/acm/latest/APIReference/API_RequestCertificate.html
-	// about DescribeCertificate not being ready to call for several seconds
-	// after a successful RequestCertificate API call...
-	waitSecondsAfterCreate = 5 * time.Second
-
 	// DNS validation only works for up to 5 chained CNAME records
 	limitDomainValidationOptionsPublic = 5
 )
@@ -63,6 +58,7 @@ func validatePublicValidationOptions(
 	return nil
 }
 
-func waitAfterSuccessfulCreate() {
-	time.Sleep(waitSecondsAfterCreate)
-}
+var (
+	syncTags = tags.SyncTags
+	listTags = tags.ListTags
+)

pkg/resource/certificate/sdk.go

@@ -0,0 +1,204 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//     http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package tags
+
+import (
+	"context"
+
+	"github.com/aws-controllers-k8s/acm-controller/apis/v1alpha1"
+	ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
+
+	"github.com/aws/aws-sdk-go/aws/request"
+	svcsdk "github.com/aws/aws-sdk-go/service/acm"
+)
+
+type metricsRecorder interface {
+	RecordAPICall(opType string, opID string, err error)
+}
+
+type tagsClient interface {
+	AddTagsToCertificateWithContext(context.Context, *svcsdk.AddTagsToCertificateInput, ...request.Option) (*svcsdk.AddTagsToCertificateOutput, error)
+	ListTagsForCertificateWithContext(context.Context, *svcsdk.ListTagsForCertificateInput, ...request.Option) (*svcsdk.ListTagsForCertificateOutput, error)
+	RemoveTagsFromCertificateWithContext(context.Context, *svcsdk.RemoveTagsFromCertificateInput, ...request.Option) (*svcsdk.RemoveTagsFromCertificateOutput, error)
+}
+
+// syncTags examines the Tags in the supplied Resource and calls the
+// TagResource and UntagResource APIs to ensure that the set of
+// associated Tags stays in sync with the Resource.Spec.Tags
+func SyncTags(
+	ctx context.Context,
+	client tagsClient,
+	mr metricsRecorder,
+	resourceID string,
+	aTags []*v1alpha1.Tag,
+	bTags []*v1alpha1.Tag,
+) (err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.syncTags")
+	defer func() { exit(err) }()
+
+	desiredTags := map[string]*string{}
+	for _, t := range aTags {
+		desiredTags[*t.Key] = t.Value
+	}
+	existingTags := map[string]*string{}
+	for _, t := range bTags {
+		existingTags[*t.Key] = t.Value
+	}
+
+	toAdd := map[string]*string{}
+	toDelete := map[string]*string{}
+
+	for k, v := range desiredTags {
+		if ev, found := existingTags[k]; !found || *ev != *v {
+			toAdd[k] = v
+		}
+	}
+
+	for k, v := range existingTags {
+		if _, found := desiredTags[k]; !found {
+			toDelete[k] = v
+		}
+	}
+
+	if len(toDelete) > 0 {
+		for k, v := range toDelete {
+			rlog.Debug("removing tag from resource", "key", k, "value", *v)
+		}
+		if err = removeTags(
+			ctx,
+			client,
+			mr,
+			resourceID,
+			toDelete,
+		); err != nil {
+			return err
+		}
+	}
+	if len(toAdd) > 0 {
+		for k, v := range toAdd {
+			rlog.Debug("adding tag to resource", "key", k, "value", *v)
+		}
+		if err = addTags(
+			ctx,
+			client,
+			mr,
+			resourceID,
+			toAdd,
+		); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// addTags adds the supplied Tags to the supplied resource
+func addTags(
+	ctx context.Context,
+	client tagsClient,
+	mr metricsRecorder,
+	resourceARN string,
+	tags map[string]*string,
+) (err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.addTag")
+	defer func() { exit(err) }()
+
+	sdkTags := []*svcsdk.Tag{}
+	for k, v := range tags {
+		k := k
+		sdkTags = append(sdkTags, &svcsdk.Tag{
+			Key:   &k,
+			Value: v,
+		})
+	}
+
+	input := &svcsdk.AddTagsToCertificateInput{
+		CertificateArn: &resourceARN,
+		Tags:           sdkTags,
+	}
+
+	_, err = client.AddTagsToCertificateWithContext(ctx, input)
+	mr.RecordAPICall("UPDATE", "AddTagsToCertificate", err)
+	return err
+}
+
+// removeTags removes the supplied Tags from the supplied resource
+func removeTags(
+	ctx context.Context,
+	client tagsClient,
+	mr metricsRecorder,
+	resourceARN string,
+	tags map[string]*string,
+) (err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.removeTag")
+	defer func() { exit(err) }()
+
+	sdkTags := []*svcsdk.Tag{}
+	for k, v := range tags {
+		k := k
+		sdkTags = append(sdkTags, &svcsdk.Tag{
+			Key:   &k,
+			Value: v,
+		})
+	}
+
+	input := &svcsdk.RemoveTagsFromCertificateInput{
+		CertificateArn: &resourceARN,
+		Tags:           sdkTags,
+	}
+	_, err = client.RemoveTagsFromCertificateWithContext(ctx, input)
+	mr.RecordAPICall("UPDATE", "RemoveTagsFromCertificate", err)
+	return err
+}
+
+// getResourceTagsPagesWithContext queries the list of tags of a given resource.
+func ListTags(
+	ctx context.Context,
+	client tagsClient,
+	mr metricsRecorder,
+	resourceARN string,
+) ([]*v1alpha1.Tag, error) {
+	var err error
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.listTags")
+	defer exit(err)
+
+	var listTagsOfResourceOutput *svcsdk.ListTagsForCertificateOutput
+	listTagsOfResourceOutput, err = client.ListTagsForCertificateWithContext(
+		ctx,
+		&svcsdk.ListTagsForCertificateInput{
+			CertificateArn: &resourceARN,
+		},
+	)
+	mr.RecordAPICall("GET", "ListTagsForCertificate", err)
+	if err != nil {
+		return nil, err
+	}
+	return resourceTagsFromSDKTags(listTagsOfResourceOutput.Tags), nil
+}
+
+// resourceTagsFromSDKTags transforms a *svcsdk.Tag array to a *v1alpha1.Tag array.
+func resourceTagsFromSDKTags(svcTags []*svcsdk.Tag) []*v1alpha1.Tag {
+	tags := make([]*v1alpha1.Tag, len(svcTags))
+	for i := range svcTags {
+		tags[i] = &v1alpha1.Tag{
+			Key:   svcTags[i].Key,
+			Value: svcTags[i].Value,
+		}
+	}
+	return tags
+}

pkg/tags/sync.go

@@ -35,3 +35,10 @@
 	} else {
 		ko.Status.DomainValidations = nil
 	}
+	ko.Spec.Tags, err = listTags(
+		ctx, rm.sdkapi, rm.metrics, 
+		string(*r.ko.Status.ACKResourceMetadata.ARN), 
+	)
+	if err != nil {
+		return nil, err
+	}

templates/hooks/certificate/sdk_read_one_pre_set_output.go.tpl

@@ -0,0 +1,14 @@
+	if delta.DifferentAt("Spec.Tags") {
+		err := syncTags(
+			ctx, rm.sdkapi, rm.metrics, 
+			string(*desired.ko.Status.ACKResourceMetadata.ARN), 
+			desired.ko.Spec.Tags, latest.ko.Spec.Tags,
+		)
+		if err != nil {
+			return nil, err
+		}
+	}
+	// If nothing else has changed, we shouldn't send an update.
+    if !delta.DifferentExcept("Spec.Tags") {
+        return desired, nil
+    }

templates/hooks/certificate/sdk_update_pre_build_request.go.tpl

@@ -118,17 +118,17 @@ def get(certificate_arn):
         return None
 
 
-def get_tags(db_instance_arn):
+def get_tags(certificate_arn):
     """Returns a dict containing the Certificate's tag records from the ACM
     API.
 
     If no such Certificate exists, returns None.
     """
-    c = boto3.client('rds')
+    c = boto3.client('acm')
     try:
-        resp = c.list_tags_for_resource(
-            ResourceName=db_instance_arn,
+        resp = c.list_tags_for_certificate(
+            CertificateArn=certificate_arn,
         )
-        return resp['TagList']
-    except c.exceptions.DBCertificateNotFoundFault:
+        return resp['Tags']
+    except c.exceptions.ResourceNotFoundException:
         return None

test/e2e/certificate.py

@@ -7,5 +7,5 @@ spec:
   # NOTE(jaypipes): Having an empty certificateAuthorityARN field indicates
   # that this is a public certificate request...
   tags:
-    - key: environment
-      value: dev
+  - key: environment
+    value: dev