Skip to content

fix(generator): Filter third-party services by active products#17971

Open
JamesDuncanNz wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
JamesDuncanNz:fix/generator-filter-services
Open

fix(generator): Filter third-party services by active products#17971
JamesDuncanNz wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
JamesDuncanNz:fix/generator-filter-services

Conversation

@JamesDuncanNz

Copy link
Copy Markdown
Contributor

Description

This PR fixes a bug in the Magic Modules generator where handwritten files for Beta-only (or version-restricted) services were being leaked into the GA provider, causing compilation failures - example.

The Problem

When introducing a new Beta-only service (such as the upcoming agentregistry) that includes handwritten files in third_party/terraform/services/<service_name>/ (e.g., agent_registry_operation.go), the GitHub Actions "Provider Build and Unit Test" job fails during the GA provider compilation with:
undefined: Product

The Cause

  1. During GA provider generation, the Beta-only service is correctly not generated, meaning product.go (which defines the Product variable) is not created in the GA provider.
  2. However, the generator's CopyCommonFiles and CompileCommonFiles methods were blindly copying/compiling the entire third_party/terraform/services/ directory to the downstream provider.
  3. This resulted in the Beta-only service's handwritten files (like agent_registry_operation.go) being copied to the GA provider (google/services/agentregistry/).
  4. Since these handwritten files commonly reference the Product variable (which doesn't exist in GA for this service), the GA provider failed to compile.

The Solution

This PR modifies the generator to only copy and compile third-party service files for products that are actually active in the target version being generated:

  1. Updated Signatures: Updated CopyCommonFiles in the Provider interface and all its implementations to accept the list of active products []*api.Product.
  2. Filtered Copying: Modified getCommonCopyFiles in terraform.go to remove the global third_party/terraform/services copy. Instead, it now iterates through the active products and only copies third_party/terraform/services/<product_name> if it exists.
  3. Filtered Compilation: Modified getCommonCompileFiles in terraform.go similarly to only compile templates for active products.
  4. Helper Added: Introduced a dirExists helper in terraform.go to safely check if a service directory exists in the template filesystem before attempting to copy/compile it (as some services do not have handwritten files).
    This ensures that the GA provider remains completely free of any Beta-only service code, preventing compilation leaks.

Test/Verification Results

Verified locally by running:

  1. make provider VERSION=ga (GA Generation) -> Verified that the google/services/agentregistry directory is no longer created in the GA provider, resolving the compilation leak.
  2. make provider VERSION=beta (Beta Generation) -> Verified that the google-beta/services/agentregistry directory is correctly created and contains all generated and handwritten files.

Release Note Template for Downstream PRs (will be copied)

@modular-magician

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes for commit ea638db:

Diff report

Your PR generated the following diffs in downstream repositories:

Repository Diff Link Changes
google provider View Diff 60 files changed, 1450 deletions(-)

@JamesDuncanNz JamesDuncanNz force-pushed the fix/generator-filter-services branch from ea638db to 2bc9266 Compare June 16, 2026 01:37
@modular-magician

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 2bc9266:

Diff report

Your PR generated the following diffs in downstream repositories:

Repository Diff Link Changes
google provider View Diff 63 files changed, 1453 deletions(-)

@JamesDuncanNz JamesDuncanNz marked this pull request as ready for review June 16, 2026 10:59
@github-actions github-actions Bot requested a review from melinath June 16, 2026 10:59
@github-actions

Copy link
Copy Markdown

Googlers: For automatic test runs see go/terraform-auto-test-runs.

@melinath, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@melinath melinath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This isn't exactly a bug in the provider generator... but it does seem like a reasonable change that fixes a class of potentially-confusing compilation errors. The downstream diff looks good.

@roaks3 do you think this would cause any problems with nightly?

@melinath melinath requested a review from roaks3 June 16, 2026 21:44
@roaks3

roaks3 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

I can't immediately tell, is this fixing an urgent issue? If not, I would honestly probably hold off. It seems like more of a nice-to-have (if I'm understanding) that is touching a lot of core files with an LLM, and could break private providers in ways that are not obvious.

@melinath

Copy link
Copy Markdown
Member

I'm not aware of an urgent issue this is fixing. +1 that it seems like a nice to have. Holding off seems reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants