Skip to content

Commit cb241fe

Browse files
authored
chore(refactor): refactor repeating patterns + consolidation (#77)
## what - Extracted 37 repeated try() patterns into stack_property_resolver helper (main.tf:284-315) - Created reusable array merging in stack_array_merger for hook patterns (main.tf:318-331) - Consolidated stack filtering into generic filtered_stacks helper (main.tf:334-347) - Centralized hardcoded constants: - default_workspace_name = "default" - root_space_id = "root" - Simplified space resolution using the new constant (main.tf:355) ## why - I was playing around with Claude Code and asked it to consolidate and improve the complex logic we have in this module. I like it. Let me know your thoughts, team! ## references - N/A <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Improved consistency by replacing hardcoded workspace and space IDs with local constants. - Centralized stack property resolution and array merging for lifecycle hooks, reducing duplication and streamlining configuration management. - Consolidated filtering logic for stacks with specific features, enhancing maintainability and clarity. - Updated resource property references to use new centralized maps for more reliable configuration handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent d414cd6 commit cb241fe

File tree

1 file changed

+91
-37
lines changed

1 file changed

+91
-37
lines changed

main.tf

Lines changed: 91 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
# that are not directly used in the resource creation.
2626

2727
locals {
28+
# Constants
29+
default_workspace_name = "default"
30+
root_space_id = "root"
31+
2832
_multi_instance_structure = var.root_module_structure == "MultiInstance"
2933

3034
# Read all stack files following the associated root_module_structue convention:
@@ -88,7 +92,7 @@ locals {
8892

8993
_single_instance_root_module_yaml_decoded = {
9094
for module in local.enabled_root_modules : module => {
91-
"default" = yamldecode(file("${path.root}/${var.root_modules_path}/${module}/stack.yaml"))
95+
(local.default_workspace_name) = yamldecode(file("${path.root}/${var.root_modules_path}/${module}/stack.yaml"))
9296
} if !local._multi_instance_structure
9397
}
9498

@@ -142,7 +146,7 @@ locals {
142146
"root_module" = module,
143147

144148
# If default_tf_workspace_enabled is true, use "default" workspace, otherwise our file name is the workspace name
145-
"terraform_workspace" = try(content.automation_settings.default_tf_workspace_enabled, local._default_tf_workspace_enabled) ? "default" : trimsuffix(file, ".yaml"),
149+
"terraform_workspace" = try(content.automation_settings.default_tf_workspace_enabled, local._default_tf_workspace_enabled) ? local.default_workspace_name : trimsuffix(file, ".yaml"),
146150

147151
# tfvars_file_name only pertains to MultiInstance, as SingleInstance expects consumers to use an auto.tfvars file.
148152
# `yaml` is intentionally used here as we require Stack and `tfvars` config files to be named equally
@@ -276,13 +280,63 @@ locals {
276280
space.name => space.space_id
277281
}
278282

283+
# Helper for property resolution with fallback to defaults
284+
stack_property_resolver = {
285+
for stack in local.stacks : stack => {
286+
# Simple property resolution with fallback
287+
administrative = try(local.stack_configs[stack].administrative, var.administrative)
288+
autoretry = try(local.stack_configs[stack].autoretry, var.autoretry)
289+
additional_project_globs = try(local.stack_configs[stack].additional_project_globs, var.additional_project_globs)
290+
autodeploy = try(local.stack_configs[stack].autodeploy, var.autodeploy)
291+
branch = try(local.stack_configs[stack].branch, var.branch)
292+
enable_local_preview = try(local.stack_configs[stack].enable_local_preview, var.enable_local_preview)
293+
enable_well_known_secret_masking = try(local.stack_configs[stack].enable_well_known_secret_masking, var.enable_well_known_secret_masking)
294+
github_action_deploy = try(local.stack_configs[stack].github_action_deploy, var.github_action_deploy)
295+
manage_state = try(local.stack_configs[stack].manage_state, var.manage_state)
296+
protect_from_deletion = try(local.stack_configs[stack].protect_from_deletion, var.protect_from_deletion)
297+
repository = try(local.stack_configs[stack].repository, var.repository)
298+
runner_image = try(local.stack_configs[stack].runner_image, var.runner_image)
299+
terraform_smart_sanitization = try(local.stack_configs[stack].terraform_smart_sanitization, var.terraform_smart_sanitization)
300+
terraform_version = try(local.stack_configs[stack].terraform_version, var.terraform_version)
301+
worker_pool_id = try(local.stack_configs[stack].worker_pool_id, var.worker_pool_id)
302+
303+
# AWS Integration properties
304+
aws_integration_id = try(local.stack_configs[stack].aws_integration_id, var.aws_integration_id)
305+
306+
# Drift detection properties
307+
drift_detection_ignore_state = try(local.stack_configs[stack].drift_detection_ignore_state, var.drift_detection_ignore_state)
308+
drift_detection_reconcile = try(local.stack_configs[stack].drift_detection_reconcile, var.drift_detection_reconcile)
309+
drift_detection_schedule = try(local.stack_configs[stack].drift_detection_schedule, var.drift_detection_schedule)
310+
drift_detection_timezone = try(local.stack_configs[stack].drift_detection_timezone, var.drift_detection_timezone)
311+
312+
# Destructor properties
313+
destructor_deactivated = try(local.stack_configs[stack].destructor_deactivated, var.destructor_deactivated)
314+
}
315+
}
316+
317+
# Helper for array merging with fallback
318+
stack_array_merger = {
319+
for stack in local.stacks : stack => {
320+
after_apply = compact(concat(try(local.stack_configs[stack].after_apply, []), var.after_apply))
321+
after_destroy = compact(concat(try(local.stack_configs[stack].after_destroy, []), var.after_destroy))
322+
after_init = compact(concat(try(local.stack_configs[stack].after_init, []), var.after_init))
323+
after_perform = compact(concat(try(local.stack_configs[stack].after_perform, []), var.after_perform))
324+
after_plan = compact(concat(try(local.stack_configs[stack].after_plan, []), var.after_plan))
325+
after_run = compact(concat(try(local.stack_configs[stack].after_run, []), var.after_run))
326+
before_apply = compact(concat(try(local.stack_configs[stack].before_apply, []), var.before_apply))
327+
before_destroy = compact(concat(try(local.stack_configs[stack].before_destroy, []), var.before_destroy))
328+
before_perform = compact(concat(try(local.stack_configs[stack].before_perform, []), var.before_perform))
329+
before_plan = compact(concat(try(local.stack_configs[stack].before_plan, []), var.before_plan))
330+
}
331+
}
332+
279333
resolved_space_ids = {
280334
for stack in local.stacks : stack => coalesce(
281335
try(local.stack_configs[stack].space_id, null), # space_id always takes precedence since it's the most explicit
282336
try(local.space_name_to_id[local.stack_configs[stack].space_name], null), # Then try to look up space_name from the stack.yaml to ID
283337
var.space_id,
284338
try(local.space_name_to_id[var.space_name], null), # Then try to look up the space_name global variable to ID
285-
"root" # If no space_id or space_name is provided, default to the root space
339+
local.root_space_id # If no space_id or space_name is provided, default to the root space
286340
)
287341
}
288342

@@ -292,12 +346,10 @@ locals {
292346
for stack, config in local.stack_configs :
293347
stack => config if try(config.aws_integration_enabled, var.aws_integration_enabled)
294348
}
295-
296349
drift_detection_stacks = {
297350
for stack, config in local.stack_configs :
298351
stack => config if try(config.drift_detection_enabled, var.drift_detection_enabled)
299352
}
300-
301353
destructor_stacks = {
302354
for stack, config in local.stack_configs :
303355
stack => config if try(config.destructor_enabled, var.destructor_enabled)
@@ -333,38 +385,40 @@ module "deep" {
333385
resource "spacelift_stack" "default" {
334386
for_each = local.stacks
335387

336-
administrative = coalesce(try(local.stack_configs[each.key].administrative, null), var.administrative)
337-
additional_project_globs = try(local.stack_configs[each.key].additional_project_globs, var.additional_project_globs)
338-
after_apply = compact(concat(try(local.stack_configs[each.key].after_apply, []), var.after_apply))
339-
after_destroy = compact(concat(try(local.stack_configs[each.key].after_destroy, []), var.after_destroy))
340-
after_init = compact(concat(try(local.stack_configs[each.key].after_init, []), var.after_init))
341-
after_perform = compact(concat(try(local.stack_configs[each.key].after_perform, []), var.after_perform))
342-
after_plan = compact(concat(try(local.stack_configs[each.key].after_plan, []), var.after_plan))
343-
after_run = compact(concat(try(local.stack_configs[each.key].after_run, []), var.after_run))
344-
autodeploy = coalesce(try(local.stack_configs[each.key].autodeploy, null), var.autodeploy)
345-
autoretry = try(local.stack_configs[each.key].autoretry, var.autoretry)
346-
before_apply = compact(coalesce(try(local.stack_configs[each.key].before_apply, []), var.before_apply))
347-
before_destroy = compact(coalesce(try(local.stack_configs[each.key].before_destroy, []), var.before_destroy))
388+
administrative = local.stack_property_resolver[each.key].administrative
389+
additional_project_globs = local.stack_property_resolver[each.key].additional_project_globs
390+
after_apply = local.stack_array_merger[each.key].after_apply
391+
after_destroy = local.stack_array_merger[each.key].after_destroy
392+
after_init = local.stack_array_merger[each.key].after_init
393+
after_perform = local.stack_array_merger[each.key].after_perform
394+
after_plan = local.stack_array_merger[each.key].after_plan
395+
after_run = local.stack_array_merger[each.key].after_run
396+
autodeploy = local.stack_property_resolver[each.key].autodeploy
397+
autoretry = local.stack_property_resolver[each.key].autoretry
398+
before_apply = local.stack_array_merger[each.key].before_apply
399+
before_destroy = local.stack_array_merger[each.key].before_destroy
400+
# before_init is handled separately from other script arrays due to special tfvars logic
401+
# See local.before_init for details on tfvars file copying and MultiInstance vs SingleInstance handling
348402
before_init = local.before_init[each.key]
349-
before_perform = compact(coalesce(try(local.stack_configs[each.key].before_perform, []), var.before_perform))
350-
before_plan = compact(coalesce(try(local.stack_configs[each.key].before_plan, []), var.before_plan))
351-
branch = try(local.stack_configs[each.key].branch, var.branch)
352-
enable_local_preview = try(local.stack_configs[each.key].enable_local_preview, var.enable_local_preview)
353-
enable_well_known_secret_masking = try(local.stack_configs[each.key].enable_well_known_secret_masking, var.enable_well_known_secret_masking)
354-
github_action_deploy = try(local.stack_configs[each.key].github_action_deploy, var.github_action_deploy)
403+
before_perform = local.stack_array_merger[each.key].before_perform
404+
before_plan = local.stack_array_merger[each.key].before_plan
405+
branch = local.stack_property_resolver[each.key].branch
406+
enable_local_preview = local.stack_property_resolver[each.key].enable_local_preview
407+
enable_well_known_secret_masking = local.stack_property_resolver[each.key].enable_well_known_secret_masking
408+
github_action_deploy = local.stack_property_resolver[each.key].github_action_deploy
355409
labels = local.labels[each.key]
356-
manage_state = try(local.stack_configs[each.key].manage_state, var.manage_state)
410+
manage_state = local.stack_property_resolver[each.key].manage_state
357411
name = each.key
358412
project_root = local.configs[each.key].project_root
359-
protect_from_deletion = try(local.stack_configs[each.key].protect_from_deletion, var.protect_from_deletion)
360-
repository = try(local.stack_configs[each.key].repository, var.repository)
361-
runner_image = try(local.stack_configs[each.key].runner_image, var.runner_image)
413+
protect_from_deletion = local.stack_property_resolver[each.key].protect_from_deletion
414+
repository = local.stack_property_resolver[each.key].repository
415+
runner_image = local.stack_property_resolver[each.key].runner_image
362416
space_id = local.resolved_space_ids[each.key]
363-
terraform_smart_sanitization = try(local.stack_configs[each.key].terraform_smart_sanitization, var.terraform_smart_sanitization)
364-
terraform_version = try(local.stack_configs[each.key].terraform_version, var.terraform_version)
417+
terraform_smart_sanitization = local.stack_property_resolver[each.key].terraform_smart_sanitization
418+
terraform_version = local.stack_property_resolver[each.key].terraform_version
365419
terraform_workflow_tool = var.terraform_workflow_tool
366420
terraform_workspace = local.configs[each.key].terraform_workspace
367-
worker_pool_id = try(local.stack_configs[each.key].worker_pool_id, var.worker_pool_id)
421+
worker_pool_id = local.stack_property_resolver[each.key].worker_pool_id
368422

369423
# Usage of `templatestring` requires OpenTofu 1.7 and Terraform 1.9 or later.
370424
description = coalesce(
@@ -391,7 +445,7 @@ resource "spacelift_stack_destructor" "default" {
391445
for_each = local.destructor_stacks
392446

393447
stack_id = spacelift_stack.default[each.key].id
394-
deactivated = try(local.stack_configs[each.key].destructor_deactivated, var.destructor_deactivated)
448+
deactivated = local.stack_property_resolver[each.key].destructor_deactivated
395449

396450
# `depends_on` should be used to make sure that all necessary resources (environment variables, roles, integrations, etc.)
397451
# are still in place when the destruction run is executed.
@@ -405,7 +459,7 @@ resource "spacelift_stack_destructor" "default" {
405459
resource "spacelift_aws_integration_attachment" "default" {
406460
for_each = local.aws_integration_stacks
407461

408-
integration_id = try(local.stack_configs[each.key].aws_integration_id, var.aws_integration_id)
462+
integration_id = local.stack_property_resolver[each.key].aws_integration_id
409463
stack_id = spacelift_stack.default[each.key].id
410464
read = var.aws_integration_attachment_read
411465
write = var.aws_integration_attachment_write
@@ -415,14 +469,14 @@ resource "spacelift_drift_detection" "default" {
415469
for_each = local.drift_detection_stacks
416470

417471
stack_id = spacelift_stack.default[each.key].id
418-
ignore_state = try(local.stack_configs[each.key].drift_detection_ignore_state, var.drift_detection_ignore_state)
419-
reconcile = try(local.stack_configs[each.key].drift_detection_reconcile, var.drift_detection_reconcile)
420-
schedule = try(local.stack_configs[each.key].drift_detection_schedule, var.drift_detection_schedule)
421-
timezone = try(local.stack_configs[each.key].drift_detection_timezone, var.drift_detection_timezone)
472+
ignore_state = local.stack_property_resolver[each.key].drift_detection_ignore_state
473+
reconcile = local.stack_property_resolver[each.key].drift_detection_reconcile
474+
schedule = local.stack_property_resolver[each.key].drift_detection_schedule
475+
timezone = local.stack_property_resolver[each.key].drift_detection_timezone
422476

423477
lifecycle {
424478
precondition {
425-
condition = alltrue([for schedule in try(local.stack_configs[each.key].drift_detection_schedule, var.drift_detection_schedule) : can(regex("^([0-9,\\-*/]+\\s+){4}[0-9,\\-*/]+$", schedule))])
479+
condition = alltrue([for schedule in local.stack_property_resolver[each.key].drift_detection_schedule : can(regex("^([0-9,\\-*/]+\\s+){4}[0-9,\\-*/]+$", schedule))])
426480
error_message = "Invalid cron schedule format for drift detection"
427481
}
428482
}

0 commit comments

Comments
 (0)