Merge pull request #211 from ekristen/issue-208

fix: elasticache resources using cluster id vs arn for tag lookup
ekristen · Jul 8, 2024 · c6f4c17 · c6f4c17
2 parents 7e900eb + bc30720
commit c6f4c17
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 6 deletions.
diff --git a/pkg/testsuite/logrus.go b/pkg/testsuite/logrus.go
@@ -0,0 +1,39 @@
+package testsuite
+
+import (
+	"testing"
+
+	"github.com/sirupsen/logrus"
+)
+
+type GlobalHook struct {
+	T  *testing.T
+	TF func(t *testing.T, e *logrus.Entry)
+}
+
+func (h *GlobalHook) Levels() []logrus.Level {
+	return logrus.AllLevels
+}
+
+func (h *GlobalHook) Fire(e *logrus.Entry) error {
+	if h.TF != nil {
+		h.TF(h.T, e)
+	}
+
+	return nil
+}
+
+func (h *GlobalHook) Cleanup() {
+	logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks))
+}
+
+// NewGlobalHook creates a new GlobalHook for logrus testing
+func NewGlobalHook(t *testing.T, tf func(t *testing.T, e *logrus.Entry)) *GlobalHook {
+	gh := &GlobalHook{
+		T:  t,
+		TF: tf,
+	}
+	logrus.SetReportCaller(true)
+	logrus.AddHook(gh)
+	return gh
+}
diff --git a/resources/elasticache-memcacheclusters.go b/resources/elasticache-memcacheclusters.go
@@ -54,7 +54,7 @@ func (l *ElasticacheCacheClusterLister) List(_ context.Context, o interface{}) (
 	var resources []resource.Resource
 	for _, cacheCluster := range resp.CacheClusters {
 		tags, err := svc.ListTagsForResource(&elasticache.ListTagsForResourceInput{
-			ResourceName: cacheCluster.CacheClusterId,
+			ResourceName: cacheCluster.ARN,
 		})
 		if err != nil {
 			logrus.WithError(err).Error("unable to retrieve tags")

diff --git a/resources/elasticache-memcacheclusters_mock_test.go b/resources/elasticache-memcacheclusters_mock_test.go
@@ -2,16 +2,20 @@ package resources
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/elasticache"
 
 	"github.com/ekristen/aws-nuke/v3/mocks/mock_elasticacheiface"
 	"github.com/ekristen/aws-nuke/v3/pkg/nuke"
+	"github.com/ekristen/aws-nuke/v3/pkg/testsuite"
 )
 
 func Test_Mock_ElastiCache_CacheCluster_Remove(t *testing.T) {
@@ -48,14 +52,15 @@ func Test_Mock_ElastiCache_CacheCluster_List_NoTags(t *testing.T) {
 	mockElastiCache.EXPECT().DescribeCacheClusters(gomock.Any()).Return(&elasticache.DescribeCacheClustersOutput{
 		CacheClusters: []*elasticache.CacheCluster{
 			{
+				ARN:                aws.String("arn:aws:elasticache:us-west-2:123456789012:cluster:foobar"),
 				CacheClusterId:     aws.String("foobar"),
 				CacheClusterStatus: aws.String("available"),
 			},
 		},
 	}, nil)
 
 	mockElastiCache.EXPECT().ListTagsForResource(&elasticache.ListTagsForResourceInput{
-		ResourceName: aws.String("foobar"),
+		ResourceName: aws.String("arn:aws:elasticache:us-west-2:123456789012:cluster:foobar"),
 	}).Return(&elasticache.TagListMessage{}, nil)
 
 	resources, err := cacheClusterLister.List(context.TODO(), &nuke.ListerOpts{})
@@ -80,13 +85,14 @@ func Test_Mock_ElastiCache_CacheCluster_List_WithTags(t *testing.T) {
 	mockElastiCache.EXPECT().DescribeCacheClusters(gomock.Any()).Return(&elasticache.DescribeCacheClustersOutput{
 		CacheClusters: []*elasticache.CacheCluster{
 			{
+				ARN:            aws.String("arn:aws:elasticache:us-west-2:123456789012:cluster:foobar"),
 				CacheClusterId: aws.String("foobar"),
 			},
 		},
 	}, nil)
 
 	mockElastiCache.EXPECT().ListTagsForResource(&elasticache.ListTagsForResourceInput{
-		ResourceName: aws.String("foobar"),
+		ResourceName: aws.String("arn:aws:elasticache:us-west-2:123456789012:cluster:foobar"),
 	}).Return(&elasticache.TagListMessage{
 		TagList: []*elasticache.Tag{
 			{
@@ -110,3 +116,47 @@ func Test_Mock_ElastiCache_CacheCluster_List_WithTags(t *testing.T) {
 	a.Equal("foobar", resource.Properties().Get("tag:Name"))
 	a.Equal("test", resource.Properties().Get("tag:aws-nuke"))
 }
+
+func Test_Mock_ElastiCache_CacheCluster_List_TagsInvalidARN(t *testing.T) {
+	called := false
+
+	th := testsuite.NewGlobalHook(t, func(t *testing.T, e *logrus.Entry) {
+		if !strings.HasSuffix(e.Caller.Function, "resources.(*ElasticacheCacheClusterLister).List") {
+			return
+		}
+
+		assert.Equal(t, "unable to retrieve tags", e.Message)
+
+		called = true
+	})
+	defer th.Cleanup()
+
+	a := assert.New(t)
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockElastiCache := mock_elasticacheiface.NewMockElastiCacheAPI(ctrl)
+
+	cacheClusterLister := ElasticacheCacheClusterLister{
+		mockSvc: mockElastiCache,
+	}
+
+	mockElastiCache.EXPECT().DescribeCacheClusters(gomock.Any()).Return(&elasticache.DescribeCacheClustersOutput{
+		CacheClusters: []*elasticache.CacheCluster{
+			{
+				ARN:            aws.String("foobar:invalid:arn"),
+				CacheClusterId: aws.String("foobar"),
+			},
+		},
+	}, nil)
+
+	mockElastiCache.EXPECT().ListTagsForResource(&elasticache.ListTagsForResourceInput{
+		ResourceName: aws.String("foobar:invalid:arn"),
+	}).Return(nil, awserr.New(elasticache.ErrCodeInvalidARNFault, elasticache.ErrCodeInvalidARNFault, nil))
+
+	resources, err := cacheClusterLister.List(context.TODO(), &nuke.ListerOpts{})
+	a.Nil(err)
+	a.Len(resources, 0)
+
+	a.True(called, "expected global hook called and log message to be found")
+}
diff --git a/resources/elasticache-subnetgroups.go b/resources/elasticache-subnetgroups.go
@@ -51,7 +51,7 @@ func (l *ElasticacheSubnetGroupLister) List(_ context.Context, o interface{}) ([
 	var resources []resource.Resource
 	for _, subnetGroup := range resp.CacheSubnetGroups {
 		tags, err := svc.ListTagsForResource(&elasticache.ListTagsForResourceInput{
-			ResourceName: subnetGroup.CacheSubnetGroupName,
+			ResourceName: subnetGroup.ARN,
 		})
 		if err != nil {
 			logrus.WithError(err).Error("unable to retrieve tags")

diff --git a/resources/elasticache-subnetgroups_mock_test.go b/resources/elasticache-subnetgroups_mock_test.go
@@ -49,13 +49,14 @@ func Test_Mock_ElastiCache_SubnetGroup_List_NoTags(t *testing.T) {
 	mockElastiCache.EXPECT().DescribeCacheSubnetGroups(gomock.Any()).Return(&elasticache.DescribeCacheSubnetGroupsOutput{
 		CacheSubnetGroups: []*elasticache.CacheSubnetGroup{
 			{
+				ARN:                  aws.String("arn:aws:elasticache:us-west-2:123456789012:subnet-group:foobar"),
 				CacheSubnetGroupName: aws.String("foobar"),
 			},
 		},
 	}, nil)
 
 	mockElastiCache.EXPECT().ListTagsForResource(&elasticache.ListTagsForResourceInput{
-		ResourceName: aws.String("foobar"),
+		ResourceName: aws.String("arn:aws:elasticache:us-west-2:123456789012:subnet-group:foobar"),
 	}).Return(&elasticache.TagListMessage{}, nil)
 
 	resources, err := subnetGroupsLister.List(context.TODO(), &nuke.ListerOpts{})
@@ -82,13 +83,14 @@ func Test_Mock_ElastiCache_SubnetGroup_List_WithTags(t *testing.T) {
 	mockElastiCache.EXPECT().DescribeCacheSubnetGroups(gomock.Any()).Return(&elasticache.DescribeCacheSubnetGroupsOutput{
 		CacheSubnetGroups: []*elasticache.CacheSubnetGroup{
 			{
+				ARN:                  aws.String("arn:aws:elasticache:us-west-2:123456789012:subnet-group:foobar"),
 				CacheSubnetGroupName: aws.String("foobar"),
 			},
 		},
 	}, nil)
 
 	mockElastiCache.EXPECT().ListTagsForResource(&elasticache.ListTagsForResourceInput{
-		ResourceName: aws.String("foobar"),
+		ResourceName: aws.String("arn:aws:elasticache:us-west-2:123456789012:subnet-group:foobar"),
 	}).Return(&elasticache.TagListMessage{
 		TagList: []*elasticache.Tag{
 			{