[braghettos]

A green status over unrendered templates

SyncRepos logged a warning and carried on when loadValuesFromConfigMap failed:

values, err = e.loadValuesFromConfigMap(ctx, spec.ConfigMapKeyRef)
if err != nil {
    e.log.Warn("Unable to load configmap with template data", "msg", err.Error())
    e.rec.Event(cr, plumbingevent.Warning("CannotLoadConfigMap", ...))
}   // <-- no return

That function returns (nil, err) on every error path. So values is nil, the if values != nil guard below installs no templating engine, and the copy, commit and push all still run — publishing files with their {{ }} placeholders intact and then setting Available / Synced.

And it is terminal. TargetCommitId now matches the commit that was just pushed, so Observe sees an up-to-date resource. It never retries and never corrects itself. The operator sees READY=True SYNCED=True over a target repo full of unrendered templates, with nothing but a warning in a log they have no reason to read.

The trigger is a typo

This is not an exotic path. provider-runtime's GetConfigMapValue ends:

return string(cm.Data[ref.Key]), nil

For an absent key that is "" with a nil error. So a typo in configMapKeyRef.key never surfaces as "missing key" — it reaches json.Unmarshal([]byte("")) and fails as unexpected end of JSON input. Three realistic triggers, all the same outcome:

1. a typo in configMapKeyRef.key
2. YAML written into a key the controller parses as JSON
3. a ConfigMap that is missing or unreadable by the provider's RBAC

The change

A CR that sets configMapKeyRef has asked for substitution. If the values cannot be read, the honest outcome is a failed sync — not a green one over unrendered output.

The warning log and the CannotLoadConfigMap event are kept; only the silent continue becomes failSync, which sets Unavailable + ReconcileError and returns the error so the resource is retried.

Tests

internal/controllers/repo/configmap_values_test.go — plain unit tests with a fake client. (The existing repo_test.go is //go:build integration and needs Docker; these do not.)

test	pins
TestLoadValuesFromConfigMapErrorsOnMissingKey	a typo'd key is an error, not an empty value set
TestLoadValuesFromConfigMapErrorsOnNonJSON	YAML in a JSON key is an error
TestLoadValuesFromConfigMapReturnsValues	a valid key still returns its values
TestLoadFailureYieldsNilValuesWhichWouldSkipTemplating	the precondition that made the silent skip possible — asserted so a future refactor dropping the early return has something to fail against

go build ./..., go vet and go test ./internal/... clean — 7 packages ok.

Provenance

Found by an adversarial review of the write-path templating. It was handed to a skeptic told to refute it, who attacked it on five axes — reachability, a guard elsewhere, does the claimed input produce the claimed output, error shadowing, inflated severity — and it survived all five. Notably the stale non-nil err is not an accidental escape hatch: it is reassigned at co, err := copier.NewCopier(...) before it could ever be returned.

Related: #8 (unconditional templating in the localresource controller). Same family — the two controllers disagreeing about when to template — but independent changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JumEqTGGJ2hf3vjE3xFQsv