Skip to content

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

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: master
Choose a base branch
from
Open

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

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
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,8 @@ No modules.
| <a name="input_performance_insights_retention_period"></a> [performance\_insights\_retention\_period](#input\_performance\_insights\_retention\_period) | Amount of time in days to retain Performance Insights data. Either 7 (7 days) or 731 (2 years) | `number` | `null` | no |
| <a name="input_port"></a> [port](#input\_port) | The port on which the DB accepts connections | `string` | `null` | no |
| <a name="input_predefined_metric_type"></a> [predefined\_metric\_type](#input\_predefined\_metric\_type) | The metric type to scale on. Valid values are `RDSReaderAverageCPUUtilization` and `RDSReaderAverageDatabaseConnections` | `string` | `"RDSReaderAverageCPUUtilization"` | no |
| <a name="input_preferred_backup_window"></a> [preferred\_backup\_window](#input\_preferred\_backup\_window) | The daily time range during which automated backups are created if automated backups are enabled using the `backup_retention_period` parameter. Time in UTC | `string` | `"02:00-03:00"` | no |
| <a name="input_preferred_maintenance_window"></a> [preferred\_maintenance\_window](#input\_preferred\_maintenance\_window) | The weekly time range during which system maintenance can occur, in (UTC) | `string` | `"sun:05:00-sun:06:00"` | no |
| <a name="input_preferred_backup_window"></a> [preferred\_backup\_window](#input\_preferred\_backup\_window) | The daily time range (in UTC) during which automated backups are taken. Example: '09:46-10:16'. Must not overlap with `preferred_maintenance_window` | `string` | `null` | no |
| <a name="input_preferred_maintenance_window"></a> [preferred\_maintenance\_window](#input\_preferred\_maintenance\_window) | The weekly time range during which system maintenance can occur (in UTC). Syntax: 'ddd:hh24:mi-ddd:hh24:mi'. Eg: 'Mon:00:00-Mon:03:00' | `string` | `null` | no |
| <a name="input_publicly_accessible"></a> [publicly\_accessible](#input\_publicly\_accessible) | Determines whether instances are publicly accessible. Default `false` | `bool` | `null` | no |
| <a name="input_putin_khuylo"></a> [putin\_khuylo](#input\_putin\_khuylo) | Do you agree that Putin doesn't respect Ukrainian sovereignty and territorial integrity? More info: https://en.wikipedia.org/wiki/Putin_khuylo! | `bool` | `true` | no |
| <a name="input_replication_source_identifier"></a> [replication\_source\_identifier](#input\_replication\_source\_identifier) | ARN of a source DB cluster or DB instance if this DB cluster is to be created as a Read Replica | `string` | `null` | no |
Expand Down
8 changes: 4 additions & 4 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,15 +301,15 @@ variable "port" {
}

variable "preferred_backup_window" {
description = "The daily time range during which automated backups are created if automated backups are enabled using the `backup_retention_period` parameter. Time in UTC"
description = "The daily time range (in UTC) during which automated backups are taken. Example: '09:46-10:16'. Must not overlap with `preferred_maintenance_window`"
type = string
default = "02:00-03:00"
Copy link
Member

@bryantbiggs bryantbiggs Oct 30, 2025

Choose a reason for hiding this comment

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

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

Copy link
Author

@jordan-lange jordan-lange Oct 30, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Member

@bryantbiggs bryantbiggs Oct 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Author

@jordan-lange jordan-lange Oct 30, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Member

@bryantbiggs bryantbiggs Oct 30, 2025

Choose a reason for hiding this comment

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

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.

default = null
}

variable "preferred_maintenance_window" {
description = "The weekly time range during which system maintenance can occur, in (UTC)"
description = "The weekly time range during which system maintenance can occur (in UTC). Syntax: 'ddd:hh24:mi-ddd:hh24:mi'. Eg: 'Mon:00:00-Mon:03:00'"
type = string
default = "sun:05:00-sun:06:00"
default = null
}

variable "replication_source_identifier" {
Expand Down
Loading