feat: Adding custom default template #3658 #4035
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request introduces several changes across the codebase. A typographical error is fixed in the catalog module’s constant name. The catalog configuration is enhanced with a new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as Scaffold Command
participant P as prepareBoilerplateFiles
participant D as downloadTemplate
participant G as generateDefaultTemplate
U->>S: Execute `terragrunt scaffold`
S->>P: Call prepareBoilerplateFiles with template URL and tempDir
alt Template URL provided
P->>D: Invoke downloadTemplate
D-->>P: Return downloaded template path
else No template URL provided
P->>G: Check catalog config for DefaultTemplate
G-->>P: Return generated default template path
end
P-->>S: Return final boilerplate files path
Assessment against linked issues
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
config/catalog.go (1)
48-59: Consider extending normalize method for DefaultTemplate path.The
normalizemethod currently only transforms relative paths to absolute ones for URLs, but it doesn't do the same for theDefaultTemplatefield. If DefaultTemplate can also be a relative path, consider applying the same normalization.func (cfg *CatalogConfig) normalize(configPath string) { configDir := filepath.Dir(configPath) // transform relative paths to absolute ones for i, url := range cfg.URLs { url := filepath.Join(configDir, url) if files.FileExists(url) { cfg.URLs[i] = url } } + + // Normalize DefaultTemplate path if it's not empty and exists + if cfg.DefaultTemplate != "" { + path := filepath.Join(configDir, cfg.DefaultTemplate) + if files.FileExists(path) { + cfg.DefaultTemplate = path + } + } }docs/_docs/02_features/06-scaffold.md (1)
71-76: Refactor the bullet list structure to avoid repetition and enhance clarity.The current bullet list repeats "1." for each item, and each item starts with "You can" in close succession. Consider applying sequential numbering or using another format while rephrasing to reduce redundancy. For example:
-1. You can specify a custom boilerplate template ... -1. You can define a custom boilerplate template ... -1. You can define a default custom boilerplate template ... +1. You can specify a custom boilerplate template ... +2. You can define a custom boilerplate template ... +3. You can define a default custom boilerplate template ...🧰 Tools
🪛 LanguageTool
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... subfolder of your Terraform module. 1. You can define a default custom boilerplate...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/_docs/02_features/05-catalog.md (2)
26-26: Fix grammatical errors in the repository search description.Use “is” instead of “are” and resolve capitalization to match sentence boundaries:
-1. The repository list are searched in the [config file](#configuration) ... - if `terragrunt.hcl` does not exist in the current directory, the config are searched ... +1. The repository list is searched in the [config file](#configuration) ... + If `terragrunt.hcl` does not exist in the current directory, the config is searched ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~26-~26: The verb “are” doesn’t seem to fit in this context, “is” is probably more formally correct.
Context: ...ory, otherwise: 1. The repository list are searched in the [config file](#configur...(AI_HYDRA_LEO_CPT_ARE_IS)
[uncategorized] ~26-~26: The verb “are” doesn’t seem to fit in this context, “is” is probably more formally correct.
Context: ...st in the current directory, the config are searched in the parent directories. 1. ...(AI_HYDRA_LEO_CPT_ARE_IS)
65-65: Add missing article and punctuation for clarity.Insert "the" before "flag" and a comma after "provided":
-If flag is not provided current working directory is selected. +If the flag is not provided, the current working directory is selected.🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: You might be missing the article “a” here.
Context: ...tion for generatedterragrunt.hcl. If flag is not provided current working directo...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~65-~65: Possible missing comma found.
Context: ...eratedterragrunt.hcl. If flag is not provided current working directory is selected.(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~65-~65: Possible missing article found.
Context: ...erragrunt.hcl`. If flag is not provided current working directory is selected.(AI_HYDRA_LEO_MISSING_THE)
cli/commands/scaffold/action.go (1)
268-315: Validate fallback flows inprepareBoilerplateFiles.This method has multiple fallback paths when neither a custom template URL nor a default template is specified. Consider adding a dedicated unit test (or expanding existing tests) to verify each fallback scenario for correctness and to guard against regressions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cli/commands/catalog/module/module.go(2 hunks)cli/commands/scaffold/action.go(1 hunks)config/catalog.go(1 hunks)config/catalog_test.go(1 hunks)docs/_docs/02_features/05-catalog.md(2 hunks)docs/_docs/02_features/06-scaffold.md(1 hunks)test/fixtures/catalog/config4.hcl(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.go`: Review the Go code for quality and correctness. M...
**/*.go: Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
config/catalog.goconfig/catalog_test.gocli/commands/catalog/module/module.gocli/commands/scaffold/action.go
`docs/**/*.md`: Review the documentation for clarity, gramma...
docs/**/*.md: Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration underway from the Jekyll based documentation indocsto the Starlight + Astro based documentation indocs-starlight. Whenever changes are made to thedocsdirectory, ensure that an equivalent change is made in thedocs-starlightdirectory to keep thedocs-starlightdocumentation accurate.
docs/_docs/02_features/06-scaffold.mddocs/_docs/02_features/05-catalog.md
🪛 LanguageTool
docs/_docs/02_features/06-scaffold.md
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... subfolder of your Terraform module. 1. You can define a default custom boilerplate...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/_docs/02_features/05-catalog.md
[uncategorized] ~26-~26: The verb “are” doesn’t seem to fit in this context, “is” is probably more formally correct.
Context: ...ory, otherwise: 1. The repository list are searched in the [config file](#configur...
(AI_HYDRA_LEO_CPT_ARE_IS)
[uncategorized] ~26-~26: The verb “are” doesn’t seem to fit in this context, “is” is probably more formally correct.
Context: ...st in the current directory, the config are searched in the parent directories. 1. ...
(AI_HYDRA_LEO_CPT_ARE_IS)
[uncategorized] ~65-~65: You might be missing the article “a” here.
Context: ...tion for generated terragrunt.hcl. If flag is not provided current working directo...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~65-~65: Possible missing comma found.
Context: ...erated terragrunt.hcl. If flag is not provided current working directory is selected.
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~65-~65: Possible missing article found.
Context: ...erragrunt.hcl`. If flag is not provided current working directory is selected.
(AI_HYDRA_LEO_MISSING_THE)
🔇 Additional comments (8)
config/catalog.go (2)
40-41: LGTM: New DefaultTemplate field added properly.This correctly adds the
DefaultTemplatefield to theCatalogConfigstruct with appropriate HCL and CTY tags for serialization, aligning with the PR objectives for supporting custom default templates.
44-46: String() method updated to include the DefaultTemplate field.The String() method is correctly updated to include the new DefaultTemplate field in its representation, maintaining consistency in the struct's string output.
test/fixtures/catalog/config4.hcl (1)
1-7: LGTM: Test fixture for DefaultTemplate created.This test fixture correctly defines the
catalogblock with thedefault_templatefield, which aligns with the changes made to theCatalogConfigstruct. Thelocalsblock with thebaseRepovariable is also properly defined.config/catalog_test.go (1)
128-134: LGTM: Test case added for DefaultTemplate configuration.This test case properly validates that the
DefaultTemplatefield is correctly parsed from the configuration file. It checks that when readingconfig4.hcl, the resultingCatalogConfigobject has theDefaultTemplatefield set to "/test/fixtures/scaffold/external-template" with no error.cli/commands/catalog/module/module.go (2)
15-15: Fixed typo in constant name: maxDescriptionLength.Good catch correcting the typo in the constant name from
maxDescriptionLenghttomaxDescriptionLength. This improves code readability and maintainability.
89-89: Updated usage of renamed constant.The usage of the renamed constant is properly updated in the Description method.
cli/commands/scaffold/action.go (2)
224-236: Well-structured approach to generating default files.The
generateDefaultTemplatefunction correctly writes bothterragrunt.hclandboilerplate.ymlwith proper permissions and handles errors gracefully.
238-266: Good modular design for template parsing and file retrieval.Splitting out
parseTemplateURLclarifies responsibilities and promotes maintainability. The error handling is consistent and well-scoped.
cli/commands/scaffold/action.go
Outdated
| // downloading templateURL | ||
| opts.Logger.Infof("Using template from %s", templateURL) | ||
|
|
||
| if _, err := getter.GetAny(ctx, templateDir, templateURL); err != nil { |
There was a problem hiding this comment.
NIT: Consider integrating into the CAS experiment to save you reclones of the template.
There was a problem hiding this comment.
I can have a look at doing that if I have some time, or otherwise I'll ask you for some pointers next week.
cli/commands/scaffold/action.go
Outdated
| } | ||
|
|
||
| // if boilerplate dir is not found, create one with default template | ||
| if !files.IsExistingDir(boilerplateDir) { | ||
| // no default boilerplate dir, create one | ||
| defaultTempDir, err := os.MkdirTemp("", "boilerplate") | ||
| config, err := config.ReadCatalogConfig(ctx, opts) |
There was a problem hiding this comment.
Is this the right behavior here? Should scaffold always fail if the catalog config discovered in the parent directory is invalid?
There was a problem hiding this comment.
That's a good question. I'll leave that to your judgment. If we get to this point in the code, no Boilerplate template was provided as a cli input, and the terraform module also doesn't contain a .boilerplate folder. That leaves one of two options: either we use the custom default template in the catalog config (if there is one), or we stick to the hardcoded default template.
I can see the argument for falling back to the hardcoded custom template in case of an invalid catalog config, but that may surprise a user who thought they defined a custom default config. I would personally lean towards flagging the issue with the catalog config though, even if it doesn't actually contain a custom template. As a user I'd like to know if my config is invalid. Sooner rather than later.
|
Moving to in-draft, as @tgeijg and I are going to go over this and testing for it next week. |
tgeijg
force-pushed
the
feat/adding-custom-default-template
branch
from
679fa0c to
4132c5e
Compare
tgeijg
force-pushed
the
feat/adding-custom-default-template
branch
from
4132c5e to
dfb27d0
Compare
yhakbar
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/_docs/02_features/06-scaffold.md (1)
71-75: Consider rewording repeated sentences and fixing the "sub-directory" spelling.The code currently repeats “You can …” at the beginning of each sentence and uses “sub-directory” with a hyphen. This can be more concise and consistent:
-1. You can specify a custom Boilerplate template to use as the second argument of the `scaffold` command. -1. You can define a custom Boilerplate template in a `.boilerplate` sub-directory of any OpenTofu/Terraform module. -1. You can define a default custom Boilerplate template in the [catalog config]({{site.baseurl}}/docs/features/catalog/#configuration) +1. Provide a custom Boilerplate template as the second argument of the `scaffold` command. +1. Place a custom Boilerplate template in a `.boilerplate` subdirectory of any OpenTofu/Terraform module. +1. Specify a default custom Boilerplate template in the [catalog config]({{site.baseurl}}/docs/features/catalog/#configuration)🧰 Tools
🪛 LanguageTool
[misspelling] ~74-~74: This word is normally spelled as one.
Context: ...oilerplate template in a.boilerplatesub-directory of any OpenTofu/Terraform module. 1. Yo...(EN_COMPOUNDS_SUB_DIRECTORY)
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ry of any OpenTofu/Terraform module. 1. You can define a default custom Boilerplate...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/_docs/02_features/05-catalog.md (2)
41-41: Use “subdirectory” as one word.To align with standard spelling norms, remove the hyphen in “sub-directory”:
-1. You can define a custom Boilerplate template in a `.boilerplate` sub-directory of any OpenTofu/Terraform module. +1. You can define a custom Boilerplate template in a `.boilerplate` subdirectory of any OpenTofu/Terraform module.🧰 Tools
🪛 LanguageTool
[misspelling] ~41-~41: This word is normally spelled as one.
Context: ...oilerplate template in a.boilerplatesub-directory of any OpenTofu/Terraform module. 1. Yo...(EN_COMPOUNDS_SUB_DIRECTORY)
66-66: Add the missing article “the” when referencing the flag.Improve clarity by specifying "the flag" rather than "flag":
-- `--output-folder` - Location for the scaffolded configurations. If flag is not provided current working directory is selected. +- `--output-folder` - Location for the scaffolded configurations. If the flag is not provided, the current working directory is selected.🧰 Tools
🪛 LanguageTool
[uncategorized] ~66-~66: You might be missing the article “a” here.
Context: ...n for the scaffolded configurations. If flag is not provided current working directo...(AI_EN_LECTOR_MISSING_DETERMINER_A)
cli/commands/scaffold/scaffold.go (1)
238-266: Consider wrapping errors with contextual information.When errors occur (e.g., downloading the template), returning them with additional context helps with debugging. For instance:
- return "", errors.New(err) + return "", errors.Wrapf(err, "failed to download template from %s", templateURL)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cli/commands/catalog/module/module.go(2 hunks)cli/commands/scaffold/scaffold.go(1 hunks)config/catalog.go(2 hunks)config/catalog_test.go(1 hunks)docs/_docs/02_features/05-catalog.md(2 hunks)docs/_docs/02_features/06-scaffold.md(1 hunks)test/fixtures/catalog/config4.hcl(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- config/catalog.go
- config/catalog_test.go
- test/fixtures/catalog/config4.hcl
- cli/commands/catalog/module/module.go
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.go`: Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
**/*.go: Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
cli/commands/scaffold/scaffold.go
`docs/**/*.md`: Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration under...
docs/**/*.md: Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration underway from the Jekyll based documentation indocsto the Starlight + Astro based documentation indocs-starlight. Whenever changes are made to thedocsdirectory, ensure that an equivalent change is made in thedocs-starlightdirectory to keep thedocs-starlightdocumentation accurate.
docs/_docs/02_features/05-catalog.mddocs/_docs/02_features/06-scaffold.md
🪛 LanguageTool
docs/_docs/02_features/05-catalog.md
[misspelling] ~41-~41: This word is normally spelled as one.
Context: ...oilerplate template in a .boilerplate sub-directory of any OpenTofu/Terraform module. 1. Yo...
(EN_COMPOUNDS_SUB_DIRECTORY)
[uncategorized] ~66-~66: You might be missing the article “a” here.
Context: ...n for the scaffolded configurations. If flag is not provided current working directo...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
docs/_docs/02_features/06-scaffold.md
[misspelling] ~74-~74: This word is normally spelled as one.
Context: ...oilerplate template in a .boilerplate sub-directory of any OpenTofu/Terraform module. 1. Yo...
(EN_COMPOUNDS_SUB_DIRECTORY)
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ry of any OpenTofu/Terraform module. 1. You can define a default custom Boilerplate...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: Pull Request has non-contributor approval
🔇 Additional comments (2)
cli/commands/scaffold/scaffold.go (2)
224-236: Implementation looks good.File creation, permission settings, and error handling are all correctly handled.
268-315: Logic for handling fallback templates and default configurations appears correct.The approach of checking for a
.boilerplatefolder, then theDefaultTemplate, and finally reverting to the internal default is well-structured.
| @@ -23,13 +23,31 @@ terragrunt catalog <repo-url> [--no-include-root] [--root-file-name] [--output-f | |||
|
|
|||
| If `<repo-url>` is provided, the repository will be cloned into a temporary directory, otherwise: | |||
|
|
|||
| 1. The repository list are searched in the config file `terragrunt.hcl`. if `terragrunt.hcl` does not exist in the current directory, the config are searched in the parent directories. | |||
| 1. The `urls` listed in the `catalog` configuration of your [parent configuration file](#configuration) are used. If the root configuration file does not exist in the current directory, parent directories are recursively searched. | |||
There was a problem hiding this comment.
- @yhakbar Make sure to copy this update to the starlight docs.
Description
Fixes #3658.
Adds support for a custom default boilerplate template
TODOs
Release Notes (draft)
Added support for a custom default boilerplate template
Summary by CodeRabbit
Bug Fixes
New Features
Documentation