Skip to content

Conversation

@Quixotical
Copy link

what

Remove the s3_settings block & variable from the module

why

Terraform has completely deprecated and remove s3_settings from the aws_dms_endpoint resource and now any builds result in an error of:

Error: Unsupported block type
│ 
│   on .terraform/modules/dms-endpoint/main.tf line 118, in resource "aws_dms_endpoint" "default":
│  118:   dynamic "s3_settings" {
│ 
│ Blocks of type "s3_settings" are not expected here.

references

Latest version showing that s3_settings is no longer there:
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/dms_endpoint

Previous version showing that it is there but should be removed from code because they will remove it themselves soon:
https://registry.terraform.io/providers/hashicorp/aws/5.100.0/docs/resources/dms_endpoint

The s3_settings argument is deprecated, may not be maintained, and will be removed in a future version. Use the aws_dms_s3_endpoint resource instead.

@Quixotical Quixotical requested review from a team as code owners June 23, 2025 20:38
@Quixotical Quixotical requested review from RoseSecurity and oycyc June 23, 2025 20:38
@mergify mergify bot added the triage Needs triage label Jun 23, 2025
@Quixotical Quixotical force-pushed the remove_deprecated_s3_settings branch from 044bc74 to cd3f6e2 Compare June 23, 2025 20:42
@RoseSecurity
Copy link

/terratest

@RoseSecurity
Copy link

With this removal, we lose the s3_settings functionality. The deprecation documentation also suggests transitioning to the aws_dms_s3_endpoint. This migration might necessitate an extensive transition to the new resource to preserve the existing functionality, which could result in a breaking change for this module.

@Quixotical
Copy link
Author

With this removal, we lose the s3_settings functionality. The deprecation documentation also suggests transitioning to the aws_dms_s3_endpoint. This migration might necessitate an extensive transition to the new resource to preserve the existing functionality, which could result in a breaking change for this module.

Currently if you are using aws provider version 6.0.0, this module is broken because that s3_settings is no longer valid. I'll be happy to make adjustments to the PR but I don't follow what changes should be made.

Here is where my head is at:

If we leave it in, anyone who continues to use the latest versions of the aws provider can not use this module, if we remove s3_settings and create a new release version for the module, then anyone using aws provider version 5.* can use up to aws-dms-endpoint module version v1.3.1, and anyone using 6.* and higher can use any later module versions. So removing this from a new release version wouldn't break anything unless a user specifically changes the module version to something > v1.3.1

I don't see a way that we can have this module functional for those who are using the most current aws provider unless it's removed, but like I said I'm very happy to make any adjustments to get this fixed

}
}

dynamic "s3_settings" {

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage Needs triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants