generated from masterpointio/terraform-module-template
-
Notifications
You must be signed in to change notification settings - Fork 2
feat: support prefix for project root #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
gberenice
wants to merge
3
commits into
main
Choose a base branch
from
feat/project_prefix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 9 additions & 0 deletions
9
tests/fixtures/multi-instance/root-module-a/stacks/custom-project-root.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| kind: StackConfigV1 | ||
| stack_settings: | ||
| # Custom project_root should take precedence over project_root_prefix | ||
| project_root: custom/path/to/root-module-a | ||
| labels: | ||
| - custom_project_root_label | ||
|
|
||
| automation_settings: | ||
| tfvars_enabled: false |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| # Tests for the project_root_prefix variable | ||
| # This variable allows setting a global prefix for project_root when root_modules_path | ||
| # uses relative paths for local scanning but the repo structure is different. | ||
|
|
||
| mock_provider "spacelift" { | ||
| mock_data "spacelift_spaces" { | ||
| defaults = { | ||
| spaces = [] | ||
| } | ||
| } | ||
|
|
||
| mock_data "spacelift_worker_pools" { | ||
| defaults = { | ||
| worker_pools = [] | ||
| } | ||
| } | ||
|
|
||
| mock_data "spacelift_aws_integrations" { | ||
| defaults = { | ||
| integrations = [] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| mock_provider "jsonschema" { | ||
| mock_data "jsonschema_validator" { | ||
| defaults = { | ||
| validated = "{}" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| variables { | ||
| repository = "terraform-spacelift-automation" | ||
| github_enterprise = { | ||
| namespace = "masterpointio" | ||
| } | ||
| aws_integration_enabled = false | ||
| } | ||
|
|
||
| # Test default behavior without project_root_prefix (null) | ||
| # project_root should be calculated from root_modules_path with "../" stripped | ||
| run "test_project_root_without_prefix" { | ||
| command = plan | ||
|
|
||
| variables { | ||
| root_modules_path = "./tests/fixtures/multi-instance" | ||
| all_root_modules_enabled = true | ||
| } | ||
|
|
||
| # project_root should be based on root_modules_path | ||
| assert { | ||
| condition = spacelift_stack.default["root-module-a-test"].project_root == "./tests/fixtures/multi-instance/root-module-a" | ||
| error_message = "Project root without prefix should use root_modules_path: ${spacelift_stack.default["root-module-a-test"].project_root}" | ||
| } | ||
| } | ||
|
|
||
| # Test project_root_prefix with relative root_modules_path | ||
| # This is the main use case: local scanning with relative paths, but different repo structure | ||
| run "test_project_root_with_prefix" { | ||
| command = plan | ||
|
|
||
| variables { | ||
| root_modules_path = "./tests/fixtures/multi-instance" | ||
| project_root_prefix = "terraform/root-modules" | ||
| all_root_modules_enabled = true | ||
| } | ||
|
|
||
| # project_root should use the prefix instead of the stripped root_modules_path | ||
| assert { | ||
| condition = spacelift_stack.default["root-module-a-test"].project_root == "terraform/root-modules/root-module-a" | ||
| error_message = "Project root with prefix should use project_root_prefix: ${spacelift_stack.default["root-module-a-test"].project_root}" | ||
| } | ||
| } | ||
|
|
||
| # Test that project_root_prefix works with nested directories | ||
| run "test_project_root_prefix_with_nested_directories" { | ||
| command = plan | ||
|
|
||
| variables { | ||
| root_modules_path = "./tests/fixtures/nested-multi-instance" | ||
| project_root_prefix = "infra/terraform/modules" | ||
| root_module_structure = "MultiInstance" | ||
| all_root_modules_enabled = true | ||
| } | ||
|
|
||
| # project_root should preserve nested path with the prefix | ||
| assert { | ||
| condition = spacelift_stack.default["parent/nested-dev"].project_root == "infra/terraform/modules/parent/nested" | ||
| error_message = "Nested project_root with prefix incorrect: ${spacelift_stack.default["parent/nested-dev"].project_root}" | ||
| } | ||
| } | ||
|
|
||
| # Test that per-stack project_root in YAML takes precedence over project_root_prefix | ||
| run "test_stack_project_root_takes_precedence_over_prefix" { | ||
| command = plan | ||
|
|
||
| variables { | ||
| root_modules_path = "./tests/fixtures/multi-instance" | ||
| project_root_prefix = "should/be/ignored" | ||
| all_root_modules_enabled = true | ||
| } | ||
|
|
||
| # Stack with custom project_root in YAML should use that, not the prefix | ||
| assert { | ||
| condition = spacelift_stack.default["root-module-a-custom-project-root"].project_root == "custom/path/to/root-module-a" | ||
| error_message = "Per-stack project_root should take precedence over prefix: ${spacelift_stack.default["root-module-a-custom-project-root"].project_root}" | ||
| } | ||
|
|
||
| # Stack without custom project_root should use the prefix | ||
| assert { | ||
| condition = spacelift_stack.default["root-module-a-test"].project_root == "should/be/ignored/root-module-a" | ||
| error_message = "Stack without custom project_root should use prefix: ${spacelift_stack.default["root-module-a-test"].project_root}" | ||
| } | ||
| } | ||
|
|
||
| # Test that project_root_prefix works with SingleInstance structure | ||
| run "test_project_root_prefix_with_single_instance" { | ||
| command = plan | ||
|
|
||
| variables { | ||
| root_module_structure = "SingleInstance" | ||
| root_modules_path = "./tests/fixtures/single-instance" | ||
| project_root_prefix = "terraform/single" | ||
| all_root_modules_enabled = true | ||
| } | ||
|
|
||
| assert { | ||
| condition = spacelift_stack.default["root-module-a"].project_root == "terraform/single/root-module-a" | ||
| error_message = "SingleInstance project_root with prefix incorrect: ${spacelift_stack.default["root-module-a"].project_root}" | ||
| } | ||
| } | ||
|
|
||
| # Test that project_root_prefix replaces the fallback behavior that strips "../" | ||
| # This validates the main use case: root_modules_path with "../" for local scanning, | ||
| # project_root_prefix for the actual repo path | ||
| run "test_project_root_prefix_replaces_fallback" { | ||
| command = plan | ||
|
|
||
| variables { | ||
| root_modules_path = "./tests/fixtures/multi-instance" | ||
| project_root_prefix = "actual/repo/path" | ||
| all_root_modules_enabled = true | ||
| } | ||
|
|
||
| # With prefix set, the fallback logic is bypassed entirely | ||
| assert { | ||
| condition = spacelift_stack.default["root-module-a-test"].project_root == "actual/repo/path/root-module-a" | ||
| error_message = "Prefix should replace fallback entirely: ${spacelift_stack.default["root-module-a-test"].project_root}" | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm confused while reading this logic: Why is this necessary? if the only difference between using
var.project_root_prefixandvar.root_modules_pathhere is that we're replacing"../"... can we just passvar.project_root_prefixwithout the local directory paths? Does that not accomplish the same thing or am I missing something?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And let me clarify the above:
Can we just pass
var.root_modules_pathWITHOUT the../../and pass it asterraform/root-modules?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we can't.
root_modules_pathhelps to discover sibling root modules and theirstacks/*.yaml, whileproject_roottells Spacelift where to find the Terraform code in the repo for each stack.So from
spacelift-automation/:../../root-modules→ correctly findsnetwork/,tfstate-backend/, etc. for discoveryterraform/root-modules/network(from repo root) to run the stackproject_root_prefixbridges this gap:I think the
root_modules_pathname is a bit confusing here. I updated variables description to clarify the difference.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @gberenice, alongside the upcoming major revision to fix stack naming that I'm asking to do as part of #101... I'm thinking that we introduce another variable here that provides what you're looking for and removes the usage of root module's path. If we do that as a breaking change, then we can keep this code cleaner while still providing the functionality that you need here.
What are your thoughts on that? Do you want to continue the conversation live?