bramble
bramble copied to clipboard
Allow @boundary enum, @boundary scalar, and @boundary input
Shared enums would be really useful to define standard values across services. For example:
enum Gender @boundary {
MALE
FEMALE
OTHER
UNKNOWN
}
Similarly, shared scalars would be really useful to standardize the encoding of dates and timestamps for example. For example:
scalar ISODate @boundary
Shared input types are a bit less useful, but one great use case is to standardize pagination:
input PageInput @boundary {
cursor: ID
pageSize: Int! = 100
}
I think the proper semantics for these new boundary types is that they each appear exactly once in the resulting merged schema. The schema merge operation would fail if the definitions of the boundary enums / scalars / inputs differ in any way. This does mean that changing the definition of any of these down the line requires a simultaneous upgrade of all the affected services, but I think that's a price worth paying.
Thoughts @Ackar @pkqk ?
For things like PageInput
I think the ability to have a shared bit of schema that you could include would help.
It also needs the ability to be defined in multiple services so will need @boundary
or similar directive.
Yeah I think reusing @boundary
is fine here.
Yeah having the same directive is a good idea I think, since the semantics are essentially the same.
There's two things here, we want to be able to have scalar
s, enum
s and input
s with the same type name in various services, and possibly share definitions to reduce duplication of definition and the associated drift.
In the scalar
case this is easy as it's just a statement that this type name exists.
In the enum
case we can union all the values from various services into the final enum, although this means that services could get values they are not expecting in their own subset of type system?
Same with input
types, if we do a union of the input
s fields then again one service can receive fields it doesn't expect.
It might be safer to have one service that defines the common types we use, mark these with a @boundary
directive, and then have a way to include this services schema into others as a common package?
In the enum case we can union all the values from various services into the final enum, although this means that services could get values they are not expecting in their own subset of type system?
I would only allow @boundary
enums to be defined as the exact same set of values in all services. The alternative causes more issues than is worth dealing with IMHO.
Same with input types, if we do a union of the inputs fields then again one service can receive fields it doesn't expect.
I was confused here for a sec, I thought you meant the GraphQL union, not the set union :)
I think we should take the more conservative approach of only allowing @boundary
inputs that are exactly the same in all services, just like enums.
and then have a way to include this services schema into others as a common package?
I don't think this is necessary. I think it's fine (and even preferable) for each service schema to be defined independently. Checking that a service's schema is mergeable in the graph should be done during CI.
My reasoning is there is already quite a bit of boilerplate that needs to be included in each service to join the gateway. eg.
directive @boundary on OBJECT
directive @namespace on OBJECT
interface Node {
id: ID!
}
type Service {
name: String! # unique name for the service
version: String! # any string
schema: String! # the full schema for the service
}
type Query {
service: Service!
node(id: ID!): Node
}
The locations which we have to update this if we start adding/changing things will start growing, it may even make it difficult to roll out an addition. Eg: If we add a value to an enum, then that services definition doesn't match the others so it won't be accepted into the gateway, meaning we'd have to pull all services out of the gateway that define it, deploy the new service, update all the other services that also use that enum and roll them out.
@pkqk I totally get your point that rolling out changes to @boundary
will be a tricky operation. I do believe, however, that centralizing the definition in one spot will make this worse and not better. The issue is that, by centralizing the enum definition, you get lured into a false sense of security - you may think that updating the one common definition is sufficient. In reality, though, each service will need to be carefully and individually reviewed when such a change takes place.
Summary from IRL discussion:
We decided for our use case we could handle the downtime of removing some services from the gateway and rolling out schema updates in a co-ordinated way.
We might later want to be able to do this in a highly available way, one idea we thought of was to have a way to tell bramble a schema change was acceptable and any new services joining with the change would be accepted.
FWIW, I wrote these sorts of validations for the Node GraphQL Tools schema stitching package, and landed on the following rules:
-
boundary scalars are fine as long as they’re treated as strings. Limiting custom scalars from appearing across serivces is prohibitively restrictive. These aren’t even really boundaries, IMO. We actually went so far as to allow scalar mappings, so that —say— an ID in the gateway could proxy a String in a subservice.
-
for boundary inputs, schema stitching traditionally allowed input divergence because inputs are always filtered to a subschema, and invalid input was dropped. We kept that for backwards compatibility, but added strong warnings. My preference would be to always require arguments and inputs to be consistent.
-
then, boundary enums with divergent values are fine in read-only contexts, meaning an enum is allowed divergent values as long as it never appears as an argument or input value. Once an enum is used as input, it must be consistent across services.
To distill this down to a basic 2.0 spec, my suggestion would be:
- Custom scalars are allowed to appear across services, and they shouldn’t require a boundary declaration. They’re just leaf values of a common type.
- Input objects should be verified consistent across services with no divergence allowed, IMO.
- Enum values SHOULD allow divergence… if for no other reason than the logistics of rolling out a new value across services one at a time. Apollo even broke on this logistical hurtle in Federation v2. A validation against divergent enums appearing as input values safeguards against practical repercussions.
@gmac wouldn't disallowing divergence in Input objects also making rolling out changes to the object one service at a time difficult. Our current plan is removing the affected services and adding all the new ones but it's not a method I'm happy with.
If one service adds a new field to an Input object, but it's being passed to a service that doesn't yet accept it, the two outcomes I can think of are:
- deny the request, even though it matches the merged schema
- remove the field values before passing to the service that doesn't recognize them
Both options could be surprising, though removing the fields is similar to your comment about divergent Enum values?
That’s a fair point on the input divergence also being blocking. I know GQL Tools handles this by always filtering inputs to the subservice schema, but I don’t love that the approach requires subservice schemas to be individually present for gateway operations (which works against https://github.com/movio/bramble/issues/99). Maybe the gateway composition could cache a minimal delta of fields that must be removed from specific services when this applies? Denying requests feels like a tricky outcome that could have small mistakes translate into large outages.