Skip to content
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

chore: adds root module example + more docs #1

Merged
merged 4 commits into from
Dec 16, 2024

Conversation

gberenice
Copy link
Member

@gberenice gberenice commented Dec 11, 2024

what

  • Adds root-module example.
  • Moves child module example to terraform-random-pet to reflect our recommendations.
  • Updates READMEs accordingly.

why

  • We want to provide both, child and root modules examples and best practices.

references

Summary by CodeRabbit

  • New Features

    • Updated repository name and documentation links for clarity.
    • Introduced new README files for the root module and the terraform-random-pet module.
    • Added example directories for child and root modules.
    • New output for generated random pet name.
    • New configuration for the random provider.
  • Bug Fixes

    • Improved overall organization and clarity of documentation.
  • Chores

    • Introduced new configuration for markdownlint to ignore specific rules.
    • Updated version constraints for Terraform and providers.

@gberenice gberenice requested a review from Gowiem December 11, 2024 17:24
Copy link

coderabbitai bot commented Dec 11, 2024

Walkthrough

The pull request introduces significant changes to a Terraform module repository, including renaming the repository, restructuring documentation, and adding new files. The README.md file is updated to reflect the new repository name and includes examples for both child and root modules. A new README for the root module is created, along with additional files like providers.tf, variables.tf, and versions.tf. The terraform-random-pet module is also enhanced with new variables, outputs, and documentation. Notably, the previous versions.tf file is deleted, and markdownlint configurations are updated.

Changes

File Change Summary
README.md Renamed from terraform-child-module-template to client-tf-templates. Updated links and added examples for child and root modules. Removed sections on requirements, providers, modules, resources, inputs, and outputs.
root-module/README.md New README file created for the root module, detailing documentation recommendations and module usage.
root-module/example.auto.tfvars New file added to provide sample input variable values, defining length with a value of 1.
root-module/main.tf Updated to include a local variable prefix and a module call to random_pet.
root-module/providers.tf New provider configuration added for the "random" provider.
root-module/variables.tf New variable length defined with validation for positive numbers.
root-module/versions.tf Updated to specify required Terraform version 1.8.7 and random provider version constraint.
terraform-random-pet/README.md New README file created for the terraform-random-pet module, including documentation recommendations and usage examples.
terraform-random-pet/main.tf Added prefix and length attributes to the random_pet resource definition.
terraform-random-pet/outputs.tf New output block random_pet_name introduced to retrieve the generated random pet name.
terraform-random-pet/variables.tf New variables length and prefix defined with validation for positive numbers.
terraform-random-pet/versions.tf New file created to define minimum required versions for Terraform and providers.
versions.tf Deleted the previous versions.tf file that specified required versions for Terraform and providers.
.trunk/configs/.markdownlint.yaml Added rule to ignore markdownlint rule MD029 regarding ordered list item prefixes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Terraform
    participant RandomPetModule

    User->>Terraform: Apply configuration
    Terraform->>RandomPetModule: Create resource with length and prefix
    RandomPetModule-->>Terraform: Return generated random pet name
    Terraform-->>User: Display output
Loading

🐇 "In the garden, changes bloom,
With templates fresh, there's room!
From child to root, we leap and play,
New names and paths light the way.
With random pets and names so bright,
Our Terraform dreams take flight!" 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (8)
root-module/versions.tf (1)

9-9: Consider tightening the Terraform version constraint.

The current constraint ~> 1.0 allows any 1.x version, which might be too permissive. Consider using a more specific constraint like ~> 1.5 to ensure compatibility while still allowing patch updates.

-  required_version = "~> 1.0"
+  required_version = "~> 1.5"
terraform-random-pet/main.tf (1)

5-8: Consider adding module-specific best practices.

While the current best practices are good, consider adding guidance specific to the random_pet resource usage and naming conventions.

root-module/providers.tf (1)

11-13: Consider documenting why the random provider block is empty.

While the random provider might not need specific configuration, it would be helpful to add a comment explaining this.

 provider "random" {
-  # Configuration options
+  # The random provider doesn't require any configuration options
+  # See: https://registry.terraform.io/providers/hashicorp/random/latest/docs
 }
terraform-random-pet/variables.tf (3)

1-9: Consider enhancing documentation with examples

The documentation header is well-structured and informative. Consider adding example usage for each variable to make it even more user-friendly.

 # Best Practices:
 # - Descriptive variables: Use meaningful names and description attributes.
 # - Default values: Provide reasonable defaults when possible. For mandatory inputs, omit defaults to enforce explicit user input.
 # - Type constraints and validation: Use type constraints and validation blocks to catch incorrect inputs early.
 # - Group related variables: Organize variables logically, adding comments to separate sections if many variables exist.
+#
+# Examples:
+# length = 2      # Creates a two-word pet name
+# prefix = "dev"  # Results in names like "dev-happy-elephant"

10-18: Consider adding upper bound validation for length

While the current validation ensures positive numbers, consider adding an upper bound to prevent excessively long resource names.

 variable "length" {
   description = "The length of the random name."
   type        = number
   default     = 2
   validation {
-    condition     = var.length > 0
-    error_message = "The length must be a positive number."
+    condition     = var.length > 0 && var.length <= 5
+    error_message = "The length must be a positive number not exceeding 5."
   }
 }

20-24: Add validation for prefix format

Consider adding validation to ensure the prefix follows naming conventions and contains only valid characters.

 variable "prefix" {
   description = "A string to prefix the name with."
   type        = string
   default     = null
+  validation {
+    condition     = var.prefix == null || can(regex("^[a-zA-Z0-9-]*$", var.prefix))
+    error_message = "The prefix must contain only alphanumeric characters and hyphens."
+  }
 }
README.md (2)

1-9: Enhance module examples section

While the links to example directories are helpful, consider adding quick-start examples directly in the README for better visibility.

 Root module example is provided in [root-module](./root-module/) directory.

+## Quick Start
+
+### Child Module Usage
+```hcl
+module "pet" {
+  source = "./terraform-random-pet"
+  length = 2
+  prefix = "dev"
+}
+```
+
+### Root Module Usage
+```hcl
+module "root" {
+  source = "./root-module"
+  length = 2
+}
+```
+
 This README.md serves as the module's primary documentation and entry point.

21-31: Consider adding more context about providers.tf

The structure section mentions providers.tf is for root modules only, but it would be helpful to explain why and provide guidance on provider configuration.

 ├── outputs.tf
 ├── providers.tf   (root module only)
 ├── variables.tf
-└── versions.tf
+└── versions.tf
+
+### About providers.tf
+The `providers.tf` file in root modules is responsible for:
+- Configuring provider authentication
+- Setting provider-specific parameters
+- Managing provider aliases
+
+Example:
+```hcl
+provider "aws" {
+  region = var.aws_region
+}
+```
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e4c0224 and 1097d94.

📒 Files selected for processing (16)
  • README.md (2 hunks)
  • root-module/README.md (1 hunks)
  • root-module/data.tf (1 hunks)
  • root-module/example.auto.tfvars (1 hunks)
  • root-module/main.tf (1 hunks)
  • root-module/outputs.tf (1 hunks)
  • root-module/providers.tf (1 hunks)
  • root-module/variables.tf (1 hunks)
  • root-module/versions.tf (1 hunks)
  • terraform-random-pet/README.md (1 hunks)
  • terraform-random-pet/data.tf (1 hunks)
  • terraform-random-pet/main.tf (1 hunks)
  • terraform-random-pet/outputs.tf (1 hunks)
  • terraform-random-pet/variables.tf (1 hunks)
  • terraform-random-pet/versions.tf (1 hunks)
  • versions.tf (0 hunks)
💤 Files with no reviewable changes (1)
  • versions.tf
✅ Files skipped from review due to trivial changes (9)
  • root-module/example.auto.tfvars
  • root-module/outputs.tf
  • terraform-random-pet/data.tf
  • terraform-random-pet/outputs.tf
  • terraform-random-pet/README.md
  • root-module/data.tf
  • root-module/variables.tf
  • terraform-random-pet/versions.tf
  • root-module/README.md
🔇 Additional comments (3)
root-module/versions.tf (1)

1-7: LGTM! Well-documented best practices.

The comments provide clear guidance on version management and maintenance expectations.

terraform-random-pet/main.tf (1)

11-12: LGTM! Clean variable usage.

The addition of the prefix variable follows Terraform best practices for configuration flexibility.

root-module/providers.tf (1)

4-10: LGTM! Excellent security considerations.

The best practices section provides valuable guidance on secure provider configuration, especially regarding credential management.

root-module/main.tf Show resolved Hide resolved
root-module/main.tf Show resolved Hide resolved
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get to finish this review, but including a bunch of comments now before I end my day! Let's keep working through!

README.md Outdated Show resolved Hide resolved
root-module/README.md Outdated Show resolved Hide resolved
root-module/README.md Outdated Show resolved Hide resolved
root-module/README.md Outdated Show resolved Hide resolved
root-module/README.md Outdated Show resolved Hide resolved
root-module/README.md Outdated Show resolved Hide resolved
@gberenice
Copy link
Member Author

I didn't get to finish this review, but including a bunch of comments now before I end my day! Let's keep working through!

Addressed the first batch of suggestions 👍

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (4)
README.md (2)

5-7: Fix grammar in example directory references

Add the article "The" for better readability.

-Child module example is provided in [terraform-random-pet](./terraform-random-pet/) directory.
+The child module example is provided in the [terraform-random-pet](./terraform-random-pet/) directory.

-Root module example is provided in [root-module](./root-module/) directory.
+The root module example is provided in the [root-module](./root-module/) directory.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...t](./terraform-random-pet/) directory. Root module example is provided in [root-mod...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)


Line range hint 14-14: Update remaining HashiCorp documentation link to OpenTofu

For consistency, update the HashiCorp documentation link to use OpenTofu documentation instead.

-- Usage Instructions: Include code snippets demonstrating how to call the module from a [root module](https://developer.hashicorp.com/terraform/language/modules#the-root-module).
+- Usage Instructions: Include code snippets demonstrating how to call the module from a [root module](https://opentofu.org/docs/language/modules/#the-root-module).
🧰 Tools
🪛 LanguageTool

[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...t](./terraform-random-pet/) directory. Root module example is provided in [root-mod...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

terraform-random-pet/README.md (2)

24-24: Fix grammar in naming guidelines

Add the article "the" for better readability.

-1. Keep name short and focused: While it should be descriptive, avoid overly long names. The goal is to convey the module's purpose concisely:
+1. Keep the name short and focused: While it should be descriptive, avoid overly long names. The goal is to convey the module's purpose concisely:
🧰 Tools
🪛 LanguageTool

[uncategorized] ~24-~24: Possible missing article found.
Context: ...hens to separate words. Also: 1. Keep name short and focused: While it should be d...

(AI_HYDRA_LEO_MISSING_THE)


46-48: Add specific version constraint

The comment suggests pinning to a specific version, but no version is specified. Consider adding a specific version constraint for better stability.

 module "sandbox_pet" {
   source  = "masterpointio/random/pet"
-  # We recommend to pin the specific version
-  # version = "X.X.X"
+  version = "0.1.0"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1097d94 and 009e7c6.

📒 Files selected for processing (3)
  • README.md (2 hunks)
  • root-module/README.md (1 hunks)
  • terraform-random-pet/README.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
terraform-random-pet/README.md

[uncategorized] ~24-~24: Possible missing article found.
Context: ...hens to separate words. Also: 1. Keep name short and focused: While it should be d...

(AI_HYDRA_LEO_MISSING_THE)

README.md

[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...t](./terraform-random-pet/) directory. Root module example is provided in [root-mod...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

Comment on lines +5 to +9
## Documentation Recommendations (DO NOT INCLUDE THIS INTO THE REAL README)

- Module description: Briefly explain what the root module sets up (e.g., infrastructure for RDS Postgres instances).
- Use [terraform-docs](https://github.com/terraform-docs/terraform-docs) to ensure that variables, outputs, child module, and resource documentation is included.
- Maintain current information: Keep the README updated as the infrastructure evolves.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove documentation recommendations from template

The "Documentation Recommendations" section with the note "(DO NOT INCLUDE THIS INTO THE REAL README)" should not be part of the template itself. Consider moving these recommendations to a separate documentation guide or contributing guidelines document.

Comment on lines +1 to +3
# root-module

This is a template root module.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance the module description

The current description "This is a template root module" is too generic. Consider providing a more specific description that explains the purpose and functionality of this root module example.

-This is a template root module.
+# Example Root Module
+
+This root module demonstrates how to compose child modules together to create a complete infrastructure. It showcases best practices for:
+- Module composition
+- Variable management
+- Resource organization
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# root-module
This is a template root module.
# root-module
# Example Root Module
This root module demonstrates how to compose child modules together to create a complete infrastructure. It showcases best practices for:
- Module composition
- Variable management
- Resource organization

Comment on lines 5 to 29
## Documentation Recommendations (DO NOT INCLUDE THIS INTO THE REAL README)

### Naming

The repository/directory name should follow this pattern:

```sh
terraform-<PROVIDER>-<NAME>
```

Here’s what this means:

1. The repository should start with `terraform-` if your module should be [published to and discovered on the Registry](https://opentofu.org/docs/language/modules/develop/publish/). Even if you don’t intend to publish the module, following this pattern is a good practice that helps differentiate your Terraform child modules from other code in your projects.
2. Include the provider name: After the prefix, specify the primary provider your module is for, such as aws, google, datadog, etc.
3. Use descriptive name: Follow the provider name with a clear and concise identifier that describes the module’s purpose.
4. Use hyphens to separate words.

Also:

1. Keep name short and focused: While it should be descriptive, avoid overly long names. The goal is to convey the module’s purpose concisely:
- Good: terraform-aws-internal-lb
- Not so good: terraform-aws-internal-misc-module
- Too long: terraform-aws-internal-application-load-balancer-with-extra-rules
2. Module names should reflect their purpose rather than environment-specific details.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Move documentation recommendations to a separate guide

The "Documentation Recommendations" section with naming guidelines should not be part of the template itself. Consider moving these recommendations to a separate documentation guide or contributing guidelines document.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~24-~24: Possible missing article found.
Context: ...hens to separate words. Also: 1. Keep name short and focused: While it should be d...

(AI_HYDRA_LEO_MISSING_THE)

Comment on lines +1 to +3
# terraform-random-pet

This is a template child module.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance the module description

The current description "This is a template child module" is too generic. Consider providing a more specific description that explains the purpose and functionality of this random pet name generator module.

-This is a template child module.
+# Random Pet Name Generator Module
+
+This Terraform module generates random pet names that can be used as unique identifiers for your infrastructure resources. It supports customizable name length and optional prefixing for organizational purposes.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# terraform-random-pet
This is a template child module.
# terraform-random-pet
# Random Pet Name Generator Module
This Terraform module generates random pet names that can be used as unique identifiers for your infrastructure resources. It supports customizable name length and optional prefixing for organizational purposes.

Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gberenice this looks great. Two larger requests:

  1. Let's move the per-file documentation surrounding purpose + examples from the files themselves and to the main README's "Structure" section. This way we remove some of the duplication and consumers of this template won't clone / copy those file descriptions into each file, which they'll then need to delete every time.

  2. Let's have a specific section in the root README on how to think about versioning. Here are my thoughts:

## Versioning TF + Providers

We’re particular about how we version providers and terraform in child modules and root modules — We recommend the following:

### Child Modules

Since child-modules are intended to be used many times throughout your code, it’s important to make it so that they create as little restrictions on the consuming consuming root module as possible. This means you should find the earliest versions of Terraform / OpenTofu and the providers that your code supports and then use the greater than or equal operator `>=` to make sure that consumers are at least using those versions. This enables Root Modules to choose and upgrade their TF and provider versions without needing their underlying child-modules to be updated.

### Root Modules

Root modules are intended to be planned and applied and therefore they should be more prescriptive so that they’re called consistently in each case that you instantiate a new root module instance (i.e. a state file). To accomplish that, you should do the following:

1. Explicitly pin the latest version of Terraform / OpenTofu that your Root Module supports. You’ll need to upgrade this version each time you want to use a new TF version across your code base.
2. Find the highest provider versions that your root module supports (e.g. AWS Provider `5.81.0`) and use [[the pessimistic version operator](https://developer.hashicorp.com/terraform/language/expressions/version-constraints#operators)](https://developer.hashicorp.com/terraform/language/expressions/version-constraints#operators) `~>` to make sure that you continue to get any bug fixes to that provider. 

Doing the above ensures that you won’t easily introduce any break changes into the actual TF code that you’re applying and you won’t have inconsistencies show up when you run a `tofu init -upgrade`.

root-module/versions.tf Outdated Show resolved Hide resolved
root-module/versions.tf Outdated Show resolved Hide resolved
terraform-random-pet/README.md Show resolved Hide resolved
terraform-random-pet/README.md Outdated Show resolved Hide resolved
terraform-random-pet/README.md Show resolved Hide resolved
terraform-random-pet/README.md Outdated Show resolved Hide resolved
terraform-random-pet/README.md Outdated Show resolved Hide resolved
@gberenice gberenice force-pushed the chore/add-root-module-example branch 3 times, most recently from 6633928 to c8f3cc3 Compare December 13, 2024 13:00
@gberenice gberenice force-pushed the chore/add-root-module-example branch from c8f3cc3 to 2bfcfde Compare December 13, 2024 13:04
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gberenice great work! Couple small things, but then let's :shipit:

README.md Outdated
- Best Practices:
- Descriptive variables: Use meaningful names and description attributes.
- Default values: Provide reasonable defaults when possible. For mandatory inputs, omit defaults to enforce explicit user input.
- Type vonstraints and validation: Use type constraints and validation blocks to catch incorrect inputs early.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Type vonstraints and validation: Use type constraints and validation blocks to catch incorrect inputs early.
- Type constraints and validation: Use type constraints and validation blocks to catch incorrect inputs early.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
terraform-random-pet/variables.tf Outdated Show resolved Hide resolved
terraform-random-pet/README.md Show resolved Hide resolved
@gberenice
Copy link
Member Author

@gberenice great work! Couple small things, but then let's :shipit:

@Gowiem all addressed 👍

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (7)
terraform-random-pet/main.tf (1)

1-4: Consider adding variable validation and resource description.

To improve maintainability and prevent potential issues:

  1. Add a description comment above the resource to document its purpose
  2. Consider adding validation blocks for the variables to enforce constraints
+# Creates a random pet name with a timestamp prefix for unique resource naming
 resource "random_pet" "template" {
   length = var.length
   prefix = var.prefix
 }

And in your variables.tf file:

variable "length" {
  description = "The number of words in the pet name"
  type        = number
  
  validation {
    condition     = var.length > 0
    error_message = "Length must be greater than 0."
  }
}

variable "prefix" {
  description = "A prefix to add to the pet name (typically a timestamp)"
  type        = string
  
  validation {
    condition     = can(regex("^[0-9]{8}", var.prefix))
    error_message = "Prefix must start with 8 digits (YYYYMMDD format)."
  }
}
root-module/variables.tf (2)

1-4: Enhance variable documentation for clarity.

The implementation looks good, but since this is a root module example, consider enhancing the description to be more specific about:

  • The purpose (generating random pet names)
  • The impact on the generated name
  • Any constraints on the value
 variable "length" {
-  description = "The length of the random name"
+  description = "The number of words to use in the generated pet name (e.g., length of 2 might generate 'happy-turtle')"
   type        = number
   default     = 2

5-8: Consider adding an upper bound validation.

While validating for positive numbers is good, consider adding an upper bound to prevent excessively long pet names that might cause issues in resource naming.

   validation {
-    condition     = var.length > 0
-    error_message = "The length must be a positive number."
+    condition     = var.length > 0 && var.length <= 5
+    error_message = "The length must be between 1 and 5 words."
   }
terraform-random-pet/README.md (1)

36-54: Enhance the usage example

The usage example is good but could be improved by:

  1. Adding comments explaining the purpose of each variable
  2. Including more realistic values
  3. Showing how to reference the outputs in different scenarios
 module "sandbox_pet" {
   source  = "masterpointio/random/pet"
   # We recommend to pin the specific version
   # version = "X.X.X"

-  # Input variables
-  length = 2
-  prefix = "sandbox"
+  # Number of words in the pet name (e.g., "happy-dog" has length=2)
+  length = 2
+
+  # Optional prefix for organizational or environment identification
+  prefix = "sandbox"
 }

 # Example outputs
 output "pet_name" {
   description = "The generated pet name, useful for unique resource naming"
   value       = module.sandbox_pet.random_pet_name
 }

+# Example: Use in resource names
+resource "aws_s3_bucket" "example" {
+  bucket = module.sandbox_pet.random_pet_name
+  # ... other configuration ...
+}
README.md (3)

126-126: Fix duplicate word

There's a duplicate word "consuming" in the sentence.

-Since child-modules are intended to be used many times throughout your code, it's important to make it so that they create as little restrictions on the consuming consuming root module as possible.
+Since child-modules are intended to be used many times throughout your code, it's important to make it so that they create as little restrictions on the consuming root module as possible.
🧰 Tools
🪛 LanguageTool

[duplication] ~126-~126: Possible typo: you repeated a word
Context: ...ey create as little restrictions on the consuming consuming root module as possible. This means yo...

(ENGLISH_WORD_REPEAT_RULE)


180-180: Fix grammar in examples directory reference

The article "an" cannot be used with the plural noun "examples".

-- Testing and Examples: Consider adding an examples/ directory with sample configurations and a test/ directory
+- Testing and Examples: Consider adding examples/ directory with sample configurations and a test/ directory
🧰 Tools
🪛 LanguageTool

[grammar] ~180-~180: The plural noun “examples” cannot be used with the article “an”. Did you mean “an example” or “examples”?
Context: ...- Testing and Examples: Consider adding an examples/ directory with sample configurations a...

(A_NNS)


154-154: Add missing punctuation

Add commas to improve readability in compound sentences.

-Root modules are intended to be planned and applied and therefore they should be more prescriptive
+Root modules are intended to be planned and applied, and therefore they should be more prescriptive

-In this example Terraform is pinned exactly at 1.3.7
+In this example, Terraform is pinned exactly at 1.3.7

Also applies to: 176-176

🧰 Tools
🪛 LanguageTool

[uncategorized] ~154-~154: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...s are intended to be planned and applied and therefore they should be more prescript...

(COMMA_COMPOUND_SENTENCE)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 009e7c6 and 801d195.

📒 Files selected for processing (13)
  • .trunk/configs/.markdownlint.yaml (1 hunks)
  • README.md (2 hunks)
  • root-module/README.md (1 hunks)
  • root-module/example.auto.tfvars (1 hunks)
  • root-module/main.tf (1 hunks)
  • root-module/providers.tf (1 hunks)
  • root-module/variables.tf (1 hunks)
  • root-module/versions.tf (1 hunks)
  • terraform-random-pet/README.md (1 hunks)
  • terraform-random-pet/main.tf (1 hunks)
  • terraform-random-pet/outputs.tf (1 hunks)
  • terraform-random-pet/variables.tf (1 hunks)
  • terraform-random-pet/versions.tf (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .trunk/configs/.markdownlint.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
  • root-module/providers.tf
  • root-module/versions.tf
  • root-module/example.auto.tfvars
  • root-module/main.tf
  • terraform-random-pet/variables.tf
  • terraform-random-pet/versions.tf
🧰 Additional context used
🪛 LanguageTool
README.md

[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...t](./terraform-random-pet/) directory. Root module example is provided in [root-mod...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)


[duplication] ~126-~126: Possible typo: you repeated a word
Context: ...ey create as little restrictions on the consuming consuming root module as possible. This means yo...

(ENGLISH_WORD_REPEAT_RULE)


[uncategorized] ~154-~154: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...s are intended to be planned and applied and therefore they should be more prescript...

(COMMA_COMPOUND_SENTENCE)


[uncategorized] ~176-176: A comma might be missing here.
Context: ... = "
> 5.81.0" } } } ``` In this example Terraform is pinned exactly at 1.3.7, t...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[grammar] ~180-~180: The plural noun “examples” cannot be used with the article “an”. Did you mean “an example” or “examples”?
Context: ...- Testing and Examples: Consider adding an examples/ directory with sample configurations a...

(A_NNS)

terraform-random-pet/README.md

[uncategorized] ~21-~21: You might be missing the article “a” here.
Context: ...ch as aws, google, datadog, etc. 3. Use descriptive name: Follow the provider name with a c...

(AI_EN_LECTOR_MISSING_DETERMINER_A)


[uncategorized] ~26-~26: Possible missing article found.
Context: ...hens to separate words. Also: 1. Keep name short and focused: While it should be d...

(AI_HYDRA_LEO_MISSING_THE)

🔇 Additional comments (4)
terraform-random-pet/main.tf (1)

1-4: LGTM! Clean and well-structured resource configuration.

The implementation follows Terraform best practices by using variables instead of hardcoded values.

terraform-random-pet/outputs.tf (1)

1-4: LGTM! Verify the referenced resource.

The output definition follows Terraform best practices with proper naming, description, and formatting.

Let's verify that the referenced resource exists:

✅ Verification successful

✅ Resource reference verified and valid

The output correctly references the random_pet.template resource which is properly defined in main.tf. The resource is configured to use the module's input variables for length and prefix, making this a valid and well-structured Terraform module.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the random_pet.template resource exists in the module
# Expected: Find the resource definition in main.tf or other .tf files

# Search for the random_pet resource named "template"
rg 'resource\s+"random_pet"\s+"template"\s+{' terraform-random-pet/

Length of output: 213


Script:

#!/bin/bash
# Let's try a simpler pattern to find the random_pet resource
rg 'resource "random_pet" "template"' terraform-random-pet/

# Also list all .tf files in the module to ensure we're searching in the right place
fd '\.tf$' terraform-random-pet/

# Let's see the content of any .tf files to understand the module structure
fd '\.tf$' terraform-random-pet/ -x cat {}

Length of output: 1030

root-module/README.md (1)

1-3: 🛠️ Refactor suggestion

Enhance the module description

The current description "This is a template root module" is too generic. Consider providing a more specific description that explains the purpose and functionality of this root module example.

-# root-module
-
-This is a template root module.
+# Example Root Module
+
+This root module demonstrates how to compose child modules together to create a complete infrastructure. It showcases:
+- Module composition and organization
+- Variable management and validation
+- Provider configuration best practices

Likely invalid or redundant comment.

terraform-random-pet/README.md (1)

1-3: 🛠️ Refactor suggestion

Enhance the module description

The current description "This is a template child module" is too generic. Consider providing a more specific description that explains the purpose and functionality of this random pet name generator module.

-# terraform-random-pet
-
-This is a template child module.
+# Random Pet Name Generator Module
+
+This Terraform module generates random pet names that can be used as unique identifiers for your infrastructure resources. It provides customizable name length and optional prefixing capabilities.

Likely invalid or redundant comment.

Comment on lines +5 to +11
<!-- README TEMPLATE: AFTER READING THE BELOW SECTION, DELETE THE BELOW SECTION AND REPLACE WITH YOUR OWN CONTENT -->

## Documentation Recommendations (DO NOT INCLUDE THIS INTO THE REAL README)

- Module description: Briefly explain what the root module sets up (e.g., infrastructure for RDS Postgres instances).
- Use [terraform-docs](https://github.com/terraform-docs/terraform-docs) to ensure that variables, outputs, child module, and resource documentation is included.
- Maintain current information: Keep the README updated as the infrastructure evolves.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Move documentation recommendations to a separate guide

The "Documentation Recommendations" section marked with "(DO NOT INCLUDE THIS INTO THE REAL README)" should be moved to a separate documentation guide or contributing guidelines document.

Consider creating a new file CONTRIBUTING.md in the repository root to house these guidelines.

Comment on lines +7 to +31
## Documentation Recommendations (DO NOT INCLUDE THIS INTO THE REAL README)

### Naming

The repository/directory name should follow this pattern:

```sh
[terraform-]<PROVIDER>-<NAME>
```

Here’s what this means:

1. [If it's a separate repository] The repository should start with `terraform-` if your module should be [published to and discovered on the Registry](https://opentofu.org/docs/language/modules/develop/publish/). Even if you don’t intend to publish the module, following this pattern is a good practice that helps differentiate your Terraform child modules from other code in your projects.
2. Include the provider name: Specify the primary provider your module is for, such as aws, google, datadog, etc.
3. Use descriptive name: Follow the provider name with a clear and concise identifier that describes the module’s purpose.
4. Use hyphens to separate words.

Also:

1. Keep name short and focused: While it should be descriptive, avoid overly long names. The goal is to convey the module’s purpose concisely:
- Good: `terraform-aws-internal-lb`.
- Not so good: `terraform-aws-internal-misc-module`.
- Too long: `terraform-aws-internal-application-load-balancer-with-extra-rules`.
2. Module names should reflect their purpose rather than environment-specific details.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Move naming guidelines to a separate guide

The comprehensive naming guidelines section should be moved to a separate documentation guide. While the information is valuable, it makes the module's README too lengthy and detracts from its primary purpose.

Consider moving this content to a new file docs/MODULE_NAMING.md in the repository root.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~21-~21: You might be missing the article “a” here.
Context: ...ch as aws, google, datadog, etc. 3. Use descriptive name: Follow the provider name with a c...

(AI_EN_LECTOR_MISSING_DETERMINER_A)


[uncategorized] ~26-~26: Possible missing article found.
Context: ...hens to separate words. Also: 1. Keep name short and focused: While it should be d...

(AI_HYDRA_LEO_MISSING_THE)

Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@Gowiem Gowiem merged commit bbc4f32 into main Dec 16, 2024
3 checks passed
@Gowiem Gowiem deleted the chore/add-root-module-example branch December 16, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants