elasticstack_elasticsearch_index_mappings: state stores template-injected dynamic_templates, not just user-declared subset

[tobio]

Summary

elasticstack_elasticsearch_index_mappings's schema documents: "Only the keys and fields declared here are tracked; dynamic extras added by Elasticsearch are ignored." This holds for properties (via intersectProperties), but not for dynamic_templates: after a Read, state ends up containing the full API-returned dynamic_templates array — including entries contributed by an index template — instead of just the templates the user declared in config.

Root cause

internal/elasticsearch/index/indexmappings/intersect.go's intersectMappings special-cases properties (name-aware intersection), but falls back to index.FieldSemanticallyEqual(stateVal, apiVal) for every other top-level key:

if index.FieldSemanticallyEqual(stateVal, apiVal) {
    result[key] = stateVal
    continue
}
result[key] = apiVal

FieldSemanticallyEqual/fieldSemanticallyEqual type-asserts both sides to map[string]any. dynamic_templates is a []any, so the assertion always fails, FieldSemanticallyEqual always returns false, and result[key] = apiVal unconditionally stores the entire API array (user templates + template-injected extras) into state.

Repro

Given:

• stateMap: {"dynamic_templates": [{"text_ja_example": {...}}]} (user-declared)
• apiMap: {"dynamic_templates": [{"template_default": {...}}, {"text_ja_example": {...}}]} (API, with an index-template-injected extra)

intersectMappings(apiMap, stateMap) returns dynamic_templates containing both template_default and text_ja_example, not just the user-owned text_ja_example.

Confirmed with a standalone probe test against intersectMappings:

result: {"dynamic_templates":[{"template_default":{"mapping":{"type":"keyword"},"match_mapping_type":"string"}},{"text_ja_example":{"mapping":{"type":"text"},"path_match":"hoge.example_field.freetext"}}]}

Impact

• terraform state show / terraform show -json for this resource can contain dynamic_templates entries the user never declared and doesn't own, contradicting the documented "user-declared subset" contract.
• The existing acceptance test coverage doesn't catch this: checkStateMappingsDynamicTemplates(minCount) in acc_test.go asserts len(templates) >= minCount, which can't distinguish "state has just the user's template" from "state has the user's template plus injected extras."

Suggested fix

Reuse the name-keyed comparison introduced in #4581 (dynamicTemplatesByName in internal/elasticsearch/index/mappings_value.go) inside intersectMappings/intersectProperties so dynamic_templates is intersected by template name the same way properties is intersected by field name — keeping only the user-declared template names in state. Also tighten checkStateMappingsDynamicTemplates to assert an exact set of template names rather than a minimum count, so a regression here is actually caught.

Related

• Found during review of fix(elasticsearch-index): compare dynamic templates semantically #4581, which fixed the adjacent (but distinct) issue of dynamic_templates causing false "Provider produced inconsistent result after apply" errors during the plan/apply consistency check. That fix is in mappings_value.go's MappingsSemanticallyEqual/StringSemanticEquals path and does not touch intersect.go, so this state-persistence issue is pre-existing and unaffected by fix(elasticsearch-index): compare dynamic templates semantically #4581.

[github-actions]

Thanks for this detailed report! I've added the needs-spec label to this issue.

This issue is well-specified enough to move directly into the spec-writing phase — the root cause is pinpointed to intersectMappings in internal/elasticsearch/index/indexmappings/intersect.go, a repro confirms the behavior, and the suggested fix (reusing the name-keyed dynamicTemplatesByName comparison from #4581 and tightening checkStateMappingsDynamicTemplates) gives an implementer enough to start without further research. A maintainer will review it and queue it for implementation planning.

If I've misclassified this or you have additional context, please leave a comment — a maintainer can adjust the label manually.

[github-actions]

✅ Research Factory Issue Intake completed successfully!

Generated by Research Factory Issue Intake for #4588

[github-actions]

Implementation research

Run: 2026-08-26T05:20:36Z · workflow run

Edits inside this comment are read as input on the next run but are not preserved verbatim. For durable feedback, post a comment or edit the issue body.

Problem framing

Read for elasticstack_elasticsearch_index_mappings should keep only the user-declared subset of mappings. That holds for properties (intersectProperties, name-aware, recursive) but not dynamic_templates: in internal/elasticsearch/index/indexmappings/intersect.go:38-64, intersectMappings special-cases only properties (line 47); every other top-level key falls through to index.FieldSemanticallyEqual(stateVal, apiVal) (line 57), which type-asserts to map[string]any. dynamic_templates is []any, so that assertion always fails and result[key] = apiVal (line 61) stores the entire API array — user templates plus anything an index template injected.

Both maintainer decision rounds recorded on this issue are now resolved, and together they fully pin down the fix:

1. Consolidate packages — move intersect.go into internal/elasticsearch/index (not the reverse), so the fix reuses dynamicTemplatesByName (added in fix(elasticsearch-index): compare dynamic templates semantically #4581, mappings_value.go:427-452) in-package rather than exporting it as new public API surface.
2. Drop-on-missing — a declared dynamic_templates name absent from the API (deleted out-of-band) is dropped from state, mirroring intersectProperties's existing omit-on-missing at intersect.go:73-76. Drift then surfaces on the next plan instead of state silently pinning a stale value.
3. Invalid state shape (duplicate template name, or a template value that isn't an object — only reachable via manual state edits/import, since Elasticsearch itself won't produce this shape) — fall back to today's whole-array passthrough for that key, not drop the key outright.
4. Test tightening — checkStateMappingsDynamicTemplates (acc_test.go:636-651) moves from a len >= minCount assertion to an exact-name-set assertion, must additionally assert template_default is absent from state, and the existing TestAccResourceIndexMappings_dynamicTemplatesFromIndexTemplate fixture gets an added step exercising decision 2 (a declared name deleted out-of-band).

This closes out every open question from the prior research round; the design below reflects the resolved shape rather than re-presenting alternatives already ruled out.

Approaches considered

Approach A: Move intersect.go into index, add a name-keyed dynamic_templates case reusing dynamicTemplatesByName

Move intersect.go and intersect_test.go from indexmappings into internal/elasticsearch/index, alongside MappingsValue and dynamicTemplatesByName/dynamicTemplatesSemanticallyEqual (mappings_value.go). intersectMappings becomes exported IntersectMappings (call site indexmappings/read.go:73 changes to index.IntersectMappings(apiMap, stateMap)); intersectProperties and the new dynamic-templates helper stay unexported since everything is now one package — dynamicTemplatesByName needs no new exported seam.

Add a dynamic_templates case parallel to the existing properties case:

if key == dynamicTemplatesKey {
    if templates, ok := intersectDynamicTemplates(apiVal, stateVal); ok {
        if len(templates) > 0 {
            result[key] = templates
        }
        continue
    }
    // unparseable shape (decision 3) — fall through to today's passthrough below
}

intersectDynamicTemplates walks the state's []any in its original order (not dynamicTemplatesByName's map, which would lose declared ordering — template order affects Elasticsearch's first-match-wins semantics), looks up each declared name in dynamicTemplatesByName(apiVal), keeps the API's definition for names found (mirroring intersectProperties's "filter by name, keep API's definition" pattern rather than dynamicTemplatesSemanticallyEqual's equality check), and omits names absent from the API (decision 2). If either side fails to parse via dynamicTemplatesByName (decision 3's duplicate-name/non-object case), ok is false and the caller passes through apiVal unfiltered, unchanged from today.

• Pros: reuses the exact name-keying already proven correct by fix(elasticsearch-index): compare dynamic templates semantically #4581's plan-time path, so "same template" can't be judged differently by Read's projection vs. MappingsSemanticallyEqual's drift suppression; satisfies decision 1 by construction (no new public API, single package owns both comparisons); every one of decisions 2–4 maps to a small, independently testable branch.
• Cons: real (if small) diff — moving a file across packages touches imports in read.go and the test file; unit tests for intersectMappings/intersectProperties move packages too (index_test or in-package index).

Approach B: Export dynamicTemplatesByName from index, keep intersect.go in indexmappings

Add an exported wrapper (e.g. index.DynamicTemplatesByName) and use it from a new dynamic_templates branch inside intersect.go, without relocating any files.

• Pros: minimal diff — one new exported function, no cross-package file moves, no test-file relocation.
• Cons: directly contradicts decision 1, which explicitly rejected "export a helper" in favor of consolidating the packages, and decision on consolidation direction explicitly named moving intersect.go into index rather than the reverse. Recorded to show why it's rejected, not as a live alternative.

Recommendation

Approach A. It's the only approach consistent with all four resolved maintainer decisions — file relocation direction, drop-on-missing semantics, invalid-shape fallback, and the test-tightening scope are all decided, leaving Approach A as direct implementation of those decisions rather than a judgment call.

Open questions

• Naming: is IntersectMappings the right exported name after the move, or would the maintainers prefer it stay lowercase with a package-level test-only seam (e.g. via an internal test package)? Non-blocking — either compiles and behaves identically; only affects the diff's surface API.
• Confirm that preserving the state's declared order for kept dynamic_templates entries (rather than the API's array order) is the intended behavior — this wasn't explicitly asked in either maintainer round, but matters because dynamic template order is semantically significant to Elasticsearch (first match wins). Non-blocking for correctness of the fix itself (order only matters if the user relies on it downstream), but worth a one-line confirmation before merge.

Out of scope

• Plan/apply consistency path (MappingsSemanticallyEqual/StringSemanticEquals, RequiresMappingsUpdate) — already fixed by fix(elasticsearch-index): compare dynamic templates semantically #4581 and untouched by this change.
• properties intersection — already correctly name-aware; not modified beyond possibly moving packages.
• Import path (priorMappingsEmpty short-circuit in read.go:54-57) — first read after import stores the full API mappings unfiltered by design; this fix only changes behavior once a prior dynamic_templates mask exists in state.
• Any change to the openspec/specs/elasticstack-elasticsearch-index-mappings/spec.md REQ-004 wording beyond what's needed to document dynamic_templates getting the same recursive-filtering treatment as properties — a spec update is likely needed but its exact wording isn't decided here.

References

• internal/elasticsearch/index/indexmappings/intersect.go (intersectMappings L38-64, intersectProperties L66-106)
• internal/elasticsearch/index/indexmappings/intersect_test.go (existing unit coverage that would move with the file)
• internal/elasticsearch/index/indexmappings/read.go (L73, call site to update on relocation)
• internal/elasticsearch/index/mappings_value.go (dynamicTemplatesByName L427-452, dynamicTemplatesSemanticallyEqual L403-421, MappingsSemanticallyEqual L358-397) — from fix(elasticsearch-index): compare dynamic templates semantically #4581
• internal/elasticsearch/index/indexmappings/acc_test.go (TestAccResourceIndexMappings_dynamicTemplatesFromIndexTemplate L196-243, checkStateMappingsDynamicTemplates L636-651, setDynamicTemplatesOutOfBand L706+)
• openspec/specs/elasticstack-elasticsearch-index-mappings/spec.md (REQ-004, "Read — user-declared subset only", L80-96)
• Issue comment thread (2026-08-26, two rounds): maintainer decisions on package consolidation direction, drop-on-missing semantics, invalid-shape fallback, and test scope

🤖 Pipeline metadata

{
  "schema_version": "1.0",
  "recommendation": {
    "spine": "name-keyed-dynamic-templates-intersection-in-index-package",
    "confidence": "high",
    "approach_index": 0
  },
  "open_questions": [
    {"id": "exported-name", "text": "Should the relocated intersectMappings be exported as index.IntersectMappings, or kept unexported with a test-only seam?", "blocking": false},
    {"id": "declared-order-preservation", "text": "Confirm kept dynamic_templates entries should preserve the state's declared order rather than the API's array order.", "blocking": false}
  ],
  "affected_capabilities": ["elasticstack-elasticsearch-index-mappings"],
  "estimated_scope": "small",
  "references": [
    {"type": "file", "path": "internal/elasticsearch/index/indexmappings/intersect.go"},
    {"type": "file", "path": "internal/elasticsearch/index/indexmappings/intersect_test.go"},
    {"type": "file", "path": "internal/elasticsearch/index/indexmappings/read.go"},
    {"type": "file", "path": "internal/elasticsearch/index/mappings_value.go"},
    {"type": "file", "path": "internal/elasticsearch/index/indexmappings/acc_test.go"},
    {"type": "path", "path": "openspec/specs/elasticstack-elasticsearch-index-mappings/spec.md"}
  ]
}

[tobio]

Export dynamicTemplatesByName from internal/elasticsearch/index for reuse, or consolidate the two packages so mapping-comparison logic doesn't need a public seam? (non-blocking)

Consolidate the packages

When a declared template name is missing from the API entirely (deleted out-of-band), should Read keep the stale declared value (proposed) or drop it and let drift surface on next plan? (blocking — changes plan behavior)

Drop it

Should checkStateMappingsDynamicTemplates be tightened to an exact-name-set assertion in the same PR, so a regression is actually caught? (non-blocking, but recommended in-PR)

Yes