From e70b098e127bf30e1a911bd58a3c9e04998215ef Mon Sep 17 00:00:00 2001 From: Jonathan Moss <2729151+jwmoss@users.noreply.github.com> Date: Wed, 16 Sep 2026 14:18:42 -0400 Subject: [PATCH 1/3] fix: validate sql-warehouse module inputs --- README.md | 11 +++++++ main.tf | 7 +++++ tests/module.tftest.hcl | 56 ++++++++++++++++++++++++++++++++++++ variables.tf | 63 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 136 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 265da72..d224a13 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,17 @@ The workspace pattern module checks the complete DataTF contract and its integra [Apache-2.0](LICENSE). +## Input safeguards + +The module rejects blank required names and invalid access inputs during the plan. +Cross-input preconditions preserve the Terraform 1.5 minimum and existing resource addresses. +Provider and API checks still apply. These checks do not prove complete permission visibility. + +Each permission needs exactly one nonblank principal and a supported resource-specific permission level. +`databricks_permissions` manages the object's permission set. Keep one state owner for that set. +Empty permissions omit the permission resource. +See [provider permission semantics](https://github.com/databricks/terraform-provider-databricks/blob/v1.130.0/docs/resources/permissions.md). + ## Requirements diff --git a/main.tf b/main.tf index 8addb4a..9d0cfc6 100644 --- a/main.tf +++ b/main.tf @@ -3,6 +3,13 @@ locals { } resource "databricks_sql_endpoint" "this" { + lifecycle { + precondition { + condition = var.max_num_clusters >= var.min_num_clusters + error_message = "max_num_clusters must be at least min_num_clusters." + } + } + name = var.name cluster_size = var.cluster_size min_num_clusters = var.min_num_clusters diff --git a/tests/module.tftest.hcl b/tests/module.tftest.hcl index c64cd7f..d1249e0 100644 --- a/tests/module.tftest.hcl +++ b/tests/module.tftest.hcl @@ -47,3 +47,59 @@ run "without_access" { error_message = "Empty access must omit the access resources." } } + +run "reject_blank_name" { + command = plan + variables { + name = " " + } + expect_failures = [var.name] +} + +run "reject_missing_principal" { + command = plan + variables { + permissions = [{ permission_level = "CAN_USE" }] + } + expect_failures = [var.permissions] +} + +run "reject_multiple_principals" { + command = plan + variables { + permissions = [{ permission_level = "CAN_USE", user_name = "user@example.com", group_name = "readers" }] + } + expect_failures = [var.permissions] +} + +run "reject_blank_principal" { + command = plan + variables { + permissions = [{ permission_level = "CAN_USE", group_name = " " }] + } + expect_failures = [var.permissions] +} + +run "reject_invalid_permission" { + command = plan + variables { + permissions = [{ permission_level = "INVALID", group_name = "readers" }] + } + expect_failures = [var.permissions] +} + +run "reject_reversed_cluster_limits" { + command = plan + variables { + min_num_clusters = 4 + } + expect_failures = [databricks_sql_endpoint.this] +} + +run "reject_group_owner" { + command = plan + variables { + permissions = [{ permission_level = "IS_OWNER", group_name = "readers" }] + } + expect_failures = [var.permissions] +} diff --git a/variables.tf b/variables.tf index 910144a..5a61c65 100644 --- a/variables.tf +++ b/variables.tf @@ -1,47 +1,89 @@ variable "name" { description = "SQL warehouse name." type = string + nullable = false + + validation { + condition = try(length(trimspace(var.name)) > 0, false) + error_message = "name must not be empty or blank." + } } variable "cluster_size" { description = "Warehouse size, for example 2X-Small, Small, or Medium." type = string + nullable = false + + validation { + condition = try(length(trimspace(var.cluster_size)) > 0, false) + error_message = "cluster_size must not be empty or blank." + } } variable "min_num_clusters" { description = "Minimum number of clusters the warehouse runs." type = number + nullable = false + + validation { + condition = try(var.min_num_clusters >= 1 && floor(var.min_num_clusters) == var.min_num_clusters, false) + error_message = "min_num_clusters must be an integer of at least 1." + } } variable "max_num_clusters" { description = "Maximum number of clusters the warehouse scales to." type = number + nullable = false + + validation { + condition = try(var.max_num_clusters >= 1 && floor(var.max_num_clusters) == var.max_num_clusters, false) + error_message = "max_num_clusters must be an integer of at least 1." + } } variable "auto_stop_mins" { description = "Minutes of inactivity before the warehouse stops. 0 disables auto stop." type = number + nullable = false + + validation { + condition = try(var.auto_stop_mins >= 0 && floor(var.auto_stop_mins) == var.auto_stop_mins, false) + error_message = "auto_stop_mins must be an integer of at least 0." + } } variable "warehouse_type" { description = "Warehouse type: CLASSIC or PRO." type = string + nullable = false + + validation { + condition = contains(["CLASSIC", "PRO"], var.warehouse_type) + error_message = "warehouse_type must be CLASSIC or PRO." + } } variable "enable_photon" { description = "Run queries on the Photon engine." type = bool + nullable = false } variable "enable_serverless_compute" { description = "Run the warehouse on serverless compute." type = bool + nullable = false } variable "spot_instance_policy" { description = "Spot policy: COST_OPTIMIZED or RELIABILITY_OPTIMIZED." type = string default = null + validation { + condition = var.spot_instance_policy == null ? true : contains(["COST_OPTIMIZED", "RELIABILITY_OPTIMIZED"], var.spot_instance_policy) + error_message = "spot_instance_policy must be COST_OPTIMIZED or RELIABILITY_OPTIMIZED." + } } variable "tags" { @@ -60,5 +102,24 @@ variable "permissions" { service_principal_name = optional(string) })) - default = [] + default = [] + nullable = false + + validation { + condition = try(alltrue([for permission in var.permissions : + length([for principal in [permission.group_name, permission.user_name, permission.service_principal_name] : + principal if principal != null + ]) == 1 && + alltrue([for principal in [permission.group_name, permission.user_name, permission.service_principal_name] : + principal == null ? true : length(trimspace(principal)) > 0 + ]) && contains(["CAN_USE", "CAN_MONITOR", "CAN_MANAGE", "CAN_VIEW", "IS_OWNER"], permission.permission_level) + ]), false) + error_message = "Each permission needs exactly one nonblank principal and a supported level: CAN_USE, CAN_MONITOR, CAN_MANAGE, CAN_VIEW, IS_OWNER." + } + validation { + condition = alltrue([for permission in var.permissions : + permission.permission_level != "IS_OWNER" || permission.group_name == null + ]) && length([for permission in var.permissions : permission if permission.permission_level == "IS_OWNER"]) <= 1 + error_message = "A warehouse can have at most one explicit owner; a group cannot own a warehouse." + } } From 0d268c56a62da715d410ceeab0515bcf8d1ea3bc Mon Sep 17 00:00:00 2001 From: Jonathan Moss <2729151+jwmoss@users.noreply.github.com> Date: Wed, 16 Sep 2026 14:28:40 -0400 Subject: [PATCH 2/3] fix: preserve provider defaults in input validation --- README.md | 1 + main.tf | 2 +- tests/module.tftest.hcl | 12 ++++++++++++ variables.tf | 14 ++++---------- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index d224a13..d7876a2 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,7 @@ The workspace pattern module checks the complete DataTF contract and its integra The module rejects blank required names and invalid access inputs during the plan. Cross-input preconditions preserve the Terraform 1.5 minimum and existing resource addresses. +Null remains valid for inputs where the provider supplies a default. Provider and API checks still apply. These checks do not prove complete permission visibility. Each permission needs exactly one nonblank principal and a supported resource-specific permission level. diff --git a/main.tf b/main.tf index 9d0cfc6..a606c3d 100644 --- a/main.tf +++ b/main.tf @@ -5,7 +5,7 @@ locals { resource "databricks_sql_endpoint" "this" { lifecycle { precondition { - condition = var.max_num_clusters >= var.min_num_clusters + condition = coalesce(var.max_num_clusters, 1) >= coalesce(var.min_num_clusters, 1) error_message = "max_num_clusters must be at least min_num_clusters." } } diff --git a/tests/module.tftest.hcl b/tests/module.tftest.hcl index d1249e0..7166b15 100644 --- a/tests/module.tftest.hcl +++ b/tests/module.tftest.hcl @@ -103,3 +103,15 @@ run "reject_group_owner" { } expect_failures = [var.permissions] } + +run "accept_provider_defaults" { + command = plan + variables { + min_num_clusters = null + max_num_clusters = null + auto_stop_mins = null + warehouse_type = null + enable_photon = null + enable_serverless_compute = null + } +} diff --git a/variables.tf b/variables.tf index 5a61c65..6ed4a7c 100644 --- a/variables.tf +++ b/variables.tf @@ -23,10 +23,9 @@ variable "cluster_size" { variable "min_num_clusters" { description = "Minimum number of clusters the warehouse runs." type = number - nullable = false validation { - condition = try(var.min_num_clusters >= 1 && floor(var.min_num_clusters) == var.min_num_clusters, false) + condition = var.min_num_clusters == null ? true : try(var.min_num_clusters >= 1 && floor(var.min_num_clusters) == var.min_num_clusters, false) error_message = "min_num_clusters must be an integer of at least 1." } } @@ -34,10 +33,9 @@ variable "min_num_clusters" { variable "max_num_clusters" { description = "Maximum number of clusters the warehouse scales to." type = number - nullable = false validation { - condition = try(var.max_num_clusters >= 1 && floor(var.max_num_clusters) == var.max_num_clusters, false) + condition = var.max_num_clusters == null ? true : try(var.max_num_clusters >= 1 && floor(var.max_num_clusters) == var.max_num_clusters, false) error_message = "max_num_clusters must be an integer of at least 1." } } @@ -45,10 +43,9 @@ variable "max_num_clusters" { variable "auto_stop_mins" { description = "Minutes of inactivity before the warehouse stops. 0 disables auto stop." type = number - nullable = false validation { - condition = try(var.auto_stop_mins >= 0 && floor(var.auto_stop_mins) == var.auto_stop_mins, false) + condition = var.auto_stop_mins == null ? true : try(var.auto_stop_mins >= 0 && floor(var.auto_stop_mins) == var.auto_stop_mins, false) error_message = "auto_stop_mins must be an integer of at least 0." } } @@ -56,10 +53,9 @@ variable "auto_stop_mins" { variable "warehouse_type" { description = "Warehouse type: CLASSIC or PRO." type = string - nullable = false validation { - condition = contains(["CLASSIC", "PRO"], var.warehouse_type) + condition = var.warehouse_type == null ? true : contains(["CLASSIC", "PRO"], var.warehouse_type) error_message = "warehouse_type must be CLASSIC or PRO." } } @@ -67,13 +63,11 @@ variable "warehouse_type" { variable "enable_photon" { description = "Run queries on the Photon engine." type = bool - nullable = false } variable "enable_serverless_compute" { description = "Run the warehouse on serverless compute." type = bool - nullable = false } variable "spot_instance_policy" { From efa0b4964086fb7d4022f9148c59adc11e49a0c1 Mon Sep 17 00:00:00 2001 From: Jonathan Moss <2729151+jwmoss@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:46:09 -0400 Subject: [PATCH 3/3] chore: prepare v1.0.1 release --- CHANGELOG.md | 6 ++++++ README.md | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eec0654..60a9548 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 1.0.1 - 2026-09-16 + +- Validate module inputs and access rules before provider API requests. +- Reject invalid permissions and conflicting settings without changing resource addresses. +- Preserve provider defaults for optional values. + ## 1.0.0 - 2026-09-09 - Declare the module contract stable. The inputs, outputs, resource addresses, and provider diff --git a/README.md b/README.md index d7876a2..b0c53ca 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ The resource addresses above are part of the DataTF import contract. Do not rena ```hcl module "warehouse" { source = "536tech/sql-warehouse/databricks" - version = "1.0.0" + version = "1.0.1" name = "Analytics WH" cluster_size = "Small"