Skip to content

engine: tolerate unknown trusted doc ids in Allow mode#2512

Merged
tomhoule merged 1 commit intomainfrom
tomhoule-lxvrmpulowsp
Jan 20, 2025
Merged

engine: tolerate unknown trusted doc ids in Allow mode#2512
tomhoule merged 1 commit intomainfrom
tomhoule-lxvrmpulowsp

Conversation

@tomhoule
Copy link
Contributor

@tomhoule tomhoule commented Jan 20, 2025

In allow / audit mode, before this commit, we try to fetch the trusted document every time there is a trusted document id in the query, and we return an error if it doesn't exist. This is not in the spirit of allow mode: we have a setting to log, in that case. We should allow for a soft transition.

This commit changes the behaviour to try and fetch the trusted document from the trusted document id, but without hard error in case of failure, unless there is no inline document — there would be nothing to execute in that case, so we can't proceed to handle the request.

closes GB-8282

@linear
Copy link

linear bot commented Jan 20, 2025

@github-actions
Copy link

github-actions bot commented Jan 20, 2025

TestsPassed ✅Skipped ⏭️Failed ❌
Federation Audit Report179 ran168 passed0 skipped11 failed

@tomhoule tomhoule marked this pull request as ready for review January 20, 2025 07:17
@tomhoule tomhoule requested a review from a team as a code owner January 20, 2025 07:17
@tomhoule tomhoule requested a review from Finistere January 20, 2025 07:17
@tomhoule
Copy link
Contributor Author

This should really be tested, adding a test.

In allow / audit mode, before this commit, we try to fetch the trusted document every time there is a trusted document id in the query, and we return an error if it doesn't exist. This is not in the spirit of allow mode: we have a setting to log, in that case. We should be allow for a soft transition.

This commit changes the behaviour to try and fetch the trusted document from the trusted document id, but without hard error in case of failure, unless there is no inline document — there would be nothing to execute in that case, so we can't proceed to handle the request.

closes GB-8282
@tomhoule tomhoule force-pushed the tomhoule-lxvrmpulowsp branch from 8e9372a to 0948c72 Compare January 20, 2025 08:20
@tomhoule
Copy link
Contributor Author

This should really be tested, adding a test.

done

@tomhoule tomhoule merged commit 8bdb41b into main Jan 20, 2025
16 checks passed
@tomhoule tomhoule deleted the tomhoule-lxvrmpulowsp branch January 20, 2025 08:33
tomhoule added a commit that referenced this pull request Jan 20, 2025
## Features

### Websocket connection init payload forwarding

The gateway implements the [graphql-transport-ws
protocol](https://github.com/graphql/graphql-over-http/blob/main/rfcs/GraphQLOverWebSocket.md).
In that protocol, the first message on a websocket connection will be
connection_init. That message contains an optional payload field that is
a JSON object with arbitrary values. Typically, information that would
go in HTTP headers for queries and mutations flows instead in that
connection_init message's payload. Data like tokens that would otherwise
go into the `Authorization` header are passed there instead.

Before this release, the ConnectionInit payload was not forwarded to the
subgraph resolving the subscription. It is now forwarded by default.
That behaviour can be disabled with the following configuration:

```toml
[websockets]
forward_connection_init_payload = false
```

Docs are available at
https://grafbase.com/docs/reference/gateway/configuration/websockets

Implemented in #2508

## Improvements

### Trusted document incremental adoption

When `trusted_documents.enabled = true` and `trusted_documents.enforced
= false`, the gateway fetches the trusted document whenever a trusted
document id is present in the request. In 0.25.0 and 0.25.1, the gateway
would return a hard error when no trusted document exists with that id,
even when an inline document was in the query.

This is not in the spirit of audit mode: there is the
`document_id_unknown_log_level` setting to log, potentailly with error
level, to detect this problem, but it should not block the request. We
now allow for a soft transition: when no trusted document is found, but
an inline document is present, we will use the inline document to
execute the query. This is useful for organizations who want to
gradually adopt trusted documents.

Docs:
https://grafbase.com/docs/reference/gateway/configuration/trusted-documents

Implemented in #2512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants