Skip to content
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.

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

Add validation compilation phase to ensure federated schema validity #19

Open
kdawgwilk opened this issue Sep 22, 2021 · 10 comments
Open
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@kdawgwilk
Copy link
Collaborator

It would be great if we had a schema phase that validated the federated aspects of the schema to give end users more insight into why the schema may be an invalid federated schema

@kdawgwilk kdawgwilk added enhancement New feature or request help wanted Extra attention is needed labels Sep 22, 2021
@kdawgwilk kdawgwilk added this to the v1.0 milestone Sep 29, 2021
@kdawgwilk
Copy link
Collaborator Author

I would refer mostly to the federation spec to determine which kinds of validations we want to enforce are in place https://www.apollographql.com/docs/federation/federation-spec an example would be that @key uses fields that actually exist on the GQL type so you can't have something like this

object :product do
  key_fields("upc")
  field :id, non_null(:id)
end

Since the field upc doesnt exist on the type. Another validation we could do is if a type is marked @extends the @key(fields: "upc") must also be @external

An example compliation validation phase you can reference would be this one from absinthe https://github.com/absinthe-graphql/absinthe/blob/master/lib/absinthe/phase/schema/validation/directives_must_be_valid.ex#L1

@scottming
Copy link
Contributor

I'll take this.

@kzlsakal
Copy link
Collaborator

I didn't want to open a new issue since this one covers it. If we add duplicate directives for a field like @deprecate or @tag, it will break the composition. It will help to have a validation for making sure there are no duplicate directives for a field.

@scottming
Copy link
Contributor

Do we need to verify anything else now?

@maartenvanvliet
Copy link

I didn't want to open a new issue since this one covers it. If we add duplicate directives for a field like @deprecate or @tag, it will break the composition. It will help to have a validation for making sure there are no duplicate directives for a field.

There's a validation in Absinthe to ensure that directives not marked as repeatable will return an error when applied multiple times. How does this break for you?

@kdawgwilk
Copy link
Collaborator Author

@maartenvanvliet maybe we run our phases after that validation phase? I wouldn't think we do but we are also doing some weird stuff like hand crafting the directive blueprint structs ourselves because of the lack of support in absinthe which may play into this

@kzlsakal
Copy link
Collaborator

kzlsakal commented May 14, 2022

I will try to replicate this in an example repo to demonstrate it when I have a chance.

I know that it doesn’t get caught by Apollo’s managed schema checks either, but it breaks IntrospectAndCompose of Federation Gateway since 2.0.

@kzlsakal
Copy link
Collaborator

kzlsakal commented May 15, 2022

I pushed the branch where I replicated the issue here: https://github.com/kzlsakal/federation_poc/tree/break-composition

If you start the products service it will compile and start successfully, but then starting the gateway will produce the following error:

IntrospectAndCompose failed to update supergraph with the following error: A valid schema couldn't be composed. The following composition errors were found:
        [products] The directive "@deprecated" can only be used once at this location.
Error: A valid schema couldn't be composed. The following composition errors were found:
        [products] The directive "@deprecated" can only be used once at this location.

Here's how it looks like in the schema (mix absinthe.federation.schema.sdl --schema ProductsWeb.Schema):

type Product @key(fields: "upc") {
  upc: String!
  name: String!
  price: Int @deprecated(reason: "This field is truly deprecated") @deprecated(reason: "This field is deprecated")
}

Here's another example branch with duplicate directives causing the same error on the gateway:
https://github.com/kzlsakal/federation_poc/tree/break-composition-2

IntrospectAndCompose failed to update supergraph with the following error: A valid schema couldn't be composed. The following composition errors were found:
        [products] The directive "@requires" can only be used once at this location.
Error: A valid schema couldn't be composed. The following composition errors were found:
        [products] The directive "@requires" can only be used once at this location.

And here's how it looks like in the schema definition:

type Product @key(fields: "upc") {
  upc: String!
  name: String!
  price: Int @requires(fields: ["bar"]) @requires(fields: ["foo"])
}

@kdawgwilk
Copy link
Collaborator Author

Absinthe issue to add validation for this absinthe-graphql/absinthe#1177

@maartenvanvliet
Copy link

Absinthe 1.7.1 is released with the fix for this. When I run the break-composition branch link above I get the following message:

== Compilation error in file lib/products_web/schema.ex ==
** (Absinthe.Schema.Error) Compilation failed:
---------------------------------------
## Locations
Column 0, Line 20

Directive `deprecated' cannot be applied repeatedly.
---------------------------------------
## Locations
Column 0, Line 19

Directive `deprecated' cannot be applied repeatedly.

So Absinthe now handles this correctly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants