Skip to content

fix(blueprints): detect a remote name conflict from the leaf stack list - #2129

Open
AnsoCode wants to merge 1 commit into
mainfrom
fix/blueprint-remote-name-conflict
Open

AnsoCode wants to merge 1 commit into
mainfrom
fix/blueprint-remote-name-conflict

Conversation

@AnsoCode

Copy link
Copy Markdown
Collaborator

Summary

On a remote node, a Blueprint name conflict was never detected. The ownership probe read the leaf's GET /api/stacks response as an array of objects carrying a name, but that endpoint answers with the stack names themselves. The match therefore matched nothing, and the probe reported every remote name as clear.

Three places were wrong as a result:

  • Check again on a name_conflict row answered cleared: true and moved the row to pending while the conflicting stack was still there, so the next reconcile tick parked it in conflict again.
  • Preview never flagged a remote create row as blocked_name_conflict.
  • The deploy pre-flight in deployToNode and deployAuthorizedMaterialization never ran on a remote node.

Nothing was ever written over: the leaf re-checks ownership under its per-stack lock and refuses with a 409. The defect cost a wrong answer in three places plus a doomed round trip, not data.

The probe now reads the names the leaf sends. Where the answer is not readable it fails closed at the point that is still undecided. A name found among the readable entries is answered from the marker as before. A name absent from a list that also held entries this version cannot read is an unverified probe rather than a clear name. A non-array body counts as unreadable rather than as an empty node, which is what let the object-shape read pass. An empty list stays a real answer, a node with no stacks. The local arm is unchanged.

One consequence worth expecting: a plan that creates a Blueprint on a remote node holding an unmanaged same-named stack now shows a blocker, and POST /:id/apply answers PLAN_BLOCKED. That is what the same collision already does on a local node.

Type of change

  • Bug fix

Related issue

Related to #2118, which added Check again and is where this was noticed. The probe defect predates it, and this is not a tracked issue, so there is nothing to close.

Testing performed

Backend: npx tsc --noEmit clean; npm test reports 11,306 tests passing. The one failing suite is git-transport-ssh.integration.test.ts, which refuses to run because loopback port 22 already has a listener on this machine. That is unrelated to this change, and the suite touches none of these files.

Regression proof: with the service change reverted, seven of the new and updated tests fail, and the negative controls (no such stack, empty list, name free) keep passing in both directions. So the new tests fail for the reason the fix addresses rather than passing by construction.

Behaviour pinned by the new tests, through the public entry points rather than internals:

  • a leaf listing the stack names with the target among them, and a marker read reporting the leaf's not-found code, is a conflict, and a rechecked remote row stays in name_conflict
  • a leaf listing no such stack reports no conflict and never issues the marker read
  • a leaf with no stacks reports no conflict
  • a non-array body, an array in the old object shape, an array of non-strings, and a mixed list whose readable entries do not contain the name are all refusals rather than clear names
  • a name present beside an unreadable sibling is still answered from the marker
  • preview marks a remote create as a blocker when the leaf lists the stack, and leaves it as a create when the leaf does not
  • a deploy whose ownership cannot be verified is recorded failed, not name_conflict, so a transport problem is not reported as an unowned stack sitting on the node

Two fixtures that had been answering /api/stacks in a shape the leaf never sends were corrected. They are why the probe's own tests agreed with the defect: one of them was also asserting the message of the wrong failure path for the same reason.

No user-visible label, path, or default changed, and the operator docs already describe the behavior this restores. No UI change, so no browser pass was needed.

Checklist

  • Tests added or updated
  • Docs updated if user-facing behavior, labels, or defaults changed (nothing user-visible changed)
  • Lint and typecheck pass locally
  • Commit messages and the PR title follow Conventional Commits
  • This PR stays on one concern and contains no secrets

The ownership probe's remote arm read GET /api/stacks as an array of objects
carrying a name, but the endpoint answers with the stack names themselves. The
match therefore matched nothing, so every remote name read as clear: Check again
released a name_conflict row while the conflicting stack was still there, the
preview never flagged a remote create as blocked, and the deploy pre-flight
never ran. Only the leaf's own apply-local check held, which is why nothing was
ever written over.

Read the names the leaf sends, and fail closed when the answer is not readable:
a name found among the readable entries is still answered from the marker, while
a name absent from a list holding entries this version cannot read is an
unverified probe rather than a clear name. A non-array body counts as unreadable
rather than as an empty node. The local arm is unchanged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant