aws-samples / aws-samples/sample-collaborative-ai-dlc
[Bug]: global_resource_name_suffix is set in tfvars but never declared, so it is ignored
- Dominant language
- JavaScript
- Stars
- 75
- Forks
- 23
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 24
Description
### Description
An environment tfvars file that sets `global_resource_name_suffix` has no effect. The root module never declares a variable of that name, so Terraform emits a warning and ignores the value.
`terraform/variables.tf` on `main` declares only `aws_region`, `project_name`, `environment`, `bedrock_model`, `codex_model`, and `aidlc_repo_ref`. Terraform treats a value assigned to an undeclared variable in a named or `-var-file` tfvars as a warning rather than an error, so an operator reading their tfvars would reasonably believe the suffix was applied when it never was.
Expected: the suffix is appended to account-scoped resource names, so a second stack can coexist with an existing one in the same AWS account.
Actual: the value is discarded, and the resources keep their unsuffixed names.
The fix is larger than declaring the variable, for two reasons.
**Declaring it alone is unsafe for existing stacks.** Once declared, the value becomes effective on the next apply and resource names change. `name` on `aws_iam_role` is `ForceNew`, so Terraform destroys and recreates the role and its ARN changes. Lambda functions and the ECS service reference those roles. Depending on apply ordering this can cause authorization failures during the apply window. Any environment that already sets a suffix in tfvars is running unsuffixed today, so declaring the variable would rename live resources on the next apply.
**Coverage would need to be complete to be useful.** The goal is to avoid collisions on names that are account-scoped rather than region-scoped. IAM role names are account-global. There are 21 `aws_iam_role` resources under `terraform/`, plus one in the AgentCore module. Suffixing a subset does not help: the second stack still fails on the first unsuffixed name with `EntityAlreadyExists`.
Resources in that category:
| Resource | Location |
|---|---|
| `aws_iam_role.users` | `terraform/modules/api/lambda/main.tf:113` |
| `aws_iam_role.trackers` | `terraform/modules/api/lambda/main.tf:241` |
| `aws_iam_role.neptune_artifacts` | `terraform/modules/api/lambda/main.tf:481` |
| `aws_iam_role.neptune_tasks` | `terraform/modules/api/lambda/main.tf:627` |
| `aws_iam_role.blocks` | `terraform/modules/api/lambda/main.tf:658` |
| `aws_iam_role.source_control` | `terraform/modules/api/lambda/main.tf:719` |
| `aws_iam_role.credential_broker` | `terraform/modules/api/lambda/main.tf:840` |
| `aws_iam_role.gitlab_connector` | `terraform/modules/api/lambda/main.tf:1378` |
| `aws_iam_role.discussions` | `terraform/modules/api/lambda/main.tf:1550` |
| `aws_iam_role.intents` | `terraform/modules/api/lambda/main.tf:1896` |
| `aws_iam_role.v2_orchestrator` | `terraform/modules/api/lambda/main.tf:2214` |
| `aws_iam_role.agentcore` | `terraform/modules/compute/agentcore/main.tf:253` |
| `aws_ecr_repository` (agentcore) | `terraform/modules/compute/agentcore/main.tf:133` |
| `aws_ecr_repository` (yjs-server) | `terraform/modules/realtime/yjs-server/main.tf:50` |
| `aws_ecs_cluster.main` | `terraform/modules/realtime/yjs-server/main.tf:108` |
| `aws_lb.yjs_server` | `terraform/modules/realtime/yjs-server/main.tf:306` |
| `aws_lb_target_group.yjs_server` | `terraform/modules/realtime/yjs-server/main.tf:315` |
The AgentCore module is not passed a suffix variable from `terraform/main.tf` and does not declare one, so its role and ECR repository cannot be suffixed at all without wiring that first.
S3 buckets are correctly out of scope: they already append `random_id.bucket_suffix`, so they do not collide.
Two specific traps for whoever implements this:
**The ALB name is already at the 32-character limit.** `terraform/modules/realtime/yjs-server/main.tf:10` computes `yjs_lb_name_raw = "${var.project_name}-yjs-${var.environment}"`, then falls back to a truncated name plus a sha1 fragment when that exceeds 32 characters. With `project_name = "collaborative-ai-dlc"` and `environment = "staging"` the raw name is exactly 32 characters. A suffix must be folded into `yjs_lb_name_raw` before the truncation branch so the existing sha1 fallback absorbs it. Appending after truncation would exceed the limit and fail at apply.
**The suffix value needs validation.** A value of `-v2` would produce a double hyphen and therefore a different name than intended. A value containing characters that are invalid in IAM role names would only fail at apply.
### Steps to reproduce
1. `git checkout main` at `b9ab0b6d`
2. Add `global_resource_name_suffix = "v2"` to an environment tfvars file
3. `terraform init -backend-config=terraform/environments/.s3.tfbackend`
4. `terraform plan -var-file=terraform/environments/.tfvars`
5. Terraform warns that a value was assigned to an undeclared variable, and no resource name in the plan carries the suffix
### Logs or screenshots
`terraform/variables.tf` on `main` contains no such declaration:
```
$ grep -c global_resource_name_suffix terraform/variables.tf
0
```
Expected Terraform behaviour for the undeclared value:
```
Warning: Value for undeclared variable
The root module does not declare a variable named "global_resource_name_suffix"
but a value was found in file "terraform/environments/.tfvars".
```
### Environment
- `main` at `b9ab0b6d`
- Terraform `>= 1.0` (`terraform/main.tf:2`); CI pins `1.15.0`
- Deployment tfvars files are gitignored, so this affects any environment where an operator has set the suffix locally
### Suggested path
1. Decide the migration story first, because it determines everything else: do existing stacks adopt suffixed names, or does the suffix only ever apply to newly created stacks? Renaming live IAM roles on an existing environment needs a coordinated plan, so the second option is likely preferable.
2. Declare the variable with validation on shape and length.
3. Cover all account-scoped names in one change, including wiring the AgentCore module. A partial implementation is worse than none: it fails late, during apply, on whichever name was missed.
4. Fold the suffix into the ALB name before the 32-character truncation.
Happy to pick this up once step 1 is decided.
Contributor guide
Research direction
First resolve the migration question for existing stacks, then read terraform/variables.tf and terraform/main.tf to trace the root variable into the modules. Review the listed resources in terraform/modules/api/lambda/main.tf, terraform/modules/compute/agentcore/main.tf, and terraform/modules/realtime/yjs-server/main.tf, including the ALB naming logic, and run the documented plan reproduction. Done means the chosen migration behavior is explicit and all listed account-scoped names are covered without invalid names or collisions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, terraform
- Domain
- cloud, infrastructure
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100