From cf987dc13e6a124b8e69b6f83d714bcf1ce7a7c8 Mon Sep 17 00:00:00 2001 From: Sarah Sicard Date: Wed, 15 Jul 2026 13:55:47 -0500 Subject: [PATCH 1/3] feat: add cloudflare_worker_secret migration support Adds v4-to-v5 migration for cloudflare_workers_secret and the deprecated cloudflare_worker_secret resources. In v5, standalone worker secrets no longer exist; they are managed as secret_text bindings on the cloudflare_workers_script resource. Migration behavior: - Cross-resource merge: secrets are automatically folded into their parent workers_script bindings list when both are in the same file - Existing bindings preserved via concat() - Parent matching by resource reference or literal script_name - Orphan secrets (parent not in file) get removed block + diagnostic - PhaseOneTransformer implemented for two-phase migration (required because the v5 provider has no schema for workers_secret) Handles both cloudflare_workers_secret (plural) and cloudflare_worker_secret (singular/deprecated) identically. Ref: APIX-1259 --- .../workers_secret/expected/workers_secret.tf | 174 ++++++++ .../workers_secret/input/workers_secret.tf | 122 ++++++ internal/registry/registry.go | 2 + internal/resources/workers_script/v4_to_v5.go | 9 + internal/resources/workers_secret/README.md | 104 +++++ internal/resources/workers_secret/v4_to_v5.go | 352 ++++++++++++++++ .../resources/workers_secret/v4_to_v5_test.go | 392 ++++++++++++++++++ 7 files changed, 1155 insertions(+) create mode 100644 integration/v4_to_v5/testdata/workers_secret/expected/workers_secret.tf create mode 100644 integration/v4_to_v5/testdata/workers_secret/input/workers_secret.tf create mode 100644 internal/resources/workers_secret/README.md create mode 100644 internal/resources/workers_secret/v4_to_v5.go create mode 100644 internal/resources/workers_secret/v4_to_v5_test.go diff --git a/integration/v4_to_v5/testdata/workers_secret/expected/workers_secret.tf b/integration/v4_to_v5/testdata/workers_secret/expected/workers_secret.tf new file mode 100644 index 00000000..bbe1d4cf --- /dev/null +++ b/integration/v4_to_v5/testdata/workers_secret/expected/workers_secret.tf @@ -0,0 +1,174 @@ +variable "cloudflare_account_id" { + description = "Cloudflare account ID" + type = string +} + +variable "cloudflare_zone_id" { + description = "Cloudflare zone ID" + type = string +} + +variable "cloudflare_domain" { + description = "Cloudflare domain for testing" + type = string +} + +# ======================================== +# Pattern 1: Single secret with parent script in same file +# ======================================== + +resource "cloudflare_workers_script" "basic_worker" { + account_id = var.cloudflare_account_id + content = "addEventListener('fetch', event => { event.respondWith(new Response('Hello')); });" + script_name = "cftftest-basic-worker" + bindings = [ + { + type = "secret_text" + name = "API_KEY" + text = "my-api-key-value" + } + ] +} + + +# ======================================== +# Pattern 2: Multiple secrets for the same script +# ======================================== + +resource "cloudflare_workers_script" "multi_secret_worker" { + account_id = var.cloudflare_account_id + content = "addEventListener('fetch', event => { event.respondWith(new Response('Multi')); });" + script_name = "cftftest-multi-secret-worker" + bindings = [ + { + type = "secret_text" + name = "DB_PASSWORD" + text = "super-secret-db-password" + }, { + type = "secret_text" + name = "JWT_SECRET" + text = "jwt-signing-key" + } + ] +} + + + +# ======================================== +# Pattern 3: Deprecated singular form (cloudflare_worker_secret) +# ======================================== + + + +# ======================================== +# Pattern 4: Secret with script that has existing bindings +# ======================================== + +resource "cloudflare_workers_kv_namespace" "test_kv" { + account_id = var.cloudflare_account_id + title = "cftftest-kv-for-secret-test" +} + +resource "cloudflare_workers_script" "worker_with_bindings" { + account_id = var.cloudflare_account_id + content = "addEventListener('fetch', event => { event.respondWith(new Response('Bindings')); });" + + + script_name = "cftftest-worker-with-bindings" + bindings = concat([ + { + type = "kv_namespace" + name = "MY_KV" + namespace_id = cloudflare_workers_kv_namespace.test_kv.id + }, { + type = "plain_text" + name = "ENV" + text = "production" + } + ], [ + { + type = "secret_text" + name = "EXTRA_SECRET" + text = "extra-secret-value" + } + ]) +} + + +# ======================================== +# Pattern 5: Secret referencing script via depends_on (literal name) +# ======================================== + +resource "cloudflare_workers_script" "literal_match_worker" { + account_id = var.cloudflare_account_id + content = "addEventListener('fetch', event => { event.respondWith(new Response('Literal')); });" + script_name = "cftftest-literal-match" + bindings = [ + { + type = "secret_text" + name = "LITERAL_SECRET" + text = "literal-secret-value" + } + ] +} + + +removed { + from = cloudflare_workers_secret.basic_secret + lifecycle { + destroy = false + } +} + +removed { + from = cloudflare_workers_secret.db_password + lifecycle { + destroy = false + } +} + +removed { + from = cloudflare_workers_secret.jwt_secret + lifecycle { + destroy = false + } +} + +resource "cloudflare_workers_script" "singular_worker" { + account_id = var.cloudflare_account_id + content = "addEventListener('fetch', event => { event.respondWith(new Response('Singular')); });" + script_name = "cftftest-singular-worker" + bindings = [ + { + type = "secret_text" + name = "SINGULAR_SECRET" + text = "singular-secret-value" + } + ] +} + +moved { + from = cloudflare_worker_script.singular_worker + to = cloudflare_workers_script.singular_worker +} + +removed { + from = cloudflare_worker_secret.singular_secret + lifecycle { + destroy = false + } +} + +removed { + from = cloudflare_workers_secret.binding_secret + lifecycle { + destroy = false + } +} + +removed { + from = cloudflare_workers_secret.literal_match_secret + lifecycle { + destroy = false + } +} diff --git a/integration/v4_to_v5/testdata/workers_secret/input/workers_secret.tf b/integration/v4_to_v5/testdata/workers_secret/input/workers_secret.tf new file mode 100644 index 00000000..3196e189 --- /dev/null +++ b/integration/v4_to_v5/testdata/workers_secret/input/workers_secret.tf @@ -0,0 +1,122 @@ +variable "cloudflare_account_id" { + description = "Cloudflare account ID" + type = string +} + +variable "cloudflare_zone_id" { + description = "Cloudflare zone ID" + type = string +} + +variable "cloudflare_domain" { + description = "Cloudflare domain for testing" + type = string +} + +# ======================================== +# Pattern 1: Single secret with parent script in same file +# ======================================== + +resource "cloudflare_workers_script" "basic_worker" { + account_id = var.cloudflare_account_id + name = "cftftest-basic-worker" + content = "addEventListener('fetch', event => { event.respondWith(new Response('Hello')); });" +} + +resource "cloudflare_workers_secret" "basic_secret" { + account_id = var.cloudflare_account_id + script_name = cloudflare_workers_script.basic_worker.name + name = "API_KEY" + secret_text = "my-api-key-value" +} + +# ======================================== +# Pattern 2: Multiple secrets for the same script +# ======================================== + +resource "cloudflare_workers_script" "multi_secret_worker" { + account_id = var.cloudflare_account_id + name = "cftftest-multi-secret-worker" + content = "addEventListener('fetch', event => { event.respondWith(new Response('Multi')); });" +} + +resource "cloudflare_workers_secret" "db_password" { + account_id = var.cloudflare_account_id + script_name = cloudflare_workers_script.multi_secret_worker.name + name = "DB_PASSWORD" + secret_text = "super-secret-db-password" +} + +resource "cloudflare_workers_secret" "jwt_secret" { + account_id = var.cloudflare_account_id + script_name = cloudflare_workers_script.multi_secret_worker.name + name = "JWT_SECRET" + secret_text = "jwt-signing-key" +} + +# ======================================== +# Pattern 3: Deprecated singular form (cloudflare_worker_secret) +# ======================================== + +resource "cloudflare_worker_script" "singular_worker" { + account_id = var.cloudflare_account_id + name = "cftftest-singular-worker" + content = "addEventListener('fetch', event => { event.respondWith(new Response('Singular')); });" +} + +resource "cloudflare_worker_secret" "singular_secret" { + account_id = var.cloudflare_account_id + script_name = cloudflare_worker_script.singular_worker.name + name = "SINGULAR_SECRET" + secret_text = "singular-secret-value" +} + +# ======================================== +# Pattern 4: Secret with script that has existing bindings +# ======================================== + +resource "cloudflare_workers_kv_namespace" "test_kv" { + account_id = var.cloudflare_account_id + title = "cftftest-kv-for-secret-test" +} + +resource "cloudflare_workers_script" "worker_with_bindings" { + account_id = var.cloudflare_account_id + name = "cftftest-worker-with-bindings" + content = "addEventListener('fetch', event => { event.respondWith(new Response('Bindings')); });" + + kv_namespace_binding { + name = "MY_KV" + namespace_id = cloudflare_workers_kv_namespace.test_kv.id + } + + plain_text_binding { + name = "ENV" + text = "production" + } +} + +resource "cloudflare_workers_secret" "binding_secret" { + account_id = var.cloudflare_account_id + script_name = cloudflare_workers_script.worker_with_bindings.name + name = "EXTRA_SECRET" + secret_text = "extra-secret-value" +} + +# ======================================== +# Pattern 5: Secret referencing script via depends_on (literal name) +# ======================================== + +resource "cloudflare_workers_script" "literal_match_worker" { + account_id = var.cloudflare_account_id + name = "cftftest-literal-match" + content = "addEventListener('fetch', event => { event.respondWith(new Response('Literal')); });" +} + +resource "cloudflare_workers_secret" "literal_match_secret" { + account_id = var.cloudflare_account_id + script_name = "cftftest-literal-match" + name = "LITERAL_SECRET" + secret_text = "literal-secret-value" + depends_on = [cloudflare_workers_script.literal_match_worker] +} diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 659cedf4..a4f98ef4 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -61,6 +61,7 @@ import ( "github.com/cloudflare/tf-migrate/internal/resources/workers_custom_domain" "github.com/cloudflare/tf-migrate/internal/resources/workers_for_platforms_dispatch_namespace" "github.com/cloudflare/tf-migrate/internal/resources/workers_kv" + "github.com/cloudflare/tf-migrate/internal/resources/workers_secret" "github.com/cloudflare/tf-migrate/internal/resources/workers_kv_namespace" "github.com/cloudflare/tf-migrate/internal/resources/workers_script" "github.com/cloudflare/tf-migrate/internal/resources/zero_trust_access_application" @@ -163,6 +164,7 @@ func RegisterAllMigrations() { workers_kv.NewV4ToV5Migrator() workers_kv_namespace.NewV4ToV5Migrator() workers_script.NewV4ToV5Migrator() + workers_secret.NewV4ToV5Migrator() workers_for_platforms_dispatch_namespace.NewV4ToV5Migrator() zero_trust_access_application.NewV4ToV5Migrator() zero_trust_access_group.NewV4ToV5Migrator() diff --git a/internal/resources/workers_script/v4_to_v5.go b/internal/resources/workers_script/v4_to_v5.go index 953efacd..c14af8f1 100644 --- a/internal/resources/workers_script/v4_to_v5.go +++ b/internal/resources/workers_script/v4_to_v5.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/hcl/v2/hclwrite" "github.com/cloudflare/tf-migrate/internal" + "github.com/cloudflare/tf-migrate/internal/resources/workers_secret" "github.com/cloudflare/tf-migrate/internal/transform" tfhcl "github.com/cloudflare/tf-migrate/internal/transform/hcl" ) @@ -79,6 +80,14 @@ Use resource tags via cloudflare_workers_script_tags if needed.`, // Transform placement block → object attribute m.transformPlacement(body) + // Process cross-resource migration - merge workers_secret resources into workers_script bindings. + // This runs AFTER transformBindings so that inline binding blocks are already converted to the + // unified bindings list. The merge will then append secret bindings via concat(). + // This is idempotent - safe to call multiple times. + if ctx.CFGFile != nil { + workers_secret.ProcessCrossResourceConfigMigration(ctx.CFGFile) + } + // Generate moved block if the resource was renamed (singular → plural) if wasSingular { _, newType := m.GetResourceRename() diff --git a/internal/resources/workers_secret/README.md b/internal/resources/workers_secret/README.md new file mode 100644 index 00000000..89f8df4d --- /dev/null +++ b/internal/resources/workers_secret/README.md @@ -0,0 +1,104 @@ +# workers_secret Migration (v4 -> v5) + +## Overview + +The `cloudflare_workers_secret` (and deprecated `cloudflare_worker_secret`) +resource has been **removed** in the v5 provider. Worker secrets are now +managed as `secret_text` bindings on the `cloudflare_workers_script` resource. + +## v4 Configuration + +```hcl +resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + name = "my-worker" + content = file("worker.js") +} + +resource "cloudflare_workers_secret" "api_key" { + account_id = "abc123" + script_name = cloudflare_workers_script.my_worker.name + name = "API_KEY" + secret_text = "my-api-key" +} +``` + +## v5 Configuration (after migration) + +```hcl +resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = file("worker.js") + bindings = [ + { + type = "secret_text" + name = "API_KEY" + text = "my-api-key" + } + ] +} + +removed { + from = cloudflare_workers_secret.api_key + lifecycle { + destroy = false + } +} +``` + +## Attribute Mapping + +| v4 (`cloudflare_workers_secret`) | v5 (`cloudflare_workers_script.bindings[]`) | +|---|---| +| `name` | `name` | +| `secret_text` | `text` | +| (implicit) | `type = "secret_text"` | +| `script_name` | used to find parent script | +| `account_id` | dropped (already on parent) | + +## Migration Behavior + +### Cross-Resource Merge + +When the parent `cloudflare_workers_script` is in the same file, the migrator +automatically merges the secret into the script's `bindings` list: + +- **No existing bindings**: creates a new `bindings = [...]` attribute +- **Existing bindings**: wraps in `concat(existing, [new_secret])` to preserve + both the original bindings and the merged secret + +### Parent Matching + +The migrator matches secrets to their parent script by: + +1. **Reference matching**: parses `script_name = cloudflare_workers_script.NAME.script_name` + to extract the resource name (supports both v4 singular and v5 plural prefixes) +2. **Literal matching**: compares the literal `script_name` value against each + script's `script_name` attribute + +### Orphan Secrets + +If the parent script is not in the same file, the migrator: + +- Generates a `removed {}` block +- Emits a diagnostic warning with the binding snippet to add manually + +### Both v4 Names Supported + +Both `cloudflare_workers_secret` (preferred) and `cloudflare_worker_secret` +(deprecated singular) are handled identically. + +## Architecture + +This migrator follows the cross-resource merge pattern established by +`zero_trust_split_tunnel` (merged into device profiles): + +1. **`TransformConfig`**: generates `removed` block + diagnostic for each secret +2. **`ProcessCrossResourceConfigMigration`**: called from the `workers_script` + migrator after its own binding transformation completes; scans the file, + matches secrets to scripts, and merges them + +The cross-resource merge only processes scripts that have already been migrated +(identified by the presence of `script_name` instead of `name`). This ensures +correct ordering when the pipeline processes blocks sequentially. diff --git a/internal/resources/workers_secret/v4_to_v5.go b/internal/resources/workers_secret/v4_to_v5.go new file mode 100644 index 00000000..866c86f4 --- /dev/null +++ b/internal/resources/workers_secret/v4_to_v5.go @@ -0,0 +1,352 @@ +package workers_secret + +import ( + "fmt" + "strings" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclwrite" + + "github.com/cloudflare/tf-migrate/internal" + "github.com/cloudflare/tf-migrate/internal/transform" + tfhcl "github.com/cloudflare/tf-migrate/internal/transform/hcl" +) + +// V4ToV5Migrator handles migration of cloudflare_worker_secret and +// cloudflare_workers_secret resources from v4 to v5. +// +// In v5, standalone worker secret resources no longer exist. Secrets are +// managed as bindings on the cloudflare_workers_script resource: +// +// bindings = [ +// { +// type = "secret_text" +// name = "MY_SECRET" +// text = "secret-value" +// } +// ] +// +// The workers_script migrator calls ProcessCrossResourceConfigMigration to +// merge secret resources into their parent workers_script bindings list. +type V4ToV5Migrator struct{} + +type secretBinding struct { + block *hclwrite.Block + name string // the secret name expression + text string // the secret_text expression +} + +type parentScriptInfo struct { + block *hclwrite.Block + scriptName string // the script_name or name attribute value +} + +func NewV4ToV5Migrator() transform.ResourceTransformer { + migrator := &V4ToV5Migrator{} + internal.RegisterMigrator("cloudflare_workers_secret", "v4", "v5", migrator) + internal.RegisterMigrator("cloudflare_worker_secret", "v4", "v5", migrator) + return migrator +} + +func (m *V4ToV5Migrator) GetResourceType() string { + return "" +} + +func (m *V4ToV5Migrator) CanHandle(resourceType string) bool { + return resourceType == "cloudflare_workers_secret" || resourceType == "cloudflare_worker_secret" +} + +func (m *V4ToV5Migrator) Preprocess(content string) string { + return content +} + +// GetResourceRename implements the ResourceRenamer interface. +// workers_secret is removed in v5 (folded into workers_script bindings). +func (m *V4ToV5Migrator) GetResourceRename() ([]string, string) { + return []string{"cloudflare_workers_secret", "cloudflare_worker_secret"}, "" +} + +// TransformPhaseOne implements the PhaseOneTransformer interface. +// Because cloudflare_workers_secret does not exist in the v5 provider, Terraform +// cannot read existing state entries after the provider upgrade. Phase 1 appends +// a removed {} block while the v4 provider is still active so Terraform can drop +// the state entry without destroying infrastructure. The caller comments out the +// original resource block. +func (m *V4ToV5Migrator) TransformPhaseOne(ctx *transform.Context, block *hclwrite.Block) (*transform.TransformResult, error) { + if len(block.Labels()) < 2 { + return nil, fmt.Errorf("invalid resource block: expected 2 labels, got %d", len(block.Labels())) + } + resourceType := block.Labels()[0] + resourceName := block.Labels()[1] + + removedAddr := resourceType + "." + resourceName + removedBlock := tfhcl.CreateRemovedBlock(removedAddr) + return &transform.TransformResult{ + Blocks: []*hclwrite.Block{removedBlock}, + RemoveOriginal: false, + }, nil +} + +func (m *V4ToV5Migrator) TransformConfig(ctx *transform.Context, block *hclwrite.Block) (*transform.TransformResult, error) { + originalResourceType := tfhcl.GetResourceType(block) + resourceName := tfhcl.GetResourceName(block) + + // Generate a removed block for the secret resource + from := originalResourceType + "." + resourceName + removedBlock := tfhcl.CreateRemovedBlock(from) + + // Build the binding snippet for the diagnostic message + body := block.Body() + bindingSnippet := buildBindingSnippet(body) + + scriptRef := extractScriptReference(body) + if scriptRef == "" { + scriptRef = "(unknown)" + } + + ctx.Diagnostics = append(ctx.Diagnostics, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf("Resource removed: %s.%s", originalResourceType, resourceName), + Detail: fmt.Sprintf(`The %s resource has been removed in v5. Secrets are now managed as +bindings on the cloudflare_workers_script resource (script_name = %s). + +If the parent cloudflare_workers_script is in the same file, the secret has +been automatically merged into its bindings list. Otherwise, add the +following binding to the parent resource manually: + +%s + +After applying, run 'terraform state rm %s' to remove the old state entry.`, + originalResourceType, scriptRef, bindingSnippet, from), + }) + + return &transform.TransformResult{ + Blocks: []*hclwrite.Block{removedBlock}, + RemoveOriginal: true, + }, nil +} + +// ProcessCrossResourceConfigMigration merges workers_secret resources into +// their parent cloudflare_workers_script resources' bindings lists. +// +// Called from the workers_script migrator after its own transformation is +// complete, following the same pattern as zero_trust_split_tunnel. +func ProcessCrossResourceConfigMigration(file *hclwrite.File) { + body := file.Body() + + // Collect workers_script resources and workers_secret resources + scriptResources := make(map[string]*parentScriptInfo) // keyed by resource name + var secretBlocks []*hclwrite.Block + + for _, block := range body.Blocks() { + if block.Type() != "resource" || len(block.Labels()) < 2 { + continue + } + resourceType := block.Labels()[0] + resourceName := block.Labels()[1] + + switch resourceType { + case "cloudflare_workers_script", "cloudflare_worker_script": + // In v5 the attribute is script_name; in v4 it's name. + // After workers_script migration runs, it will be script_name. + sn := extractAttrString(block.Body(), "script_name") + if sn == "" { + sn = extractAttrString(block.Body(), "name") + } + scriptResources[resourceName] = &parentScriptInfo{ + block: block, + scriptName: sn, + } + case "cloudflare_workers_secret", "cloudflare_worker_secret": + secretBlocks = append(secretBlocks, block) + } + } + + if len(secretBlocks) == 0 { + return + } + + // Group secrets by their parent script resource name + secretsByScript := make(map[string][]secretBinding) // keyed by script resource name + var orphanSecrets []*hclwrite.Block + + for _, secretBlock := range secretBlocks { + secretBody := secretBlock.Body() + parentScript := findParentScriptResource(secretBody, scriptResources) + + nameVal := extractAttrExpr(secretBody, "name") + textVal := extractAttrExpr(secretBody, "secret_text") + + if parentScript != "" { + secretsByScript[parentScript] = append(secretsByScript[parentScript], secretBinding{ + block: secretBlock, + name: nameVal, + text: textVal, + }) + } else { + orphanSecrets = append(orphanSecrets, secretBlock) + } + } + + // Merge secrets into parent script bindings. + // Only merge into scripts that have already been migrated (have script_name + // attribute). Scripts still using the v4 "name" attribute haven't had their + // transformBindings run yet, so merging now would be overwritten. Those + // scripts will trigger another call to this function after their own + // transformation completes. + for scriptName, secrets := range secretsByScript { + info := scriptResources[scriptName] + if info == nil { + continue + } + + // Check if the script has already been migrated by looking for script_name. + // If it still has "name" (v4), skip -- it will be handled on a later call. + if info.block.Body().GetAttribute("script_name") == nil { + continue + } + + mergeSecretsIntoScript(info.block, secrets) + + // Remove the secret resource blocks + for _, s := range secrets { + body.RemoveBlock(s.block) + } + } + + // For orphan secrets (parent not in same file), just remove the block. + // The TransformConfig already emitted a removed block and diagnostic. + for _, block := range orphanSecrets { + body.RemoveBlock(block) + } +} + +// mergeSecretsIntoScript adds secret_text bindings to a workers_script resource. +func mergeSecretsIntoScript(scriptBlock *hclwrite.Block, secrets []secretBinding) { + scriptBody := scriptBlock.Body() + + // Build binding objects for each secret + var bindingObjects []string + for _, s := range secrets { + obj := buildBindingObject(s.name, s.text) + bindingObjects = append(bindingObjects, obj) + } + + // Check if the script already has a bindings attribute + existingBindings := scriptBody.GetAttribute("bindings") + if existingBindings != nil { + // Append to existing bindings using concat() + existingExpr := strings.TrimSpace(string(existingBindings.Expr().BuildTokens(nil).Bytes())) + newBindings := "[\n " + strings.Join(bindingObjects, ", ") + "\n]" + + var concatExpr string + if strings.HasPrefix(existingExpr, "concat(") { + // Already a concat expression - add our bindings as another argument + concatExpr = existingExpr[:len(existingExpr)-1] + ", " + newBindings + ")" + } else { + concatExpr = "concat(" + existingExpr + ", " + newBindings + ")" + } + tfhcl.SetAttributeFromExpressionString(scriptBody, "bindings", concatExpr) + } else { + // No existing bindings - create new list + bindingsValue := "[\n " + strings.Join(bindingObjects, ", ") + "\n]" + tfhcl.SetAttributeFromExpressionString(scriptBody, "bindings", bindingsValue) + } +} + +// buildBindingObject creates a v5 binding object string for a secret. +func buildBindingObject(name, text string) string { + var attrs []string + attrs = append(attrs, `type = "secret_text"`) + if name != "" { + attrs = append(attrs, "name = "+name) + } + if text != "" { + attrs = append(attrs, "text = "+text) + } + return "{\n " + strings.Join(attrs, "\n ") + "\n }" +} + +// buildBindingSnippet creates a human-readable binding snippet for diagnostics. +func buildBindingSnippet(body *hclwrite.Body) string { + name := extractAttrExpr(body, "name") + text := extractAttrExpr(body, "secret_text") + + var lines []string + lines = append(lines, " {") + lines = append(lines, ` type = "secret_text"`) + if name != "" { + lines = append(lines, " name = "+name) + } + if text != "" { + lines = append(lines, " text = "+text) + } + lines = append(lines, " }") + return strings.Join(lines, "\n") +} + +// findParentScriptResource finds the parent workers_script resource name +// by matching the script_name attribute of the secret to a script resource. +func findParentScriptResource(secretBody *hclwrite.Body, scripts map[string]*parentScriptInfo) string { + scriptNameAttr := secretBody.GetAttribute("script_name") + if scriptNameAttr == nil { + return "" + } + + scriptNameExpr := strings.TrimSpace(string(scriptNameAttr.Expr().BuildTokens(nil).Bytes())) + + // Check for direct reference: cloudflare_workers_script.NAME.script_name + // or cloudflare_workers_script.NAME.name (v4 form) + // or cloudflare_worker_script.NAME.name (v4 singular form) + for _, prefix := range []string{ + "cloudflare_workers_script.", + "cloudflare_worker_script.", + } { + if strings.HasPrefix(scriptNameExpr, prefix) { + rest := scriptNameExpr[len(prefix):] + // Extract resource name (before the next dot) + parts := strings.SplitN(rest, ".", 2) + if len(parts) >= 1 { + resourceName := strings.TrimSpace(parts[0]) + if _, ok := scripts[resourceName]; ok { + return resourceName + } + } + } + } + + // Check for literal script_name match + scriptNameLiteral := extractAttrString(secretBody, "script_name") + if scriptNameLiteral != "" { + for name, info := range scripts { + if info.scriptName == scriptNameLiteral { + return name + } + } + } + + return "" +} + +// extractScriptReference returns the script_name expression as a string for diagnostics. +func extractScriptReference(body *hclwrite.Body) string { + attr := body.GetAttribute("script_name") + if attr == nil { + return "" + } + return strings.TrimSpace(string(attr.Expr().BuildTokens(nil).Bytes())) +} + +// extractAttrExpr returns the raw expression string for an attribute. +func extractAttrExpr(body *hclwrite.Body, name string) string { + attr := body.GetAttribute(name) + if attr == nil { + return "" + } + return strings.TrimSpace(string(attr.Expr().BuildTokens(nil).Bytes())) +} + +// extractAttrString returns the unquoted string value for an attribute, or empty if not a literal. +func extractAttrString(body *hclwrite.Body, name string) string { + return tfhcl.ExtractStringFromAttribute(body.GetAttribute(name)) +} diff --git a/internal/resources/workers_secret/v4_to_v5_test.go b/internal/resources/workers_secret/v4_to_v5_test.go new file mode 100644 index 00000000..1483f4d9 --- /dev/null +++ b/internal/resources/workers_secret/v4_to_v5_test.go @@ -0,0 +1,392 @@ +package workers_secret + +import ( + "strings" + "testing" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclwrite" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/cloudflare/tf-migrate/internal/testhelpers" + "github.com/cloudflare/tf-migrate/internal/transform" +) + +func TestV4ToV5Transformation(t *testing.T) { + t.Run("ConfigTransformation", func(t *testing.T) { + t.Run("WorkersSecretRemoved", testWorkersSecretRemoved) + t.Run("WorkerSecretSingularRemoved", testWorkerSecretSingularRemoved) + }) + + t.Run("PhaseOne", func(t *testing.T) { + t.Run("WorkersSecretPhaseOne", testWorkersSecretPhaseOne) + t.Run("WorkerSecretSingularPhaseOne", testWorkerSecretSingularPhaseOne) + }) + + t.Run("CrossResourceMigration", func(t *testing.T) { + t.Run("SingleSecretMergedIntoScript", testSingleSecretMergedIntoScript) + t.Run("MultipleSecretsMergedIntoScript", testMultipleSecretsMergedIntoScript) + t.Run("SecretMergedIntoScriptWithExistingBindings", testSecretMergedIntoScriptWithExistingBindings) + t.Run("OrphanSecretRemovedWhenNoParent", testOrphanSecretRemovedWhenNoParent) + t.Run("SecretMatchedByLiteralScriptName", testSecretMatchedByLiteralScriptName) + t.Run("SingularWorkerSecretMergedIntoScript", testSingularWorkerSecretMergedIntoScript) + t.Run("SecretWithReferenceToSingularWorkerScript", testSecretWithReferenceToSingularWorkerScript) + }) +} + +func testWorkersSecretRemoved(t *testing.T) { + migrator := NewV4ToV5Migrator() + + tests := []testhelpers.ConfigTestCase{ + { + Name: "workers_secret should produce removed block", + Input: `resource "cloudflare_workers_secret" "my_secret" { + account_id = "abc123" + script_name = "my-worker" + name = "MY_SECRET" + secret_text = "super-secret" +}`, + Expected: `removed { + from = cloudflare_workers_secret.my_secret + + lifecycle { + destroy = false + } +}`, + }, + } + + testhelpers.RunConfigTransformTests(t, tests, migrator) +} + +func testWorkerSecretSingularRemoved(t *testing.T) { + migrator := NewV4ToV5Migrator() + + tests := []testhelpers.ConfigTestCase{ + { + Name: "worker_secret (singular) should produce removed block", + Input: `resource "cloudflare_worker_secret" "my_secret" { + account_id = "abc123" + script_name = "my-worker" + name = "MY_SECRET" + secret_text = "super-secret" +}`, + Expected: `removed { + from = cloudflare_worker_secret.my_secret + + lifecycle { + destroy = false + } +}`, + }, + } + + testhelpers.RunConfigTransformTests(t, tests, migrator) +} + +// Phase-one tests + +func testWorkersSecretPhaseOne(t *testing.T) { + migrator := NewV4ToV5Migrator() + p1, ok := migrator.(transform.PhaseOneTransformer) + require.True(t, ok, "migrator must implement PhaseOneTransformer") + + input := `resource "cloudflare_workers_secret" "my_secret" { + account_id = "abc123" + script_name = "my-worker" + name = "MY_SECRET" + secret_text = "super-secret" +}` + file, diags := hclwrite.ParseConfig([]byte(input), "test.tf", hcl.InitialPos) + require.False(t, diags.HasErrors()) + + block := file.Body().Blocks()[0] + ctx := &transform.Context{Filename: "test.tf", CFGFile: file} + result, err := p1.TransformPhaseOne(ctx, block) + require.NoError(t, err) + assert.False(t, result.RemoveOriginal, "PhaseOne must not remove the original block") + require.Len(t, result.Blocks, 1) + assert.Equal(t, "removed", result.Blocks[0].Type()) +} + +func testWorkerSecretSingularPhaseOne(t *testing.T) { + migrator := NewV4ToV5Migrator() + p1, ok := migrator.(transform.PhaseOneTransformer) + require.True(t, ok, "migrator must implement PhaseOneTransformer") + + input := `resource "cloudflare_worker_secret" "my_secret" { + account_id = "abc123" + script_name = "my-worker" + name = "MY_SECRET" + secret_text = "super-secret" +}` + file, diags := hclwrite.ParseConfig([]byte(input), "test.tf", hcl.InitialPos) + require.False(t, diags.HasErrors()) + + block := file.Body().Blocks()[0] + ctx := &transform.Context{Filename: "test.tf", CFGFile: file} + result, err := p1.TransformPhaseOne(ctx, block) + require.NoError(t, err) + assert.False(t, result.RemoveOriginal) + require.Len(t, result.Blocks, 1) + + // Verify the removed block uses the original (singular) resource type + removedBody := result.Blocks[0].Body() + fromAttr := removedBody.GetAttribute("from") + require.NotNil(t, fromAttr) + fromExpr := strings.TrimSpace(string(fromAttr.Expr().BuildTokens(nil).Bytes())) + assert.Contains(t, fromExpr, "cloudflare_worker_secret.my_secret") +} + +// Cross-resource migration tests + +func testSingleSecretMergedIntoScript(t *testing.T) { + input := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" +} + +resource "cloudflare_workers_secret" "my_secret" { + account_id = "abc123" + script_name = cloudflare_workers_script.my_worker.script_name + name = "MY_SECRET" + secret_text = "super-secret" +}` + + expected := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" + bindings = [ + { + type = "secret_text" + name = "MY_SECRET" + text = "super-secret" + } +] +}` + + runCrossResourceTest(t, input, expected) +} + +func testMultipleSecretsMergedIntoScript(t *testing.T) { + input := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" +} + +resource "cloudflare_workers_secret" "secret_one" { + account_id = "abc123" + script_name = cloudflare_workers_script.my_worker.script_name + name = "SECRET_ONE" + secret_text = "first-secret" +} + +resource "cloudflare_workers_secret" "secret_two" { + account_id = "abc123" + script_name = cloudflare_workers_script.my_worker.script_name + name = "SECRET_TWO" + secret_text = "second-secret" +}` + + expected := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" + bindings = [ + { + type = "secret_text" + name = "SECRET_ONE" + text = "first-secret" + }, { + type = "secret_text" + name = "SECRET_TWO" + text = "second-secret" + } +] +}` + + runCrossResourceTest(t, input, expected) +} + +func testSecretMergedIntoScriptWithExistingBindings(t *testing.T) { + input := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" + bindings = [ + { + type = "kv_namespace" + name = "MY_KV" + namespace_id = "kv-id-123" + } + ] +} + +resource "cloudflare_workers_secret" "my_secret" { + account_id = "abc123" + script_name = cloudflare_workers_script.my_worker.script_name + name = "MY_SECRET" + secret_text = "super-secret" +}` + + expected := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" + bindings = concat([ + { + type = "kv_namespace" + name = "MY_KV" + namespace_id = "kv-id-123" + } + ], [ + { + type = "secret_text" + name = "MY_SECRET" + text = "super-secret" + } +]) +}` + + runCrossResourceTest(t, input, expected) +} + +func testOrphanSecretRemovedWhenNoParent(t *testing.T) { + input := `resource "cloudflare_workers_secret" "orphan_secret" { + account_id = "abc123" + script_name = "some-other-worker" + name = "ORPHAN_SECRET" + secret_text = "orphan-value" +}` + + // Orphan secret should be removed (no parent script in file) + expected := `` + + runCrossResourceTest(t, input, expected) +} + +func testSecretMatchedByLiteralScriptName(t *testing.T) { + input := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" +} + +resource "cloudflare_workers_secret" "my_secret" { + account_id = "abc123" + script_name = "my-worker" + name = "MY_SECRET" + secret_text = "super-secret" +}` + + expected := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" + bindings = [ + { + type = "secret_text" + name = "MY_SECRET" + text = "super-secret" + } +] +}` + + runCrossResourceTest(t, input, expected) +} + +func testSingularWorkerSecretMergedIntoScript(t *testing.T) { + input := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" +} + +resource "cloudflare_worker_secret" "my_secret" { + account_id = "abc123" + script_name = cloudflare_workers_script.my_worker.script_name + name = "MY_SECRET" + secret_text = "super-secret" +}` + + expected := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" + bindings = [ + { + type = "secret_text" + name = "MY_SECRET" + text = "super-secret" + } +] +}` + + runCrossResourceTest(t, input, expected) +} + +func testSecretWithReferenceToSingularWorkerScript(t *testing.T) { + // After the workers_script migrator runs, cloudflare_worker_script is renamed + // to cloudflare_workers_script and "name" becomes "script_name". + // ProcessCrossResourceConfigMigration only merges into already-migrated scripts. + input := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" +} + +resource "cloudflare_workers_secret" "my_secret" { + account_id = "abc123" + script_name = cloudflare_worker_script.my_worker.name + name = "MY_SECRET" + secret_text = "super-secret" +}` + + // The secret references cloudflare_worker_script (v4 name) but the script + // has already been renamed to cloudflare_workers_script. The cross-resource + // merge should still match because it checks both prefixes. + expected := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" + bindings = [ + { + type = "secret_text" + name = "MY_SECRET" + text = "super-secret" + } +] +}` + + runCrossResourceTest(t, input, expected) +} + +// runCrossResourceTest parses input HCL, runs ProcessCrossResourceConfigMigration, +// and compares the output to expected. +func runCrossResourceTest(t *testing.T, input, expected string) { + t.Helper() + + file, diags := hclwrite.ParseConfig([]byte(input), "test.tf", hcl.InitialPos) + require.False(t, diags.HasErrors(), "Failed to parse input HCL: %v", diags) + + ProcessCrossResourceConfigMigration(file) + + output := string(hclwrite.Format(file.Bytes())) + output = strings.TrimSpace(output) + + if expected == "" { + assert.Empty(t, output, "Expected empty output but got:\n%s", output) + return + } + + expectedFile, diags := hclwrite.ParseConfig([]byte(expected), "expected.tf", hcl.InitialPos) + require.False(t, diags.HasErrors(), "Failed to parse expected HCL: %v", diags) + expectedOutput := string(hclwrite.Format(expectedFile.Bytes())) + expectedOutput = strings.TrimSpace(expectedOutput) + + assert.Equal(t, expectedOutput, output) +} From ea65ea49d942c9d4c14f595791aba4cbad7644bc Mon Sep 17 00:00:00 2001 From: Sarah Sicard Date: Wed, 15 Jul 2026 15:26:11 -0500 Subject: [PATCH 2/3] fix(e2e): add workers_secret to obsolete state cleanup The v5 provider has no schema for cloudflare_workers_secret or cloudflare_worker_secret. The E2E runner must remove these state entries before running terraform init/plan with the v5 provider, matching the existing pattern for zone_settings_override and split_tunnel. --- internal/e2e-runner/runner.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/e2e-runner/runner.go b/internal/e2e-runner/runner.go index 722e2d28..bf902aa6 100644 --- a/internal/e2e-runner/runner.go +++ b/internal/e2e-runner/runner.go @@ -319,6 +319,8 @@ func RunE2ETests(cfg *RunConfig) error { "cloudflare_access_policy": true, // Application-scoped policies with application_id cannot be migrated; removed{} blocks handle state cleanup "cloudflare_split_tunnel": true, // Dissolved into device profile exclude/include attributes in v5 "cloudflare_zero_trust_split_tunnel": true, // Newer v4 name for split_tunnel — also dissolved in v5 + "cloudflare_workers_secret": true, // Folded into workers_script bindings in v5 + "cloudflare_worker_secret": true, // Deprecated singular form — also folded into workers_script bindings } stateFilePath := filepath.Join(v5Dir, "terraform.tfstate") if removed, err := removeObsoleteStateEntries(stateFilePath, obsoleteTypes); err != nil { From e0e6af7d309ef1e1855c0b9278f2911880203974 Mon Sep 17 00:00:00 2001 From: Sarah Sicard Date: Wed, 15 Jul 2026 17:53:44 -0500 Subject: [PATCH 3/3] fix(workers_secret): drift exemptions, diagnostic message, and test coverage - Add drift exemptions targeting cloudflare_workers_script (not cloudflare_workers_secret) since secrets are folded into script bindings - Suppress resource_type mismatch warning when resource_name_patterns provides intentional cross-resource scoping - Fix diagnostic: replace incorrect 'terraform state rm' instruction with note about removed block handling state cleanup - Add redeployment behavior note to diagnostic warning - Harden concat() append with suffix validation - Add idempotency test for ProcessCrossResourceConfigMigration - Add diagnostic content test - Fix import ordering in registry.go - Remove README.md (docs belong in provider migration guides) --- e2e/drift-exemptions/workers_secret.yaml | 40 +++++++ internal/e2e-runner/drift.go | 8 +- internal/registry/registry.go | 4 +- internal/resources/workers_secret/README.md | 104 ------------------ internal/resources/workers_secret/v4_to_v5.go | 15 ++- .../resources/workers_secret/v4_to_v5_test.go | 61 ++++++++++ .../drift-exemptions/workers_secret.yaml | 40 +++++++ 7 files changed, 160 insertions(+), 112 deletions(-) create mode 100644 e2e/drift-exemptions/workers_secret.yaml delete mode 100644 internal/resources/workers_secret/README.md create mode 100644 internal/verifydrift/exemptions/drift-exemptions/workers_secret.yaml diff --git a/e2e/drift-exemptions/workers_secret.yaml b/e2e/drift-exemptions/workers_secret.yaml new file mode 100644 index 00000000..ffb13d30 --- /dev/null +++ b/e2e/drift-exemptions/workers_secret.yaml @@ -0,0 +1,40 @@ +# Drift Exemptions for workers_secret resource +# +# v4 cloudflare_workers_secret / cloudflare_worker_secret resources are folded +# into cloudflare_workers_script bindings in v5. The secrets were managed via +# the Workers Secrets API in v4 but become secret_text bindings on the parent +# script in v5. This causes expected drift on the first plan: +# +# - New secret_text bindings appear as additions to the bindings list +# - Existing bindings may reorder (concat merges secret bindings at the end) +# +# Stabilizes after first apply. + +version: 1 + +exemptions: + - name: "workers_secret_folded_into_bindings" + description: "Worker secrets are folded into workers_script bindings in v5. The first plan will show binding additions and reordering. This is expected and resolves after terraform apply." + resource_types: + - "cloudflare_workers_script" + resource_name_patterns: + - "module.workers_secret.*" + patterns: + - 'type.*=.*"secret_text"' + - 'text.*=.*\(sensitive value\)' + - 'name.*=.*"API_KEY"' + - 'name.*=.*"DB_PASSWORD"' + - 'name.*=.*"JWT_SECRET"' + - 'name.*=.*"SINGULAR_SECRET"' + - 'name.*=.*"LITERAL_SECRET"' + - 'name.*=.*"EXTRA_SECRET"' + - 'name.*=.*"MY_KV"' + - 'name.*=.*"ENV"' + - 'namespace_id' + - 'type.*=.*"kv_namespace"' + - 'type.*=.*"plain_text"' + enabled: true + +settings: + apply_exemptions: true + verbose_exemptions: false diff --git a/internal/e2e-runner/drift.go b/internal/e2e-runner/drift.go index 36089bdf..7bc3c523 100644 --- a/internal/e2e-runner/drift.go +++ b/internal/e2e-runner/drift.go @@ -154,9 +154,13 @@ func loadResourceExemptions(repoRoot string, resource string) (*DriftExemptionsC // Validate resource_type if specified if config.Version >= 1 { expectedType := "cloudflare_" + resource - // Check if any exemption has a different resource type restriction + // Check if any exemption has a different resource type restriction. + // Skip the warning when the exemption also has resource_name_patterns, + // because that means the author is intentionally targeting a different + // resource type (e.g. workers_secret.yaml targets cloudflare_workers_script + // since secrets are folded into script bindings in v5). for _, e := range config.Exemptions { - if len(e.ResourceTypes) > 0 { + if len(e.ResourceTypes) > 0 && len(e.ResourceNamePatterns) == 0 { found := false for _, rt := range e.ResourceTypes { if rt == expectedType { diff --git a/internal/registry/registry.go b/internal/registry/registry.go index a4f98ef4..8a800613 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -61,9 +61,9 @@ import ( "github.com/cloudflare/tf-migrate/internal/resources/workers_custom_domain" "github.com/cloudflare/tf-migrate/internal/resources/workers_for_platforms_dispatch_namespace" "github.com/cloudflare/tf-migrate/internal/resources/workers_kv" - "github.com/cloudflare/tf-migrate/internal/resources/workers_secret" "github.com/cloudflare/tf-migrate/internal/resources/workers_kv_namespace" "github.com/cloudflare/tf-migrate/internal/resources/workers_script" + "github.com/cloudflare/tf-migrate/internal/resources/workers_secret" "github.com/cloudflare/tf-migrate/internal/resources/zero_trust_access_application" "github.com/cloudflare/tf-migrate/internal/resources/zero_trust_access_group" "github.com/cloudflare/tf-migrate/internal/resources/zero_trust_access_identity_provider" @@ -164,8 +164,8 @@ func RegisterAllMigrations() { workers_kv.NewV4ToV5Migrator() workers_kv_namespace.NewV4ToV5Migrator() workers_script.NewV4ToV5Migrator() - workers_secret.NewV4ToV5Migrator() workers_for_platforms_dispatch_namespace.NewV4ToV5Migrator() + workers_secret.NewV4ToV5Migrator() zero_trust_access_application.NewV4ToV5Migrator() zero_trust_access_group.NewV4ToV5Migrator() zero_trust_access_identity_provider.NewV4ToV5Migrator() diff --git a/internal/resources/workers_secret/README.md b/internal/resources/workers_secret/README.md deleted file mode 100644 index 89f8df4d..00000000 --- a/internal/resources/workers_secret/README.md +++ /dev/null @@ -1,104 +0,0 @@ -# workers_secret Migration (v4 -> v5) - -## Overview - -The `cloudflare_workers_secret` (and deprecated `cloudflare_worker_secret`) -resource has been **removed** in the v5 provider. Worker secrets are now -managed as `secret_text` bindings on the `cloudflare_workers_script` resource. - -## v4 Configuration - -```hcl -resource "cloudflare_workers_script" "my_worker" { - account_id = "abc123" - name = "my-worker" - content = file("worker.js") -} - -resource "cloudflare_workers_secret" "api_key" { - account_id = "abc123" - script_name = cloudflare_workers_script.my_worker.name - name = "API_KEY" - secret_text = "my-api-key" -} -``` - -## v5 Configuration (after migration) - -```hcl -resource "cloudflare_workers_script" "my_worker" { - account_id = "abc123" - script_name = "my-worker" - content = file("worker.js") - bindings = [ - { - type = "secret_text" - name = "API_KEY" - text = "my-api-key" - } - ] -} - -removed { - from = cloudflare_workers_secret.api_key - lifecycle { - destroy = false - } -} -``` - -## Attribute Mapping - -| v4 (`cloudflare_workers_secret`) | v5 (`cloudflare_workers_script.bindings[]`) | -|---|---| -| `name` | `name` | -| `secret_text` | `text` | -| (implicit) | `type = "secret_text"` | -| `script_name` | used to find parent script | -| `account_id` | dropped (already on parent) | - -## Migration Behavior - -### Cross-Resource Merge - -When the parent `cloudflare_workers_script` is in the same file, the migrator -automatically merges the secret into the script's `bindings` list: - -- **No existing bindings**: creates a new `bindings = [...]` attribute -- **Existing bindings**: wraps in `concat(existing, [new_secret])` to preserve - both the original bindings and the merged secret - -### Parent Matching - -The migrator matches secrets to their parent script by: - -1. **Reference matching**: parses `script_name = cloudflare_workers_script.NAME.script_name` - to extract the resource name (supports both v4 singular and v5 plural prefixes) -2. **Literal matching**: compares the literal `script_name` value against each - script's `script_name` attribute - -### Orphan Secrets - -If the parent script is not in the same file, the migrator: - -- Generates a `removed {}` block -- Emits a diagnostic warning with the binding snippet to add manually - -### Both v4 Names Supported - -Both `cloudflare_workers_secret` (preferred) and `cloudflare_worker_secret` -(deprecated singular) are handled identically. - -## Architecture - -This migrator follows the cross-resource merge pattern established by -`zero_trust_split_tunnel` (merged into device profiles): - -1. **`TransformConfig`**: generates `removed` block + diagnostic for each secret -2. **`ProcessCrossResourceConfigMigration`**: called from the `workers_script` - migrator after its own binding transformation completes; scans the file, - matches secrets to scripts, and merges them - -The cross-resource merge only processes scripts that have already been migrated -(identified by the presence of `script_name` instead of `name`). This ensures -correct ordering when the pipeline processes blocks sequentially. diff --git a/internal/resources/workers_secret/v4_to_v5.go b/internal/resources/workers_secret/v4_to_v5.go index 866c86f4..c41a06ce 100644 --- a/internal/resources/workers_secret/v4_to_v5.go +++ b/internal/resources/workers_secret/v4_to_v5.go @@ -48,6 +48,8 @@ func NewV4ToV5Migrator() transform.ResourceTransformer { return migrator } +// GetResourceType returns empty string because workers_secret is removed in v5 +// (folded into workers_script bindings), so there is no v5 resource type. func (m *V4ToV5Migrator) GetResourceType() string { return "" } @@ -116,8 +118,13 @@ following binding to the parent resource manually: %s -After applying, run 'terraform state rm %s' to remove the old state entry.`, - originalResourceType, scriptRef, bindingSnippet, from), +A 'removed' block has been generated to clean up the state entry during the next apply. + +Note: In v4, secrets were managed via a separate API and could be updated +without redeploying the Worker script. In v5, secret_text bindings are part +of the script resource, so future secret changes will trigger a script +redeployment.`, + originalResourceType, scriptRef, bindingSnippet), }) return &transform.TransformResult{ @@ -240,9 +247,9 @@ func mergeSecretsIntoScript(scriptBlock *hclwrite.Block, secrets []secretBinding newBindings := "[\n " + strings.Join(bindingObjects, ", ") + "\n]" var concatExpr string - if strings.HasPrefix(existingExpr, "concat(") { + if strings.HasPrefix(existingExpr, "concat(") && strings.HasSuffix(existingExpr, ")") { // Already a concat expression - add our bindings as another argument - concatExpr = existingExpr[:len(existingExpr)-1] + ", " + newBindings + ")" + concatExpr = strings.TrimSuffix(existingExpr, ")") + ", " + newBindings + ")" } else { concatExpr = "concat(" + existingExpr + ", " + newBindings + ")" } diff --git a/internal/resources/workers_secret/v4_to_v5_test.go b/internal/resources/workers_secret/v4_to_v5_test.go index 1483f4d9..4fccb11c 100644 --- a/internal/resources/workers_secret/v4_to_v5_test.go +++ b/internal/resources/workers_secret/v4_to_v5_test.go @@ -32,6 +32,11 @@ func TestV4ToV5Transformation(t *testing.T) { t.Run("SecretMatchedByLiteralScriptName", testSecretMatchedByLiteralScriptName) t.Run("SingularWorkerSecretMergedIntoScript", testSingularWorkerSecretMergedIntoScript) t.Run("SecretWithReferenceToSingularWorkerScript", testSecretWithReferenceToSingularWorkerScript) + t.Run("IdempotentMigration", testCrossResourceMigrationIsIdempotent) + }) + + t.Run("Diagnostics", func(t *testing.T) { + t.Run("DiagnosticContent", testDiagnosticContent) }) } @@ -365,6 +370,62 @@ resource "cloudflare_workers_secret" "my_secret" { runCrossResourceTest(t, input, expected) } +func testCrossResourceMigrationIsIdempotent(t *testing.T) { + input := `resource "cloudflare_workers_script" "my_worker" { + account_id = "abc123" + script_name = "my-worker" + content = "addEventListener('fetch', event => {});" +} + +resource "cloudflare_workers_secret" "my_secret" { + account_id = "abc123" + script_name = cloudflare_workers_script.my_worker.script_name + name = "MY_SECRET" + secret_text = "super-secret" +}` + + file, diags := hclwrite.ParseConfig([]byte(input), "test.tf", hcl.InitialPos) + require.False(t, diags.HasErrors()) + + ProcessCrossResourceConfigMigration(file) + result1 := strings.TrimSpace(string(hclwrite.Format(file.Bytes()))) + + // Run again on the output — should be identical + file2, diags := hclwrite.ParseConfig([]byte(result1), "test.tf", hcl.InitialPos) + require.False(t, diags.HasErrors()) + ProcessCrossResourceConfigMigration(file2) + result2 := strings.TrimSpace(string(hclwrite.Format(file2.Bytes()))) + + assert.Equal(t, result1, result2, "Expected idempotent migration") +} + +func testDiagnosticContent(t *testing.T) { + migrator := NewV4ToV5Migrator() + + input := `resource "cloudflare_workers_secret" "my_secret" { + account_id = "abc123" + script_name = cloudflare_workers_script.my_worker.name + name = "MY_SECRET" + secret_text = "super-secret" +}` + file, diags := hclwrite.ParseConfig([]byte(input), "test.tf", hcl.InitialPos) + require.False(t, diags.HasErrors()) + + block := file.Body().Blocks()[0] + ctx := &transform.Context{Filename: "test.tf", CFGFile: file} + _, err := migrator.(*V4ToV5Migrator).TransformConfig(ctx, block) + require.NoError(t, err) + + require.Len(t, ctx.Diagnostics, 1) + diag := ctx.Diagnostics[0] + assert.Equal(t, hcl.DiagWarning, diag.Severity) + assert.Contains(t, diag.Summary, "Resource removed") + assert.Contains(t, diag.Summary, "cloudflare_workers_secret.my_secret") + assert.Contains(t, diag.Detail, "secret_text") + assert.Contains(t, diag.Detail, "removed") + assert.NotContains(t, diag.Detail, "terraform state rm") +} + // runCrossResourceTest parses input HCL, runs ProcessCrossResourceConfigMigration, // and compares the output to expected. func runCrossResourceTest(t *testing.T, input, expected string) { diff --git a/internal/verifydrift/exemptions/drift-exemptions/workers_secret.yaml b/internal/verifydrift/exemptions/drift-exemptions/workers_secret.yaml new file mode 100644 index 00000000..ffb13d30 --- /dev/null +++ b/internal/verifydrift/exemptions/drift-exemptions/workers_secret.yaml @@ -0,0 +1,40 @@ +# Drift Exemptions for workers_secret resource +# +# v4 cloudflare_workers_secret / cloudflare_worker_secret resources are folded +# into cloudflare_workers_script bindings in v5. The secrets were managed via +# the Workers Secrets API in v4 but become secret_text bindings on the parent +# script in v5. This causes expected drift on the first plan: +# +# - New secret_text bindings appear as additions to the bindings list +# - Existing bindings may reorder (concat merges secret bindings at the end) +# +# Stabilizes after first apply. + +version: 1 + +exemptions: + - name: "workers_secret_folded_into_bindings" + description: "Worker secrets are folded into workers_script bindings in v5. The first plan will show binding additions and reordering. This is expected and resolves after terraform apply." + resource_types: + - "cloudflare_workers_script" + resource_name_patterns: + - "module.workers_secret.*" + patterns: + - 'type.*=.*"secret_text"' + - 'text.*=.*\(sensitive value\)' + - 'name.*=.*"API_KEY"' + - 'name.*=.*"DB_PASSWORD"' + - 'name.*=.*"JWT_SECRET"' + - 'name.*=.*"SINGULAR_SECRET"' + - 'name.*=.*"LITERAL_SECRET"' + - 'name.*=.*"EXTRA_SECRET"' + - 'name.*=.*"MY_KV"' + - 'name.*=.*"ENV"' + - 'namespace_id' + - 'type.*=.*"kv_namespace"' + - 'type.*=.*"plain_text"' + enabled: true + +settings: + apply_exemptions: true + verbose_exemptions: false