Skip to content

Conversation

@oycyc
Copy link
Contributor

@oycyc oycyc commented Aug 27, 2025

what

  • Add resource "aws_iam_role_policy" "custom" to allow for a custom policy document to be attached to the role that is associated with the EC2 instance.

why

  • There are times where commands will be ran in the EC2 instance as the ec2-user, either manually or via the user_data. This additional IAM policy document will allow that.

Summary by CodeRabbit

  • New Features
    • Optional custom permissions: You can now attach a custom inline policy to the default role by supplying a JSON policy.
    • Configurable policy name: Provide a custom name for the attached policy when using a custom policy.
    • Non-intrusive default: If no custom policy is provided, nothing changes for existing setups.
    • Flexible integration: Enables tailoring role permissions to specific organizational or workload needs without altering core configuration.

@oycyc oycyc requested a review from a team as a code owner August 27, 2025 21:27
@oycyc oycyc requested a review from gberenice August 27, 2025 21:27
@coderabbitai
Copy link

coderabbitai bot commented Aug 27, 2025

Walkthrough

Adds optional inline IAM policy attachment to the default IAM role. Introduces aws_iam_role_policy "custom" in main.tf, created conditionally when var.custom_policy_document is non-empty (count uses length(var.custom_policy_document) > 0 ? 1 : 0). The policy name concatenates module.role_label.id and var.custom_policy_name, attaches to aws_iam_role.default.name, and uses var.custom_policy_document as the policy JSON. Adds two new input variables in variables.tf: custom_policy_document (string, default "") and custom_policy_name (string, default "custom-policy").

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/allow-additional-custom-policy

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ 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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

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.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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: 0

🧹 Nitpick comments (3)
variables.tf (2)

216-220: Validate and trim the custom policy JSON to avoid accidental creation with whitespace/invalid JSON

Currently, any non-empty (even whitespace-only) string triggers resource creation and may fail at apply. Add validation and trimspace.

Apply this diff:

 variable "custom_policy_document" {
   description = "JSON policy document for custom permissions to attach to the SSM Agent role. If not provided, no custom policy will be attached."
   type        = string
   default     = ""
+  validation {
+    condition     = var.custom_policy_document == "" || can(jsondecode(trimspace(var.custom_policy_document)))
+    error_message = "custom_policy_document must be empty or a valid JSON IAM policy document."
+  }
 }

222-226: Constrain policy name to IAM limits and characters

Guard against invalid characters and excessive length for the name component. This prevents surprises when composing the final name in main.tf.

Apply this diff:

 variable "custom_policy_name" {
   description = "Name for the custom policy. Only used if custom_policy_document is provided."
   type        = string
   default     = "custom-policy"
+  validation {
+    condition     = length(var.custom_policy_name) > 0 && length(var.custom_policy_name) <= 128 && can(regex("^[-_A-Za-z0-9+=,.@]+$", var.custom_policy_name))
+    error_message = "custom_policy_name must be 1–128 chars and match [-_A-Za-z0-9+=,.@]."
+  }
 }
main.tf (1)

142-148: Harden conditional creation and sanitize inputs; add safety check for composed name length

  • Use trimspace so whitespace-only input doesn’t create a broken inline policy.
  • Trim the policy payload to avoid leading/trailing whitespace.
  • Optional: add a lifecycle precondition to ensure the composed name stays within IAM’s 128-char limit.

Apply this diff:

 resource "aws_iam_role_policy" "custom" {
-  count = length(var.custom_policy_document) > 0 ? 1 : 0
+  count = length(trimspace(var.custom_policy_document)) > 0 ? 1 : 0
 
   name   = "${module.role_label.id}-${var.custom_policy_name}"
   role   = aws_iam_role.default.name
-  policy = var.custom_policy_document
+  policy = trimspace(var.custom_policy_document)
+  lifecycle {
+    precondition {
+      condition     = length("${module.role_label.id}-${var.custom_policy_name}") <= 128
+      error_message = "Composed inline policy name exceeds 128 characters. Consider a shorter custom_policy_name."
+    }
+  }
 }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ce0441b and 2e88a0d.

📒 Files selected for processing (2)
  • main.tf (1 hunks)
  • variables.tf (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tf

⚙️ CodeRabbit configuration file

**/*.tf: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.

Files:

  • main.tf
  • variables.tf
🧠 Learnings (1)
📚 Learning: 2024-11-21T13:30:01.588Z
Learnt from: gberenice
PR: masterpointio/terraform-aws-tailscale#41
File: main.tf:0-0
Timestamp: 2024-11-21T13:30:01.588Z
Learning: In this Terraform module (`main.tf`), read permissions (`ssm:GetParameter`) for SSM parameters are managed by the SSM Agent module (`masterpointio/ssm-agent/aws`), so adding `ssm:GetParameter` permissions in the custom `ssm_policy` module is unnecessary.

Applied to files:

  • main.tf
  • variables.tf
🔇 Additional comments (1)
main.tf (1)

142-148: Feature addition looks good and is minimally invasive

Conditional inline policy on the role aligns with the module’s design and Cloud Posse patterns (label usage, count-based toggle). With the small hardening above, this should be solid.

}
}

variable "custom_policy_document" {
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this updating the README on commit? Do we need to add the trunk action to this repo maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I didn't init Trunk in this repo locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the Git hook didn't block it.

@oycyc oycyc requested a review from Gowiem August 27, 2025 21:47
@oycyc oycyc merged commit 93a7757 into main Aug 28, 2025
5 checks passed
@oycyc oycyc deleted the feat/allow-additional-custom-policy branch August 28, 2025 01:27
oycyc pushed a commit that referenced this pull request Aug 28, 2025
🤖 I have created a release *beep* *boop*
---


##
[1.6.0](v1.5.0...v1.6.0)
(2025-08-28)


### Features

* allow additional custom IAM policy to attached EC2 role
([#52](#52))
([93a7757](93a7757))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: masterpointbot[bot] <177651640+masterpointbot[bot]@users.noreply.github.com>
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.

3 participants