graphile / federation

Apollo federation support for PostGraphile [UNMAINTAINED]
MIT License
43 stars 12 forks source link

Federating two postgraphile endpoints with Apollo Federation results in "Query.query can only be defined once" error with Relay support or "Union type _Entity" error without Relay support #8

Open cedricvidal opened 4 years ago

cedricvidal commented 4 years ago

Hello guys,

I'm trying to federate two postgraphile endpoints with the Apollo Federation gateway but can't seem to find a working combination.

Postgraphile with both Federation and Relay support

Given I started the postgraphile endpoints with Federation support (using FederationPlugin) and Relay support (default NodePlugin activated), when I start the Apollo Federation gateway, it complains with the following error:

(node:19865) UnhandledPromiseRejectionWarning: GraphQLSchemaValidationError: Field "Query.query" can only be defined once.

Field "Query.nodeId" can only be defined once.

Field "Query.node" can only be defined once.
    at ApolloGateway.createSchema (/home/cvidal/Documents/Sources/pocs/postgraphile/federation/node_modules/@apollo/gateway/dist/index.js:210:19)
    at ApolloGateway.<anonymous> (/home/cvidal/Documents/Sources/pocs/postgraphile/federation/node_modules/@apollo/gateway/dist/index.js:181:32)
    at Generator.next (<anonymous>)
    at fulfilled (/home/cvidal/Documents/Sources/pocs/postgraphile/federation/node_modules/@apollo/gateway/dist/index.js:5:58)
    at Object.dynatraceRegularInvoke [as doInvoke] (/opt/dynatrace/oneagent/agent/res/nodeagent/nodejsagent.js:1732:20)
    at Object.a.safeInvoke (/opt/dynatrace/oneagent/agent/res/nodeagent/nodejsagent.js:1802:29)
    at /opt/dynatrace/oneagent/agent/res/nodeagent/nodejsagent.js:6952:25
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

I investigated a bit and looking at the Apollo Federation demo here https://github.com/apollographql/federation-demo, the federated services don't expose node and nodeId fields in the Query type.

Postgraphile with Federation support but without Relay support

I therefore tested starting the two postgraphile endpoints without Relay support (skipping the NodePlugin plugin) but the FederationPlugin plugin fails with :

Error: Union type _Entity must define one or more member types.
    at assertValidSchema (/home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/graphql/type/validate.js:71:11)
    at Object.validate (/home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/graphql/validation/validate.js:54:35)
    at parseQuery (/home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/postgraphile/src/postgraphile/http/createPostGraphileHttpRequestHandler.ts:352:32)
    at /home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/postgraphile/src/postgraphile/http/createPostGraphileHttpRequestHandler.ts:690:55
    at Array.map (<anonymous>)
    at requestHandler (/home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/postgraphile/src/postgraphile/http/createPostGraphileHttpRequestHandler.ts:650:20)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

Apparently, this is a known problem in Apollo Federation that Relay is not supported: https://spectrum.chat/apollo/apollo-federation/federation-with-relays-node-interface~c0ae3cb1-d243-491a-ac58-17307629e31e

Conclusion

So, this problem may be spread between Postgraphile and Apollo Federation Gateway.

That being said, I can't find a working combination so far, with or without Relay support.

phryneas commented 4 years ago

Heyo, you're entering experimental territory there :)

"Postgraphile with Federation support but without Relay support" should be working if you try it with #3 (Feedback very much appreciated).

For the "Postgraphile with both Federation and Relay support", I want to get a PR into graphile-engine, so that you can disable and/or rename the fields "Query.nodeId" and "Query.node" per service. I hope to do that the coming Weekend, but cannot make any guarantees.

cedricvidal commented 4 years ago

Hey, thank you for the quick feedback!

Postgraphile with Federation support but without Relay support

I'll give a look at your PR #3 and give you feedback as soon as I get the chance.

Postgraphile with both Federation and Relay support

Not sure renaming "Query.nodeId" and "Query.node" is a good idea. It may break the Relay support at the gateway level. It may actually be more the responsibility of the federation gateway to handle that use case.

There is an attempt here at fixing apollo-federation to support relay: https://github.com/victorandree/apollo-federation-relay

phryneas commented 4 years ago

Apollo-Federation-Relay looks very interesting.

The following is based on these two conversations if someone seeing this want to read them up:

As a result of the second discussion, I think you have two options. Let me explain: Currently, the nodeId is generated in getNodeIdForTypeAndIdentifiers. It usually looks like base64(JSON.stringify([tableName,primaryKey1,...])). The following method getTypeAndIdentifiersFromNodeId is reverting that process. Both those methods are added to the builder object in the build phase. Actually, by default, the above would be base64(JSON.stringify([typeName,primaryKey1,...])) (note: typeName, not tableName) instead, but this is changed (due to compatiblity with postgraphile 3 in the PgNodeAliasPostGraphile plugin. Unfortunately, the getNodeAlias/setNodeAlias methods used there can only modify the first element of the JSON-encoded array, not the structure itself.

So, from that realization, you have two methods going forward:


export default (function NodePlugin(
  builder
) {

  builder.hook(
    "build",
    (build) => {
      return build.extend(
        build,
        {
          getNodeIdForTypeAndIdentifiers(Type, ...identifiers) {
            return base64(
              `${Type}:`+JSON.stringify([this.getNodeAlias(Type), ...identifiers])
            );
          },
          getTypeAndIdentifiersFromNodeId(nodeId) {
            const decoded = base64Decode(nodeId);
            const [alias, ...identifiers] = JSON.parse(decoded.slice(decoded.indexOf(':')));
            return {
              Type: this.getNodeType(alias),
              identifiers,
            };
          },
        },
        `Describe what you are doing`
      );
    },
    ["Your plugin name"],
    [],
    ["Node"] // run this AFTER "Node"
  );

});

As you are already experimenting with this, it would be really cool if you tried one (or both) of those ways and reported back!

cedricvidal commented 4 years ago

Thank you @phryneas for this very detailed step by step explanation! This is very helpful! I'll try to test this this week, I feel bad since your explanation is so detailed but I have design docs to write first. I will let you know!

phryneas commented 4 years ago

No worries, Open Source takes time :) I just gave you the full rundown immediately because later I'd have probably forgotten about it ^^

stlbucket commented 4 years ago

i have just encountered this same problem, i think. tho i am not using the node plugin.

@phryneas - i could put my schemas in a repo for you if that would help. not sure it's adding anything tho.

i have not tried to dig into the issue deeper - maybe it's time for me to learn something!

this occurs when i have the FederationPlugin included, whether i try to use the apollo gateway or whether i just try to hit graphiql from the server.

if i remove the plugin, then graphiql finds my schema just fine.

stlbucket commented 4 years ago

my stack trace is different, tho:

Error: Union type _Entity must define one or more member types.
    at assertValidSchema (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/type/validate.js:71:11)
    at Object.validate (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/validation/validate.js:54:35)
    at parseQuery (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:254:48)
    at /Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:547:63
    at Array.map (<anonymous>)
    at requestHandler (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:510:52)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)
phryneas commented 4 years ago

I'm on the road so I can take a look only in a few days, but then your schemas could be interesting. It sounds to me like you are not exposing any types. You need to expose at least one to avoid that error -- Sent from my Android phone with K-9 Mail. Please excuse my brevity.

stlbucket commented 4 years ago

i thought that might be the case so i've done some more investigating. my original db had two schemas - one with jwt_token and an id generator function, and another with one table. i've consolidated all into one schema (below). i now get a slightly different error (also below). the overall behaviour is the same - postgraphile works fine w/o the federation plugin.

dbscript;

create extension if not exists pgcrypto;
drop schema if exists iot cascade;
create schema iot;
grant usage on schema iot to app_anonymous;
ALTER SCHEMA iot OWNER TO app;

--
-- Name: generate_ulid(); Type: FUNCTION; Schema: iot; Owner: app
--

CREATE FUNCTION iot.generate_ulid() RETURNS text
    LANGUAGE plpgsql
    AS $$
DECLARE
  -- Crockford's Base32
  encoding   BYTEA = '0123456789ABCDEFGHJKMNPQRSTVWXYZ';
  timestamp  BYTEA = E'\\000\\000\\000\\000\\000\\000';
  output     TEXT = '';

  unix_time  BIGINT;
  ulid       BYTEA;
BEGIN
  -- 6 timestamp bytes
  unix_time = (EXTRACT(EPOCH FROM NOW()) * 1000)::BIGINT;
  timestamp = SET_BYTE(timestamp, 0, (unix_time >> 40)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 1, (unix_time >> 32)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 2, (unix_time >> 24)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 3, (unix_time >> 16)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 4, (unix_time >> 8)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 5, unix_time::BIT(8)::INTEGER);

  -- 10 entropy bytes
  ulid = timestamp || gen_random_bytes(10);

  -- Encode the timestamp
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 0) & 224) >> 5));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 0) & 31)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 1) & 248) >> 3));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 1) & 7) << 2) | ((GET_BYTE(ulid, 2) & 192) >> 6)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 2) & 62) >> 1));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 2) & 1) << 4) | ((GET_BYTE(ulid, 3) & 240) >> 4)));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 3) & 15) << 1) | ((GET_BYTE(ulid, 4) & 128) >> 7)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 4) & 124) >> 2));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 4) & 3) << 3) | ((GET_BYTE(ulid, 5) & 224) >> 5)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 5) & 31)));

  -- Encode the entropy
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 6) & 248) >> 3));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 6) & 7) << 2) | ((GET_BYTE(ulid, 7) & 192) >> 6)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 7) & 62) >> 1));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 7) & 1) << 4) | ((GET_BYTE(ulid, 8) & 240) >> 4)));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 8) & 15) << 1) | ((GET_BYTE(ulid, 9) & 128) >> 7)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 9) & 124) >> 2));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 9) & 3) << 3) | ((GET_BYTE(ulid, 10) & 224) >> 5)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 10) & 31)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 11) & 248) >> 3));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 11) & 7) << 2) | ((GET_BYTE(ulid, 12) & 192) >> 6)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 12) & 62) >> 1));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 12) & 1) << 4) | ((GET_BYTE(ulid, 13) & 240) >> 4)));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 13) & 15) << 1) | ((GET_BYTE(ulid, 14) & 128) >> 7)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 14) & 124) >> 2));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 14) & 3) << 3) | ((GET_BYTE(ulid, 15) & 224) >> 5)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 15) & 31)));

  RETURN output;
END
$$;

CREATE TYPE iot.jwt_token AS (
    role text,
    app_user_id text,
    app_tenant_id text,
    token text
);

CREATE TABLE iot.thing (
    id text DEFAULT iot.generate_ulid() NOT NULL,
    app_tenant_id text NOT NULL,
    created_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL,
    updated_at timestamp with time zone NOT NULL,
    name text not null,
    CONSTRAINT ck_thing_name CHECK ((name <> ''::text))
);

CREATE FUNCTION iot.fn_timestamp_update_thing() RETURNS trigger
    LANGUAGE plpgsql
    AS $$
  BEGIN
    NEW.updated_at = current_timestamp;
    RETURN NEW;
  END; $$;
ALTER FUNCTION iot.fn_timestamp_update_thing() OWNER TO app;
CREATE TRIGGER tg_timestamp_update_thing BEFORE INSERT OR UPDATE ON iot.thing FOR EACH ROW EXECUTE PROCEDURE iot.fn_timestamp_update_thing();

grant all on iot.thing to app_anonymous;

current error trace:

Error: Union type _Entity must define one or more member types.
    at assertValidSchema (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/type/validate.js:71:11)
    at Object.validate (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/validation/validate.js:54:35)
    at parseQuery (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:254:48)
    at /Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:547:63
    at Array.map (<anonymous>)
    at requestHandler (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:510:52)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)
Error: Union type _Entity must define one or more member types.
    at assertValidSchema (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/type/validate.js:71:11)
    at Object.validate (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/validation/validate.js:54:35)
    at parseQuery (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:254:48)
    at /Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:547:63
    at Array.map (<anonymous>)
    at requestHandler (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:510:52)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)
stlbucket commented 4 years ago

figured it out...

not that i shouldn't have done so for other reasons... but you have to have a primary key on your table in order for the Node interface to implemented in order for it to be included in federation types...

... so, my bad...

and now i am back to the first error:

(node:98342) UnhandledPromiseRejectionWarning: GraphQLSchemaValidationError: Field "Query.query" can only be defined once.

Field "Query.nodeId" can only be defined once.

Field "Query.node" can only be defined once.
    at ApolloGateway.createSchema (/Users/buckfactor/node_modules/@apollo/gateway/dist/index.js:207:19)
    at ApolloGateway.<anonymous> (/Users/buckfactor/node_modules/@apollo/gateway/dist/index.js:178:32)
    at Generator.next (<anonymous>)
    at fulfilled (/Users/buckfactor/node_modules/@apollo/gateway/dist/index.js:5:58)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)
(node:98342) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:98342) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
stlbucket commented 4 years ago

https://github.com/apollographql/federation-jvm/issues/20#issuecomment-515556164

this sounds like it might be a bug in the gateway itself

phryneas commented 4 years ago

"Query.query" can only be defined once.

Field "Query.nodeId" can only be defined once.

Field "Query.node" can only be defined once.

You are getting these because multiple microservices are defining those and federation cannot decide which one to use.

The apollo-federation-relay package linked above might be a fix for is, but it won't work out-of-the box with postgraphile. My comment above explains how to possibly get around that.

By the way, @cedricvidal is there any progress from your side? :)

stlbucket commented 4 years ago

@phryneas - thanks, and i'll try that out when i get back to this (hopefully this weekend), unless it moves forward by other paths.

Pinqvin commented 4 years ago

For people who don't depend on the node and nodeId support for their queries, you can bruteforce solve this issue by just removing the conflicting fields:

import { Plugin } from "graphile-build";
import { omit } from "lodash";

export default (function RemoveNodeAndQueryFieldsPlugin(builder) {
  builder.hook("GraphQLObjectType:fields", (fields, _, { Self }) => {
    if (Self.name !== "Query") {
      return fields;
    }

    return omit(fields, ["node", "nodeId", "query"]);
  });
} as Plugin);

We need to remove the Node interface as well:

import { Plugin } from "graphile-build";

export default (function StripNodeInterfacePlugin(builder) {
  builder.hook("GraphQLObjectType:interfaces", interfaces => {
    return interfaces.filter(int => int.name !== "Node");
  });
} as Plugin);

These two plugins should allow you to federate your services. Obviously this doesn't really help you if you need that functionality