Skip to content

feat: accept url in schema check #6582

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 7, 2025
Merged

feat: accept url in schema check #6582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9001332
feat: accept url in schema check
jdolle Mar 5, 2025
0f69e39
prettier
jdolle Mar 5, 2025
e7d4d47
Add changeset
jdolle Mar 6, 2025
c70108c
Merge branch 'main' into check-url
jdolle Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/red-needles-know.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@graphql-hive/cli': patch
'hive': patch
---

Adds optional url argument to schema checks
32 changes: 32 additions & 0 deletions integration-tests/tests/cli/__snapshots__/schema.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@ stdout--------------------------------------------:
__NONE__
`;

exports[`FEDERATION > schema:check accepts a \`--url\` argument in distributed projects 1`] = `
:::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::

stdout--------------------------------------------:
✔ Schema registry is empty, nothing to compare your schema with.
View full report:
http://__URL__
`;

exports[`FEDERATION > schema:check should notify user when registry is empty > schemaCheck 1`] = `
:::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::

Expand Down Expand Up @@ -261,6 +270,20 @@ stdout--------------------------------------------:
__NONE__
`;

exports[`SINGLE > schema:check rejects a \`--url\` argument in single projects 1`] = `
:::::::::::::::: CLI FAILURE OUTPUT :::::::::::::::
exitCode------------------------------------------:
1
stderr--------------------------------------------:
__NONE__
stdout--------------------------------------------:
✖ Detected 1 error

- url is only supported by distributed projects


`;

exports[`SINGLE > schema:check should notify user when registry is empty > schemaCheck 1`] = `
:::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::

Expand Down Expand Up @@ -433,6 +456,15 @@ stdout--------------------------------------------:
__NONE__
`;

exports[`STITCHING > schema:check accepts a \`--url\` argument in distributed projects 1`] = `
:::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::

stdout--------------------------------------------:
✔ Schema registry is empty, nothing to compare your schema with.
View full report:
http://__URL__
`;

exports[`STITCHING > schema:check should notify user when registry is empty > schemaCheck 1`] = `
:::::::::::::::: CLI SUCCESS OUTPUT :::::::::::::::::

Expand Down
70 changes: 70 additions & 0 deletions integration-tests/tests/cli/schema.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,76 @@ describe.each([ProjectType.Stitching, ProjectType.Federation, ProjectType.Single
await expect(fetchCmd).resolves.toMatchSnapshot('latest sdl');
},
);

test.skipIf(projectType !== ProjectType.Single)(
'schema:check rejects a `--url` argument in single projects',
async ({ expect }) => {
const { createOrg } = await initSeed().createOwner();
const { inviteAndJoinMember, createProject } = await createOrg();
await inviteAndJoinMember();
const { createTargetAccessToken } = await createProject(projectType);
const { secret } = await createTargetAccessToken({});

await expect(
schemaCheck(
[
'--registry.accessToken',
secret,
'--service',
'example',
'--url',
'https://example.graphql-hive.com/graphql',
'--author',
'Kamil',
'fixtures/init-schema.graphql',
],
{
// set these environment variables to "emulate" a GitHub actions environment
// We set GITHUB_EVENT_PATH to "" because on our CI it can be present and we want
// consistent snapshot output behaviour.
GITHUB_ACTIONS: '1',
GITHUB_REPOSITORY: 'foo/foo',
GITHUB_EVENT_PATH: '',
},
),
).rejects.toMatchSnapshot();
},
);

test.skipIf(projectType === ProjectType.Single)(
'schema:check accepts a `--url` argument in distributed projects',
async ({ expect }) => {
const { createOrg } = await initSeed().createOwner();
const { inviteAndJoinMember, createProject } = await createOrg();
await inviteAndJoinMember();
const { createTargetAccessToken } = await createProject(projectType);
const { secret } = await createTargetAccessToken({});

await expect(
schemaCheck(
[
'--registry.accessToken',
secret,
'--service',
'example',
'--url',
'https://example.graphql-hive.com/graphql',
'--author',
'Kamil',
'fixtures/init-schema.graphql',
],
{
// set these environment variables to "emulate" a GitHub actions environment
// We set GITHUB_EVENT_PATH to "" because on our CI it can be present and we want
// consistent snapshot output behaviour.
GITHUB_ACTIONS: '1',
GITHUB_REPOSITORY: 'foo/foo',
GITHUB_EVENT_PATH: '',
},
),
).resolves.toMatchSnapshot();
},
);
Comment on lines +358 to +426
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify that URL validation is properly implemented in the API layer.

The tests indicate that URL validation should be enforced - rejecting URLs for single projects while accepting them for distributed projects. However, I don't see the validation logic in the provided code snippets that would reject URLs for single projects.

🏁 Script executed:

#!/bin/bash
# Search for validation logic that rejects URL for single projects in schema-publisher.ts

echo "Searching for schema validation logic in schema-publisher.ts file..."
fd "schema-publisher\.ts$" --exec grep -A 10 -B 10 "url.*validation" {} \; || echo "No validation pattern found"

echo "Searching for validation logic by looking for ProjectType.Single checks..."
fd "schema-publisher\.ts$" --exec grep -A 5 -B 5 "ProjectType\.Single" {} \; || echo "No ProjectType.Single pattern found"

echo "Searching for URL validation in internalCheck method..."
fd "schema-publisher\.ts$" --exec grep -A 15 -B 5 "internalCheck" {} \; || echo "No internalCheck method found"

Length of output: 2605

URL Validation Missing in API Layer
The integration tests expect that providing a --url argument is rejected in single projects (and accepted in distributed projects). However, our search in schema-publisher.ts did not reveal any URL validation code (e.g., no checks for ProjectType.Single or explicit URL validation logic). Please verify if the URL validation is implemented elsewhere; if not, add the required validations in the API layer to ensure that URL arguments are correctly rejected for single projects.

},
);

Expand Down
5 changes: 5 additions & 0 deletions packages/libraries/cli/src/commands/schema/check.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ export default class SchemaCheck extends Command<typeof SchemaCheck> {
' This can either be a slug following the format "$organizationSlug/$projectSlug/$targetSlug" (e.g "the-guild/graphql-hive/staging")' +
' or an UUID (e.g. "a0f4c605-6541-4350-8cfe-b31f21a4bf80").',
}),
url: Flags.string({
description:
'If checking a service, then you can optionally provide the service URL to see the difference in the supergraph during the check.',
}),
};

static args = {
Expand Down Expand Up @@ -273,6 +277,7 @@ export default class SchemaCheck extends Command<typeof SchemaCheck> {
: null,
contextId: flags.contextId ?? undefined,
target,
url: flags.url,
},
},
});
Expand Down
4 changes: 4 additions & 0 deletions packages/services/api/src/modules/schema/module.graphql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,10 @@ export default gql`
Manually approved breaking changes will be memorized for schema checks with the same context id.
"""
contextId: String
"""
Optional url if wanting to show subgraph url changes inside checks.
"""
url: String
}

input SchemaDeleteInput {
Expand Down
5 changes: 4 additions & 1 deletion packages/services/api/src/modules/schema/providers/models/composite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export class CompositeModel {
input: {
sdl: string;
serviceName: string;
url: string | null;
};
selector: {
organizationId: string;
Expand Down Expand Up @@ -146,7 +147,9 @@ export class CompositeModel {
sdl: input.sdl,
service_name: input.serviceName,
service_url:
latest?.schemas?.find(s => s.service_name === input.serviceName)?.service_url ?? 'temp',
input.url ??
latest?.schemas?.find(s => s.service_name === input.serviceName)?.service_url ??
'temp',
action: 'PUSH',
metadata: null,
};
Expand Down
22 changes: 22 additions & 0 deletions packages/services/api/src/modules/schema/providers/schema-publisher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,27 @@ export class SchemaPublisher {
});
}

// if url is provided but this is not a distributed project
if (
input.url != null &&
!(project.type === ProjectType.FEDERATION || project.type === ProjectType.STITCHING)
) {
this.logger.debug('url is only supported by distributed projects (type=%s)', project.type);
increaseSchemaCheckCountMetric('rejected');

return {
__typename: 'SchemaCheckError',
valid: false,
changes: [],
warnings: [],
errors: [
{
message: 'url is only supported by distributed projects',
},
],
} as const;
}

if (
(project.type === ProjectType.FEDERATION || project.type === ProjectType.STITCHING) &&
input.service == null
Expand Down Expand Up @@ -548,6 +569,7 @@ export class SchemaPublisher {
input: {
sdl,
serviceName: input.service,
url: input.url ?? null,
},
selector,
latest: latestVersion
Expand Down
Loading