fix: Don't use a specific default value for preferred maintenance/backup windows for clusters

[jordan-lange]

Description

This aligns the behavior with the instance level module. When not set, new resources will use service provided (regional) default window(s).

https://github.com/terraform-aws-modules/terraform-aws-rds/blob/master/modules/db_instance/variables.tf#L308

Motivation and Context

The main motivation is just to align the instance level and aurora level modules. Secondary to that it seems beneficial for these not to be set by default because the service provided defaults are regional and generally fall outside of business hours whereas these defaults may land in less convenient times in some regions.

Breaking Changes

Setting the default to null means terraform will use the provided values for any existing resources where it wasn't specified. So it won't break management of existing resources.

For new resources the behavior will change to use the service provided default which might be considered a breaking change.

How Has This Been Tested?

• Tested that the basic change works by not setting a value for either when not specified in the module config
  The plan for a new example resource on the old code looks like this:

>   + resource "aws_rds_cluster" "this" {
...
      + preferred_backup_window               = "02:00-03:00"
      + preferred_maintenance_window          = "sun:05:00-sun:06:00"
}

  + resource "aws_rds_cluster_instance" "this" {
...
      + preferred_backup_window               = (known after apply)
      + preferred_maintenance_window          = "sun:05:00-sun:06:00"
}

With the proposed code both resources (cluster and instance) show:

      + preferred_backup_window               = (known after apply)
      + preferred_maintenance_window          = (known after apply)

After applying both resources show:

    preferred_backup_window               = "08:27-08:57"
    preferred_maintenance_window          = "tue:12:11-tue:12:41"

• Tested that with an existing resource created on the old code with no preferred windows set in the module config, after applying the proposed code changes the plan shows:

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

• Tested a scenario where there was no maintenance window set in the config on the old code, then applied the proposed code, and added a preferred maintenance window to the config.

  # module.test_cluster.aws_rds_cluster.this[0] will be updated in-place
  ~ resource "aws_rds_cluster" "this" {
        id                                    = "test-cluster"
      ~ preferred_maintenance_window          = "sun:05:00-sun:06:00" -> "mon:05:00-mon:05:30"
        tags                                  = {}
        # (57 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

  # module.test_cluster.aws_rds_cluster_instance.this["one"] will be updated in-place
  ~ resource "aws_rds_cluster_instance" "this" {
        id                                    = "test-cluster-one"
      ~ preferred_maintenance_window          = "sun:05:00-sun:06:00" -> "mon:05:00-mon:05:30"
        tags                                  = {}
        # (33 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

[bryantbiggs]

we can consider adopting this at the next breaking change

however, your commit/PR description is incorrect. we do not "hardcode" anything here - we have set a default value because a "sane" default value was not available at the time when this was originally set. users are free to set preferred_backup_window = null to achieve the desired effect today without any code changes

[jordan-lange]

So this would be considered a breaking change because the default behavior is changing for newly created resources? I can update the description.

On the second part you just mean that it's a misuse of the word "hardcoded" since it's a configuration that you can override?

[bryantbiggs]

So this would be considered a breaking change because the default behavior is changing for newly created resources? I can update the description.

Not just for new resources, its a change in default behavior of new or existing resources

On the second part you just mean that it's a misuse of the word "hardcoded" since it's a configuration that you can override?

Hardcoded means its codified in such a way that users cannot change the value. Currently this is specified through a variable with a default value - very standard for Terraform.

[jordan-lange]

Not just for new resources, its a change in default behavior of new or existing resources

This won't affect existing resources unless they're destroyed and re-created as far as I know. Is that what you mean? Or perhaps I'm missing something.

[bryantbiggs]

I have not tested it to know for certain what happens if the value changes from some provided string value to null. either way, its a change in the default behavior and that is breaking.