Allow backup plans to be unset.#116
Conversation
We (NDSPCDSP) for example don't use DynamoDB, EBS volumes or ParameterStore,
so we have no need for those backup plans, and it seems silly to create
them no matter what.
So to make sure they don't gets created, I want/need to set:
```
module "source" {
[...]
backup_plan_config = { "enable": false }
backup_plan_config_dynamodb = { "enable": false }
backup_plan_config_ebsvol = { "enable": false }
backup_plan_config_parameter_store = { "enable": false }
}
```
All the other `backup_plan_config_*` resources had a `count` to
create them or not, but not the "main" one, so add that. That lead
to the resources being an array, so make sure they're `moved`
automatically.
It also required *all* options in the variables to be `optional()`,
which should (!?) be ok, since there's defaults.
HOWEVER, the `default` is only used if/when the variable is completely
unset! NOT if/when some values are missing. As in, TF won't substitute
empty/unset values with one from the `default`.
That unfortunately lead to those three values required (`selection_tag`,
`compliance_resource_types` and `rules` in the case of the "main" config)
can't be required anymore. Because they have defaults, there's no way
to check (`validation {}`) if they're set or not. Because they always will,
wether `enable` is `true` or `false` :(.
| type = object({ | ||
| enable = bool | ||
| selection_tag = string | ||
| selection_tag = optional(string) |
There was a problem hiding this comment.
can't be optional, would fail on aws_backup_framework creation as tag keys can't be null
There was a problem hiding this comment.
It can, if there's a default for it (which there are). Besides, the selection_tag = optional(string) is what's in all the other configs.
There was a problem hiding this comment.
The default would only work if backup_plan_config isn't passed, if backup_plan_config is passed but without selection_tag it would read it as null
There was a problem hiding this comment.
But that's true for all the other backup_plan_config* variables. THEY have these optional()s..
There was a problem hiding this comment.
I see the requirement for optional in the scenario where the enable is set to false. some of the keys need to be set when enable is set to true, or terraform will fail. would it be possible to add those defaults on the optional. so optional(string, "default") to add that fallback
There was a problem hiding this comment.
That's the point of this PR, to allow these (all) to be unset, and then their enable = false.
2 participants
We (CDS-NDSP) for example don't use DynamoDB, EBS volumes or ParameterStore, so we have no need for those backup plans, and it seems silly to create them no matter what.
So to make sure they don't gets created, I want/need to set:
All the other
backup_plan_config_*resources had acountto create them or not, but not the "main" one, so add that. That lead to the resources being an array, so make sure they'removedautomatically.It also required all options in the variables to be
optional(), which should (!?) be ok, since there's defaults.HOWEVER, the
defaultis only used if/when the variable is completely unset! NOT if/when some values are missing. As in, TF won't substitute empty/unset values with one from thedefault.That unfortunately lead to those three values required (
selection_tag,compliance_resource_typesandrulesin the case of the "main" config) can't be required anymore. Because they have defaults, there's no way to check (validation {}) if they're set or not. Because they always will, wetherenableistrueorfalse:(.Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.