Skip to content
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.

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

feat: Logging and Snapshot copy resources converted to standalone resource equivalents, MSV of Terraform raised to v1.3 #99

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,15 @@ module "redshift" {

| Name | Version |
|------|---------|
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0 |
| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 5.35 |
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.3 |
| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 5.45 |
| <a name="requirement_random"></a> [random](#requirement\_random) | >= 3.0 |

## Providers

| Name | Version |
|------|---------|
| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 5.35 |
| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 5.45 |
| <a name="provider_random"></a> [random](#provider\_random) | >= 3.0 |

## Modules
Expand All @@ -204,8 +204,10 @@ No modules.
| [aws_redshift_cluster.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_cluster) | resource |
| [aws_redshift_cluster_iam_roles.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_cluster_iam_roles) | resource |
| [aws_redshift_endpoint_access.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_endpoint_access) | resource |
| [aws_redshift_logging.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_logging) | resource |
| [aws_redshift_parameter_group.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_parameter_group) | resource |
| [aws_redshift_scheduled_action.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_scheduled_action) | resource |
| [aws_redshift_snapshot_copy.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_snapshot_copy) | resource |
| [aws_redshift_snapshot_schedule.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_snapshot_schedule) | resource |
| [aws_redshift_snapshot_schedule_association.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_snapshot_schedule_association) | resource |
| [aws_redshift_subnet_group.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/redshift_subnet_group) | resource |
Expand Down
132 changes: 132 additions & 0 deletions UPGRADE-6.0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
# Upgrade from v5.x to v6.x

Please consult the `examples` directory for reference example configurations. If you find a bug, please open an issue with supporting configuration to reproduce.

## List of backwards incompatible changes

- Minimum supported version of Terraform AWS provider updated to `v5.45` to support latest resources
- Minimum supported version of Terraform raised to `v1.3`
- logging block within the `aws_redshift_cluster` resource has been replaced with a standalone resource. After upgrade, a new resource for logging will be created.
- snapshot_copy block within the `aws_redshift_cluster` resource has been replaced with a standalone resource. After upgrade, to prevent errors due to already existing `snapshot_copy` configurations, import of the new resource is required.

## Additional changes

### Added

- `aws_redshift_logging` has been added to replace `logging` block in `aws_redshift_cluster`
- `aws_redshift_snapshot_copy` has been added to replace `snapshot_copy` block in `aws_redshift_cluster`

### Modified

- None

### Removed

- `logging` block in `aws_redshift_cluster`
- `snapshot_copy` block in `aws_redshift_cluster`

### Variable and output changes

1. Removed variables:

- Cluster
- `var.logging.enable` has been removed
antonbabenko marked this conversation as resolved.
Show resolved Hide resolved

2. Renamed variables:

- None

3. Added variables:

- Snapshot Copy
- `var.snapshot_copy.manual_snapshot_retention_period`

4. Removed outputs:

- None

5. Renamed outputs:

- None

6. Added outputs:

- None

## Upgrade Migration

### Before v5.x Example

```hcl
module "redshift" {
source = "terraform-aws-modules/redshift/aws"
version = "~> 5.0"

snapshot_copy = {
destination_region = "us-east-1"
grant_name = aws_redshift_snapshot_copy_grant.useast1.snapshot_copy_grant_name
}

logging = {
enable = true
bucket_name = module.s3_logs.s3_bucket_id
s3_key_prefix = local.s3_prefix
}
}
```

### After v6.x Example

```hcl
module "redshift" {
source = "terraform-aws-modules/redshift/aws"
version = "~> 6.0"

snapshot_copy = {
destination_region = "us-east-1"
grant_name = aws_redshift_snapshot_copy_grant.useast1.snapshot_copy_grant_name
}

logging = {
bucket_name = module.s3_logs.s3_bucket_id
s3_key_prefix = local.s3_prefix
}
}
```

### Diff of Before vs After

```diff
# module.redshift.aws_redshift_logging.this[0] will be created
+ resource "aws_redshift_logging" "this" {
+ bucket_name = "ex-complete20240414012816938100000003"
+ cluster_identifier = "ex-complete"
+ id = (known after apply)
+ s3_key_prefix = "redshift/ex-complete/"
}

# module.redshift.aws_redshift_snapshot_copy.this[0] will be created
+ resource "aws_redshift_snapshot_copy" "this" {
+ cluster_identifier = "ex-complete"
+ destination_region = "us-east-1"
+ id = (known after apply)
+ manual_snapshot_retention_period = (known after apply)
+ retention_period = (known after apply)
+ snapshot_copy_grant_name = "ex-complete-us-east-1"
}
```
The `aws_redshift_logging` can be applied or imported. If setting the `log_destination_type`, an apply following an import will be required to clear the remaining diff.
The `aws_redshift_snapshot_copy` resource requires importing if an existing snapshot_copy configuration exists.

### State Move Commands

None required

### State Import Commands

During the migration to v6.x of this module, logging and snapshot_copy resources will be created by this module if those settings are configured. In order to guarantee the best experience and prevent data loss, you will need to import them into terraform state using commands like these:

```bash
terraform import 'module.redshift.aws_redshift_logging.this[0]' <cluster-id>
terraform import 'module.redshift.aws_redshift_snapshot_copy.this[0]' <cluster-id>
Copy link
Member Author

Choose a reason for hiding this comment

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

For those who have snapshot_copy previously configured, this will require an import or it will produce an error that snapshot copy is already enabled when it attempts to create the new resource. I don't mind removing this guide but just want to confirm that we don't want to consider this a breaking change.

╷
│ Error: creating Amazon Redshift Snapshot Copy ("ex-complete"): operation error Redshift: EnableSnapshotCopy, https response error StatusCode: 400, RequestID: caf48715-ac18-4a26-9b28-5c8b647cc5e7, SnapshotCopyAlreadyEnabledFault: Snapshot Copy is already enabled on Cluster ex-complete
│ 
│   with module.redshift.aws_redshift_snapshot_copy.this[0],
│   on ../../main.tf line 321, in resource "aws_redshift_snapshot_copy" "this":
│  321: resource "aws_redshift_snapshot_copy" "this" {
│ 
│ operation error Redshift: EnableSnapshotCopy, https response error StatusCode: 400, RequestID: caf48715-ac18-4a26-9b28-5c8b647cc5e7, SnapshotCopyAlreadyEnabledFault: Snapshot Copy is already enabled on Cluster ex-complete
╵

Copy link
Member

Choose a reason for hiding this comment

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

I see. I thought these resources had the same adoptable behavior as S3 bucket resources like "logging," "versioning," etc. Thank you for the explanation.

```
8 changes: 4 additions & 4 deletions examples/complete/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ Note that this example may create resources which cost money. Run `terraform des

| Name | Version |
|------|---------|
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0 |
| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 5.35 |
| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.3 |
| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 5.45 |

## Providers

| Name | Version |
|------|---------|
| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 5.35 |
| <a name="provider_aws.us_east_1"></a> [aws.us\_east\_1](#provider\_aws.us\_east\_1) | >= 5.35 |
| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 5.45 |
| <a name="provider_aws.us_east_1"></a> [aws.us\_east\_1](#provider\_aws.us\_east\_1) | >= 5.45 |

## Modules

Expand Down
2 changes: 0 additions & 2 deletions examples/complete/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ module "redshift" {
}

logging = {
enable = true
bucket_name = module.s3_logs.s3_bucket_id
s3_key_prefix = local.s3_prefix
}
Expand Down Expand Up @@ -221,7 +220,6 @@ module "with_cloudwatch_logging" {
create_cloudwatch_log_group = true
cloudwatch_log_group_retention_in_days = 7
logging = {
enable = true
log_destination_type = "cloudwatch"
log_exports = ["connectionlog", "userlog", "useractivitylog"]
}
Expand Down
4 changes: 2 additions & 2 deletions examples/complete/versions.tf
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
terraform {
required_version = ">= 1.0"
required_version = ">= 1.3"

required_providers {
aws = {
source = "hashicorp/aws"
version = ">= 5.35"
version = ">= 5.45"
}
}
}
50 changes: 28 additions & 22 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,6 @@ resource "aws_redshift_cluster" "this" {

# iam_roles and default_iam_roles are managed in the aws_redshift_cluster_iam_roles resource below

dynamic "logging" {
for_each = can(var.logging.enable) ? [var.logging] : []

content {
bucket_name = try(logging.value.bucket_name, null)
enable = logging.value.enable
log_destination_type = try(logging.value.log_destination_type, null)
log_exports = try(logging.value.log_exports, null)
s3_key_prefix = try(logging.value.s3_key_prefix, null)
}
}

maintenance_track_name = var.maintenance_track_name
manual_snapshot_retention_period = var.manual_snapshot_retention_period
manage_master_password = var.manage_master_password ? var.manage_master_password : null
Expand All @@ -74,16 +62,6 @@ resource "aws_redshift_cluster" "this" {
skip_final_snapshot = var.skip_final_snapshot
snapshot_cluster_identifier = var.snapshot_cluster_identifier

dynamic "snapshot_copy" {
for_each = length(var.snapshot_copy) > 0 ? [var.snapshot_copy] : []

content {
destination_region = snapshot_copy.value.destination_region
grant_name = try(snapshot_copy.value.grant_name, null)
retention_period = try(snapshot_copy.value.retention_period, null)
}
}

snapshot_identifier = var.snapshot_identifier
vpc_security_group_ids = var.vpc_security_group_ids

Expand Down Expand Up @@ -322,6 +300,34 @@ resource "aws_redshift_authentication_profile" "this" {
authentication_profile_content = jsonencode(each.value.content)
}

################################################################################
# Logging
################################################################################

resource "aws_redshift_logging" "this" {
count = var.create && length(var.logging) > 0 ? 1 : 0

cluster_identifier = aws_redshift_cluster.this[0].id
bucket_name = try(var.logging.bucket_name, null)
log_destination_type = try(var.logging.log_destination_type, null)
log_exports = try(var.logging.log_exports, null)
s3_key_prefix = try(var.logging.s3_key_prefix, null)
}

################################################################################
# Snapshot Copy
################################################################################

resource "aws_redshift_snapshot_copy" "this" {
count = var.create && length(var.snapshot_copy) > 0 ? 1 : 0

cluster_identifier = aws_redshift_cluster.this[0].id
destination_region = var.snapshot_copy.destination_region
manual_snapshot_retention_period = try(var.snapshot_copy.manual_snapshot_retention_period, null)
retention_period = try(var.snapshot_copy.retention_period, null)
snapshot_copy_grant_name = try(var.snapshot_copy.grant_name, null)
}

################################################################################
# CloudWatch Log Group
################################################################################
Expand Down
4 changes: 2 additions & 2 deletions versions.tf
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
terraform {
required_version = ">= 1.0"
required_version = ">= 1.3"

required_providers {
aws = {
source = "hashicorp/aws"
version = ">= 5.35"
version = ">= 5.45"
}

random = {
Expand Down