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

generateApolloSources fails with 4.0.1 and federation sub-graph graphqls #6221

Closed
xcq1 opened this issue Oct 25, 2024 · 11 comments
Closed

generateApolloSources fails with 4.0.1 and federation sub-graph graphqls #6221

xcq1 opened this issue Oct 25, 2024 · 11 comments

Comments

@xcq1
Copy link

xcq1 commented Oct 25, 2024

Version

4.0.1

Summary

I'm using Apollo client to run tests against one of my federation sub-graph servers. For this the server's .graphqls looks like this:

extend schema
@link(url: "https://specs.apollo.dev/federation/v2.4",
    import: ["@key", "@extends", "@external"])

type Query {
...
}

type Mutation {
...
}

This worked fine with 4.0.0, but it suddenly throws an error in 4.0.1. According to all the docs I can find, this is the way you're supposed to include federation directives. The server works perfectly fine with the Apollo Gateway, so I don't think there is an error in the rest of the schema. Using the SDL also seems to be the preferred schema definition.

Steps to reproduce the behavior

Bump dependency version from 4.0.0 to 4.0.1

Logs

Caused by: com.apollographql.apollo.ast.SourceAwareException: e: /home/runner/_work/my-service/service/server/src/main/resources/graphql/schema.graphqls: (2, 1): Apollo: unknown foreign schema 'federation/v2.4'
----------------------------------------------------
[1]:extend schema
[2]:@link(url: "https://specs.apollo.dev/federation/v2.4",
[3]:    import: ["@key", "@extends", "@external"])
----------------------------------------------------
	at com.apollographql.apollo.ast.IssueKt.checkEmpty(Issue.kt:162)
	at com.apollographql.apollo.compiler.ApolloCompiler.buildCodegenSchema(ApolloCompiler.kt:133)
	at com.apollographql.apollo.gradle.internal.GenerateSources.execute(SourceFile:19)
	at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
	at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:54)
	at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:48)
	at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:100)
	at org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:48)
	at org.gradle.workers.internal.IsolatedClassloaderWorker.run(IsolatedClassloaderWorker.java:49)
	at org.gradle.workers.internal.IsolatedClassloaderWorker.run(IsolatedClassloaderWorker.java:30)
	at org.gradle.workers.internal.IsolatedClassloaderWorkerFactory$1.lambda$execute$0(IsolatedClassloaderWorkerFactory.java:57)
	at org.gradle.workers.internal.AbstractWorker$1.call(AbstractWorker.java:44)
	at org.gradle.workers.internal.AbstractWorker$1.call(AbstractWorker.java:41)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:209)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
	at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:166)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
	at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
	at org.gradle.workers.internal.AbstractWorker.executeWrappedInBuildOperation(AbstractWorker.java:41)
	at org.gradle.workers.internal.IsolatedClassloaderWorkerFactory$1.execute(IsolatedClassloaderWorkerFactory.java:49)
	at org.gradle.workers.internal.DefaultWorkerExecutor.lambda$submitWork$0(DefaultWorkerExecutor.java:174)
	at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.runExecution(DefaultConditionalExecutionQueue.java:195)
	at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.access$700(DefaultConditionalExecutionQueue.java:128)
	at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner$1.run(DefaultConditionalExecutionQueue.java:170)
	at org.gradle.internal.Factories$1.create(Factories.java:31)
	at org.gradle.internal.work.DefaultWorkerLeaseService.withLocks(DefaultWorkerLeaseService.java:267)
	at org.gradle.internal.work.DefaultWorkerLeaseService.runAsWorkerThread(DefaultWorkerLeaseService.java:131)
	at org.gradle.internal.work.DefaultWorkerLeaseService.runAsWorkerThread(DefaultWorkerLeaseService.java:136)
	at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.runBatch(DefaultConditionalExecutionQueue.java:165)
	at org.gradle.internal.work.DefaultConditionalExecutionQueue$ExecutionRunner.run(DefaultConditionalExecutionQueue.java:134)
	... 2 more

@martinbonnin
Copy link
Contributor

martinbonnin commented Oct 25, 2024

Thanks for sending this 🙏 . This a side effect of #6164. Apollo Kotlin is now more strict about schema validation and throws on unknown directives.

Since federation is transparent from a client point of view, I didn't add builtin support for the federation directives (unlike the nullability directives for an example).

The immediate fix for this issue is to use the API schema SDL instead of the supergraph schema SDL.

Something else we could consider is to ignore the federation directives altogether but that might become an issue the day we want to use @key to auto-configure the normalized cache for an example (we are very far from that but this has been mentioned before). I'll discuss this with the team.

In the meantime, can you try using the API schema? Is it an option for you?

@martinbonnin
Copy link
Contributor

Hey @xcq1 👋 Did you get any luck with the API schema? We have a version around the corner and I'm trying to gauge how much of an issue this is.

@xcq1
Copy link
Author

xcq1 commented Nov 4, 2024

Thanks for the quick reply, and sorry for my delayed response.

I don't think using an API SDL would be straightforward here. What I'm using here is not the supergraph definition, this is one subgraph definition in one spring-graphql server (of many) that is part of a federation. Apollo-kotlin is used in the integration tests in the CI pipeline. If I understand correctly, to get an API schema, I would need to start an ephemeral federation gateway instance with the new SDL polled in and then mock/prevent calls to other services. This seems a little bit excessive.

If there was no other way, I think I'd probably rather port the old test code over to spring-graphql's testing solution, which appears to work fine with federation annotations.
The easiest alternative would be the fix from your side, so I'll wait until I can test with that.

@martinbonnin
Copy link
Contributor

👋

What I'm using here is not the supergraph definition, this is one subgraph definition in one spring-graphql server (of many) that is part of a federation.

May I ask how you get that SDL file? Even subgraphs should have different versions of the schema. There is:

  1. "input" schema (what the backend engineer writes and is processed by spring)
  2. "output" schema (the results of spring processing the input schema and that is exposed to the frontend)

(see also the full schema RFC)

What you ideally need is a way to get 2. Introspection is a natural way to do that do you have an endpoint you can introspect maybe?

@xcq1
Copy link
Author

xcq1 commented Nov 4, 2024

To clarify, our current workflow looks like this:

  • Write the schema.graphqls file server-side where
    • the code mapping is validated by spring-graphql and
    • code and schema are tested by doing real-world requests, either with plain HTTP requests or apollo-kotlin or spring-graphql
  • During CI build of the PR the schema is checked with rover subgraph check <graph_name> --schema <local_schema_file> --name <service_name>
  • When the PR is merged, CI pushes the schema with rover subgraph publish <graph_name> --schema <local_schema_file> --name <service_name> --routing-url <deployed_server_url>
  • The schema for clients is served exclusively by a separately deployed @apollo/gateway
  • Clients then use the gateway for introspection by doing rover graph introspect <gateway_url> -H <headers> > .apollo/federated-schema.graphqls

So we essentially never come into contact with subgraph introspection. I looked into it and found spring-graphql actually provides a setting spring.graphql.schema.printer.enabled that is off by default, but allows downloading the schema including directives from <server_url>/graphql/schema.

I'm open to practical suggestions, however, I'm not sure how easy it would be to configure the schema for apollo-kotlin in the build.gradle.kts from there. The testClasses requiring a download from a built artifact sounds like unnecessary complexity and potential increase in build time, or the introspection schema would need to be checked into the repo as a redundant duplicate that can potentially go out of sync during conflicts.

I'm not sure which schema type is the one actually used with Rover/Federation? Of course it'd be better to test with that one, but I don't know of any significant difference between the two that would warrant that additional effort.

@martinbonnin
Copy link
Contributor

Thanks for the details!

spring.graphql.schema.printer.enabled

The GraphQL schema can be viewed over HTTP at "/graphql/schema"

Interesting that this requires a network call. I would have expected a build time artifact, like graphql-kotlin is doing

The testClasses requiring a download from a built artifact sounds like unnecessary complexity and potential increase in build time

Agreed. Let me dig a bit more into Spring/Rover/Federation, see if there's any solution and I'll report back.

@martinbonnin
Copy link
Contributor

Hi @xcq1 👋

As a mitigation solution until the community gets to the bottom of this problem, I just released apollo-kotlin-ffs (Federation Foreign Schemas): https://github.com/apollographql/apollo-kotlin-ffs

This Apollo Compiler Plugin patches the Apollo Kotlin compiler so that it knows about the federation directives. You can use it like so:

apollo {
  service("service") {
    packageName.set("com.example")

    plugin("com.apollographql.ffs:apollo-kotlin-ffs:0.0.0")
  }
}

Let me know how that works for you!

@martinbonnin martinbonnin removed this from the 4.1.0 milestone Nov 5, 2024
@xcq1
Copy link
Author

xcq1 commented Nov 5, 2024

Hey, thanks for the quick assistance. I can confirm this makes my tests work again with 4.0.1. The 0.0.0 is a bit spooky, but I guess I'll have to keep an eye on what the long-term solution will look like anyway. Something tells me it might not have this name, but you made me chuckle 😄.

@martinbonnin
Copy link
Contributor

As a software developer, I figured out version numbers should be 0-indexed like everything else so I'm trying to start a trend 😄 . Just missed Halloween by a couple of days 🎃

I believe the long term solution is for server frameworks to have support for both SDL variants and formalize that in the spec. That might take a bit of time, let's see 🤞

@martinbonnin
Copy link
Contributor

I'll close this one and redirect to the full schemas RFC in case anyone is interested to join the discussion in the graphql working group.

Copy link
Contributor

github-actions bot commented Nov 5, 2024

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Kotlin usage and allow us to serve you better.

@martinbonnin martinbonnin removed the ⌛ Waiting for info More information is required label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants