History

GraphQL health check: drop the schema root type as connection identity (#1817)

* Stop reporting the schema root type as a GraphQL connection identity * Cover the healthy GraphQL account row label in e2e

Rhys Sullivan committed 2 months ago

Commit 1d4d1611 parent ac2dec63 files · +71 −5Browse files

Unified · Split

3 files
  1. e2e/scenarios/graphql-introspection-health.test.ts +62 −0
  2. packages/plugins/graphql/src/sdk/plugin.test.ts +5 −1
  3. packages/plugins/graphql/src/sdk/plugin.ts +4 −4
e2e/scenarios/graphql-introspection-health.test.ts+62−0
@@ -8,6 +8,11 @@ import { graphqlHttpPlugin } from "@executor-js/plugin-graphql/api"; · 7 unmodified lines
88 import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared";
99 import { variable } from "@executor-js/sdk/http-auth";
1010
11+import {
12+ makeGreetingGraphqlSchema,
13+ serveGraphqlTestServer,
14+} from "@executor-js/plugin-graphql/testing";
15+
1116 import { scenario } from "../src/scenario";
1217 import { Api, Browser, Target } from "../src/services";
1318 import { visit } from "../src/surfaces/browser";
@@ -144,3 +149,60 @@ scenario( · 130 unmodified lines
144149 }),
145150 ),
146151 );
152+
153+scenario(
154+ "GraphQL · a healthy account row keeps the connection's name",
155+ {},
156+ Effect.scoped(
157+ Effect.gen(function* () {
158+ const target = yield* Target;
159+ const browser = yield* Browser;
160+ const { client: makeApiClient } = yield* Api;
161+ const identity = yield* target.newIdentity();
162+ const client = yield* makeApiClient(api, identity);
163+ const slug = unique("graphql_label");
164+ const upstream = yield* serveGraphqlTestServer({ schema: makeGreetingGraphqlSchema() });
165+
166+ yield* client.graphql.addIntegration({
167+ payload: {
168+ endpoint: upstream.endpoint,
169+ slug,
170+ name: "Greeting API",
171+ },
172+ });
173+
174+ yield* Effect.gen(function* () {
175+ yield* client.connections.create({
176+ payload: {
177+ owner: "org",
178+ name: ConnectionName.make("workspace"),
179+ integration: IntegrationSlug.make(slug),
180+ template: AuthTemplateSlug.make("none"),
181+ value: "unused",
182+ },
183+ });
184+
185+ yield* browser.session(identity, async ({ page, step }) => {
186+ await step("The healthy account row is labeled with the connection name", async () => {
187+ await visit(page, `/integrations/${slug}`);
188+ await page.getByRole("tab", { name: "Accounts" }).click();
189+ // Wait for the automatic health probe to land so a probe identity
190+ // would have replaced the label if the plugin still reported one.
191+ await page.getByLabel("Status: Healthy").waitFor();
192+ await page.getByText("workspace", { exact: true }).waitFor();
193+ expect(
194+ await page.getByText("GraphQL schema", { exact: false }).count(),
195+ "the schema root type never masquerades as the account label",
196+ ).toBe(0);
197+ });
198+ });
199+ }).pipe(
200+ Effect.ensuring(
201+ client.integrations
202+ .remove({ params: { slug: IntegrationSlug.make(slug) } })
203+ .pipe(Effect.ignore),
204+ ),
205+ );
206+ }),
207+ ),
208+);
packages/plugins/graphql/src/sdk/plugin.test.ts+5−1
@@ -729,8 +729,12 @@ describe("graphqlPlugin real protocol server", () => { · 728 unmodified lines
729729 expect(result).toMatchObject({
730730 status: "healthy",
731731 httpStatus: 200,
732− identity: "GraphQL schema: Query",
732+ responseSample: [{ path: "__schema.queryType.name", value: "Query" }],
733733 });
734+ // The schema's root type name is not an account identity; reporting it
735+ // as one made the accounts UI label every GraphQL connection
736+ // "GraphQL schema: Query".
737+ expect(result).not.toHaveProperty("identity");
734738 }),
735739 );
736740
packages/plugins/graphql/src/sdk/plugin.ts+4−4
@@ -737,16 +737,16 @@ const checkGraphqlHealth = (input: { · 736 unmodified lines
737737 : { ...verdict, detail: scrubCredentialValues(verdict.detail, input.credential.values) };
738738 }
739739
740+ // No `identity`: the schema's root type name ("Query" almost everywhere)
741+ // identifies nothing, and the accounts UI would show it as the account
742+ // label. Introspection proves reachability, not who the credential is.
740743 const queryType = result.introspection.__schema.queryType?.name;
741744 return {
742745 status: "healthy",
743746 httpStatus: 200,
744747 checkedAt,
745748 ...(queryType != null
746− ? {
747− identity: `GraphQL schema: ${queryType}`,
748− responseSample: [{ path: "__schema.queryType.name", value: queryType }],
749− }
749+ ? { responseSample: [{ path: "__schema.queryType.name", value: queryType }] }
750750 : {}),
751751 } satisfies HealthCheckResult;
752752 });