-
Notifications
You must be signed in to change notification settings - Fork 468
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
Added solution.md for Day 1 of TerraWeek: Introduction to Terraform Basics and AWS Setup #21
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce an updated file, Changes
Poem
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? 🪧 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
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
day01/solution.md (3)
3-3
: Adjust heading hierarchy for proper document structureThe document starts with an h3 heading (
###
). For proper document hierarchy, the main sections should start with h2 (##
) and then increment by one level at a time.Apply this change to fix the heading hierarchy:
-### **Day 1: Introduction to Terraform and Terraform Basics** +## **Day 1: Introduction to Terraform and Terraform Basics**🧰 Tools
🪛 Markdownlint (0.35.0)
3-3: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
133-137
: Consider using a more flexible version constraintThe AWS provider version is pinned to a very specific version (
5.65.0
). Consider using a more flexible version constraint to allow for minor updates and patches while maintaining compatibility.required_providers { aws = { source = "hashicorp/aws" - version = "5.65.0" + version = "~> 5.65" } }
49-51
: Enhance SSH command example clarityThe SSH command example could be more clear about the placeholders and provide guidance on finding the necessary information.
-ssh -i "your-key-pair.pem" ubuntu@<EC2-Public-IP> +ssh -i "/path/to/your-key-pair.pem" ubuntu@your-instance-public-ip +# Note: Replace: +# - /path/to/your-key-pair.pem with the path to your downloaded key pair +# - your-instance-public-ip with the Public IPv4 address from your EC2 instance details
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
day01/solution.md
(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
day01/solution.md
3-3: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time
(MD001, heading-increment)
- Create a new IAM user with **programmatic access**. | ||
- Attach the necessary policies (e.g., `AdministratorAccess` or policies tailored to your use case). | ||
- Note down the **Access Key ID** and **Secret Access Key**. |
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.
Revise IAM permissions recommendation
Recommending AdministratorAccess
policy is not aligned with the principle of least privilege. Instead, suggest creating custom IAM policies that grant only the necessary permissions for the specific infrastructure being managed.
Consider updating the documentation to recommend:
- Creating a custom IAM policy based on the resources being managed
- Using AWS managed policies specific to the services being used
- Following the principle of least privilege
- **Configure AWS CLI Credentials:** | ||
|
||
Run the following command and enter your IAM user details: | ||
```bash | ||
aws configure | ||
``` | ||
|
||
Provide: | ||
- **Access Key ID** | ||
- **Secret Access Key** | ||
- **Default region** (e.g., `us-east-1`) | ||
- Output format (default: `json`) | ||
|
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.
Recommend using IAM roles instead of access keys
When working with Terraform on EC2 instances, it's more secure to use IAM roles instead of storing AWS credentials directly on the instance.
Consider updating this section to:
- Create an IAM role with the necessary permissions
- Attach the IAM role to the EC2 instance during creation
- Remove the AWS credentials configuration steps as they won't be needed with IAM roles
data "aws_ami" "ubuntu" { | ||
most_recent = true | ||
filter { | ||
name = "name" | ||
values = ["ubuntu/images/*"] | ||
} | ||
} | ||
``` |
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.
🛠️ Refactor suggestion
Enhance AMI data source example with additional filters
The current AMI data source example is missing important filters that are typically needed for proper AMI selection, such as virtualization-type and architecture.
data "aws_ami" "ubuntu" {
most_recent = true
+ owners = ["099720109477"] # Canonical
filter {
name = "name"
values = ["ubuntu/images/*"]
}
+ filter {
+ name = "virtualization-type"
+ values = ["hvm"]
+ }
+ filter {
+ name = "architecture"
+ values = ["x86_64"]
+ }
}
📝 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.
data "aws_ami" "ubuntu" { | |
most_recent = true | |
filter { | |
name = "name" | |
values = ["ubuntu/images/*"] | |
} | |
} | |
``` | |
data "aws_ami" "ubuntu" { | |
most_recent = true | |
owners = ["099720109477"] # Canonical | |
filter { | |
name = "name" | |
values = ["ubuntu/images/*"] | |
} | |
filter { | |
name = "virtualization-type" | |
values = ["hvm"] | |
} | |
filter { | |
name = "architecture" | |
values = ["x86_64"] | |
} | |
} |
```bash | ||
wget -O - https://apt.releases.hashicorp.com/gpg | sudo gpg --dearmor -o /usr/share/keyrings/hashicorp-archive-keyring.gpg | ||
echo "deb [arch=$(dpkg --print-architecture) signed-by=/usr/share/keyrings/hashicorp-archive-keyring.gpg] https://apt.releases.hashicorp.com $(lsb_release -cs) main" | sudo tee /etc/apt/sources.list.d/hashicorp.list | ||
``` |
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.
🛠️ Refactor suggestion
Improve security of GPG key installation
The current GPG key installation pipes wget directly to sudo, which is not the most secure approach. Consider downloading the key first and then importing it.
-wget -O - https://apt.releases.hashicorp.com/gpg | sudo gpg --dearmor -o /usr/share/keyrings/hashicorp-archive-keyring.gpg
+# Download the GPG key first
+wget -O hashicorp.asc https://apt.releases.hashicorp.com/gpg
+# Verify the fingerprint (optional but recommended)
+gpg --show-keys hashicorp.asc
+# Import the key
+sudo gpg --dearmor -o /usr/share/keyrings/hashicorp-archive-keyring.gpg hashicorp.asc
+# Clean up
+rm hashicorp.asc
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
day01/solution.md (2)
1-4
: Fix heading hierarchyThe document structure jumps from h1 to h3, which violates markdown best practices. Consider restructuring the headings to maintain proper hierarchy:
-# TerraWeek Day 1 - -### **Day 1: Introduction to Terraform and Terraform Basics** +# TerraWeek Day 1: Introduction to Terraform and Terraform Basics + +## Overview🧰 Tools
🪛 Markdownlint (0.35.0)
3-3: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
132-140
: Document provider version pinning strategyWhile pinning the AWS provider version is good practice, consider adding a comment explaining:
- Why this specific version was chosen
- How to handle version updates
- The importance of reviewing the changelog before updating
Example addition:
terraform { required_providers { aws = { source = "hashicorp/aws" version = "5.65.0" # Pinned for stability + # Review changelog at https://github.com/hashicorp/terraform-provider-aws/releases + # before updating this version } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
day01/solution.md
(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
day01/solution.md
3-3: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🔇 Additional comments (4)
day01/solution.md (4)
70-73
: Improve security of GPG key installation
The current GPG key installation method is not following security best practices.
96-98
: Revise IAM permissions recommendation
107-119
: Recommend using IAM roles instead of access keys
209-216
: Enhance AMI data source example with additional filters
1. Go to the **EC2 Dashboard** on AWS. | ||
2. Click **Launch Instance**. | ||
3. Configure: | ||
- **Name:** Terraform-Setup | ||
- **AMI:** Ubuntu Server 22.04 LTS | ||
- **Instance Type:** t2.micro (free tier eligible) | ||
- **Key Pair:** Create or select an existing key pair. | ||
- **Network Settings:** Allow SSH traffic (port 22) from your IP address. | ||
- **Storage:** Keep the default 8 GiB or adjust as needed. | ||
|
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.
🛠️ Refactor suggestion
Enhance EC2 instance security recommendations
The EC2 instance setup instructions should include additional security best practices:
- Use VPC with private subnet and NAT Gateway
- Enable IMDSv2
- Apply security group rules with minimum required permissions
- Enable detailed monitoring
- Add proper resource tags
Consider adding these security configurations to the EC2 launch instructions.
3. **State:** | ||
|
||
- A file that tracks the current state of your infrastructure. It helps Terraform understand the desired state versus the actual state. | ||
|
||
- Example: | ||
|
||
Terraform generates a `terraform.tfstate` file during execution. | ||
|
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.
Add state file security warnings
The state file section should include critical security considerations:
- Warning about sensitive data in state files
- Recommendation for remote state storage (e.g., S3 with encryption)
- State file backup strategies
- Access control for state files
- State locking mechanisms
Consider expanding this section to cover these important aspects of state management.
Would you like me to provide a detailed example of secure state configuration?
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
day01/solution.md (2)
1-4
: Fix heading hierarchyThe document structure violates markdown heading hierarchy by jumping from h1 to h3. Headers should only increment by one level at a time.
Apply this diff:
-# TerraWeek Day 1 - -### **Day 1: Introduction to Terraform and Terraform Basics** +# TerraWeek Day 1 + +## Introduction to Terraform and Terraform Basics🧰 Tools
🪛 Markdownlint (0.35.0)
3-3: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
136-149
: Add version constraint best practicesWhile specifying an exact version is precise, it's recommended to use version constraints that allow for compatible updates while preventing breaking changes.
Consider updating the provider block:
terraform { required_providers { aws = { source = "hashicorp/aws" - version = "5.65.0" + version = "~> 5.65.0" # Allows minor and patch updates within 5.x } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
day01/solution.md
(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
day01/solution.md
3-3: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🔇 Additional comments (6)
day01/solution.md (6)
35-44
: Enhance EC2 instance security recommendations
The EC2 instance setup instructions should include additional security best practices:
- Use VPC with private subnet and NAT Gateway
- Enable IMDSv2
- Apply security group rules with minimum required permissions
- Enable detailed monitoring
- Add proper resource tags
Consider adding these security configurations to the EC2 launch instructions.
70-73
: Improve security of GPG key installation
The current GPG key installation pipes wget directly to sudo, which is not the most secure approach. Consider downloading the key first and then importing it.
96-98
: Revise IAM permissions recommendation
Recommending AdministratorAccess
policy is not aligned with the principle of least privilege. Instead, suggest creating custom IAM policies that grant only the necessary permissions for the specific infrastructure being managed.
112-124
: Recommend using IAM roles instead of access keys
When working with Terraform on EC2 instances, it's more secure to use IAM roles instead of storing AWS credentials directly on the instance.
186-193
: Add state file security warnings
The state file section should include critical security considerations:
- Warning about sensitive data in state files
- Recommendation for remote state storage (e.g., S3 with encryption)
- State file backup strategies
- Access control for state files
- State locking mechanisms
214-221
: Enhance AMI data source example with additional filters
The current AMI data source example is missing important filters that are typically needed for proper AMI selection.
This PR covers the Day 1 activities for TerraWeek, focusing on introducing Terraform and setting up the basics for managing infrastructure as code (IaC) with AWS.
Key Highlights
Introduction to Terraform
Installation and Setup on AWS
Core Concepts in Terraform
Terraform Configuration Example
.tf
configurations for S3 buckets and AMIs.References
This submission serves as the foundation for upcoming days of TerraWeek, focusing on practical implementation and advanced concepts.
Next Steps
Looking forward to feedback and suggestions!
Summary by CodeRabbit