fix!: various issues / cleanup + adds approve_until_date support (#42)

* fix!: removes region variable as not used and not needed

* chore: cleanup potential issues

* fix!: converts bucket_id to list(string) (fix #41) + removes label

* chore: bump s3-bucket to latest (4.0.1)

* feat: adds approve_until_date support (closes #8)

* chore: updates example to show patching (closes #40)

* feat: adds s3_bucket_prefix_scan_logs var (closes #14)

* chore: readme changes

* chore: make tf fmt happy 👍

docs/terraform.md

@@ -18,8 +18,7 @@
 |------|--------|---------|
 | <a name="module_install_window_label"></a> [install\_window\_label](#module\_install\_window\_label) | cloudposse/label/null | 0.25.0 |
 | <a name="module_scan_window_label"></a> [scan\_window\_label](#module\_scan\_window\_label) | cloudposse/label/null | 0.25.0 |
-| <a name="module_ssm_patch_log_s3_bucket"></a> [ssm\_patch\_log\_s3\_bucket](#module\_ssm\_patch\_log\_s3\_bucket) | cloudposse/s3-bucket/aws | 4.0.0 |
-| <a name="module_ssm_patch_log_s3_bucket_label"></a> [ssm\_patch\_log\_s3\_bucket\_label](#module\_ssm\_patch\_log\_s3\_bucket\_label) | cloudposse/label/null | 0.25.0 |
+| <a name="module_ssm_patch_log_s3_bucket"></a> [ssm\_patch\_log\_s3\_bucket](#module\_ssm\_patch\_log\_s3\_bucket) | cloudposse/s3-bucket/aws | 4.0.1 |
 | <a name="module_this"></a> [this](#module\_this) | cloudposse/label/null | 0.25.0 |
 
 ## Resources
@@ -47,7 +46,7 @@
 | <a name="input_approved_patches"></a> [approved\_patches](#input\_approved\_patches) | A list of explicitly approved patches for the baseline | `list(string)` | `[]` | no |
 | <a name="input_approved_patches_compliance_level"></a> [approved\_patches\_compliance\_level](#input\_approved\_patches\_compliance\_level) | Defines the compliance level for approved patches. This means that if an approved patch is reported as missing, this is the severity of the compliance violation. Valid compliance levels include the following: CRITICAL, HIGH, MEDIUM, LOW, INFORMATIONAL, UNSPECIFIED. The default value is UNSPECIFIED. | `string` | `"HIGH"` | no |
 | <a name="input_attributes"></a> [attributes](#input\_attributes) | ID element. Additional attributes (e.g. `workers` or `cluster`) to add to `id`,<br>in the order they appear in the list. New attributes are appended to the<br>end of the list. The elements of the list are joined by the `delimiter`<br>and treated as a single ID element. | `list(string)` | `[]` | no |
-| <a name="input_bucket_id"></a> [bucket\_id](#input\_bucket\_id) | The bucket ID to use for the patch log. If no bucket ID is provided, the module will create a new one. | `string` | `null` | no |
+| <a name="input_bucket_id"></a> [bucket\_id](#input\_bucket\_id) | The bucket ID to use for the patch log. If no bucket ID is provided, the module will create a new one. This is of type `list(string)` to work around #41 / https://github.com/hashicorp/terraform/issues/28962. | `list(string)` | `[]` | no |
 | <a name="input_context"></a> [context](#input\_context) | Single object for setting entire context at once.<br>See description of individual variables for details.<br>Leave string and numeric variables as `null` to use default value.<br>Individual variable settings (non-null) override settings in context object,<br>except for attributes, tags, and additional\_tag\_map, which are merged. | `any` | <pre>{<br>  "additional_tag_map": {},<br>  "attributes": [],<br>  "delimiter": null,<br>  "descriptor_formats": {},<br>  "enabled": true,<br>  "environment": null,<br>  "id_length_limit": null,<br>  "label_key_case": null,<br>  "label_order": [],<br>  "label_value_case": null,<br>  "labels_as_tags": [<br>    "unset"<br>  ],<br>  "name": null,<br>  "namespace": null,<br>  "regex_replace_chars": null,<br>  "stage": null,<br>  "tags": {},<br>  "tenant": null<br>}</pre> | no |
 | <a name="input_delimiter"></a> [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.<br>Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no |
 | <a name="input_descriptor_formats"></a> [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.<br>Map of maps. Keys are names of descriptors. Values are maps of the form<br>`{<br>   format = string<br>   labels = list(string)<br>}`<br>(Type is `any` so the map values can later be enhanced to provide additional options.)<br>`format` is a Terraform format string to be passed to the `format()` function.<br>`labels` is a list of labels, in order, to pass to `format()` function.<br>Label values will be normalized before being passed to `format()` so they will be<br>identical to how they appear in `id`.<br>Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no |
@@ -72,12 +71,12 @@
 | <a name="input_notification_events"></a> [notification\_events](#input\_notification\_events) | The different events for which you can receive notifications. Valid values: All, InProgress, Success, TimedOut, Cancelled, and Failed | `list(string)` | <pre>[<br>  "All"<br>]</pre> | no |
 | <a name="input_notification_type"></a> [notification\_type](#input\_notification\_type) | When specified with Command, receive notification when the status of a command changes. When specified with Invocation, for commands sent to multiple instances, receive notification on a per-instance basis when the status of a command changes. Valid values: Command and Invocation | `string` | `"Command"` | no |
 | <a name="input_operating_system"></a> [operating\_system](#input\_operating\_system) | Defines the operating system the patch baseline applies to. Supported operating systems include WINDOWS, AMAZON\_LINUX, AMAZON\_LINUX\_2, SUSE, UBUNTU, CENTOS, and REDHAT\_ENTERPRISE\_LINUX. The Default value is WINDOWS. | `string` | `"AMAZON_LINUX_2"` | no |
-| <a name="input_patch_baseline_approval_rules"></a> [patch\_baseline\_approval\_rules](#input\_patch\_baseline\_approval\_rules) | A set of rules used to include patches in the baseline. Up to 10 approval rules can be specified. Each `approval_rule` block requires the fields documented below. | <pre>list(object({<br>    approve_after_days : number<br>    compliance_level : string<br>    enable_non_security : bool<br>    patch_baseline_filters : list(object({<br>      name : string<br>      values : list(string)<br>    }))<br>  }))</pre> | <pre>[<br>  {<br>    "approve_after_days": 7,<br>    "compliance_level": "HIGH",<br>    "enable_non_security": true,<br>    "patch_baseline_filters": [<br>      {<br>        "name": "PRODUCT",<br>        "values": [<br>          "AmazonLinux2",<br>          "AmazonLinux2.0"<br>        ]<br>      },<br>      {<br>        "name": "CLASSIFICATION",<br>        "values": [<br>          "Security",<br>          "Bugfix",<br>          "Recommended"<br>        ]<br>      },<br>      {<br>        "name": "SEVERITY",<br>        "values": [<br>          "Critical",<br>          "Important",<br>          "Medium"<br>        ]<br>      }<br>    ]<br>  }<br>]</pre> | no |
+| <a name="input_patch_baseline_approval_rules"></a> [patch\_baseline\_approval\_rules](#input\_patch\_baseline\_approval\_rules) | A set of rules used to include patches in the baseline. Up to 10 approval rules can be specified.<br>    Each `approval_rule` block requires the fields documented below (unless marked optional).<br>    `approve_after_days` and `approve_until_date` conflict, do not set both in the same `approval_rule`.<br><br>    See https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ssm_patch_baseline#approval_rule-block for full details. | <pre>list(object({<br>    approve_after_days : optional(number)<br>    approve_until_date : optional(string)<br>    compliance_level : string<br>    enable_non_security : bool<br>    patch_baseline_filters : list(object({<br>      name : string<br>      values : list(string)<br>    }))<br>  }))</pre> | <pre>[<br>  {<br>    "approve_after_days": 7,<br>    "compliance_level": "HIGH",<br>    "enable_non_security": true,<br>    "patch_baseline_filters": [<br>      {<br>        "name": "PRODUCT",<br>        "values": [<br>          "AmazonLinux2",<br>          "AmazonLinux2.0"<br>        ]<br>      },<br>      {<br>        "name": "CLASSIFICATION",<br>        "values": [<br>          "Security",<br>          "Bugfix",<br>          "Recommended"<br>        ]<br>      },<br>      {<br>        "name": "SEVERITY",<br>        "values": [<br>          "Critical",<br>          "Important",<br>          "Medium"<br>        ]<br>      }<br>    ]<br>  }<br>]</pre> | no |
 | <a name="input_reboot_option"></a> [reboot\_option](#input\_reboot\_option) | When you choose the RebootIfNeeded option, the instance is rebooted if Patch Manager installed new patches, or if it detected any patches with a status of INSTALLED\_PENDING\_REBOOT during the Install operation. Possible values : RebootIfNeeded, NoReboot | `string` | `"RebootIfNeeded"` | no |
 | <a name="input_regex_replace_chars"></a> [regex\_replace\_chars](#input\_regex\_replace\_chars) | Terraform regular expression (regex) string.<br>Characters matching the regex will be removed from the ID elements.<br>If not set, `"/[^a-zA-Z0-9-]/"` is used to remove all characters other than hyphens, letters and digits. | `string` | `null` | no |
-| <a name="input_region"></a> [region](#input\_region) | AWS region | `string` | n/a | yes |
 | <a name="input_rejected_patches"></a> [rejected\_patches](#input\_rejected\_patches) | A list of rejected patches | `list(string)` | `[]` | no |
-| <a name="input_s3_bucket_prefix_install_logs"></a> [s3\_bucket\_prefix\_install\_logs](#input\_s3\_bucket\_prefix\_install\_logs) | The Amazon S3 bucket subfolder | `string` | `"install"` | no |
+| <a name="input_s3_bucket_prefix_install_logs"></a> [s3\_bucket\_prefix\_install\_logs](#input\_s3\_bucket\_prefix\_install\_logs) | The Amazon S3 bucket subfolder for install logs | `string` | `"install"` | no |
+| <a name="input_s3_bucket_prefix_scan_logs"></a> [s3\_bucket\_prefix\_scan\_logs](#input\_s3\_bucket\_prefix\_scan\_logs) | The Amazon S3 bucket subfolder for scan logs | `string` | `"scanning"` | no |
 | <a name="input_scan_maintenance_window_cutoff"></a> [scan\_maintenance\_window\_cutoff](#input\_scan\_maintenance\_window\_cutoff) | The number of hours before the end of the Maintenance Window that Systems Manager stops scheduling new tasks for execution | `number` | `1` | no |
 | <a name="input_scan_maintenance_window_duration"></a> [scan\_maintenance\_window\_duration](#input\_scan\_maintenance\_window\_duration) | The duration of the maintenence windows (hours) | `number` | `3` | no |
 | <a name="input_scan_maintenance_window_schedule"></a> [scan\_maintenance\_window\_schedule](#input\_scan\_maintenance\_window\_schedule) | The schedule of the Maintenance Window in the form of a cron or rate expression. | `string` | `"cron(0 0 18 ? * WED *)"` | no |

examples/complete/main.tf

@@ -37,16 +37,15 @@ module "ec2_instance" {
   ssm_patch_manager_s3_log_bucket = format("%s-%s-%s-%s", module.this.namespace, module.this.environment, module.this.stage, module.this.name)
 
   tags = {
-    "TOSCAN"  = "true",
-    "TOPATCH" = "true"
+    "Patch Group" : "TOPATCH",
   }
+
   context = module.this.context
 
 }
 
 module "ssm_patch_manager" {
   source = "../.."
-  region = var.region
 
   context = module.this.context
 }

main.tf

@@ -53,7 +53,7 @@ resource "aws_ssm_maintenance_window_task" "task_scan_patches" {
         values = ["NoReboot"]
       }
       output_s3_bucket     = local.bucket_id
-      output_s3_key_prefix = "scaning"
+      output_s3_key_prefix = var.s3_bucket_prefix_scan_logs
       service_role_arn     = var.sns_notification_role_arn
 
       dynamic "notification_config" {
@@ -188,6 +188,7 @@ resource "aws_ssm_patch_baseline" "baseline" {
     content {
 
       approve_after_days  = approval_rule.value.approve_after_days
+      approve_until_date  = approval_rule.value.approve_until_date
       compliance_level    = approval_rule.value.compliance_level
       enable_non_security = approval_rule.value.enable_non_security
 
@@ -202,7 +203,8 @@ resource "aws_ssm_patch_baseline" "baseline" {
       }
     }
   }
-  tags = var.tags
+
+  tags = module.this.tags
 }
 
 resource "aws_ssm_patch_group" "install_patchgroup" {

outputs.tf

@@ -10,40 +10,40 @@ output "ssm_patch_log_s3_bucket_arn" {
 
 output "scan_maintenance_window_task_id" {
   description = "SSM Patch Manager scan maintenance windows task ID"
-  value       = aws_ssm_maintenance_window_task.task_scan_patches[0].id
+  value       = try(aws_ssm_maintenance_window_task.task_scan_patches[0].id, "")
 }
 
 output "install_maintenance_window_task_id" {
   description = "SSM Patch Manager install maintenance windows task ID"
-  value       = aws_ssm_maintenance_window_task.task_install_patches[0].id
+  value       = try(aws_ssm_maintenance_window_task.task_install_patches[0].id, "")
 }
 
 output "scan_maintenance_window_target_id" {
   description = "SSM Patch Manager scan maintenance window target ID"
-  value       = aws_ssm_maintenance_window_target.target_scan[0].id
+  value       = try(aws_ssm_maintenance_window_target.target_scan[0].id, "")
 }
 
 output "install_maintenance_window_target_id" {
   description = "SSM Patch Manager install maintenance window target ID"
-  value       = aws_ssm_maintenance_window_target.target_install[0].id
+  value       = try(aws_ssm_maintenance_window_target.target_install[0].id, "")
 }
 
 output "install_maintenance_window_id" {
   description = "SSM Patch Manager install maintenance window ID"
-  value       = aws_ssm_maintenance_window.install_window[0].id
+  value       = try(aws_ssm_maintenance_window.install_window[0].id, "")
 }
 
 output "patch_baseline_arn" {
   description = "SSM Patch Manager patch baseline ARN"
-  value       = aws_ssm_patch_baseline.baseline[0].arn
+  value       = try(aws_ssm_patch_baseline.baseline[0].arn, "")
 }
 
 output "install_patch_group_id" {
   description = "SSM Patch Manager install patch group ID"
-  value       = aws_ssm_patch_group.install_patchgroup[0].id
+  value       = try(aws_ssm_patch_group.install_patchgroup[0].id, "")
 }
 
 output "scan_patch_group_id" {
   description = "SSM Patch Manager scan patch group ID"
-  value       = aws_ssm_patch_group.scan_patchgroup[0].id
+  value       = try(aws_ssm_patch_group.scan_patchgroup[0].id, "")
 }

ssm_log_bucket.tf

@@ -1,20 +1,11 @@
 locals {
   account_id        = join("", data.aws_caller_identity.current[*].account_id)
   aws_partition     = join("", data.aws_partition.current[*].partition)
-  create_log_bucket = local.enabled && var.bucket_id == null
-  bucket_id         = var.bucket_id != null ? var.bucket_id : module.ssm_patch_log_s3_bucket_label.id
+  create_log_bucket = local.enabled && length(var.bucket_id) > 0
+  bucket_id         = local.create_log_bucket ? var.bucket_id[0] : module.this.id
   bucket_policy     = var.ssm_bucket_policy != null ? var.ssm_bucket_policy : try(data.aws_iam_policy_document.bucket_policy[0].json, "")
 }
 
-
-module "ssm_patch_log_s3_bucket_label" {
-  source  = "cloudposse/label/null"
-  version = "0.25.0"
-
-  enabled = local.create_log_bucket
-  # attributes = ["scan-window"]
-  context = module.this.context
-}
 data "aws_iam_policy_document" "bucket_policy" {
   count = local.create_log_bucket ? 1 : 0
   statement {
@@ -27,8 +18,8 @@ data "aws_iam_policy_document" "bucket_policy" {
     ]
 
     resources = [
-      format("arn:%s:s3:::%s", local.aws_partition, module.ssm_patch_log_s3_bucket_label.id),
-      format("arn:%s:s3:::%s/*", local.aws_partition, module.ssm_patch_log_s3_bucket_label.id)
+      format("arn:%s:s3:::%s", local.aws_partition, module.this.id),
+      format("arn:%s:s3:::%s/*", local.aws_partition, module.this.id)
     ]
 
     principals {
@@ -41,10 +32,10 @@ data "aws_iam_policy_document" "bucket_policy" {
 module "ssm_patch_log_s3_bucket" {
   count   = local.create_log_bucket ? 1 : 0
   source  = "cloudposse/s3-bucket/aws"
-  version = "4.0.0"
+  version = "4.0.1"
 
   acl                     = "private"
   versioning_enabled      = var.ssm_bucket_versioning_enable
   source_policy_documents = [local.bucket_policy]
-  context                 = module.ssm_patch_log_s3_bucket_label.context
+  context                 = module.this.context
 }

variables.tf

@@ -1,8 +1,3 @@
-variable "region" {
-  type        = string
-  description = "AWS region"
-}
-
 variable "scan_maintenance_window_duration" {
   description = "The duration of the maintenence windows (hours)"
   type        = number
@@ -122,11 +117,17 @@ variable "install_maintenance_window_schedule" {
 }
 
 variable "s3_bucket_prefix_install_logs" {
-  description = "The Amazon S3 bucket subfolder"
+  description = "The Amazon S3 bucket subfolder for install logs"
   type        = string
   default     = "install"
 }
 
+variable "s3_bucket_prefix_scan_logs" {
+  description = "The Amazon S3 bucket subfolder for scan logs"
+  type        = string
+  default     = "scanning"
+}
+
 variable "task_install_priority" {
   description = "The priority of the task in the Maintenance Window, the lower the number the higher the priority. Tasks in a Maintenance Window are scheduled in priority order with tasks that have the same priority scheduled in parallel."
   type        = number
@@ -169,9 +170,16 @@ variable "rejected_patches" {
 }
 
 variable "patch_baseline_approval_rules" {
-  description = "A set of rules used to include patches in the baseline. Up to 10 approval rules can be specified. Each `approval_rule` block requires the fields documented below."
+  description = <<EOT
+    A set of rules used to include patches in the baseline. Up to 10 approval rules can be specified.
+    Each `approval_rule` block requires the fields documented below (unless marked optional).
+    `approve_after_days` and `approve_until_date` conflict, do not set both in the same `approval_rule`.
+
+    See https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ssm_patch_baseline#approval_rule-block for full details.
+  EOT
   type = list(object({
-    approve_after_days : number
+    approve_after_days : optional(number)
+    approve_until_date : optional(string)
     compliance_level : string
     enable_non_security : bool
     patch_baseline_filters : list(object({
@@ -216,13 +224,13 @@ variable "ssm_bucket_policy" {
 }
 
 variable "bucket_id" {
-  type        = string
-  description = "The bucket ID to use for the patch log. If no bucket ID is provided, the module will create a new one."
-  default     = null
+  type        = list(string)
+  description = "The bucket ID to use for the patch log. If no bucket ID is provided, the module will create a new one. This is of type `list(string)` to work around #41 / https://github.com/hashicorp/terraform/issues/28962."
+  default     = []
 }
 
 variable "ssm_bucket_versioning_enable" {
   type        = string
   description = "To enable or disable S3 bucket versioning for the log bucket."
   default     = true
-}
+}