diff --git a/.github/instructions/terraform-modules.instructions.md b/.github/instructions/terraform-modules.instructions.md index b471a32e..a489cea4 100644 --- a/.github/instructions/terraform-modules.instructions.md +++ b/.github/instructions/terraform-modules.instructions.md @@ -132,12 +132,14 @@ Every module must contain: | `context.tf` | Tags context (copied from `tags/exports/context.tf`) | Yes | | `data.tf` | Data sources (e.g., `data.aws_*`, `data.local_file`) | Only if data sources exist | | `locals.tf` | Derived/computed values (naming, defaults) | Only if `locals {}` blocks exist | +| `validations.tf` | Cross-variable `terraform_data` preconditions | Only if cross-variable constraints exist | | `README.md` | Usage documentation with examples | Yes | **Conditional file guidance:** - **`data.tf`**: Create this file if the module queries external data (e.g., `data.aws_availability_zones`, `data.aws_ami`). Store all data sources here for clarity. - **`locals.tf`**: Create this file only if the module defines `locals {}` blocks for computed values, naming logic, or CIDR calculations. If no locals are needed, omit the file entirely. +- **`validations.tf`**: Create this file when the module needs to enforce constraints that span multiple input variables (e.g., mutual exclusivity, co-required inputs). Use a `terraform_data` resource with `lifecycle { precondition { ... } }` blocks. See the Cross-Variable Validation section below. For any newly created module, `context.tf` must come from `infrastructure/modules/tags/exports/context.tf`, and the copied file must reference `source = "../tags"`. @@ -176,6 +178,44 @@ variable "recovery_window_in_days" { } ``` +## Cross-Variable Validation (`validations.tf`) + +Terraform `validation` blocks can only reference the variable they are declared on. When a constraint spans multiple variables (e.g., mutual exclusivity, co-required inputs), use a `terraform_data` resource with `lifecycle { precondition { ... } }` blocks in a dedicated `validations.tf` file. + +```hcl +################################################################ +# Input validation +# +# Validates cross-variable constraints that cannot be expressed +# through individual variable validation blocks: +# +# * +# * +################################################################ + +resource "terraform_data" "validations" { + count = module.this.enabled ? 1 : 0 + + lifecycle { + precondition { + condition = !(var.option_a && var.option_b != null) + error_message = "option_a and option_b are mutually exclusive; set only one." + } + precondition { + condition = !var.enable_feature || var.feature_config != null + error_message = "enable_feature requires feature_config to be set." + } + } +} +``` + +**Rules:** + +- Gate with `count = module.this.enabled ? 1 : 0` so checks only run when the module is active. +- `terraform_data` is a built-in resource requiring no extra provider (available since Terraform 1.4). +- Add a reference in the `main.tf` header comment: `# Cross-variable input constraints are enforced in validations.tf.` +- Document the constraints in a `## Validation` section in `README.md`. + ## Outputs - Include `description` on all outputs. @@ -220,6 +260,7 @@ Module READMEs should include: - Advanced/edge example (e.g., restore-from-snapshot, records-only mode) 4. **Conventions** section explaining naming, context behaviour, and notable defaults. 5. **What this module does NOT do** section listing intentional exclusions/guardrails. +6. **Validation** section (when `validations.tf` exists) listing each precondition constraint in plain English. READMEs that do not include a `## Usage` section are incomplete and must be updated before merge. diff --git a/.github/skills/terraform-module-patterns.skill.md b/.github/skills/terraform-module-patterns.skill.md index 5c6f9e93..9d07688e 100644 --- a/.github/skills/terraform-module-patterns.skill.md +++ b/.github/skills/terraform-module-patterns.skill.md @@ -185,7 +185,47 @@ variable "meaningful_name" { - `data.tf` is mandatory only when data sources exist in the module (for example `data.aws_*`, `data.local_file`, `data.external`). - `locals.tf` is mandatory only when the module defines one or more `locals {}` blocks. -- When present, keep all data sources in `data.tf` and all local values in `locals.tf`. +- `validations.tf` is mandatory when cross-variable constraints exist (mutual exclusivity, co-required inputs). See below. +- When present, keep all data sources in `data.tf`, all local values in `locals.tf`, and all cross-variable preconditions in `validations.tf`. + +## Cross-Variable Validation Pattern (`validations.tf`) + +Individual `variable` validation blocks can only reference the variable they are declared on. For constraints spanning multiple variables, use a `terraform_data` resource in `validations.tf`: + +```hcl +################################################################ +# Input validation +# +# Validates cross-variable constraints that cannot be expressed +# through individual variable validation blocks: +# +# * +# * +################################################################ + +resource "terraform_data" "validations" { + count = module.this.enabled ? 1 : 0 + + lifecycle { + precondition { + condition = !(var.option_a && var.option_b != null) + error_message = "option_a and option_b are mutually exclusive; set only one." + } + precondition { + condition = !var.enable_feature || var.feature_config != null + error_message = "enable_feature requires feature_config to be set." + } + } +} +``` + +**Key rules:** + +- Gate with `count = module.this.enabled ? 1 : 0` — checks only run when the module is active. +- `terraform_data` is a Terraform built-in; no extra provider required (available since Terraform 1.4, well within `>= 1.13`). +- `lifecycle { precondition }` on `module` blocks is **not supported** — always use a `terraform_data` resource. +- Reference `validations.tf` in the `main.tf` header comment: `# Cross-variable input constraints are enforced in validations.tf.` +- Document each constraint in a `## Validation` section in `README.md`. ## Locals Pattern (When Locals Are Needed) @@ -314,6 +354,14 @@ module "example" { * * + +## Validation + +*(Include this section only when `validations.tf` exists.)* + +The following constraints are enforced at `plan` time via preconditions in `validations.tf`: + +* ****: . ``` ## Security Patterns diff --git a/infrastructure/modules/secrets-manager/README.md b/infrastructure/modules/secrets-manager/README.md index df6f7eeb..fcb530f6 100644 --- a/infrastructure/modules/secrets-manager/README.md +++ b/infrastructure/modules/secrets-manager/README.md @@ -61,11 +61,31 @@ module "api_key" { } ``` +### Random password + +```hcl +module "generated_password" { + source = "git::https://github.com/NHSDigital/screening-terraform-modules-aws.git//infrastructure/modules/secrets-manager?ref=" + + context = module.this.context + stack = "database" + name = "db-password" + + description = "Auto-generated database password" + kms_key_id = module.rds_kms.key_arn + create_random_password = true + + # Optional: customise the generated password + random_password_length = 48 + random_password_override_special = "!@#$%&*()-_=+[]{}<>:?" +} +``` + ### Secret with rotation ```hcl module "rotated_password" { - source = "git::https://github.com/NHSDigital/screening-terraform-modules-aws.git//infrastructure/modules/secrets-manager?ref=" + source = "git::https://github.com/NHSDigital/screening-terraform-modules-aws.git//infrastructure/modules/secrets-manager?ref=" context = module.this.context stack = "database" @@ -75,6 +95,7 @@ module "rotated_password" { ignore_secret_changes = true # let the rotation Lambda manage the value enable_rotation = true + rotate_immediately = true rotation_lambda_arn = "arn:aws:lambda:eu-west-2:123456789012:function:rotate-db-secret" rotation_rules = { automatically_after_days = 30 @@ -90,6 +111,9 @@ module "rotated_password" { - `secret_string` is the plaintext secret value (as a string or JSON-encoded object); use Terraform variables and `sensitive = true` to avoid exposure. - `ignore_secret_changes` should be set to `true` when enabling rotation so Terraform does not overwrite the rotated value on the next apply. - `create_random_password` generates a random password as the secret value; do not set `secret_string` when using this option. +- `random_password_length` controls the length of the generated password (default `32`; must be 8–4096). +- `random_password_override_special` overrides the default special character set used during password generation. +- `rotate_immediately` controls whether rotation fires immediately (`true`) or waits for the next scheduled window (`false`); only relevant when `enable_rotation = true`. - `create_policy` defaults to `false`; set to `true` and provide `policy_statements` to attach a resource-based policy. - Secret names are derived from `module.this.id` for consistency with other screening modules. @@ -99,7 +123,13 @@ module "rotated_password" { - Create rotation Lambda functions; you must create the function separately and provide its ARN via `rotation_lambda_arn`. - Populate secret values automatically; you must provide `secret_string` or enable `create_random_password`. - Manage secret replicas across regions (use native `aws_secretsmanager_secret_rotation` resources if multi-region replication is required). -- Retrieve secret values; use `data.aws_secretsmanager_secret_version` in consumer stacks to read secrets. + +## Validation + +The following constraints are enforced at `plan` time via preconditions in `validations.tf`: + +- **Secret value mutual exclusivity**: `secret_string`, `secret_string_wo`, and `create_random_password` are mutually exclusive — setting more than one will produce a clear error before any resource is created. +- **Rotation completeness**: Setting `enable_rotation = true` requires both `rotation_lambda_arn` and `rotation_rules` to be provided. @@ -114,7 +144,9 @@ module "rotated_password" { ## Providers -No providers. +| Name | Version | +| ---- | ------- | +| [terraform](#provider\_terraform) | n/a | ## Modules @@ -125,7 +157,9 @@ No providers. ## Resources -No resources. +| Name | Type | +| ---- | ---- | +| [terraform_data.validations](https://registry.terraform.io/providers/hashicorp/terraform/latest/docs/resources/data) | resource | ## Inputs @@ -159,9 +193,12 @@ No resources. | [policy\_statements](#input\_policy\_statements) | A map of IAM policy statements to attach to the secret policy. Only used when create\_policy is true. |
map(object({
sid = optional(string)
actions = optional(list(string))
not_actions = optional(list(string))
effect = optional(string)
resources = optional(list(string))
not_resources = optional(list(string))
principals = optional(list(object({
type = string
identifiers = list(string)
})))
not_principals = optional(list(object({
type = string
identifiers = list(string)
})))
condition = optional(list(object({
test = string
values = list(string)
variable = string
})))
}))
| `{}` | no | | [project](#input\_project) | ID element. A project identifier, indicating the name or role of the project the resource is for, such as `website` or `api` | `string` | `null` | no | | [public\_facing](#input\_public\_facing) | Whether this resource is public facing | `bool` | `false` | no | +| [random\_password\_length](#input\_random\_password\_length) | The length of the randomly-generated password. Only used when create\_random\_password is true. | `number` | `32` | no | +| [random\_password\_override\_special](#input\_random\_password\_override\_special) | Supply your own list of special characters for random password generation. Overrides the default special character set. Only used when create\_random\_password is true. | `string` | `"!@#$%&*()-_=+[]{}<>:?"` | no | | [recovery\_window\_in\_days](#input\_recovery\_window\_in\_days) | Number of days AWS Secrets Manager waits before permanently deleting the secret. Valid values: 0 (immediate deletion) or 7-30. | `number` | `30` | no | | [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.
Characters matching the regex will be removed from the ID elements.
If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no | | [region](#input\_region) | ID element \_(Rarely used, not included by default)\_. Usually an abbreviation of the selected AWS region e.g. 'uw2', 'ew2' or 'gbl' for resources like IAM roles that have no region | `string` | `null` | no | +| [rotate\_immediately](#input\_rotate\_immediately) | When enable\_rotation is true, specifies whether to rotate the secret immediately or wait until the next scheduled rotation window. Defaults to immediate rotation when not set. | `bool` | `null` | no | | [rotation\_lambda\_arn](#input\_rotation\_lambda\_arn) | ARN of the Lambda function that rotates the secret. Required when enable\_rotation is true. | `string` | `""` | no | | [rotation\_rules](#input\_rotation\_rules) | Rotation schedule for the secret. Provide either automatically\_after\_days or a schedule\_expression (cron/rate). Required when enable\_rotation is true. |
object({
automatically_after_days = optional(number)
duration = optional(string)
schedule_expression = optional(string)
})
| `null` | no | | [secret\_name](#input\_secret\_name) | Optional explicit name for the secret. When null, the name is derived from context labels via module.this.id. | `string` | `null` | no | @@ -184,6 +221,7 @@ No resources. | [secret\_arn](#output\_secret\_arn) | The ARN of the secret | | [secret\_id](#output\_secret\_id) | The ID of the secret (same as the ARN) | | [secret\_name](#output\_secret\_name) | The name of the secret | +| [secret\_string](#output\_secret\_string) | The secret string value. Sensitive — only use where required. | | [secret\_version\_id](#output\_secret\_version\_id) | The unique identifier of the current version of the secret | diff --git a/infrastructure/modules/secrets-manager/main.tf b/infrastructure/modules/secrets-manager/main.tf index f7bc1b10..c484a466 100644 --- a/infrastructure/modules/secrets-manager/main.tf +++ b/infrastructure/modules/secrets-manager/main.tf @@ -11,6 +11,8 @@ # # Inputs intentionally NOT exposed (hardcoded below): # - block_public_policy → always true; callers cannot override +# +# Cross-variable input constraints are enforced in validations.tf. ################################################################ module "secret" { @@ -27,11 +29,13 @@ module "secret" { recovery_window_in_days = var.recovery_window_in_days # Secret value - create_random_password = var.create_random_password - secret_string = var.secret_string - secret_string_wo = var.secret_string_wo - secret_string_wo_version = var.secret_string_wo_version - ignore_secret_changes = var.ignore_secret_changes + create_random_password = var.create_random_password + random_password_length = var.random_password_length + random_password_override_special = var.random_password_override_special + secret_string = var.secret_string + secret_string_wo = var.secret_string_wo + secret_string_wo_version = var.secret_string_wo_version + ignore_secret_changes = var.ignore_secret_changes # Policy create_policy = var.create_policy @@ -40,6 +44,7 @@ module "secret" { # Rotation enable_rotation = var.enable_rotation + rotate_immediately = var.rotate_immediately rotation_lambda_arn = var.rotation_lambda_arn rotation_rules = var.rotation_rules diff --git a/infrastructure/modules/secrets-manager/outputs.tf b/infrastructure/modules/secrets-manager/outputs.tf index 5363e619..b30554bf 100644 --- a/infrastructure/modules/secrets-manager/outputs.tf +++ b/infrastructure/modules/secrets-manager/outputs.tf @@ -17,3 +17,9 @@ output "secret_version_id" { description = "The unique identifier of the current version of the secret" value = module.secret.secret_version_id } + +output "secret_string" { + description = "The secret string value. Sensitive — only use where required." + sensitive = true + value = module.secret.secret_string +} diff --git a/infrastructure/modules/secrets-manager/validations.tf b/infrastructure/modules/secrets-manager/validations.tf new file mode 100644 index 00000000..0a758f1f --- /dev/null +++ b/infrastructure/modules/secrets-manager/validations.tf @@ -0,0 +1,34 @@ +################################################################ +# Input validation +# +# Validates cross-variable constraints that cannot be expressed +# through individual variable validation blocks: +# +# * secret_string, secret_string_wo, and create_random_password +# are mutually exclusive — at most one may be set +# * enable_rotation requires both rotation_lambda_arn and +# rotation_rules to be provided +################################################################ + +resource "terraform_data" "validations" { + count = module.this.enabled ? 1 : 0 + + lifecycle { + precondition { + condition = !(var.create_random_password && var.secret_string != null) + error_message = "create_random_password and secret_string are mutually exclusive; set only one." + } + precondition { + condition = !(var.create_random_password && var.secret_string_wo != null) + error_message = "create_random_password and secret_string_wo are mutually exclusive; set only one." + } + precondition { + condition = !(var.secret_string != null && var.secret_string_wo != null) + error_message = "secret_string and secret_string_wo are mutually exclusive; set only one." + } + precondition { + condition = !var.enable_rotation || (var.rotation_lambda_arn != "" && var.rotation_rules != null) + error_message = "enable_rotation requires both rotation_lambda_arn and rotation_rules to be set." + } + } +} diff --git a/infrastructure/modules/secrets-manager/variables.tf b/infrastructure/modules/secrets-manager/variables.tf index 036f7f8f..2c47affd 100644 --- a/infrastructure/modules/secrets-manager/variables.tf +++ b/infrastructure/modules/secrets-manager/variables.tf @@ -9,6 +9,11 @@ # - name / name_prefix → derived from module.this.id via locals.tf # - secret_binary → not supported; use secret_string # - replica → replication not required for this use case +# - version_stages → low-value for standard use cases +################################################################ + +################################################################ +# Naming ################################################################ variable "secret_name" { @@ -17,6 +22,10 @@ variable "secret_name" { description = "Optional explicit name for the secret. When null, the name is derived from context labels via module.this.id." } +################################################################ +# Secret configuration +################################################################ + variable "description" { type = string default = null @@ -39,12 +48,32 @@ variable "recovery_window_in_days" { } } +################################################################ +# Secret value +################################################################ + variable "create_random_password" { type = bool default = false description = "When true, generates a random password as the secret value. When set, secret_string and secret_string_wo should not be set." } +variable "random_password_length" { + type = number + default = 32 + description = "The length of the randomly-generated password. Only used when create_random_password is true." + validation { + condition = var.random_password_length >= 8 && var.random_password_length <= 4096 + error_message = "random_password_length must be between 8 and 4096 inclusive." + } +} + +variable "random_password_override_special" { + type = string + default = "!@#$%&*()-_=+[]{}<>:?" + description = "Supply your own list of special characters for random password generation. Overrides the default special character set. Only used when create_random_password is true." +} + variable "secret_string" { type = string default = null @@ -71,6 +100,10 @@ variable "ignore_secret_changes" { description = "When true, Terraform will ignore any changes made to the secret value outside of Terraform (e.g. by a rotation Lambda). Set to true when rotation is enabled." } +################################################################ +# Policy +################################################################ + variable "create_policy" { type = bool default = false @@ -103,12 +136,22 @@ variable "policy_statements" { description = "A map of IAM policy statements to attach to the secret policy. Only used when create_policy is true." } +################################################################ +# Rotation +################################################################ + variable "enable_rotation" { type = bool default = false description = "Whether to enable automatic secret rotation via a Lambda function." } +variable "rotate_immediately" { + type = bool + default = null + description = "When enable_rotation is true, specifies whether to rotate the secret immediately or wait until the next scheduled rotation window. Defaults to immediate rotation when not set." +} + variable "rotation_lambda_arn" { type = string default = ""