From ac49856855f6eab3da4ee7d3ee3d26c1f9786e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Taveira=20Ara=C3=BAjo?= Date: Tue, 12 Dec 2023 16:43:43 -0800 Subject: [PATCH] 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 --- cmd/subscriber/main.go | 2 +- handler/subscriber/config.go | 10 +++++---- handler/subscriber/config_test.go | 5 +++-- handler/subscriber/handler.go | 2 +- handler/subscriber/subscription.go | 4 ++-- handler/subscriber/subscription_test.go | 28 ++++++++++++++++++++++++- 6 files changed, 40 insertions(+), 11 deletions(-) diff --git a/cmd/subscriber/main.go b/cmd/subscriber/main.go index eb44c4da..9ce66e7c 100644 --- 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,noinit"` 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 index 9b7591ea..7486693b 100644 --- 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 index 15516682..d1c26ffc 100644 --- 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 index 49171178..e68d17f3 100644 --- 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 index a383d18b..bd43898d 100644 --- 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 index b93c3ec0..71b829c0 100644 --- 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)