restrictive block on public access (#5)

* add release badges

* test example, to get public block working

* improve variable name, document how to customize it

* guidance on max retention

* more README and doc polish

Author: eschultink

.github/workflows/terraform_integration.yaml

@@ -23,6 +23,7 @@ jobs:
       EXAMPLE_TENANT_SA_ID: '104184075060961394622'
       GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       AWS_REGION: 'us-west-2'
+      AWS_ACCOUNT_ID: '626567183302'
     steps:
       - name: Get timestamp
         id: timestamp
@@ -58,7 +59,10 @@ jobs:
         working-directory: examples/basic
         run: |
           terraform init
-          terraform apply -var="resource_name_prefix=tf_aws_w8s_export_ci_${{ env.timestamp}}" -var="worklytics_tenant_id=${{ env.EXAMPLE_TENANT_SA_ID }}" -auto-approve
+          terraform apply -auto-approve \
+            -var="aws_account_id=${{ env.AWS_ACCOUNT_ID }}" \
+            -var="resource_name_prefix=tf_aws_w8s_export_ci_${{ env.timestamp}}" \
+            -var="worklytics_tenant_id=${{ env.EXAMPLE_TENANT_SA_ID }}"
           echo "worklytics_export_bucket_id=$(terraform output -raw worklytics_export_bucket_id)" >> $GITHUB_OUTPUT
           echo "worklytics_tenant_aws_role_arn=$(terraform output -raw worklytics_tenant_aws_role_arn)" >> $GITHUB_OUTPUT
 
@@ -71,4 +75,7 @@ jobs:
         working-directory: examples/basic
         run: |
           aws s3 rm s3://${{ steps.terraform_apply.outputs.worklytics_export_bucket_id }} --recursive
-          terraform destroy -auto-approve -var="worklytics_tenant_id=${{ env.EXAMPLE_TENANT_SA_ID }}"
+          terraform destroy -auto-approve \
+            -var="aws_account_id=${{ env.AWS_ACCOUNT_ID }}" \
+            -var="worklytics_tenant_id=${{ env.EXAMPLE_TENANT_SA_ID }}" \
+            -var="resource_name_prefix=tf_aws_w8s_export_ci_${{ env.timestamp}}"

README.md

@@ -1,17 +1,20 @@
 # Worklytics Export to AWS Terraform Module
 
+[![Latest Release](https://img.shields.io/github/v/release/Worklytics/terraform-aws-worklytics-export)](https://github.com/Worklytics/terraform-aws-worklytics-export/releases/latest)
+![tests](https://img.shields.io/github/actions/workflow/status/Worklytics/terraform-aws-worklytics-export/terraform_integration.yaml?label=tests)
+
 This module creates infra to support exporting data from Worklytics to AWS.
 
 It is published in the Terraform Registry at:
 https://registry.terraform.io/modules/Worklytics/worklytics-export/aws/latest
 
 ## Usage
 
-from Terraform registry: (pending release)
+from Terraform registry:
 ```hcl
 module "worklytics-export" {
   source  = "terraform-aws-worklytics-export"
-  version = "~> 0.1.0"
+  version = "~> 0.2.0"
 
   # numeric ID of your Worklytics Tenant SA
   worklytics_tenant_id = "123123123123"
@@ -21,7 +24,7 @@ module "worklytics-export" {
 via GitHub:
 ```hcl
 module "worklytics-export" {
-  source  = "git::https://github.com/worklytics/terraform-aws-worklytics-export/?ref=v0.1.0"
+  source  = "git::https://github.com/worklytics/terraform-aws-worklytics-export/?ref=v0.2.0"
 
   # numeric ID of your Worklytics Tenant SA
   worklytics_tenant_id = "123123123123"
@@ -33,6 +36,10 @@ module "worklytics-export" {
 #### `worklytics_export_bucket`
 The Terraform resource created as the export bucket. See [`aws_s3_bucket`](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket) for details.
 
+This is useful to compose with the other `aws_s3_bucket_*` resources to configure retention, encryption, etc. See:
+  - [aws_s3_bucket_lifecycle_configuration](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_lifecycle_configuration)
+  - [aws_s3_bucket_service_side_encryption_configuration](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_server_side_encryption_configuration)
+
 #### `worklytics_tenant_aws_role`
 The IAM role that your Worklytics Tenant will assume before operating on your AWS infrastructure.
 
@@ -46,11 +53,10 @@ for general idea; this is the reverse direction of that (GCP --> AWS, rather tha
 This value is useful for a few scenarios:
   - if you set a CMEK to encrypt the bucket rather than relying on AWS default, you may need to
     grant encrypt / data key creation permissions to this role.
-  - if you additional IAM policies set on this account which would *deny* the permissions needed by
-    this role for S3/etc, you'll have to use this role's ARN to add exceptions to those policies
+  - if your AWS account has additional IAM policies which would *deny* the permissions needed by
+    this role for S3/etc, use this role's ARN to add exceptions to those policies
     (in AWS IAM logic, explicit deny has precedence over explicit allow)
 
-
 ## Compatibility
 
 This module is meant for use with Terraform 1.0+. If you find incompatibilities using Terraform >=
@@ -66,11 +72,54 @@ If you wish to export Worklytics data to an existing bucket, use a Terraform imp
 terraform import module.worklytics_export.aws_s3_bucket.worklytics_export <bucket_name>
 ```
 
+### Customize Public Access Block
+By default, we set a restrictive public access block on the bucket.  If you need something more
+permissive, you can disable the default block by setting the variable `enable_aws_s3_bucket_public_access_block=false`
+in your `terraform.tfvars` file and then add your own public access block as follows:
+
+```tf
+resource "aws_s3_bucket_public_access_block" "worklytics_export" {
+  bucket = module.worklytics_export.worklytics_export_bucket.id
+
+  block_public_acls       = true
+  block_public_policy     = true
+  ignore_public_acls      = true
+  restrict_public_buckets = true
+}
+```
+
+### Add a Max Retention Policy
+
+It's good practice to have a max retention policy on your bucket, even if it's really long. If you
+have a data pipeline regularly moving data from this bucket into your data warehouse, a value of 30
+or 60 days can likely lower your storage costs and reduce risk of having data in more places than it
+needs to be.
+
+```tf
+resource "aws_s3_bucket_lifecycle_configuration" "worklytics_export" {
+  bucket = module.worklytics_export.worklytics_export_bucket.id
+
+  rule {
+    id      = "max_retention_5_years"
+    enabled = true
+
+    expiration {
+      days = 5*365 # 5 years
+    }
+  }
+}
+
+```
+
 ## Development
 
 This module is written and maintained by [Worklytics, Co.](https://worklytics.co/) and intended to
 guide our customers in setting up their own infra to export data from Worklytics to AWS.
 
-Our intent is that this will be [published as a Terraform module](https://developer.hashicorp.com/terraform/registry/modules/publish), so will follow [standard Terraform
-module structure](https://developer.hashicorp.com/terraform/language/modules/develop/structure).
+As this is [published as a Terraform module](https://developer.hashicorp.com/terraform/registry/modules/publish),
+we will strive to follow [standard Terraform module structure](https://developer.hashicorp.com/terraform/language/modules/develop/structure)
+and [style conventions](https://developer.hashicorp.com/terraform/language/syntax/style).
+
+See [examples/basic/](examples/basic/) for a simple example of how to use this module.
+
 

examples/basic-remote/main.tf

@@ -6,9 +6,9 @@ terraform {
   }
 }
 
-module "worklytics-export" {
+module "worklytics_export" {
   source  = "terraform-aws-worklytics-export"
-  version = "~> 0.1.0"
+  version = "~> 0.2.0"
 
   resource_name_prefix = var.resource_name_prefix
   worklytics_tenant_id = var.worklytics_tenant_id

examples/basic/README.md

@@ -0,0 +1,24 @@
+# Basic Worklytics Export to S3 Example
+
+We don't recommend *direct* use of this example, but rather use it as a reference for how to add
+the Worklytics Export module to your own Terraform configuration or as a working example when
+developing the module itself.
+
+
+## Usage for Development
+
+Within `examples/basic/` (eg, here), create a `terraform.tfvars` file with the following content,
+customizing  AWS account id and role name as needed.
+
+```hcl
+worklytics_tenant_id = null # "123456712345671234567"
+aws_account_id       = "626567183302" # our CI account!! use your own!
+aws_role_name        = "Admin"
+```
+
+Then test the example:
+
+```shell
+terraform init
+terraform apply
+```

examples/basic/example_variables.tf

@@ -0,0 +1,12 @@
+# variables needed by the example
+
+variable "aws_account_id" {
+  type        = string
+  description = "AWS account in which to provision. Required to be explicitly specified, to reduce chance of inadvertently provisioning in the wrong account."
+}
+
+variable "aws_role_name" {
+  type        = string
+  description = "The name of the role to assume within AWS account. `null` if already auth'd as desired role/user."
+  default     = null
+}

examples/basic/main.tf

@@ -8,6 +8,20 @@ terraform {
   }
 }
 
+# provider just for example purposes; in real use, you likely already have an AWS provider block
+# in your terraform configuration
+provider "aws" {
+
+  assume_role {
+    role_arn = var.aws_role_name == null ? null : "arn:aws:iam::${var.aws_account_id}:role/${var.aws_role_name}"
+  }
+
+  allowed_account_ids = [
+    var.aws_account_id
+  ]
+}
+
+
 
 module "worklytics_export" {
   source = "../../"
@@ -23,3 +37,5 @@ output "worklytics_export_bucket_id" {
 output "worklytics_tenant_aws_role_arn" {
   value = module.worklytics_export.worklytics_tenant_aws_role.arn
 }
+
+

main.tf

@@ -44,8 +44,8 @@ resource "aws_iam_role" "for_worklytics_tenant" {
   })
 }
 
-
 resource "aws_s3_bucket" "worklytics_export" {
+
   bucket_prefix = replace(lower(var.resource_name_prefix), "_", "-")
 
   lifecycle {
@@ -56,11 +56,20 @@ resource "aws_s3_bucket" "worklytics_export" {
   }
 }
 
-resource "aws_s3_bucket_acl" "worklytics_export_private" {
+# you can use `aws_s3_bucket_public_access_block` to disable this, as these defaults are extreme.
+# if you do, we recommend setting something similar outside this module
+resource "aws_s3_bucket_public_access_block" "worklytics_export" {
+  count = var.enable_aws_s3_bucket_public_access_block ? 1 : 0
+
   bucket = aws_s3_bucket.worklytics_export.id
-  acl    = "private"
+
+  block_public_acls       = true
+  block_public_policy     = true
+  ignore_public_acls      = true
+  restrict_public_buckets = true
 }
 
+
 # TODO if key, need perm to "kms:GenerateDataKey" and "kms:Decrypt" ??
 # q - do we leave that to customer, or support it natively since pretty common case??
 

variables.tf

@@ -1,7 +1,6 @@
-
 variable "resource_name_prefix" {
   type        = string
-  description = "Prefix to give to names of infra created by this module, where appicable."
+  description = "Prefix to give to names of infra created by this module, where applicable."
   default     = "worklytics-export-"
 }
 
@@ -14,3 +13,10 @@ variable "worklytics_tenant_id" {
     error_message = "`worklytics_tenant_id` must be a 21-digit numeric value. (or `null`, for pre-production use case where you don't want external entity to be allowed to assume the role)."
   }
 }
+
+variable "enable_aws_s3_bucket_public_access_block" {
+  type        = bool
+  description = "Whether to place restrictive `aws_s3_bucket_public_access_block` on S3 bucket. Set to `false` if you wish to configure something equivalent outside this module."
+  default     = true
+}
+