diff --git a/cmd/bucket-notification-utils.go b/cmd/bucket-notification-utils.go index 04b63ee02..1cc72e322 100644 --- a/cmd/bucket-notification-utils.go +++ b/cmd/bucket-notification-utils.go @@ -164,14 +164,13 @@ func isValidTopic(topicARN arnTopic) bool { func isValidQueueID(queueARN string) bool { // Unmarshals QueueARN into structured object. sqsARN := unmarshalSqsARN(queueARN) - // AMQP queue. - if isAMQPQueue(sqsARN) { + // Is Queue identifier valid?. + if isAMQPQueue(sqsARN) { // AMQP eueue. amqpN := serverConfig.GetAMQPNotifyByID(sqsARN.AccountID) return amqpN.Enable && amqpN.URL != "" } else if isElasticQueue(sqsARN) { // Elastic queue. elasticN := serverConfig.GetElasticSearchNotifyByID(sqsARN.AccountID) return elasticN.Enable && elasticN.URL != "" - } else if isRedisQueue(sqsARN) { // Redis queue. redisN := serverConfig.GetRedisNotifyByID(sqsARN.AccountID) return redisN.Enable && redisN.Addr != "" @@ -186,13 +185,6 @@ func checkQueueConfig(qConfig queueConfig) APIErrorCode { return s3Error } - // Unmarshals QueueARN into structured object. - sqsARN := unmarshalSqsARN(qConfig.QueueARN) - // Validate if sqsARN requested any of the known supported queues. - if !isValidQueue(sqsARN) { - return ErrARNNotification - } - // Validate if the account ID is correct. if !isValidQueueID(qConfig.QueueARN) { return ErrARNNotification diff --git a/cmd/bucket-notification-utils_test.go b/cmd/bucket-notification-utils_test.go index bca6994ad..0d313e086 100644 --- a/cmd/bucket-notification-utils_test.go +++ b/cmd/bucket-notification-utils_test.go @@ -16,7 +16,162 @@ package cmd -import "testing" +import ( + "strings" + "testing" +) + +// Test validates for duplicate configs. +func TestCheckDuplicateConfigs(t *testing.T) { + testCases := []struct { + qConfigs []queueConfig + expectedErrCode APIErrorCode + }{ + // Error for duplicate queue configs. + { + qConfigs: []queueConfig{ + { + QueueARN: "arn:minio:sqs:us-east-1:1:redis", + }, + { + QueueARN: "arn:minio:sqs:us-east-1:1:redis", + }, + }, + expectedErrCode: ErrOverlappingConfigs, + }, + // Valid queue configs. + { + qConfigs: []queueConfig{ + { + QueueARN: "arn:minio:sqs:us-east-1:1:redis", + }, + }, + expectedErrCode: ErrNone, + }, + } + + // ... validate for duplicate queue configs. + for i, testCase := range testCases { + errCode := checkDuplicateQueueConfigs(testCase.qConfigs) + if errCode != testCase.expectedErrCode { + t.Errorf("Test %d: Expected %d, got %d", i+1, testCase.expectedErrCode, errCode) + } + } + + // Test cases for SNS topic config. + topicTestCases := []struct { + tConfigs []topicConfig + expectedErrCode APIErrorCode + }{ + // Error out for duplicate configs. + { + tConfigs: []topicConfig{ + { + TopicARN: "arn:minio:sns:us-east-1:1:listen", + }, + { + TopicARN: "arn:minio:sns:us-east-1:1:listen", + }, + }, + expectedErrCode: ErrOverlappingConfigs, + }, + // Valid config. + { + tConfigs: []topicConfig{ + { + TopicARN: "arn:minio:sns:us-east-1:1:listen", + }, + }, + expectedErrCode: ErrNone, + }, + } + + // ... validate for duplicate topic configs. + for i, testCase := range topicTestCases { + errCode := checkDuplicateTopicConfigs(testCase.tConfigs) + if errCode != testCase.expectedErrCode { + t.Errorf("Test %d: Expected %d, got %d", i+1, testCase.expectedErrCode, errCode) + } + } +} + +// Tests for validating filter rules. +func TestCheckFilterRules(t *testing.T) { + testCases := []struct { + rules []filterRule + expectedErrCode APIErrorCode + }{ + // Valid prefix and suffix values. + { + rules: []filterRule{ + { + Name: "prefix", + Value: "test/test1", + }, + { + Name: "suffix", + Value: ".jpg", + }, + }, + expectedErrCode: ErrNone, + }, + // Invalid filter name. + { + rules: []filterRule{ + { + Name: "unknown", + Value: "test/test1", + }, + }, + expectedErrCode: ErrFilterNameInvalid, + }, + // Cannot have duplicate prefixes. + { + rules: []filterRule{ + { + Name: "prefix", + Value: "test/test1", + }, + { + Name: "prefix", + Value: "test/test1", + }, + }, + expectedErrCode: ErrFilterNamePrefix, + }, + // Cannot have duplicate suffixes. + { + rules: []filterRule{ + { + Name: "suffix", + Value: ".jpg", + }, + { + Name: "suffix", + Value: ".txt", + }, + }, + expectedErrCode: ErrFilterNameSuffix, + }, + // Filter value cannot be bigger than > 1024. + { + rules: []filterRule{ + { + Name: "prefix", + Value: strings.Repeat("a", 1025), + }, + }, + expectedErrCode: ErrFilterValueInvalid, + }, + } + + for i, testCase := range testCases { + errCode := checkFilterRules(testCase.rules) + if errCode != testCase.expectedErrCode { + t.Errorf("Test %d: Expected %d, got %d", i+1, testCase.expectedErrCode, errCode) + } + } +} // Tests filter name validation. func TestIsValidFilterName(t *testing.T) { diff --git a/cmd/event-notifier_test.go b/cmd/event-notifier_test.go index c1ea94a6a..e2f571e70 100644 --- a/cmd/event-notifier_test.go +++ b/cmd/event-notifier_test.go @@ -324,6 +324,11 @@ func TestListenBucketNotification(t *testing.T) { t.Fatal("Unexpected error:", err) } + // Validate if minio SNS is configured for an empty topic configs. + if isMinioSNSConfigured(listenARN, nil) { + t.Fatal("SNS listen shouldn't be configured.") + } + // Check if the config is loaded notificationCfg := globalEventNotifier.GetBucketNotificationConfig(bucketName) if notificationCfg == nil {