Skip to content

feat(terraform_wrapper_module_for_each): Strip provider_meta blocks from versions.tf#967

Merged
MaxymVlasov merged 6 commits into
antonbabenko:masterfrom
pen-pal:fix/954-strip-provider-meta-from-wrapper
May 30, 2026
Merged

feat(terraform_wrapper_module_for_each): Strip provider_meta blocks from versions.tf#967
MaxymVlasov merged 6 commits into
antonbabenko:masterfrom
pen-pal:fix/954-strip-provider-meta-from-wrapper

Conversation

@pen-pal
Copy link
Copy Markdown
Contributor

@pen-pal pen-pal commented May 25, 2026

  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.

Description of your changes

The hook copies the source module's versions.tf into the wrapper verbatim. If the source declares a provider_meta block (used by terraform-aws-modules to tag the provider's user-agent), the wrapper inherits it. The wrapper makes no provider calls itself, so that propagation is redundant and skews module-usage telemetry.

Uses hcledit (already a hook dependency, see check_dependencies at hooks/terraform_wrapper_module_for_each.sh:430) to remove user_agent and module_name for the provider configured via --module-repo-provider. The edit is structural:

  • Modules with no provider_meta are untouched.
  • Non-AWS provider_meta blocks are untouched.
  • A future nested block inside provider_meta (e.g. if the AWS provider extends the schema) is preserved.

Cosmetic caveat: the (possibly now-empty) provider_meta block itself remains, because hcledit cannot remove nested blocks. Functionally harmless — no attributes ⇒ no user-agent contribution.

Fixes #954

How can we test changes

Run the hook against a module whose versions.tf has a provider_meta block (e.g. any terraform-aws-modules repo):

bash hooks/terraform_wrapper_module_for_each.sh \
  --args=--root-dir=<path> --args=--module-dir=. \
  --args=--module-repo-shortname=<name> <path>/main.tf

Check <path>/wrappers/versions.tf — provider_meta should be gone, everything else intact.

Tested against hypothetical value:

input

terraform {
  provider_meta "aws" {
    user_agent = ["github.com/terraform-aws-modules/terraform-aws-s3-bucket"]

    hypothetical_nested_block {
      key = "value"
    }
  }
}

after the hook

terraform {
  provider_meta "aws" {

    hypothetical_nested_block {
      key = "value"
    }
  }
}

user_agent cleanly removed, nested block intact.

…s from wrapper `versions.tf`

The hook copies the source module's `versions.tf` into the generated wrapper
verbatim. When the source carries a `provider_meta` block (used by
terraform-aws-modules to tag the provider user-agent), the wrapper ends up
declaring it too, so every provider call from a wrappered module sends the
same user-agent fragment twice.

Filter `provider_meta` blocks out while copying. `required_version` and
`required_providers` are preserved.

Fixes antonbabenko#954

Signed-off-by: pen-pal <unameme@proton.me>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 83bd9019-8d49-4cab-9572-60c12ba6b21c

📥 Commits

Reviewing files that changed from the base of the PR and between 3cede7a and 364281f.

📒 Files selected for processing (1)
  • hooks/terraform_wrapper_module_for_each.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • hooks/terraform_wrapper_module_for_each.sh

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Prevented duplicate provider metadata tags when wrapping modules that include version specifications: the wrapper now honors source module version info when present and falls back to a default required-version constraint when absent, ensuring consistent provider tagging and version handling.

Walkthrough

The wrapper now copies a module's versions.tf when present but strips terraform.provider_meta.<provider>.user_agent and .module_name attributes; if no versions.tf exists the wrapper still writes its default required_version block.

Changes

Provider Meta Filtering in Terraform Wrapper

Layer / File(s) Summary
Filter provider_meta attributes from versions.tf
hooks/terraform_wrapper_module_for_each.sh
Copies source versions.tf into the wrapper output but removes terraform.provider_meta.<module_repo_provider>.user_agent and .module_name attributes instead of copying them verbatim; preserves default required_version when no source file exists.

Sequence Diagram(s)

sequenceDiagram
  participant Wrapper
  participant SourceModule
  participant OutputDir
  Wrapper->>SourceModule: read versions.tf
  Wrapper->>Wrapper: strip terraform.provider_meta.<provider>.user_agent and .module_name
  Wrapper->>OutputDir: write filtered versions.tf
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested labels

hook/terraform_wrapper_module_for_each

Suggested reviewers

  • antonbabenko
  • yermulnik
  • MaxymVlasov
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: stripping provider_meta blocks from versions.tf in the terraform_wrapper_module_for_each hook.
Description check ✅ Passed The description is well-related to the changeset, explaining the bug fix, implementation approach using hcledit, behavior details, and testing instructions.
Linked Issues check ✅ Passed The PR successfully implements the objective from issue #954 by using hcledit to remove user_agent and module_name attributes from provider_meta blocks, preventing duplication of provider user-agent telemetry while preserving other content.
Out of Scope Changes check ✅ Passed All changes are scoped to the terraform_wrapper_module_for_each.sh hook and directly address the provider_meta stripping requirement from issue #954; no out-of-scope modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@pen-pal pen-pal changed the title fix(terraform_wrapper_module_for_each): strip provider_meta block… fix: Strip provider_meta blocks from wrapper versions.tf May 25, 2026
@yermulnik
Copy link
Copy Markdown
Collaborator

@pen-pal Thanks for the contribution.

so the provider's user-agent tag gets sent twice on every call.

What's wrong with such duplication? Does it impact/break something? What's the drawback of leaving provider_meta "aws" { block as is in terraform {} inside wrapped configuration?

Also given that awk is going to be yet another dependency for the wrapper, it's probably best to convert it to sed, which is already in use by the wrapper. WDYT?

Apart from that, I guess it's best to strip the provider_meta off of the exact provider, rather than from any provider_meta block which might potentially exist and be used with non-AWS providers.

Also (and it's probably my main concern) if by any reason the AWS provider's provider_meta nested block receives an update that introduces its own nested block, then the proposed solution would produce incorrect terraform {} configuration, which would result in non-obvious syntax error and might complicate usage and diagnostics for end users.
The below is what I mean with the hypothetical configuration:

> cat ~/tmp/versions.tf
terraform {
  provider_meta "aws" {
    user_agent = [
      "github.com/terraform-aws-modules/terraform-aws-s3-bucket"
    ]

    hypthetical_nested_block {
      key = "value"
    }
  }
}

> cat ~/tmp/versions.tf | awk '/^[[:space:]]*provider_meta[[:space:]]+"[^"]+"[[:space:]]*\{[[:space:]]*$/ { in_block = 1; next }; in_block && /^[[:space:]]*\}[[:space:]]*$/ { in_block = 0; next }; !in_block { print }'
terraform {
  }
}

To avoid such — though still hypothetical — confusion, we're better off scaffolding some other solution I reckon.
Off the top of my head, I see three immediate options:

  • Update the suggested code to become more intelligent to catch such conditions. This option is weird obviously, as it would introduce unnecessary complexity.
  • Since hcledit cannot be used to strip the provider_meta block directly, we can use it to drop the user_agent from this block (hcledit attribute rm terraform.provider_meta.aws.user_agent), which (the user_agent key) seemingly is the direct "culprit" of the "issue" I guess — would this do the thing to eliminate duplication and would it not produce incorrect configuration? Could you please see if you've got a chance to test and validate this? 🤔
  • Use e.g. https://github.com/tmccombs/hcl2json and jq to convert HCL to JSON, strip the provider_meta "aws" block, and convert back from JSON to HCL — this also would intro unnecessary complexity by means of additional dependencies.

…of `awk` to strip `provider_meta` attrs

Address antonbabenko#967 review:
- `hcledit` is already a hard dep of this hook, so no new tooling.
- Structural edit survives a hypothetical nested block inside
  `provider_meta` (where the `awk` filter would mangle the closing brace).
- Target only the configured provider via `${module_repo_provider}`, so
  unrelated `provider_meta` blocks (e.g. for non-AWS providers) are
  preserved.
- Strip both `user_agent` and legacy `module_name` keys.

The (possibly now-empty) `provider_meta` block itself remains since
`hcledit` cannot remove nested blocks — it is functionally harmless.

Signed-off-by: penpal <unameme@proton.me>
@pen-pal
Copy link
Copy Markdown
Contributor Author

pen-pal commented May 26, 2026

@pen-pal Thanks for the contribution.

so the provider's user-agent tag gets sent twice on every call.

What's wrong with such duplication? Does it impact/break something? What's the drawback of leaving provider_meta "aws" { block as is in terraform {} inside wrapped configuration?

Also given that awk is going to be yet another dependency for the wrapper, it's probably best to convert it to sed, which is already in use by the wrapper. WDYT?

Apart from that, I guess it's best to strip the provider_meta off of the exact provider, rather than from any provider_meta block which might potentially exist and be used with non-AWS providers.

Also (and it's probably my main concern) if by any reason the AWS provider's provider_meta nested block receives an update that introduces its own nested block, then the proposed solution would produce incorrect terraform {} configuration, which would result in non-obvious syntax error and might complicate usage and diagnostics for end users. The below is what I mean with the hypothetical configuration:

> cat ~/tmp/versions.tf
terraform {
  provider_meta "aws" {
    user_agent = [
      "github.com/terraform-aws-modules/terraform-aws-s3-bucket"
    ]

    hypthetical_nested_block {
      key = "value"
    }
  }
}

> cat ~/tmp/versions.tf | awk '/^[[:space:]]*provider_meta[[:space:]]+"[^"]+"[[:space:]]*\{[[:space:]]*$/ { in_block = 1; next }; in_block && /^[[:space:]]*\}[[:space:]]*$/ { in_block = 0; next }; !in_block { print }'
terraform {
  }
}

To avoid such — though still hypothetical — confusion, we're better off scaffolding some other solution I reckon. Off the top of my head, I see three immediate options:

  • Update the suggested code to become more intelligent to catch such conditions. This option is weird obviously, as it would introduce unnecessary complexity.
  • Since hcledit cannot be used to strip the provider_meta block directly, we can use it to drop the user_agent from this block (hcledit attribute rm terraform.provider_meta.aws.user_agent), which (the user_agent key) seemingly is the direct "culprit" of the "issue" I guess — would this do the thing to eliminate duplication and would it not produce incorrect configuration? Could you please see if you've got a chance to test and validate this? 🤔
  • Use e.g. https://github.com/tmccombs/hcl2json and jq to convert HCL to JSON, strip the provider_meta "aws" block, and convert back from JSON to HCL — this also would intro unnecessary complexity by means of additional dependencies.

@yermulnik made some updates based on suggestion.
also updated the description

Comment thread hooks/terraform_wrapper_module_for_each.sh Outdated
@yermulnik
Copy link
Copy Markdown
Collaborator

@yermulnik made some updates based on suggestion. also updated the description

Thank you.
LGTM, though I left a comment — please take a look.

yermulnik
yermulnik previously approved these changes May 28, 2026
Copy link
Copy Markdown
Collaborator

@yermulnik yermulnik left a comment

Choose a reason for hiding this comment

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

LGTM
@MaxymVlasov Could you please take a look too? Thanks

Copy link
Copy Markdown
Owner

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Minor comment, LGTM

Comment thread hooks/terraform_wrapper_module_for_each.sh Outdated
Signed-off-by: penpal <unameme@proton.me>
yermulnik
yermulnik previously approved these changes May 29, 2026
@MaxymVlasov MaxymVlasov changed the title fix: Strip provider_meta blocks from wrapper versions.tf feat(terraform_wrapper_module_for_each): Strip provider_meta blocks from versions.tf May 29, 2026
Comment thread hooks/terraform_wrapper_module_for_each.sh Outdated
Comment thread hooks/terraform_wrapper_module_for_each.sh Outdated
@MaxymVlasov MaxymVlasov enabled auto-merge (squash) May 30, 2026 00:01
@MaxymVlasov MaxymVlasov disabled auto-merge May 30, 2026 00:01
@MaxymVlasov MaxymVlasov merged commit 7710871 into antonbabenko:master May 30, 2026
44 checks passed
antonbabenko pushed a commit that referenced this pull request May 30, 2026
# [1.106.0](v1.105.0...v1.106.0) (2026-05-30)

### Features

* **`terraform_wrapper_module_for_each`:** Strip `provider_meta` blocks from `versions.tf` ([#967](#967)) ([7710871](7710871))
@antonbabenko
Copy link
Copy Markdown
Owner

This PR is included in version 1.106.0 🎉

@pen-pal pen-pal deleted the fix/954-strip-provider-meta-from-wrapper branch May 30, 2026 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review the wrapper hook to make sure it works correctly with provider_meta

4 participants