hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.82k stars 9.17k forks source link

[Enhancement]: Validations for final_snapshot properties on aws_redshift_cluster resource #39910

Open TobiasAmmann opened 1 week ago

TobiasAmmann commented 1 week ago

Terraform Core Version

v1.3.9

AWS Provider Version

5.73.0

Affected Resource(s)

resource "aws_redshift_cluster" "redshift_cluster" {
  cluster_identifier     = "redshift-cluster"
  database_name          = "redshift_db"
  master_username        = "user"
  manage_master_password = true
  node_type              = "ra3.large"
  cluster_type           = "single-node"
  publicly_accessible    = false
  encrypted              = true

  vpc_security_group_ids    = [var.redshift_security_group_id]
  cluster_subnet_group_name = aws_redshift_subnet_group.redshift_subnet_group.name
}

Expected Behavior

I think the easiest solution would be to make the skip_final_snapshot or the final_snapshot_identifier property necessary. If then skip_final_snapshot != true or final_snapshot_identifier != null the terraform plan or apply should break the process with a message that those properties are necessary.

resource "aws_redshift_cluster" "redshift_cluster" {
  cluster_identifier     = "cndd-redshift-cluster-${var.environment}"
  database_name          = "cndd_redshift_db_${var.environment}"
  master_username        = "masteruser"
  manage_master_password = true
  node_type              = "ra3.large"
  cluster_type           = "single-node"
  publicly_accessible    = false
  encrypted              = true
  skip_final_snapshot    = true  # either this needs to be set
  final_snapshot_identifier = "my-final-snapshot" # or this property. In case it none of the two are true, it should be an invalid resource

  vpc_security_group_ids    = [var.redshift_security_group_id]
  cluster_subnet_group_name = aws_redshift_subnet_group.redshift_subnet_group.name
}

Actual Behavior

Creation of this resource is ok:

resource "aws_redshift_cluster" "redshift_cluster" {
  cluster_identifier     = "redshift-cluster"
  database_name          = "redshift_db"
  master_username        = "user"
  manage_master_password = true
  node_type              = "ra3.large"
  cluster_type           = "single-node"
  publicly_accessible    = false
  encrypted              = true

  vpc_security_group_ids    = [var.redshift_security_group_id]
  cluster_subnet_group_name = aws_redshift_subnet_group.redshift_subnet_group.name
}

But when trying to delete it, the following error appears:

Error: Redshift Cluster Instance FinalSnapshotIdentifier is required when a final snapshot is required

Relevant Error/Panic Output Snippet

No response

Terraform Configuration Files

resource "aws_redshift_subnet_group" "redshift_subnet_group" {
  name       = "redshift-subnet-group"
  subnet_ids = [var.vpc_subnet_id_private_a]
}

resource "aws_redshift_cluster" "redshift_cluster" {
  cluster_identifier     = "redshift-cluster"
  database_name          = "redshift_db"
  master_username        = "user"
  manage_master_password = true
  node_type              = "ra3.large"
  cluster_type           = "single-node"
  publicly_accessible    = false
  encrypted              = true

  vpc_security_group_ids    = [var.redshift_security_group_id]
  cluster_subnet_group_name = aws_redshift_subnet_group.redshift_subnet_group.name
}

Please not not forget to set the variables

Steps to Reproduce

  1. terraform apply
  2. terraform destroy

Debug Output

No response

Panic Output

No response

Important Factoids

No response

References

No response

Would you like to implement a fix?

No

github-actions[bot] commented 1 week ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue

justinretzolk commented 1 week ago

Hey @TobiasAmmann 👋 Thanks for taking the time to raise this! This kind of a modification of behavior to an existing resource (something that might be resolved with further validations) is something that we would consider an enhancement rather than a bug. With that in mind, I'm going to update a few things about this issue. There's no further action needed from you at this point, I just like to let people know before making these kinds of modifications.