fix(source): evaluate FQDN templates on typed objects like unstructured

[zanarellidev]

What does it do?

On FQDN template execution failure, retry with an unstructured-style data shape (Spec/Status as JSON maps, plus Name/Namespace/...). Shared --fqdn-template values that use JSON keys under Spec (e.g. {{ range .Spec.hostnames }}) then work for typed gateway routes the same way they already work for the unstructured source.

Motivation

#6593: the same --fqdn-template is used for unstructured and typed sources. Unstructured wraps objects so Spec is a map with JSON keys; typed v1.HTTPRouteSpec exposes Go field names, so .Spec.hostnames fails with can't evaluate field hostnames in type v1.HTTPRouteSpec (including when hostnames are empty/absent).

More

• Yes, this PR title follows Conventional Commits
• Yes, I added unit tests
• Yes, I updated end user documentation accordingly

Validation:

• go test ./source/template/ (typed hostnames, empty hostnames, unstructured, Name+Spec mix)
• Existing TestExecFQDNExecutionError still fails closed on real template errors
• CI

Fixes #6593

[kubernetes-prow]

Hi @zanarellidev. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ivankatliarchuk]

Not too sure. The approach try, fail, retry - for a shared JSON-style template on a typed source like HTTPRoute, step 1 will always fail and step 2 will always run, on every object, every sync cycle - permanently. What are the other options considered?

[zanarellidev]

Good catch. For a shared JSON-style template against typed HTTPRoute, step 1 does fail every sync and step 2 always runs. That cost is real for that path.

Why try/fail was chosen anyway: always normalizing typed objects to the unstructured/JSON map shape before Execute fixes JSON keys in one pass, but it silently breaks Go-field templates that already work today (e.g. {{ range .Spec.Hostnames }}). On a map, .Spec.Hostnames evaluates empty with no error. Existing coverage also uses .ObjectMeta.Annotations, .Name, .Labels, and .Data on typed objects, so a naive always-convert needs a much richer view and still has the Go-vs-JSON Spec field problem.

Options considered:

1. Keep try/fail (this PR). Preserves both template styles. Cost is permanent on the JSON-on-typed path only. Unstructured already succeeds on step 1.

2. Separate --unstructured-fqdn-template (preferred long-term in Combining unstructured source with structured source for same resource can lead to templating errors #6593). Removes the shared-template conflict entirely. Larger API/docs change; gateway and unstructured no longer share one string.

3. Soft-skip: do not run the shared FQDN template for gateway (also floated in Combining unstructured source with structured source for same resource can lead to templating errors #6593: not needed for the normal Gateway source). Behavior change for anyone who does rely on FQDN templates on gateway routes.

4. Cache the failure once per template (or GVK+template): after the first typed Execute miss, subsequent objects use the unstructured shape directly. Same semantics as (1), without paying fail+retry every sync.

5. Template helpers / missingkey=zero: does not fix struct missing-field errors (can't evaluate field hostnames).

I lean toward (2) as the clean product fix, or (4) if we want to keep a single --fqdn-template and only remove the permanent retry cost. Happy to implement whichever you prefer.

[ivankatliarchuk]

I'm not too sure what the right solution should be. There are missing tests related to fqdn in source/gateway_httproute_test.go and in source/unstructured_fqdn_test.go, that syntax works aka for httproute something like .Spec.Hostnames and for unstructured .Spec.hostnames

[zanarellidev]

Good catch, that gap was real. Added an integration test through the actual gateway resolver path (not just the isolated source/template/engine_test.go unit tests): source/gateway_httproute_test.go's new "FQDNTemplate with JSON-style Spec access on typed HTTPRoute" case. Confirmed it fails with the exact #6593 error against the pre-fix engine.go, and passes with this PR's fix — proving .Spec.hostnames (JSON-style) now works via the fallback while .Spec.Hostnames (the existing typed-field path, still covered by the other test cases in the same file) keeps working unchanged.

[zanarellidev]

Ivan, adjusted as requested. Thanks for the help.

[ivankatliarchuk]

So there is currently an asymmetry in how templating behaves for different casings.

Typed HTTPRoute (--source=gateway-httproute), with this PR applied:

• .Spec.Hostnames (capital) - succeeds on the first tmpl.Execute attempt directly against the real struct, since Hostnames is the actual Go field name. No retry ever
  triggers; it just works, same as before this PR existed.
• .Spec.hostnames (lowercase) - first attempt hard-errors (can't evaluate field hostnames in type v1.HTTPRouteSpec), which is an error, so the retry fires, converts to
  the map shape, and the second attempt succeeds via the JSON key.

Unstructured source (any object, dynamic-client-backed):

• .Spec.hostnames (lowercase) - .Spec is already map[string]any with the raw JSON key hostnames; direct map lookup on the first attempt succeeds immediately.
• .Spec.Hostnames (capital) - map lookup for key "Hostnames" misses. A map miss is not an error in Go's text/template (confirmed by the throwaway test earlier: err= output="DONE", zero range iterations). So tmpl.Execute returns successfully with empty output — the retry gate (if err != nil) never fires, because there's no error to catch, and there's nowhere to fall back to anyway since the map is already the "final" shape.
• Only lowercase works; capital silently renders nothing, no error, no log line.

Most likely, we should widen the fix; make both casings resolves correctly on unstructured as well. Something like

// source/unstructured.go
 // addTitleCaseAliases makes each key in m reachable by both its native JSON
// and a naive Title-cased variant. Recurses so nested access works both ways too.
func addTitleCaseAliases(m map[string]any) map[string]any {}

...usage...
if spec, ok := u.Object["spec"].(map[string]any); ok {
        w.Spec = addTitleCaseAliases(copyMap(spec)) // copy first — see caveat below
  }

[ivankatliarchuk]

/ok-to-test

[coveralls]

Coverage Report for CI Build 31402468539

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.1%) to 81.891%

Details

• Coverage increased (+0.1%) from the base build.
• Patch coverage: No coverable lines changed in this PR.
• 22 coverage regressions across 3 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

22 previously-covered lines in 3 files lost coverage.

File	Lines Losing Coverage	Coverage
skipper_routegroup.go	13	92.07%
template/engine.go	6	80.81%
unstructured.go	3	97.09%

---

Coverage Stats

Relevant Lines:	21465
Covered Lines:	17578
Line Coverage:	81.89%
Coverage Strength:	1454.02 hits per line

---

💛 - Coveralls

[zanarellidev]

Pushed both remaining items:

• Widened the fix for the casing asymmetry you flagged: withTitleCaseAliases on the unstructured source's Spec/Status maps, so .Spec.Hostnames and .Spec.hostnames both resolve. Added a test covering that exact case plus unit tests for the alias helper.
• Simplified the retry block with a wrapTemplateErr helper and now propagate the retry's own error instead of discarding it.

One tradeoff to flag: anything that ranges over the whole .Spec/.Status map (instead of indexing a specific field) will now also see the title-cased alias as a separate key.

[ivankatliarchuk]

/lgtm

[kubernetes-prow]

New changes are detected. LGTM label has been removed.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from ivankatliarchuk. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[zanarellidev]

@mloiseleur thanks for the concrete repros, all three are real and now fixed in d6c3b2da:

• fail-open on unknown field: the unstructured retry now runs with missingkey=error (cloned template, so it only affects that retry, not the rest of the engine), so an unresolvable field fails the template instead of rendering <no value>. Added TestExecFQDNFailsClosedOnUnknownField with your three sub-cases.
• URL/DNSNames casing: titleCaseKey now upper-cases known initialisms (url -> URL, dnsNames -> DNSNames) instead of only capitalizing the first letter. Added TestTitleCaseKey_KnownInitialisms covering the cert-manager fields you named.
• alias pollution: withTitleCaseAliases no longer recurses into nested map values — it only aliases the direct children of Spec/Status now, so a template ranging or len()-ing nested data doesn't see doubled keys. Deep JSON paths (e.g. spec.endpoint.hostname) stay reachable through their literal key; I updated the one existing test that relied on a nested alias to reflect that. Added TestUnstructuredWrapper_MapIterationIsNotAliasPolluted with your exact repro.

go test -race ./source/... ./source/template/... green locally. Happy to adjust the initialism list or the alias-depth call if you'd rather it worked differently.