From 7b5ec13a100988cc73447983cd63e19ba9dbad2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Taveira=20Ara=C3=BAjo?= Date: Thu, 9 May 2024 09:46:58 -0700 Subject: [PATCH] fix(forwarder): relax DataAccessPointArn requirement This commit relaxes DataAccessPointArn as a required parameter for the Forwarder stack. The prior settings assumed the stack would always point at Filedrop. In practice, there can be usecases where an access point should not be assumed: - if we point directly at an S3 bucket, for testing or aggregation purposes. - if we point the forwarder towards an HTTP endpoint instead. --- Makefile | 2 +- apps/forwarder/template.yaml | 69 ++++++++++++++------ apps/stack/template.yaml | 18 +++--- docs/forwarder.md | 5 +- docs/stack.md | 6 +- integration/tests/forwarder.tftest.hcl | 2 +- integration/tests/forwarder_s3.tftest.hcl | 78 +++++++++++++++++++++++ integration/tests/stack.tftest.hcl | 2 +- integration/variables.tf | 2 +- 9 files changed, 146 insertions(+), 38 deletions(-) create mode 100644 integration/tests/forwarder_s3.tftest.hcl diff --git a/Makefile b/Makefile index 62fb49c0..b050def5 100644 --- a/Makefile +++ b/Makefile @@ -177,7 +177,7 @@ build-Forwarder: build-Subscriber: APP=subscriber $(MAKE) build-App -# parameters: generate doc table for cloudformation parameters +## parameters: generate doc table for cloudformation parameters parameters: $(call check_var,APP) @echo "| Parameter | Type | Description |" diff --git a/apps/forwarder/template.yaml b/apps/forwarder/template.yaml index c87c5c3d..b47faf5f 100644 --- a/apps/forwarder/template.yaml +++ b/apps/forwarder/template.yaml @@ -18,8 +18,8 @@ Metadata: - Label: default: Filedrop Configuration Parameters: - - DataAccessPointArn - DestinationUri + - DataAccessPointArn - NameOverride - Label: default: Data Sources @@ -40,16 +40,18 @@ Globals: MemorySize: 128 Parameters: - DataAccessPointArn: + DestinationUri: Type: String Description: >- - The access point ARN for your Filedrop. - AllowedPattern: "^arn:.*$" - DestinationUri: + The URI for your destination, e.g. `s3://bucket-alias/ds101/`. S3 URIs must + end in a forward slash. + AllowedPattern: "^(s3:\/\/.+\/|https:\/\/.+)$" + DataAccessPointArn: Type: String Description: >- - The S3 URI for your Filedrop, e.g. `s3://bucket-alias/ds101/` - AllowedPattern: "^(s3|https):\/\/.*" + The access point ARN for your Filedrop. + AllowedPattern: "^(arn:.*)?$" + Default: '' NameOverride: Type: String Description: >- @@ -100,7 +102,18 @@ Parameters: Default: '' AllowedPattern: "^(http(s)?:\/\/.*)?$" Conditions: - IsMaxFileSizeEmpty: !Equals [ !Ref MaxFileSize, '' ] + NoDataAccessPointArn: !Equals + - !Ref DataAccessPointArn + - '' + HasS3Destination: !Equals + - !Select [ 0, !Split [ "://", !Ref DestinationUri ] ] + - 's3' + HasS3DestinationWithoutAccessPoint: !And + - !Condition HasS3Destination + - !Condition NoDataAccessPointArn + IsMaxFileSizeEmpty: !Equals + - !Ref MaxFileSize + - '' DisableSourceS3: !Equals - !Join - '' @@ -230,19 +243,33 @@ Resources: - logs:CreateLogStream - logs:PutLogEvents Resource: !GetAtt LogGroup.Arn - - PolicyName: writer - PolicyDocument: - Version: 2012-10-17 - Statement: - - Effect: Allow - Action: - - s3:PutObject - - s3:PutObjectTagging - Resource: "*" - Condition: - StringLike: - s3:DataAccessPointArn: - - !Ref DataAccessPointArn + - !If + - HasS3Destination + - PolicyName: writer + PolicyDocument: + Version: 2012-10-17 + Statement: + - Effect: Allow + Action: + - s3:PutObject + - s3:PutObjectTagging + Resource: !If + - HasS3DestinationWithoutAccessPoint + # We enforce DestinationUri ends in a forward slash so as to ensure + # wildcard does not apply more broadly than intended. + - !Sub + - "arn:${AWS::Partition}:s3:::${Destination}*" + - Destination: !Select [ 1, !Split [ "s3://", !Ref DestinationUri ] ] + # If data access point is provided, we do not know the actual ARN. + # Restrict using a condition instead + - "*" + Condition: !If + - HasS3DestinationWithoutAccessPoint + - !Ref AWS::NoValue + - StringLike: + s3:DataAccessPointArn: + - !Ref DataAccessPointArn + - !Ref AWS::NoValue - !If - DisableSourceS3 - !Ref AWS::NoValue diff --git a/apps/stack/template.yaml b/apps/stack/template.yaml index 0523cc46..82f0a752 100644 --- a/apps/stack/template.yaml +++ b/apps/stack/template.yaml @@ -17,10 +17,10 @@ Metadata: AWS::CloudFormation::Interface: ParameterGroups: - Label: - default: Required parameters + default: Destination parameters Parameters: - - DataAccessPointArn - DestinationUri + - DataAccessPointArn - Label: default: AWS Config Parameters: @@ -48,16 +48,18 @@ Metadata: - DebugEndpoint Parameters: - DataAccessPointArn: + DestinationUri: Type: String Description: >- - The access point ARN for your Filedrop. - AllowedPattern: "^arn:.*$" - DestinationUri: + The URI for your destination, e.g. `s3://bucket-alias/ds101/`. S3 URIs must + end in a forward slash. + AllowedPattern: "^(s3:\/\/.+\/|https:\/\/.+)$" + DataAccessPointArn: Type: String Description: >- - The S3 URI for your Filedrop, e.g. `s3://bucket-alias/ds101/` - AllowedPattern: "^s3:\/\/.*$" + The access point ARN for your Filedrop. + AllowedPattern: "^(arn:.*)?$" + Default: '' SourceBucketNames: Type: CommaDelimitedList Description: >- diff --git a/docs/forwarder.md b/docs/forwarder.md index dc13cd12..755142d7 100644 --- a/docs/forwarder.md +++ b/docs/forwarder.md @@ -36,14 +36,15 @@ The forwarder stack can be configured with the following parameters: | Parameter | Type | Description | |-----------------|---------|-------------| -| **`DataAccessPointArn`** | String | The access point ARN for your Filedrop. | -| **`DestinationUri`** | String | The S3 URI for your Filedrop, e.g. `s3://bucket-alias/ds101/` | +| **`DestinationUri`** | String | The URI for your destination, e.g. `s3://bucket-alias/ds101/`. S3 URIs must end in a forward slash. | +| `DataAccessPointArn` | String | The access point ARN for your Filedrop. | | `NameOverride` | String | Name of IAM role expected by Filedrop. This name will also be applied to the SQS Queue and Lambda Function processing events. In the absence of a value, the stack name will be used. | | `SourceBucketNames` | CommaDelimitedList | A list of bucket names which the forwarder is allowed to read from. This list only affects permissions, and supports wildcards. In order to have files copied to Filedrop, you must also subscribe S3 Bucket Notifications to the forwarder. | | `SourceTopicArns` | CommaDelimitedList | A list of SNS topics the forwarder is allowed to be subscribed to. | | `MaxFileSize` | String | Max file size for objects to process (in bytes), default is 1GB | | `ContentTypeOverrides` | CommaDelimitedList | A list of key value pairs. The key is a regular expression which is applied to the S3 source (/) of forwarded files. The value is the content type to set for matching files. For example, `\.json$=application/x-ndjson` would forward all files ending in `.json` as newline delimited JSON files. | | `SourceKMSKeyArns` | CommaDelimitedList | A list of KMS Key ARNs the forwarder is allowed to use to decrypt objects in S3. | +| `DebugEndpoint` | String | Endpoint to send additional debug telemetry to. | ## Installation diff --git a/docs/stack.md b/docs/stack.md index 8484ab41..92169c1b 100644 --- a/docs/stack.md +++ b/docs/stack.md @@ -24,13 +24,13 @@ The Observe stack provisions the following components: | Parameter | Type | Description | |-----------------|---------|-------------| -| **`DataAccessPointArn`** | String | The access point ARN for your Filedrop. | -| **`DestinationUri`** | String | The S3 URI for your Filedrop, e.g. `s3://bucket-alias/ds101/` | +| `DataAccessPointArn` | String | The access point ARN for your Filedrop. | +| **`DestinationUri`** | String | The URI for your destination, e.g. `s3://bucket-alias/ds101/`. S3 URIs must end in a forward slash. | | `SourceBucketNames` | CommaDelimitedList | A list of bucket names which the forwarder is allowed to read from. | | `ContentTypeOverrides` | CommaDelimitedList | A list of key value pairs. The key is a regular expression which is applied to the S3 source (/) of forwarded files. The value is the content type to set for matching files. For example, `\.json$=application/x-ndjson` would forward all files ending in `.json` as newline delimited JSON files. | | `NameOverride` | String | Name of IAM role expected by Filedrop. This role will be created as part of this stack, and must therefore be unique within the account. | | `ConfigDeliveryBucketName` | String | If AWS Config is already enabled in this account and region, provide the S3 bucket snapshots are written to. | -| `IncludeResourceTypes` | CommaDelimitedList | Resources to collect using AWS Config. Use a wildcard to collect all supported resource types. Do not set this parameter if AWS Config is already installed for this region. | +| `IncludeResourceTypes` | CommaDelimitedList | If AWS Config is not enabled in this account and region, provide a list of resource types to collect. Use a wildcard to collect all supported resource types. | | `ExcludeResourceTypes` | CommaDelimitedList | Exclude a subset of resource types from configuration collection. This parameter can only be set if IncludeResourceTypes is wildcarded. | | `LogGroupNamePatterns` | CommaDelimitedList | Comma separated list of patterns. If not empty, the lambda function will only apply to log groups that have names that match one of the provided strings based on a case-sensitive substring search. | | `LogGroupNamePrefixes` | CommaDelimitedList | Comma separated list of prefixes. If not empty, the lambda function will only apply to log groups that start with a provided string. | diff --git a/integration/tests/forwarder.tftest.hcl b/integration/tests/forwarder.tftest.hcl index a6ed65c4..2b8fbd4f 100644 --- a/integration/tests/forwarder.tftest.hcl +++ b/integration/tests/forwarder.tftest.hcl @@ -121,7 +121,7 @@ run "install_forwarder" { app = "forwarder" parameters = { DataAccessPointArn = run.target_bucket.access_point.arn - DestinationUri = "s3://${run.target_bucket.access_point.alias}" + DestinationUri = "s3://${run.target_bucket.access_point.alias}/" SourceBucketNames = "${join(",", [for k, v in run.sources.buckets : v.id])}" SourceTopicArns = "arn:aws:sns:${run.setup.region}:${run.setup.account_id}:*" ContentTypeOverrides = "${var.override_match}=${var.override_content_type}" diff --git a/integration/tests/forwarder_s3.tftest.hcl b/integration/tests/forwarder_s3.tftest.hcl new file mode 100644 index 00000000..d530ce4d --- /dev/null +++ b/integration/tests/forwarder_s3.tftest.hcl @@ -0,0 +1,78 @@ +# This test verifies our forwarder can write to an S3 bucket directly, +# without being fronted by a DataAccessPoint +run "setup" { + module { + source = "observeinc/collection/aws//modules/testing/setup" + version = "2.9.0" + } +} + +run "target_bucket" { + module { + source = "observeinc/collection/aws//modules/testing/s3_bucket" + version = "2.9.0" + } + + variables { + setup = run.setup + } +} + +run "sources" { + module { + source = "./modules/setup_sources" + } + + variables { + setup = run.setup + } +} + +run "install_forwarder" { + variables { + setup = run.setup + app = "forwarder" + parameters = { + DestinationUri = "s3://${run.target_bucket.id}/" + SourceBucketNames = "${join(",", [for k, v in run.sources.buckets : v.id])}" + SourceTopicArns = "arn:aws:sns:${run.setup.region}:${run.setup.account_id}:*" + NameOverride = run.setup.id + } + capabilities = [ + "CAPABILITY_NAMED_IAM", + "CAPABILITY_AUTO_EXPAND", + ] + } +} + +run "subscribe_sources" { + module { + source = "./modules/subscribe_sources" + } + + variables { + sources = run.sources + queue_arn = run.install_forwarder.stack.outputs["Queue"] + } +} + + +run "check_sqs" { + module { + source = "observeinc/collection/aws//modules/testing/exec" + version = "2.9.0" + } + + variables { + command = "./scripts/check_object_diff" + env_vars = { + SOURCE = run.sources.buckets["sqs"].id + DESTINATION = run.target_bucket.id + } + } + + assert { + condition = output.exitcode == 0 + error_message = "Failed to copy object using SQS" + } +} diff --git a/integration/tests/stack.tftest.hcl b/integration/tests/stack.tftest.hcl index 8aaa75c5..bf169d9e 100644 --- a/integration/tests/stack.tftest.hcl +++ b/integration/tests/stack.tftest.hcl @@ -156,7 +156,7 @@ run "install" { app = "stack" parameters = { DataAccessPointArn = run.create_bucket.access_point.arn - DestinationUri = "s3://${run.create_bucket.access_point.alias}" + DestinationUri = "s3://${run.create_bucket.access_point.alias}/" ConfigDeliveryBucketName = "example-bucket" LogGroupNamePatterns = "*" NameOverride = run.setup.id diff --git a/integration/variables.tf b/integration/variables.tf index 1464f92f..d09c7546 100644 --- a/integration/variables.tf +++ b/integration/variables.tf @@ -27,7 +27,7 @@ variable "install_policy_json" { type = string default = null validation { - condition = can(jsondecode(var.install_policy_json)) + condition = var.install_policy_json == null || can(jsondecode(var.install_policy_json)) error_message = "must be valid JSON" } }