fix(bridges): treat bridge names case-insensitively on read and on create#22266
Open
ozpool wants to merge 1 commit intosmartcontractkit:developfrom
Open
fix(bridges): treat bridge names case-insensitively on read and on create#22266ozpool wants to merge 1 commit intosmartcontractkit:developfrom
ozpool wants to merge 1 commit intosmartcontractkit:developfrom
Conversation
…eate
Bridge names are stored case-folded by bridges.ParseBridgeName, and the
JSON-API controllers (POST /v2/bridge_types, GET/PATCH/DELETE
/v2/bridge_types/:BridgeName) use that parser end-to-end. Two paths
were skipping it:
- core/services/pipeline/task.bridge.go: getBridgeURLFromName cast
the spec-provided name straight to bridges.BridgeName and looked it
up as-is. A job spec referencing a bridge with mixed case (e.g.
"OpenWeatherMap") would therefore fail at run time with
"could not find bridge with name OpenWeatherMap", even though
the bridge had been created successfully.
- core/web/resolver/mutation.go: the GraphQL createBridge mutation
built the BridgeTypeRequest by raw-casting the input name, so
mutated-in bridges were stored under the original case while
every read path looked them up lower-cased.
Both call sites now go through bridges.ParseBridgeName, which lowercases
and validates the character set, matching the JSON-API behaviour and
the documented contract that bridge names are case-insensitive.
Closes smartcontractkit#9785.
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.
Description
Closes #9785.
bridges.ParseBridgeNamelower-cases bridge names, and the JSON-API controllers use that parser end-to-end (POST /v2/bridge_types,GET/PATCH/DELETE /v2/bridge_types/:BridgeName). The docs (External adapters) state that bridge names are case-insensitive. Two paths were skipping the parser and breaking that contract:1. Pipeline runtime —
core/services/pipeline/task.bridge.gogetBridgeURLFromNamewas raw-casting the spec-provided name tobridges.BridgeNameand looking it up as-is:A job spec referencing a mixed-case bridge (e.g.
OpenWeatherMap) would therefore fail at run time withcould not find bridge with name 'OpenWeatherMap', even though the bridge had been created successfully via the JSON API (which stored it asopenweathermap). The user in the linked issue described exactly this symptom.AssertBridgesExist(core/services/job/orm.go:142) already callsbridges.ParseBridgeNameon the same name when validating job creation, so a job referencingOpenWeatherMapvalidates fine but explodes on first run — strictly worse than failing fast.2. GraphQL —
core/web/resolver/mutation.gocreateBridgebuilt theBridgeTypeRequestwith a raw cast:Bridges created via GraphQL with mixed-case input were therefore persisted with the original case, while every read path (JSON-API show/update/delete, pipeline runtime, etc.) looks them up lower-cased — producing 404s and lookup failures.
Fix
Both call sites now route the name through
bridges.ParseBridgeName, which lower-cases and validates the character set. This matches the JSON-API behaviour and the documented contract.Diff is intentionally tight; the
updatemutation (mutation.go:481) takes the same shape but is gated behind a separateParseBridgeName(args.ID)lookup that already normalises, so its raw cast for the new name is benign here. Happy to clean that up in a follow-up if reviewers want it on the same path.Tests
I don't have a Postgres test database wired up locally, so I couldn't run the DB-backed
TestBridgeTask_ErrorIfBridgeMissing(or the broader job/bridge tests) directly. The fix preserves the existing error message shape (could not find bridge with name '<input>') so that test continues to assert the same string. CI will exercise the full suite once a maintainer triggers it.Local checks I did run:
Both clean.
Diff