fix(subscriber): set role ARN

We were not correctly configuring the role ARN when setting a subscription filter. This is only necessary when the destination is a Kinesis Firehose delivery stream. Added unit test to verify the role ARN is passed to cloudwatchlogs:PutSubscriptionFilter
observeinc · Dec 12, 2023 · 042ec2f · 042ec2f
1 parent 016257a
commit 042ec2f
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 11 deletions.
diff --git a/cmd/subscriber/main.go b/cmd/subscriber/main.go
@@ -19,7 +19,7 @@ var env struct {
 	FilterName           string   `env:"FILTER_NAME"`
 	FilterPattern        string   `env:"FILTER_PATTERN"`
 	DestinationARN       string   `env:"DESTINATION_ARN"`
-	RoleARN              string   `env:"ROLE_ARN"`
+	RoleARN              *string  `env:"ROLE_ARN"`
 	LogGroupNamePatterns []string `env:"LOG_GROUP_NAME_PATTERNS"`
 	LogGroupNamePrefixes []string `env:"LOG_GROUP_NAME_PREFIXES"`
 	QueueURL             string   `env:"QUEUE_URL,required"`

diff --git a/handler/subscriber/config.go b/handler/subscriber/config.go
@@ -35,8 +35,10 @@ type Config struct {
 	// DestinationARN to subscribe log groups to.
 	// If empty, delete any subscription filters we manage.
 	DestinationARN string
-	// RoleARN for subscription filter
-	RoleARN string
+
+	// RoleARN for subscription filter.
+	// Only required if destination is a firehose delivery stream
+	RoleARN *string
 
 	// LogGroupNamePrefixes contains a list of prefixes which restricts the log
 	// groups we operate on.
@@ -68,12 +70,12 @@ func (c *Config) Validate() error {
 		}
 	}
 
-	if c.RoleARN != "" {
+	if c.RoleARN != nil {
 		if c.DestinationARN == "" {
 			errs = append(errs, ErrMissingDestinationARN)
 		}
 
-		roleARN, err := arn.Parse(c.RoleARN)
+		roleARN, err := arn.Parse(*c.RoleARN)
 		if err != nil || roleARN.Service != "iam" || !strings.HasPrefix(roleARN.Resource, "role/") {
 			errs = append(errs, fmt.Errorf("failed to parse role: %w", ErrInvalidARN))
 		}

diff --git a/handler/subscriber/config_test.go b/handler/subscriber/config_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/observeinc/aws-sam-testing/handler/handlertest"
 	"github.com/observeinc/aws-sam-testing/handler/subscriber"
 
+	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 )
@@ -45,7 +46,7 @@ func TestConfig(t *testing.T) {
 			Config: subscriber.Config{
 				CloudWatchLogsClient: &handlertest.CloudWatchLogsClient{},
 				FilterName:           "observe-logs-subscription",
-				RoleARN:              "arn:aws:iam::123456789012:role/test",
+				RoleARN:              aws.String("arn:aws:iam::123456789012:role/test"),
 			},
 			ExpectError: subscriber.ErrMissingDestinationARN,
 		},
@@ -55,7 +56,7 @@ func TestConfig(t *testing.T) {
 				CloudWatchLogsClient: &handlertest.CloudWatchLogsClient{},
 				FilterName:           "observe-logs-subscription",
 				DestinationARN:       "arn:aws:firehose:us-west-2:123456789012:deliverystream/test",
-				RoleARN:              "arn:aws:iam::123456789012:role/test",
+				RoleARN:              aws.String("arn:aws:iam::123456789012:role/test"),
 			},
 		},
 		{

diff --git a/handler/subscriber/handler.go b/handler/subscriber/handler.go
@@ -94,7 +94,7 @@ func New(cfg *Config) (*Handler, error) {
 			FilterName:     aws.String(cfg.FilterName),
 			FilterPattern:  aws.String(cfg.FilterPattern),
 			DestinationArn: aws.String(cfg.DestinationARN),
-			RoleArn:        aws.String(cfg.RoleARN),
+			RoleArn:        cfg.RoleARN,
 		},
 		logGroupNameFilter: cfg.LogGroupFilter(),
 	}

diff --git a/handler/subscriber/subscription.go b/handler/subscriber/subscription.go
@@ -123,7 +123,7 @@ func (h *Handler) SubscriptionFilterDiff(subscriptionFilters []types.Subscriptio
 				FilterName:     h.subscriptionFilter.FilterName,
 				FilterPattern:  h.subscriptionFilter.FilterPattern,
 				DestinationArn: h.subscriptionFilter.DestinationArn,
-				RoleArn:        h.subscriptionFilter.LogGroupName,
+				RoleArn:        h.subscriptionFilter.RoleArn,
 			})
 		}
 	}
@@ -133,7 +133,7 @@ func (h *Handler) SubscriptionFilterDiff(subscriptionFilters []types.Subscriptio
 			FilterName:     h.subscriptionFilter.FilterName,
 			FilterPattern:  h.subscriptionFilter.FilterPattern,
 			DestinationArn: h.subscriptionFilter.DestinationArn,
-			RoleArn:        h.subscriptionFilter.LogGroupName,
+			RoleArn:        h.subscriptionFilter.RoleArn,
 		})
 	}
 

diff --git a/handler/subscriber/subscription_test.go b/handler/subscriber/subscription_test.go
@@ -158,6 +158,32 @@ func TestSubscriptionFilterDiff(t *testing.T) {
 				},
 			},
 		},
+		{
+			Configure: types.SubscriptionFilter{
+				FilterName:     aws.String("observe"),
+				DestinationArn: aws.String("arn:aws:firehose:us-west-2:123456789012:deliverystream/example"),
+				RoleArn:        aws.String("arn:aws:iam::123456789012:role/example"),
+			},
+			Existing: []types.SubscriptionFilter{
+				{
+					FilterName: aws.String("foo"),
+				},
+				{
+					FilterName: aws.String("observe-logs-subscription"),
+				},
+			},
+			ExpectedActions: []any{
+				&cloudwatchlogs.DeleteSubscriptionFilterInput{
+					FilterName: aws.String("observe-logs-subscription"),
+				},
+				&cloudwatchlogs.PutSubscriptionFilterInput{
+					FilterName:     aws.String("observe"),
+					FilterPattern:  aws.String(""),
+					DestinationArn: aws.String("arn:aws:firehose:us-west-2:123456789012:deliverystream/example"),
+					RoleArn:        aws.String("arn:aws:iam::123456789012:role/example"),
+				},
+			},
+		},
 		{
 			/*
 				Do nothing if we exceed the two subscription filter limit
@@ -201,7 +227,7 @@ func TestSubscriptionFilterDiff(t *testing.T) {
 					CloudWatchLogsClient: &handlertest.CloudWatchLogsClient{},
 					FilterName:           aws.ToString(tt.Configure.FilterName),
 					DestinationARN:       aws.ToString(tt.Configure.DestinationArn),
-					RoleARN:              aws.ToString(tt.Configure.RoleArn),
+					RoleARN:              tt.Configure.RoleArn,
 				})
 			if err != nil {
 				t.Fatal(err)