[rh-amarin]

Problem

Existing gateways toggled between Running and Provisioning. The control plane wrote back oidc on every reconcile as compact JSON; the oidc/route jsonb columns return normalized text, so desiredStateChanged (raw string compare) saw a spec change every time. That bumped generation past observed_generation, defeating the convergence gate, so each gateway was fully re-provisioned about every 7s (phase set to Provisioning, then promoted by the health reconciler).

This was introduced by #151

Fix

Compare oidc and route semantically (jsonEq), falling back to a string compare for non-JSON values.

Testing

• New unit test TestDesiredStateChangedIgnoresJSONNormalization
• make lint and api-server make test pass locally
• Diagnosed on a live VM: generation climbed 110 -> 114 in minutes, always ahead of observed_generation

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 3d8c3fee-9741-4412-ab20-7f8b17aa6f7b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[hypershell-delivery]

Amber review

Status: Complete

View the submitted review.

[hypershell-delivery]

Verdict

This remains a tight, correct fix. oidc/route are jsonb columns, so the PostgreSQL-normalized text (key order, spacing) never byte-equals the compact JSON the control plane writes back; the old strEq compare therefore bumped generation on every UpdateGateway, re-opening the convergence gate and flapping gateways between Running and Provisioning. Comparing those two fields semantically with jsonEq (nil-safe, byte fast-path, reflect.DeepEqual on the unmarshaled documents) resolves the spurious churn while leaving every other desired-state field on the strict strEq compare.

Findings

No new findings. The rebased head is diff-identical to the previously reviewed change: jsonEq is nil-safe (service.go:250-262), preserves the desired-state/observed-state split documented in data-model.spec.md (service.go:225-242), adds no secrets to logs, and weakens no pre-existing test assertion. The net-new desired_state_test.go is additive and covers the normalization no-op, a real audience change, and the cleared-field transition.

Cross-PR coordination

No material cross-PR coordination issue requires maintainer action.

Previous concerns

• Minor - jsonEq doc comment mischaracterized the non-JSON path as a string compare: Addressed. The comment at service.go:249 now reads "Unparseable values are treated as changed unless byte-identical," which matches the actual behavior: after the *a == *b fast path, a json.Unmarshal failure on either value returns false (service.go:254-260).

The committer addressed the previous concerns.

Findings Summary (ordered by severity, highest first):

No outstanding findings.

Convention Checklist

Convention	Result
No panic() in production code	Pass
Errors wrapped with fmt.Errorf context	Pass
No secrets in logs or responses	Pass
Input validated	Pass
Test assertions not silently weakened	Pass
Conventional commit message	Pass