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

Federation: support custom directives in federated schemas #145

Closed
lennyburdette opened this issue Jun 4, 2019 · 24 comments
Closed

Federation: support custom directives in federated schemas #145

lennyburdette opened this issue Jun 4, 2019 · 24 comments

Comments

@lennyburdette
Copy link

In @apollo/[email protected], the printSchema function throws away SDL directives that aren't @deprecated/@key/@external/@extends/@requires/@provides.

This prevents the federation gateway from knowing about directives via the _Service.sdl field.

Conveying custom directives from federated schemas to the gateway is the first step to supporting centralized logic in the gateway for functionality like unified authorization.

Thanks for the hard work!

@ausov
Copy link

ausov commented Jun 5, 2019

If isSpecifiedDirective means "custom" directives, then looks like it should it be other way round here:

https://github.com/apollographql/apollo-server/blob/ce459ce979aacf8f3486804841127d7244646f03/packages/apollo-federation/src/service/printFederatedSchema.ts#L58

Should be .filter(n => isSpecifiedDirective(n)); , right?

@lennyburdette
Copy link
Author

@ausov From what I can tell, there's some inconsistency in how directives are handled in this module.

The filtered directives in the line you're referencing are used to print the directive declarations like directive @foo on FIELD_DEFINITION. It filters out @skip/@include/@deprecated and @extends/@external/@key/@requires/@provides, printing out only custom schema directive declarations.

However, in functions like printObject and printFields, only @deprecated and the federation directives are printed.

So custom schema directives are declared, but never included where they're used!

@trevor-scheer
Copy link
Member

@lennyburdette thanks for bringing this discussion over to GitHub! As @jbaxleyiii mentioned in the original thread, we have some questions to think through about service and gateway level directives.

We would love to hear ideas from the community while we flesh out our understanding of the problem and what the right solution might be. Providing use cases will help us to drive patterns as we design support for this. Thanks for opening up the discussion 😄

@3axap4eHko
Copy link

3axap4eHko commented Jun 21, 2019

@trevor-scheer In my case I was using directive @auth(roles: [ROLE]) on FIELD_DEFINITION to control access for specific fields

type Query {
  someQuery: Result
  someOtherQuery(id: ID!): Result @auth(roles: [ADMIN, MODERATOR])
}

type Mutation {
  someMutation(id: ID!): Result @auth(roles: [ADMIN, MODERATOR])
}

Any thoughts about such drive pattern?

@lennyburdette
Copy link
Author

Here's another use case. I'm talking to a team that's concerned about restricting access to slow/brittle endpoints. They currently use service-to-service ACLs and rate-limiting, but in a federated graph, all requests would come from the same service (the gateway.)

I'm thinking the right proposal would be client-based rate limiting. It'd be great to declare rate limit rules on the fields.

extend type Query {
  slowSearchField(term: String!): [Thing] @rateLimit(qps: 10)
}

extend type Parent {
  poorlyIndexedField: [Result] @rateLimitRule(name: "key.for.rule.in.another.system")
}

If the gateway could analyze the rate limiting directives and drop fields or whole requests to the federated schemas would be 💯

@codyspate
Copy link

codyspate commented Jul 3, 2019

Can we just allow schema directives to be created and if a custom directive is trying to replace a federation directive just throw an error, unless you are extending one of the federation directives?

edit: I need this functionality sooner rather than later so just throwing out some ideas

@jbaxleyiii jbaxleyiii transferred this issue from apollographql/apollo-server Jul 8, 2019
@3axap4eHko
Copy link

3axap4eHko commented Jul 16, 2019

@trevor-scheer what if directives will be excluded only from _Service.sdl resolver but kept in introspection response and server itself?

@avocadoslab
Copy link

We started implementing federation for our service and hit this exact roadblock.

@nazar
Copy link

nazar commented Aug 1, 2019

Is there a rough ETA on when we could expect seeing custom directive support?

We are about to start a microservice oriented graphQL system and the lack of directive support is a roadblock for us, which is unfortunate as federated schemas tick so many boxes for us.
Having the Schema stitching document marked as deprecated is also adding an element of uncertainty for us.

Having a rough indication of when we could expect directive support - i.e. weeks or six months would greatly help in our architecture choices 🙏

@edelreal
Copy link

edelreal commented Aug 9, 2019

Our use case is to leverage custom directives to wrap either some or all resolvers fields in an object with some authorization code that implements access control and decides whether the original resolver will be run.

The typeDefs SDL DocumentNode we use in our current non-federated service looks something like this:

directive @authorize (
  requires: ResourcePrivilege = INTERNAL_OPERATIONS_PRIVILEGE,
) on OBJECT | FIELD_DEFINITION

type Query @authorize(requires: ACCOUNT_OPERATIONS_PRIVILEGE) {
  users: [User]
  customers: [Customer]
}

type Mutation @authorize(requires: ACCOUNT_OPERATIONS_PRIVILEGE) {
  addCustomerUser(customerId: Int!, input: AddCustomerUserInput! ): AddCustomerUserResult

  deleteCustomer(customerId: Int! ): String @authorize(requires: SYSTEM_OPERATIONS_PRIVILEGE)
}

When I attempt to convert our service to Apollo Federation, and import a DocumentNode (using babel-plugin-import-graphql), if it has directives, then buildFederatedSchema gives me an error like this one:

api_server/node_modules/graphql/validation/rules/KnownDirectives.js:86
        throw _iteratorError2;
        ^

TypeError: Cannot read property 'kind' of undefined
    at KnownDirectives (api_server/node_modules/graphql/validation/rules/KnownDirectives.js:70:15)
    at api_server/node_modules/graphql/validation/validate.js:63:12
    at Array.map (<anonymous>)
    at Object.validateSDL (api_server/node_modules/graphql/validation/validate.js:62:24)
    at Object.buildSchemaFromSDL (api_server/node_modules/apollo-graphql/src/schema/buildSchemaFromSDL.ts:69:18)
    at buildFederatedSchema (api_server/node_modules/@apollo/federation/src/service/buildFederatedSchema.ts:30:16)
    at Object.<anonymous> (api_server/api/schema.external.js:47:40)
    at Module._compile (internal/modules/cjs/loader.js:688:30)
    at Module._compile (api_server/node_modules/pirates/lib/index.js:99:24)
    at Module._extensions..js (internal/modules/cjs/loader.js:699:10)
    at Object.newLoader [as .js] (api_server/node_modules/pirates/lib/index.js:104:7)
    at Module.load (internal/modules/cjs/loader.js:598:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
    at Function.Module._load (internal/modules/cjs/loader.js:529:3)
    at Module.require (internal/modules/cjs/loader.js:636:17)
    at require (internal/modules/cjs/helpers.js:20:18)

I also tried defining my DocumentNode using a gql-tagged string instead:

    typeDefs: gql`
      directive @audit on OBJECT | FIELD_DEFINITION
      type Query @audit {
        test: String
      }
    `,
    resolvers: {
      Query: {
        test: () => 'Hello, World!',
      },
    },

and passed the directives and the federated schema to ApolloServer, something like this:

const apiServer = new ApolloServer({
  schema: testFederatedSchema,
  schemaDirectives,
})

, and while this ran my resolver without an error, it did not run the custom directive.

Lack of support for custom directives in the upstream server means converting our current service into a federated service (that can be used by a gateway) will require a decent amount of work (since it would require changing our authorization implementation).

While I don't think custom directives need to live at the gateway level (see https://medium.com/@__xuorig__/the-rise-of-graphql-overambitious-api-gateways-2c6f054e80a1), I do think they need to continue working at the individual upstream services if those services were already using directives, and we convert them to be usable by a gateway.

@trevor-scheer
Copy link
Member

Thanks everyone for all of your suggestions, use cases, and general input - it's much appreciated! Going forward, we plan to support schema directives for individual services in the future, but we do not have any plans to add this support at the gateway level.

Though not listed on the issue itself, this work is part of the Apollo Server 3.0 roadmap. Schema construction is undergoing changes as we move away from graphql-tools, and custom schema directives will be one of the things that we address at that time.

For those asking about timeframe, I can only speculate that it will be on the order of months. The work for AS3 is a top priority, however we have a lot to accomplish. There will be a time where we would (gladly!) accept and provide guidance for a contribution from the community, but we're not quite there yet. Thanks everyone for your participation and patience - I know this is exciting stuff 😄

@lethot
Copy link

lethot commented Sep 5, 2019

for those who cannot get directives running at federated service level, you can use mergeSchemas :

const federatedSchema = buildFederatedSchema([{
  typeDefs,
  resolvers,
}]);

const schema = mergeSchemas({
  schemas: [federatedSchema],
  schemaDirectives: {
    ...
  }
});

export const server = new ApolloServer({
  schema,

@cosmarc
Copy link

cosmarc commented Dec 29, 2019

Is this what are you looking for? It was released with v2.9.15. It supports just ExecutableDirectiveLocation for now though.

@trevor-scheer
Copy link
Member

Closing this, as executable directives are now supported via apollographql/apollo-server#3464 and service-level type system directives are permitted via apollographql/apollo-server#3536.

@coyotebush
Copy link

apollographql/apollo-server#3736 makes the gateway tolerate usages of type system directives in service schemas. However, federated services still don't return these usages from _Service.sdl, as discussed in the first few comments on this issue, so there is no chance for any implementation of a gateway to read them. I think that's because of this filter. Could that filter be removed now that removing it won't break the gateway?

@sponomarev
Copy link

sponomarev commented Apr 3, 2020

For all the interested parties: there is no issue using schema (aka Type System) directives on the federated services side except the inconvenient API and knotty internals of all the GraphQL tooling.

  1. A printSchema function is intended to remove schema directives. Here is the original motivation. Modified printFederatedSchema is used exactly to overcome this limitation because Federated Gateway uses federation directives to compose the final schema. So, don't expect to find your custom directives in SDL.

  2. The straightforward approach doesn't work since we provide a ready GraphQLSchema object in schema: argument. It uses it as-is and ignores schemaDirectives: completely.

// That won't enable schema directives
const server = new ApolloServer({
  schema: buildFederatedSchema([{ typeDefs, resolvers }]),
  schemaDirectives,
});

So, we should attach them directly to the schema.

Thanks @lethot, he found a workaround with mergeSchemas, but it's still a workaround and it's not idiomatic.

A better and conventional way is to use SchemaDirectiveVisitor.visitSchemaDirectives.

Here is an example:

// That enables schema directives on the federated service side

const schema = buildFederatedSchema([{ typeDefs, resolvers }])

SchemaDirectiveVisitor.visitSchemaDirectives(schema, schemaDirectives);

const server = new ApolloServer({
  schema: schema,
})

@ilmimris
Copy link

ilmimris commented May 4, 2020

Should we add schema directive to every federated schema (service) even if the service does not need the schema directive ?

@robross0606
Copy link

robross0606 commented Aug 26, 2020

This still does not appear to be working on input types as per @sponomarev suggestion. The schema indeed gets modified to show the name of the custom directive. I can set breakpoints and even see that the custom type instantiated by the SchemaDirectiveVisitor is created. And I can see that the constructor for that custom type is passing up implementations for serialize, parseValue and parseLiteral. However, none of those functions are ever hit by Apollo when submitting input. They're completely ignored.

@sponomarev
Copy link

@ilmimris No, you don't. Schema directives transform types, fields, etc. The affect only the current service.

@sponomarev
Copy link

@robross0606 Sorry, can't say anything specific without seeing the code you're talking about.

@robross0606
Copy link

robross0606 commented Aug 26, 2020

@sponomarev , I'm attempting to tie in the apollo-server-constraint-directive library to our service which is federated. The basic steps are as indicated in their README, except instead of calling makeExecutableSchema() I'm doing:

const { buildFederatedSchema } = require('@apollo/federation')
const { SchemaDirectiveVisitor } = require('graphql-tools')
const federatedSchema = buildFederatedSchema([typeDefsAndResolvers])
SchemaDirectiveVisitor.visitSchemaDirectives(federatedSchema, schemaDirectives)

There are no errors from this, and the resulting schema looks for all the world like it should work. Schema introspection shows the correct types as per the library in places where I've added constraints. So, for example, if add an @constraint(minLength: 5) to a String! field, I can see in debug mode that the original type is "wrapped" by the validating type StringType:

class ConstraintDirective extends SchemaDirectiveVisitor {
  visitInputFieldDefinition(field) {
    this.wrapType(field);
  }

  visitFieldDefinition(field) {
    this.wrapType(field);
  }

  wrapType(field) {
    const fieldName = field.astNode.name.value;
    const type = field.type.ofType || field.type;
    const isNotNull = (field.type instanceof GraphQLNonNull);
    const isScalarOfTypeString = (type === GraphQLString);
    const isScalarOfTypeNumber = (type === GraphQLInt || type === GraphQLFloat);

    if (!isScalarOfTypeString && !isScalarOfTypeNumber) {
      throw new Error(`Not a scalar of type ${type}`);
    }

    if (isScalarOfTypeString) {
      field.type = new StringType(fieldName, type, this.args);
    }

    if (isScalarOfTypeNumber) {
      field.type = new NumberType(fieldName, type, this.args);
    }

    if (isNotNull) {
      field.type = new GraphQLNonNull(field.type);
    }
  }
}

The constructor for StringType looks like this:

class StringType extends GraphQLScalarType {
  constructor(fieldName, type, args) {
    super({
      name: 'ValidateString',
      serialize(val) {
        const value = type.serialize(val);
        validate(fieldName, args, value);
        return value;
      },
      parseValue(val) {
        const value = type.parseValue(val);
        validate(fieldName, args, value);
        return type.parseValue(value);
      },
      parseLiteral(ast) {
        const value = type.parseLiteral(ast);
        validate(fieldName, args, value);
        return value;
      },
    });
  }
}

So, I can set breakpoints in debug mode and see that the correct type is created and "wraps" the original type, and I can see that the GraphQL type in introspection becomes "ValidatedString". However, making a call to the GraphQL never appears to pass through the serialize, parseValue or parseLiteral functions declared on that StringType. I've set breakpoints and they're never hit. I've added console log points and those are never hit.

I'm baffled as to why.

@makaivelli
Copy link

@robross0606 We're also using Federation and faced the same issue when I tried to add validation with graphql-constraint-directive. Did you manage to make them work together by any chance?

@robross0606
Copy link

robross0606 commented Sep 21, 2020

@makaivelli, I did not end up using apollo-server-constraint-directive but did end up using graphql-constraint-directive. That library appears to work a bit differently. IMO, this is a big problem with Apollo since they keep changing the methodology for doing the same things which causes confusion around these topics.

Here's a simplified example of how I ended up using it:

const { mergeTypeDefs, mergeResolvers } = require('@graphql-tools/merge')
const { constraintDirective, constraintDirectiveTypeDefs } = require('graphql-constraint-directive')

const schemaTypeDefs = require('./schema')
const allResolvers= require('./resolvers')

const directiveTransform = constraintDirective()
const directiveTypeDef =  schemaDef: gql`${constraintDirectiveTypeDefs}`

const allTypeDefs = mergeTypeDefs([{ schemaDefs: directiveTypeDef  }, schemaTypeDefs], { all: true })

const typeDefsAndResolvers = {
  typeDefs: allTypeDefs,
  resolvers: allResolvers
}

let federatedSchema = buildFederatedSchema([typeDefsAndResolvers])
federatedSchema = directiveTransform(federatedSchema)

module.exports = {
  federatedSchema,
  schemaDirectives
}

So, there are two parts to the directive: the directive typedefs and the directive implementation/handlers. The implementation is the bizarre part since it is "applied" by calling constraintDirective() which returns a "transform" function. That function needs to be called against the federated schema.

Not sure if this is the only (or even correct) way to use it, but it appears to work for us.

@sbilello
Copy link

Is there a way to integrate the https://github.com/teamplanes/graphql-rate-limit at apollo/gateway level?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests