fix(forwarder): relax DataAccessPointArn requirement

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 |"

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

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: >-

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 (<bucket>/<key>) 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
 

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 (<bucket>/<key>) 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. |

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}"

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"
+  }
+}

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

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"
   }
 }