Repository navigation
feat!: remove module provider "aws" block — caller owns credentials (v5.0.1) - #57
Merged
Merged
Conversation
…v5.0.1)
BREAKING (operational, not API): the module no longer declares its own
provider "aws". As a child module, a self-declared provider block takes
precedence for the modules resources and IGNORES the callers provider,
stripping the wrapper control over credentials (assume_role / profile /
region) and default_tags — the deprecated pattern, and the root cause of the
STSaaS-account 403 workaround in atlantis.yaml (the module credential-less
provider ran as ambient atlantis-sa instead of the assumed STSaaS role).
The module keeps its provider *requirement* (versions.tf required_providers).
Now every module.comet resource inherits the CALLERs provider.
Wrapper migration required when bumping to v5.0.1:
* ensure the wrapper provider "aws" carries region + assume_role/profile
(stsaasuat already does, via var.provider_assume_role_arn)
* move default_tags (Terraform / Environment / common_tags) onto the wrapper
provider — the module no longer sets them
* the atlantis.yaml assume-role-and-export-creds shell hack for the stsaas
workflow can then be replaced with a normal provider-level assume_role
No aliased/region-specific providers exist in the module (checked: no
configuration_aliases, no provider = aws.*), so single-provider inheritance is
safe. terraform init + validate pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…am eks) Replace the loose/inconsistent aws constraints (~> 6.0 root, >= 6.0 comet_eks) with an honest floor: >= 6.52, the minimum required by the pinned terraform-aws-modules/eks ~> 21.24. Anything looser was misleading — init could never resolve below 6.52 anyway. No upper bound and no bad-build exclusion (e.g. != 6.57.0) in the module: those are deployment policy and stay in the wrappers. Effective resolved constraint with the stsaasuat wrapper is >= 6.52, < 7.0, != 6.57.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s README Addresses baz review on #57: * Region consistency (medium): with the module provider block removed, the provider region now comes from the caller while several resources still key off var.region as a string — notably the Karpenter controller IAM policy (EC2 ARNs scoped to arn:aws:ec2:${var.region}:... and an aws:RequestedRegion == var.region condition). If the caller provider region diverges, those grants target the wrong region and deny. Add data.aws_region.current + a terraform_data precondition that fails fast at plan when they mismatch. (Kept var.region in the ARNs — it is the intended region; the guard just enforces the provider agrees. Lighter and clearer than threading data.aws_region through every ARN.) * Stale docs (low): modules/comet_eks/README.md advertised aws >= 5.0 and a kubernetes >= 2.10 requirement. Update to aws >= 6.52 (matches versions.tf) and drop kubernetes (removed in v5.0.0); add the time provider the module actually uses. Skipped baz driver-README direct-consumer note: this module is only consumed as a wrapper child (no backend of its own); the new precondition already fails fast on a region mismatch, covering the substantive concern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+1085
to
+1087
| condition = data.aws_region.current.region == var.region | ||
| error_message = "The AWS provider region (${data.aws_region.current.region}) must match var.region (${var.region}). This module inherits the caller's provider — set the wrapper's provider \"aws\" { region = ... } to the same region you pass as region/eks region, or the Karpenter IAM ARNs and aws:RequestedRegion condition will target the wrong region." | ||
| } |
There was a problem hiding this comment.
terraform_data.region_consistency references data.aws_region.current.region in both the precondition and error_message, but that attribute isn't exported by hashicorp/aws v6 — the region name lives in data.aws_region.current.id (or .name depending on version) — so Terraform errors with Unsupported attribute at plan time before the fail-fast check can even run. Should we switch both references to the correct exported attribute (e.g. data.aws_region.current.id) and update the error_message accordingly?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code and the installed AWS
provider version. In modules/comet_eks/main.tf around lines 1082-1089, inside the
`terraform_data.region_consistency` `lifecycle { precondition { ... } }` block, the
`condition` and `error_message` reference `data.aws_region.current.region`, which is not
a valid exported attribute on `data "aws_region" "current"` for `hashicorp/aws` v6 (the
region name is exposed as `.id`, or `.name` in some provider versions). Update both the
`condition` comparison against `var.region` and the `error_message` interpolation to use
the correct attribute (`data.aws_region.current.id`), so the region-consistency check
actually runs at plan time instead of failing with an `Unsupported attribute` error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Why
The module declared its own
provider "aws"(noassume_role). Because a child module's provider config wins for its own resources and ignores the caller's,module.comet's resources ran on the module's credential-less provider — ambientatlantis-sa(094792403439) instead of the wrapper's assumed STSaaS role (947208553405) → the 403s that atlantis.yaml works around by assuming the role in a shell step and exporting session creds. It also means a localAWS_PROFILEon the wrapper is ignored by the module. This is the deprecated pattern.Change
Delete the module's
provider "aws"block. Keep the provider requirement (required_providers). Module resources now inherit the caller's provider → the wrapper fully controls credentials/region/tags.Verified: no
configuration_aliases/provider = aws.*/ aliased providers anywhere in the module → single-provider inheritance is safe.region/common_tags/environment_tagremain used elsewhere (27/63/1 refs).init+validatepass.Wrapper migration (at
?refbump to v5.0.1)provider "aws"must carry region + assume_role/profile — stsaasuat already does (var.provider_assume_role_arn).default_tags(Terraform / Environment / common_tags) onto the wrapper provider — the module no longer sets them.assume_role.Folds into the v5.0.x line → tag v5.0.1 after merge, and the in-flight stsaasuat cutover bumps to v5.0.1 in one pass.
🤖 Generated with Claude Code
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Remove the module-level
provider "aws"configuration socomet_eksresources inherit the caller’s credentials, region, and default tags from the wrapper provider. Update the AWS provider requirements and module docs to match that ownership model, and add a plan-time check forvar.region-scoped IAM behavior.provider "aws"owns credentials, region, and tags forcomet_eksresources.Modified files (4)
Latest Contributors(2)
var.regionfor Karpenter and EC2-scoped grants.Modified files (1)
Latest Contributors(2)