fix(verify-drift): detect resource types instead of module names - #328
Open
Shubham-Padkonde wants to merge 1 commit into
Open
Shubham-Padkonde wants to merge 1 commit into
Shubham-Padkonde wants to merge 1 commit into
Conversation
verify-drift reported the module name of each changed resource
("Resources detected: dns, zone") because DetectResourcesFromPlan reused
the e2e helper, which relies on the e2e convention that every resource
lives in a module named after its type. Customer plans use arbitrary
module names and root-module resources, so read the resource type from
the address instead, skipping module paths, instance keys and data
sources.
Fixes cloudflare#325
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Description
tf-migrate verify-driftnow reports the Cloudflare resource types in a plan (dns_record, ruleset) instead of the names of the modules that contain them (dns, zone).DetectResourcesFromPlanused to callextractAffectedResources, which takes the second segment ofmodule.<name>.…. That works for the e2e runner, where every resource sits in a module named after its type, but not for customer plans, where modules have arbitrary names and root-module resources were not detected at all.DetectResourcesFromPlannow has its own parser. It reads the resource type from each plan header's address:module.<name>[key]path, including nested modules;["example.com_@_issue_x"], which may contain dots;cloudflare_prefix so the names still match the per-resource exemption files.extractAffectedResources, and therefore the e2e runner, is unchanged.In
verify-driftthis only affects the report header.loadEmbeddedExemptionsalready loads every exemption file regardless of what's detected, so which exemptions apply doesn't change.Motivation
Fixes #325
Type of change
Testing
make test-unit)make test-integration)make lint-testdatapasses: no testdata changesDetails:
TestDetectResourcesFromPlancovers:TestVerify_DetectsResourceTypesNotModuleNameschecks the same thing throughVerify.mainwithout the fix, 4 of the 5 newTestDetectResourcesFromPlancases andTestVerify_DetectsResourceTypesNotModuleNamesfail. All pass with it.make testpasses (exit 0).go vetis clean on both packages, andgofmt -lreports nothing for the changed files.Checklist
internal/registry/registry.go(n/a)cftftestprefix (n/a)# MIGRATION WARNINGcomments are documented inDIAGNOSTICS.md(n/a)This change was written with help from an AI coding assistant (Claude Code). I reproduced the bug and reviewed and tested the change as described above.
🤖 Generated with Claude Code