fix(tunnel): make hostname add idempotent for already-bound hosts#793
Open
bussyjd wants to merge 1 commit into
Open
fix(tunnel): make hostname add idempotent for already-bound hosts#793bussyjd wants to merge 1 commit into
bussyjd wants to merge 1 commit into
Conversation
Reported by a teammate during v0.14.0-rc0 field testing. https://claude.ai/code/session_01PnhCQLz7CHuDBUhWd5xF8v
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
obol tunnel hostname add <existing-host>was not idempotent. When the hostname was already tracked on the tunnel,AddHostnamereturned an error (%s is already a tunnel hostname; nothing to do) and exited 1 before reaching the storefront catch-all re-render.That short-circuit is especially harmful for the retry path used by
obol tunnel hostname add <host> --offer <ns>/<name>: the CLI bindsServiceOffer.spec.hostnamefirst so thatCreateStorefrontcan skip offer-bound hosts from the storefront catch-all. IfAddHostnameis re-run for a hostname that is already bound (idempotent retry), the early error means the catch-all is never re-swept. A stale catch-all route can keep shadowing the offer's dedicated-origin route — Gateway API breaks thePathPrefix=/tie by route age.Fix
Treat "already present" as an idempotent success:
st.HostnameSet()(normalized match, including mixed-case input).CreateStorefront(cfg, existing...)over the current set so the catch-all is re-rendered and skips any hostnames that have since become offer-bound.HostnameMutationResult{Action: "unchanged", ...}with exit 0.No new sweep function: the duplicate path reuses the same
CreateStorefrontcall the normal success path already uses.CreateStorefrontcontinues to fail open (warn only) when kubectl/cluster access is unavailable.Diagram
flowchart TD A["tunnel hostname add host"] --> B{"already bound?"} B -->|yes| C["CreateStorefront over current set<br/>skips offer-bound hosts"] C --> D["idempotent success<br/>Action=unchanged, exit 0"] B -->|no| E["normal add path"] E --> F["route DNS / render ingress"] F --> G["CreateStorefront over updated set"] G --> H["persist state"] H --> I["success<br/>Action=added"]Validation
Actual output:
gofmt -lprinted nothing (both files already formatted).go build ./...succeeded with no output.go test ./internal/tunnel/... -count=1passed.https://claude.ai/code/session_01PnhCQLz7CHuDBUhWd5xF8v