Skip to content

remove deprecated s3 settings from module #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

remove deprecated s3 settings from module #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 8 additions & 39 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

-->

Terraform modules for provisioning and managing AWS [DMS](https://aws.amazon.com/dms/) resources.
Terraform modules for provisioning and managing AWS [DMS](https://aws.amazon.com/dms/) resources.

The following DMS resources are supported:

Expand Down Expand Up @@ -250,37 +250,6 @@ For automated tests of the example using [bats](https://github.com/bats-core/bat
context = module.this.context
}

module "dms_endpoint_s3_bucket" {
source = "cloudposse/dms/aws//modules/dms-endpoint"
# Cloud Posse recommends pinning every module to a specific version
# version = "x.x.x"

endpoint_type = "target"
engine_name = "s3"

s3_settings = {
bucket_name = module.s3_bucket.bucket_id
bucket_folder = null
cdc_inserts_only = false
csv_row_delimiter = " "
csv_delimiter = ","
data_format = "parquet"
compression_type = "GZIP"
date_partition_delimiter = "NONE"
date_partition_enabled = true
date_partition_sequence = "YYYYMMDD"
include_op_for_full_load = true
parquet_timestamp_in_millisecond = true
timestamp_column_name = "timestamp"
service_access_role_arn = aws_iam_role.s3.arn
}

extra_connection_attributes = ""

attributes = ["target"]
context = module.this.context
}

module "dms_replication_task" {
source = "cloudposse/dms/aws//modules/dms-replication-task"
# Cloud Posse recommends pinning every module to a specific version
Expand All @@ -306,7 +275,7 @@ For automated tests of the example using [bats](https://github.com/bats-core/bat
sqs_dlq_enabled = false
fifo_topic = false
fifo_queue_enabled = false
encryption_enabled = false
encryption_enabled = false

allowed_aws_services_for_sns_published = [
"cloudwatch.amazonaws.com",
Expand Down Expand Up @@ -365,14 +334,14 @@ For automated tests of the example using [bats](https://github.com/bats-core/bat
}
```

__NOTE:__ If a replication task is in "Failed" state (for any reason, e.g. network connectivity issues, database table issues, configuration issues),
it can't be destroyed with Terraform (but can be updated).
__NOTE:__ If a replication task is in "Failed" state (for any reason, e.g. network connectivity issues, database table issues, configuration issues),
it can't be destroyed with Terraform (but can be updated).
The task needs to be updated/fixed and moved to any other state like "Running", "Stopped", "Starting", "Ready", etc.

You can monitor the progress of your task by checking the task status and by monitoring the task's control table.
The task status indicates the condition of an AWS DMS task and its associated resources.
It includes such indications as if the task is being created, starting, running, stopped, or failed.
It also includes the current state of the tables that the task is migrating, such as if a full load of a table has begun
You can monitor the progress of your task by checking the task status and by monitoring the task's control table.
The task status indicates the condition of an AWS DMS task and its associated resources.
It includes such indications as if the task is being created, starting, running, stopped, or failed.
It also includes the current state of the tables that the task is migrating, such as if a full load of a table has begun
or is in progress and details such as the number of inserts, deletes, and updates have occurred for the table.

Refer to [Monitoring DMS Task Status](https://docs.aws.amazon.com/dms/latest/userguide/CHAP_Monitoring.html#CHAP_Tasks.Status) for more information.
Expand Down
47 changes: 8 additions & 39 deletions README.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ references:

# Short description of this project
description: |-
Terraform modules for provisioning and managing AWS [DMS](https://aws.amazon.com/dms/) resources.
Terraform modules for provisioning and managing AWS [DMS](https://aws.amazon.com/dms/) resources.

The following DMS resources are supported:

Expand Down Expand Up @@ -265,37 +265,6 @@ examples: |2-
context = module.this.context
}

module "dms_endpoint_s3_bucket" {
source = "cloudposse/dms/aws//modules/dms-endpoint"
# Cloud Posse recommends pinning every module to a specific version
# version = "x.x.x"

endpoint_type = "target"
engine_name = "s3"

s3_settings = {
bucket_name = module.s3_bucket.bucket_id
bucket_folder = null
cdc_inserts_only = false
csv_row_delimiter = " "
csv_delimiter = ","
data_format = "parquet"
compression_type = "GZIP"
date_partition_delimiter = "NONE"
date_partition_enabled = true
date_partition_sequence = "YYYYMMDD"
include_op_for_full_load = true
parquet_timestamp_in_millisecond = true
timestamp_column_name = "timestamp"
service_access_role_arn = aws_iam_role.s3.arn
}

extra_connection_attributes = ""

attributes = ["target"]
context = module.this.context
}

module "dms_replication_task" {
source = "cloudposse/dms/aws//modules/dms-replication-task"
# Cloud Posse recommends pinning every module to a specific version
Expand All @@ -321,7 +290,7 @@ examples: |2-
sqs_dlq_enabled = false
fifo_topic = false
fifo_queue_enabled = false
encryption_enabled = false
encryption_enabled = false

allowed_aws_services_for_sns_published = [
"cloudwatch.amazonaws.com",
Expand Down Expand Up @@ -380,14 +349,14 @@ examples: |2-
}
```

__NOTE:__ If a replication task is in "Failed" state (for any reason, e.g. network connectivity issues, database table issues, configuration issues),
it can't be destroyed with Terraform (but can be updated).
__NOTE:__ If a replication task is in "Failed" state (for any reason, e.g. network connectivity issues, database table issues, configuration issues),
it can't be destroyed with Terraform (but can be updated).
The task needs to be updated/fixed and moved to any other state like "Running", "Stopped", "Starting", "Ready", etc.

You can monitor the progress of your task by checking the task status and by monitoring the task's control table.
The task status indicates the condition of an AWS DMS task and its associated resources.
It includes such indications as if the task is being created, starting, running, stopped, or failed.
It also includes the current state of the tables that the task is migrating, such as if a full load of a table has begun
You can monitor the progress of your task by checking the task status and by monitoring the task's control table.
The task status indicates the condition of an AWS DMS task and its associated resources.
It includes such indications as if the task is being created, starting, running, stopped, or failed.
It also includes the current state of the tables that the task is migrating, such as if a full load of a table has begun
or is in progress and details such as the number of inserts, deletes, and updates have occurred for the table.

Refer to [Monitoring DMS Task Status](https://docs.aws.amazon.com/dms/latest/userguide/CHAP_Monitoring.html#CHAP_Tasks.Status) for more information.
Expand Down
34 changes: 0 additions & 34 deletions examples/complete/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,40 +75,6 @@ module "dms_endpoint_aurora_postgres" {
]
}

module "dms_endpoint_s3_bucket" {
source = "../../modules/dms-endpoint"

endpoint_type = "target"
engine_name = "s3"

s3_settings = {
bucket_name = module.s3_bucket.bucket_id
bucket_folder = null
cdc_inserts_only = false
csv_row_delimiter = " "
csv_delimiter = ","
data_format = "parquet"
compression_type = "GZIP"
date_partition_delimiter = "NONE"
date_partition_enabled = true
date_partition_sequence = "YYYYMMDD"
include_op_for_full_load = true
parquet_timestamp_in_millisecond = true
timestamp_column_name = "timestamp"
service_access_role_arn = join("", aws_iam_role.s3[*].arn)
}

extra_connection_attributes = ""

attributes = ["target"]
context = module.this.context

depends_on = [
aws_iam_role.s3,
module.s3_bucket
]
}

resource "time_sleep" "wait_for_dms_endpoints" {
count = local.enabled ? 1 : 0

Expand Down
31 changes: 0 additions & 31 deletions modules/dms-endpoint/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,35 +150,4 @@ module "s3_bucket" {

context = module.this.context
}

module "dms_endpoint_s3_bucket" {
source = "cloudposse/dms/aws//modules/dms-endpoint"
# Cloud Posse recommends pinning every module to a specific version
# version = "x.x.x"

endpoint_type = "target"
engine_name = "s3"

s3_settings = {
bucket_name = module.s3_bucket.bucket_id
bucket_folder = null
cdc_inserts_only = false
csv_row_delimiter = " "
csv_delimiter = ","
data_format = "parquet"
compression_type = "GZIP"
date_partition_delimiter = "NONE"
date_partition_enabled = true
date_partition_sequence = "YYYYMMDD"
include_op_for_full_load = true
parquet_timestamp_in_millisecond = true
timestamp_column_name = "timestamp"
service_access_role_arn = aws_iam_role.s3.arn
}

extra_connection_attributes = ""

attributes = ["target"]
context = module.this.context
}
```
42 changes: 0 additions & 42 deletions modules/dms-endpoint/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,47 +115,5 @@ resource "aws_dms_endpoint" "default" {
}
}

dynamic "s3_settings" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of removing this functionality, could we migrate the s3_settings to the resource?

Copy link
Author

@Quixotical Quixotical Jun 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of removing this functionality, could we migrate the s3_settings to the resource?

I'm trying to think this through, and want to make it clear that I know sometimes in PR Reviews through text ppl can sound defensive or argumentative, so I want to say that I really appreciate your question and I'm just hoping we find a great solution to this :) And if we go with your suggestion, I'd just appreciate any additional context for the sort of change you're looking for

My first thought is that this module is aws-dms-endpoint which is a specific terraform resource, and aws-dms-s3-endpoint is a different terraform resource, so I'm not sure it makes sense to have both of these resources in a module called aws-dms-endpoint. I could see the creation of a new module for aws-dms-s3-endpoint though.

Second thought is if the users of this module had set extra_connection_attributes, that is no longer supported with the new resource and this will also cause a break in their code.

Third thought is about the logistics of behind the scenes destroying and creating a new resource for users. If we double up in this module and include both resources, I'm not sure if simply removing s3_settings would destroy the old endpoint, or if it would persist, and then another endpoint resource would attempt to be created. If that were to happen, I am wondering if there would be a name conflict since they'd both have the same name. Also, if this did work, I'm not sure users would be pleased to have their endpoint destroyed and a new one created, if this happened in production to someone, it could interrupt their service.

Last thought is to the opposite of this, which is how much more or less complicated/disruptive would it be if instead of going this route, we did the following:

  1. remove the deprecated/invalid block from the resource
  2. cut a new release that is v2.0.0 with this change

If we go this route, then all users that wish to continue to use the deprecated s3_settings block can by staying at version v1.3.1 and they can migrate to the non-deprecated newer aws_dms_s3_endpoint resource at a time of their own choosing, and everyone who is not using deprecated code can upgrade to v2.0.0

Maybe I'm misunderstanding though, is the idea more "let's include the new endpoint type in this module at a v2.0.0 and let users upgrade & change to the new resource at their convenience"?

for_each = var.s3_settings != null ? [true] : []
content {
bucket_name = var.s3_settings["bucket_name"]
add_column_name = lookup(var.s3_settings, "add_column_name", null)
bucket_folder = lookup(var.s3_settings, "bucket_folder", null)
canned_acl_for_objects = lookup(var.s3_settings, "canned_acl_for_objects", null)
cdc_inserts_and_updates = lookup(var.s3_settings, "cdc_inserts_and_updates", null)
cdc_inserts_only = lookup(var.s3_settings, "cdc_inserts_only", null)
cdc_max_batch_interval = lookup(var.s3_settings, "cdc_max_batch_interval", null)
cdc_min_file_size = lookup(var.s3_settings, "cdc_min_file_size", null)
cdc_path = lookup(var.s3_settings, "cdc_path", null)
compression_type = lookup(var.s3_settings, "compression_type", null)
csv_delimiter = lookup(var.s3_settings, "csv_delimiter", null)
csv_no_sup_value = lookup(var.s3_settings, "csv_no_sup_value", null)
csv_null_value = lookup(var.s3_settings, "csv_null_value", null)
csv_row_delimiter = lookup(var.s3_settings, "csv_row_delimiter", null)
data_format = lookup(var.s3_settings, "data_format", null)
data_page_size = lookup(var.s3_settings, "data_page_size", null)
date_partition_delimiter = lookup(var.s3_settings, "date_partition_delimiter", null)
date_partition_enabled = lookup(var.s3_settings, "date_partition_enabled", null)
date_partition_sequence = lookup(var.s3_settings, "date_partition_sequence", null)
dict_page_size_limit = lookup(var.s3_settings, "dict_page_size_limit", null)
enable_statistics = lookup(var.s3_settings, "enable_statistics", null)
encoding_type = lookup(var.s3_settings, "encoding_type", null)
encryption_mode = lookup(var.s3_settings, "encryption_mode", null)
external_table_definition = lookup(var.s3_settings, "external_table_definition", null)
include_op_for_full_load = lookup(var.s3_settings, "include_op_for_full_load", null)
max_file_size = lookup(var.s3_settings, "max_file_size", null)
parquet_timestamp_in_millisecond = lookup(var.s3_settings, "parquet_timestamp_in_millisecond", null)
parquet_version = lookup(var.s3_settings, "parquet_version", null)
preserve_transactions = lookup(var.s3_settings, "preserve_transactions", null)
rfc_4180 = lookup(var.s3_settings, "rfc_4180", null)
row_group_length = lookup(var.s3_settings, "row_group_length", null)
server_side_encryption_kms_key_id = lookup(var.s3_settings, "server_side_encryption_kms_key_id", null)
service_access_role_arn = lookup(var.s3_settings, "service_access_role_arn", null)
timestamp_column_name = lookup(var.s3_settings, "timestamp_column_name", null)
use_task_start_time_for_full_load_timestamp = lookup(var.s3_settings, "use_task_start_time_for_full_load_timestamp", null)
use_csv_no_sup_value = lookup(var.s3_settings, "use_csv_no_sup_value", null)
}
}

tags = module.this.tags
}
6 changes: 0 additions & 6 deletions modules/dms-endpoint/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,3 @@ variable "redshift_settings" {
description = "Configuration block for Redshift settings"
default = null
}

variable "s3_settings" {
type = map(any)
description = "Configuration block for S3 settings"
default = null
}
31 changes: 0 additions & 31 deletions modules/dms-event-subscription/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,37 +176,6 @@ module "s3_bucket" {
context = module.this.context
}

module "dms_endpoint_s3_bucket" {
source = "cloudposse/dms/aws//modules/dms-endpoint"
# Cloud Posse recommends pinning every module to a specific version
# version = "x.x.x"

endpoint_type = "target"
engine_name = "s3"

s3_settings = {
bucket_name = module.s3_bucket.bucket_id
bucket_folder = null
cdc_inserts_only = false
csv_row_delimiter = " "
csv_delimiter = ","
data_format = "parquet"
compression_type = "GZIP"
date_partition_delimiter = "NONE"
date_partition_enabled = true
date_partition_sequence = "YYYYMMDD"
include_op_for_full_load = true
parquet_timestamp_in_millisecond = true
timestamp_column_name = "timestamp"
service_access_role_arn = aws_iam_role.s3.arn
}

extra_connection_attributes = ""

attributes = ["target"]
context = module.this.context
}

module "dms_replication_task" {
source = "cloudposse/dms/aws//modules/dms-replication-task"
# Cloud Posse recommends pinning every module to a specific version
Expand Down
31 changes: 0 additions & 31 deletions modules/dms-replication-task/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,37 +181,6 @@ module "s3_bucket" {
context = module.this.context
}

module "dms_endpoint_s3_bucket" {
source = "cloudposse/dms/aws//modules/dms-endpoint"
# Cloud Posse recommends pinning every module to a specific version
# version = "x.x.x"

endpoint_type = "target"
engine_name = "s3"

s3_settings = {
bucket_name = module.s3_bucket.bucket_id
bucket_folder = null
cdc_inserts_only = false
csv_row_delimiter = " "
csv_delimiter = ","
data_format = "parquet"
compression_type = "GZIP"
date_partition_delimiter = "NONE"
date_partition_enabled = true
date_partition_sequence = "YYYYMMDD"
include_op_for_full_load = true
parquet_timestamp_in_millisecond = true
timestamp_column_name = "timestamp"
service_access_role_arn = aws_iam_role.s3.arn
}

extra_connection_attributes = ""

attributes = ["target"]
context = module.this.context
}

module "dms_replication_task" {
source = "cloudposse/dms/aws//modules/dms-replication-task"
# Cloud Posse recommends pinning every module to a specific version
Expand Down