Skip to content

fix(graphql): skip zero-weight nodes during introspection - #14124

Open
lordcheng10 wants to merge 5 commits into
apache:masterfrom
lordcheng10:fix/graphql-introspection-skip-zero-weight
Open

lordcheng10 wants to merge 5 commits into
apache:masterfrom
lordcheng10:fix/graphql-introspection-skip-zero-weight

Conversation

@lordcheng10

Copy link
Copy Markdown
Contributor

Description

graphql-limit-count derives an automatic schema introspection endpoint by sorting the configured upstream nodes and choosing the first address. The selector included nodes whose configured weight is zero, even though those nodes are disabled for normal upstream traffic.

With a zero-weight node that sorts before an enabled node, schema introspection could be sent to the disabled node and fail while the usable upstream remained available.

This change filters both supported node representations before sorting:

  • array nodes are included only when node.weight > 0;
  • map nodes are included only when their weight is greater than zero.

If every node is disabled, the existing no-node error path is used.

Reproduction

Against master commit ae87b942337a7e6897eb153c4f7cfb6ac1ecf3c8, call the shipped first_node selector with:

{
    ["127.0.0.1:1000"] = 0,
    ["127.0.0.1:2000"] = 1,
}

Before this change it returns 127.0.0.1:1000. After this change it returns 127.0.0.1:2000. The same behavior is covered for array-form nodes, and an all-zero set returns no candidate.

Test evidence

  • Added TEST 68 to t/plugin/graphql-limit-count2.t, covering map-form nodes, array-form nodes and an all-disabled upstream.
  • Ran a lightweight behavioral harness against the actual module with embedded Lua 5.4 (Lupa 2.8): all three assertions pass after the fix; the positive-weight assertions fail on the unmodified master function.
  • git diff --check passes.
  • The targeted Test::Nginx suite could not run locally because this macOS host does not have OpenResty or Test::Nginx::Socket::Lua::Stream installed. Repository CI will run the full test environment.
  • make lint could not start locally because the host lacks wget and the Makefile's OpenResty version probe invokes the system Bash with an unsupported -V option.

Which issue(s) this PR fixes:

Fixes #14123

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change (not needed: no public configuration or documented behavior changes)
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@lordcheng10

Copy link
Copy Markdown
Contributor Author

PTAL, thanks! @AlinsRan @nic-6443 @Baoyuantop

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.

bug: graphql-limit-count introspection can select a zero-weight upstream node

1 participant