Issue146 OIDC plan april#154
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to split GitHub Actions OIDC permissions for the hackforla/incubator Terraform workflows into separate Plan vs Apply IAM roles, reducing Plan privileges while keeping Apply capable of administering infrastructure.
Changes:
- Added distinct IAM roles for Terraform plan (
incubator-tf-plan) and apply (incubator-tf-apply) with different OIDC trust conditions and policy attachments. - Added a new custom IAM policy (
IncubatorTfPlanSecretsRead) and registered it in the custom policy module. - Introduced a new JSON policy document granting Terraform plan access to specific Secrets Manager secrets.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| terraform/aws-gha-oidc-providers.tf | Adds separate plan/apply OIDC-assumable roles and attaches policies. |
| terraform/aws-custom-policies.tf | Registers the new custom policy in the custom policies module. |
| terraform/aws-custom-policies/incubator-tf-plan-secrets-read-policy.json | Defines Secrets Manager read permissions for Terraform plan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| resource "aws_iam_role_policy_attachment" "incubator_tf_plan_readonly" { | ||
| role = aws_iam_role.incubator_tf_plan.name | ||
| policy_arn = "arn:aws:iam::aws:policy/ReadOnlyAccess" | ||
| } |
There was a problem hiding this comment.
Attaching only the AWS managed ReadOnlyAccess policy to the Terraform plan role is likely insufficient for a remote S3 backend with DynamoDB state locking (this repo configures dynamodb_table in prod.backend.tfvars). Terraform plan/init typically needs write permissions to the lock table (e.g., dynamodb:PutItem, DeleteItem, UpdateItem) and appropriate S3 backend access; otherwise CI plans will fail when locking the state. Consider adding a minimal backend-access policy (S3 state bucket + DynamoDB lock table) to this role, instead of (or in addition to) ReadOnlyAccess.
| "IncubatorTfPlanSecretsRead" = { | ||
| description = "Allows incubator tf plan role to read specific Secrets Manager secrets needed for terraform plan" | ||
| filename = "incubator-tf-plan-secrets-read-policy.json" | ||
| } |
There was a problem hiding this comment.
PR description mentions a new policy file incubator-tf-plan-secrets-read-policy.tf, but the change here references incubator-tf-plan-secrets-read-policy.json. If the PR description is outdated/typo, consider updating it to match the actual file name/type to avoid confusion for reviewers and future maintainers.
|
Terraform plan in terraform Plan: 3 to add, 0 to change, 0 to destroy.Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
+ create
Terraform will perform the following actions:
# aws_iam_role_policy_attachment.incubator_tf_plan_secrets_read will be created
+ resource "aws_iam_role_policy_attachment" "incubator_tf_plan_secrets_read" {
+ id = (known after apply)
+ policy_arn = (known after apply)
+ role = "incubator-tf-plan"
}
# module.aws_custom_policies.aws_iam_policy.custom_policy["IncubatorTfPlanSecretsRead"] will be created
+ resource "aws_iam_policy" "custom_policy" {
+ arn = (known after apply)
+ attachment_count = (known after apply)
+ description = "Allows incubator tf plan role to read specific Secrets Manager secrets needed for terraform plan"
+ id = (known after apply)
+ name = "IncubatorTfPlanSecretsRead"
+ name_prefix = (known after apply)
+ path = "/"
+ policy = jsonencode(
{
+ Statement = [
+ {
+ Action = [
+ "secretsmanager:GetSecretValue",
]
+ Effect = "Allow"
+ Resource = [
+ "arn:aws:secretsmanager:us-west-2:035866691871:secret:*",
]
+ Sid = "AllowReadSpecificSecretsForTerraformPlan"
},
]
+ Version = "2012-10-17"
}
)
+ policy_id = (known after apply)
+ tags_all = (known after apply)
}
# module.iam_oidc_gha_incubator.aws_iam_role_policy_attachment.github_actions_oidc["arn:aws:iam::aws:policy/AdministratorAccess"] will be created
+ resource "aws_iam_role_policy_attachment" "github_actions_oidc" {
+ id = (known after apply)
+ policy_arn = "arn:aws:iam::aws:policy/AdministratorAccess"
+ role = "gha-incubator"
}
Plan: 3 to add, 0 to change, 0 to destroy.📝 Plan generated in Write Terraform Plan to Pull Request #101 |
Addresses Issue 146
What changes did you make?
aws-gha-oidc-providers.tfIncubatorTfPlanSecretsRead inaws-custom-policies.tf`incubator-tf-plan-secrets-read-policy.tfWhy did you make the changes (we will use this info to test)?