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

Move persisted queries from preview to general availability #3914

Merged
merged 1 commit into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changesets/feat_glasser_persisted_queries_ga.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
### Move Persisted Queries to General Availability ([PR #3914](https://github.com/apollographql/router/pull/3914))

[Persisted Queries](https://www.apollographql.com/docs/graphos/operations/persisted-queries/) (a GraphOS Enterprise feature) is now moving to General Availability, from Preview where it has been since Apollo Router 1.25.

For more information about launch stages, please see the documentation here: https://www.apollographql.com/docs/resources/product-launch-stages/

The feature is now configured with a `persisted_queries` top-level key in the YAML configuration instead of with `preview_persisted_queries`. Existing configuration files will keep working as before, only with a warning. To fix that warning, rename the configuration section like so:

```diff
-preview_persisted_queries:
+persisted_queries:
enabled: true
```

By [@glasser](https://github.com/glasser) in https://github.com/apollographql/router/pull/3914
2 changes: 1 addition & 1 deletion apollo-router/src/configuration/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ impl Metrics {
);
log_usage_metrics!(
value.apollo.router.config.persisted_queries,
"$.preview_persisted_queries[?(@.enabled == true)]",
"$.persisted_queries[?(@.enabled == true)]",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BrynCooke I just want to double-check my understanding that this expression is only ever applied to a YAML file that has been "migrated".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct.

opt.log_unknown,
"$[?(@.log_unknown == true)]",
opt.safelist.require_id,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
description: Persisted queries are no longer preview, `preview_persisted_queries` is renamed `persisted_queries`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Text chosen for consistency with 0008, though I would probably use a semicolon and add "to" after "renamed"

actions:
- type: move
from: preview_persisted_queries
to: persisted_queries
4 changes: 2 additions & 2 deletions apollo-router/src/configuration/migrations/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ It uses [proteus](https://github.com/rust-playground/proteus) under the hood, wh

A migration has the following format:

The filename should begin with a 5 digit numerical prefix. This allows us to apply migrations in a deterministic order.
`Filename: 00001-name.yaml`
The filename should begin with a 4 digit numerical prefix. This allows us to apply migrations in a deterministic order.
`Filename: 0001-name.yaml`

The yaml consists of a description and a number of actions:
```yaml
Expand Down
32 changes: 15 additions & 17 deletions apollo-router/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,9 @@ pub struct Configuration {
#[serde(default)]
pub(crate) apq: Apq,

// NOTE: when renaming this to move out of preview, also update paths
// in `uplink/license.rs`.
/// Configures managed persisted queries
#[serde(default)]
pub preview_persisted_queries: PersistedQueries,
pub persisted_queries: PersistedQueries,

/// Configuration for operation limits, parser limits, HTTP limits, etc.
#[serde(default)]
Expand Down Expand Up @@ -228,7 +226,7 @@ impl<'de> serde::Deserialize<'de> for Configuration {
apollo_plugins: ApolloPlugins,
tls: Tls,
apq: Apq,
preview_persisted_queries: PersistedQueries,
persisted_queries: PersistedQueries,
#[serde(skip)]
uplink: UplinkConfig,
limits: Limits,
Expand All @@ -247,7 +245,7 @@ impl<'de> serde::Deserialize<'de> for Configuration {
.apollo_plugins(ad_hoc.apollo_plugins.plugins)
.tls(ad_hoc.tls)
.apq(ad_hoc.apq)
.persisted_query(ad_hoc.preview_persisted_queries)
.persisted_query(ad_hoc.persisted_queries)
.operation_limits(ad_hoc.limits)
.chaos(ad_hoc.experimental_chaos)
.uplink(ad_hoc.uplink)
Expand Down Expand Up @@ -310,7 +308,7 @@ impl Configuration {
homepage: homepage.unwrap_or_default(),
cors: cors.unwrap_or_default(),
apq: apq.unwrap_or_default(),
preview_persisted_queries: persisted_query.unwrap_or_default(),
persisted_queries: persisted_query.unwrap_or_default(),
limits: operation_limits.unwrap_or_default(),
experimental_chaos: chaos.unwrap_or_default(),
experimental_graphql_validation_mode: graphql_validation_mode.unwrap_or_default(),
Expand Down Expand Up @@ -380,7 +378,7 @@ impl Configuration {
tls: tls.unwrap_or_default(),
notify: notify.unwrap_or_default(),
apq: apq.unwrap_or_default(),
preview_persisted_queries: persisted_query.unwrap_or_default(),
persisted_queries: persisted_query.unwrap_or_default(),
uplink,
};

Expand Down Expand Up @@ -439,31 +437,31 @@ impl Configuration {
}

// PQs.
if self.preview_persisted_queries.enabled {
if self.preview_persisted_queries.safelist.enabled && self.apq.enabled {
if self.persisted_queries.enabled {
if self.persisted_queries.safelist.enabled && self.apq.enabled {
return Err(ConfigurationError::InvalidConfiguration {
message: "apqs must be disabled to enable safelisting",
error: "either set preview_persisted_queries.safelist.enabled: false or apq.enabled: false in your router yaml configuration".into()
error: "either set persisted_queries.safelist.enabled: false or apq.enabled: false in your router yaml configuration".into()
});
} else if !self.preview_persisted_queries.safelist.enabled
&& self.preview_persisted_queries.safelist.require_id
} else if !self.persisted_queries.safelist.enabled
&& self.persisted_queries.safelist.require_id
{
return Err(ConfigurationError::InvalidConfiguration {
message: "safelist must be enabled to require IDs",
error: "either set preview_persisted_queries.safelist.enabled: true or preview_persisted_queries.safelist.require_id: false in your router yaml configuration".into()
error: "either set persisted_queries.safelist.enabled: true or persisted_queries.safelist.require_id: false in your router yaml configuration".into()
});
}
} else {
// If the feature isn't enabled, sub-features shouldn't be.
if self.preview_persisted_queries.safelist.enabled {
if self.persisted_queries.safelist.enabled {
return Err(ConfigurationError::InvalidConfiguration {
message: "persisted queries must be enabled to enable safelisting",
error: "either set preview_persisted_queries.safelist.enabled: false or preview_persisted_queries.enabled: true in your router yaml configuration".into()
error: "either set persisted_queries.safelist.enabled: false or persisted_queries.enabled: true in your router yaml configuration".into()
});
} else if self.preview_persisted_queries.log_unknown {
} else if self.persisted_queries.log_unknown {
return Err(ConfigurationError::InvalidConfiguration {
message: "persisted queries must be enabled to enable logging unknown operations",
error: "either set preview_persisted_queries.log_unknown: false or preview_persisted_queries.enabled: true in your router yaml configuration".into()
error: "either set persisted_queries.log_unknown: false or persisted_queries.enabled: true in your router yaml configuration".into()
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,52 @@ expression: "&schema"
}
]
},
"persisted_queries": {
"description": "Configures managed persisted queries",
"default": {
"enabled": false,
"log_unknown": false,
"safelist": {
"enabled": false,
"require_id": false
}
},
"type": "object",
"properties": {
"enabled": {
"description": "Activates Persisted Queries (disabled by default)",
"default": false,
"type": "boolean"
},
"log_unknown": {
"description": "Enabling this field configures the router to log any freeform GraphQL request that is not in the persisted query list",
"default": false,
"type": "boolean"
},
"safelist": {
"description": "Restricts execution of operations that are not found in the Persisted Query List",
"default": {
"enabled": false,
"require_id": false
},
"type": "object",
"properties": {
"enabled": {
"description": "Enables using the persisted query list as a safelist (disabled by default)",
"default": false,
"type": "boolean"
},
"require_id": {
"description": "Enabling this field configures the router to reject any request that does not include the persisted query ID",
"default": false,
"type": "boolean"
}
},
"additionalProperties": false
}
},
"additionalProperties": false
},
"plugins": {
"description": "Plugin configuration",
"default": null,
Expand Down Expand Up @@ -1666,52 +1712,6 @@ expression: "&schema"
},
"additionalProperties": false
},
"preview_persisted_queries": {
"description": "Configures managed persisted queries",
"default": {
"enabled": false,
"log_unknown": false,
"safelist": {
"enabled": false,
"require_id": false
}
},
"type": "object",
"properties": {
"enabled": {
"description": "Activates Persisted Queries (disabled by default)",
"default": false,
"type": "boolean"
},
"log_unknown": {
"description": "Enabling this field configures the router to log any freeform GraphQL request that is not in the persisted query list",
"default": false,
"type": "boolean"
},
"safelist": {
"description": "Restricts execution of operations that are not found in the Persisted Query List",
"default": {
"enabled": false,
"require_id": false
},
"type": "object",
"properties": {
"enabled": {
"description": "Enables using the persisted query list as a safelist (disabled by default)",
"default": false,
"type": "boolean"
},
"require_id": {
"description": "Enabling this field configures the router to reject any request that does not include the persisted query ID",
"default": false,
"type": "boolean"
}
},
"additionalProperties": false
}
},
"additionalProperties": false
},
"rhai": {
"description": "Configuration for the Rhai Plugin",
"type": "object",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: apollo-router/src/configuration/tests.rs
expression: new_config
---
---
persisted_queries:
enabled: true

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apq:
enabled: false
preview_persisted_queries:
persisted_queries:
enabled: true
log_unknown: true
safelist:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
preview_persisted_queries:
enabled: true

Original file line number Diff line number Diff line change
Expand Up @@ -398,19 +398,18 @@ async fn poll_uplink(
while let Some(event) = uplink_executor.next().await {
match event {
ManifestPollEvent::NewManifest(new_manifest) => {
let freeform_graphql_behavior = if config.preview_persisted_queries.safelist.enabled
{
if config.preview_persisted_queries.safelist.require_id {
let freeform_graphql_behavior = if config.persisted_queries.safelist.enabled {
if config.persisted_queries.safelist.require_id {
FreeformGraphQLBehavior::DenyAll {
log_unknown: config.preview_persisted_queries.log_unknown,
log_unknown: config.persisted_queries.log_unknown,
}
} else {
FreeformGraphQLBehavior::AllowIfInSafelist {
safelist: FreeformGraphQLSafelist::new(&new_manifest),
log_unknown: config.preview_persisted_queries.log_unknown,
log_unknown: config.persisted_queries.log_unknown,
}
}
} else if config.preview_persisted_queries.log_unknown {
} else if config.persisted_queries.log_unknown {
FreeformGraphQLBehavior::LogUnlessInSafelist {
safelist: FreeformGraphQLSafelist::new(&new_manifest),
apq_enabled: config.apq.enabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl PersistedQueryLayer {
/// Create a new [`PersistedQueryLayer`] from CLI options, YAML configuration,
/// and optionally, an existing persisted query manifest poller.
pub(crate) async fn new(configuration: &Configuration) -> Result<Self, BoxError> {
if configuration.preview_persisted_queries.enabled {
if configuration.persisted_queries.enabled {
Ok(Self {
manifest_poller: Some(
PersistedQueryManifestPoller::new(configuration.clone()).await?,
Expand Down
2 changes: 1 addition & 1 deletion apollo-router/src/uplink/license_enforcement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl LicenseEnforcementReport {
.name("Operation aliases limiting")
.build(),
ConfigurationRestriction::builder()
.path("$.preview_persisted_queries")
.path("$.persisted_queries")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly want to double-check my understanding that this expression is only ever applied to a YAML file that has been "migrated".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, both license enforcement and metrics are applied to Configuration::validated_yaml which has already had migrations applied.

.name("Persisted queries")
.build(),
]
Expand Down
4 changes: 2 additions & 2 deletions apollo-router/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ async fn persisted_queries() {
.await;

let config = serde_json::json!({
"preview_persisted_queries": {
"persisted_queries": {
"enabled": true
},
"apq": {
Expand Down Expand Up @@ -1482,7 +1482,7 @@ async fn all_stock_router_example_yamls_are_valid() {
let mut configuration: Configuration = serde_yaml::from_str(&raw_yaml)
.unwrap_or_else(|e| panic!("unable to parse YAML {display_path}: {e}"));
let (_mock_guard, configuration) =
if configuration.preview_persisted_queries.enabled {
if configuration.persisted_queries.enabled {
let (_mock_guard, uplink_config) = mock_empty_pq_uplink().await;
configuration.uplink = Some(uplink_config);
(Some(_mock_guard), configuration)
Expand Down
3 changes: 1 addition & 2 deletions docs/source/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@
"Safelisting with persisted queries": [
"/configuration/persisted-queries",
[
"enterprise",
"preview"
"enterprise"
]
],
"Privacy and data collection": "/privacy"
Expand Down
16 changes: 7 additions & 9 deletions docs/source/configuration/persisted-queries.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ minVersion: 1.25.0

<GraphOSEnterpriseRequired />

<PreviewFeature discordLink="https://discordapp.com/channels/1022972389463687228/1143901714173407342"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that there are other references to "preview" on this page, but they are loaded via components in the docs repo.


<PQIntro />

## Differences from automatic persisted queries
Expand Down Expand Up @@ -40,21 +38,21 @@ For more information on other configuration aspects, see the [GraphOS persisted

The router provides four configuration options that you can combine to create the recommended [security levels](#router-security-levels). This section details each configuration option. Refer to the [security levels](#router-security-levels) section for recommended combinations.

#### `preview_persisted_queries`
#### `persisted_queries`

This base configuration enables the feature. All other configuration options build off this one.

```yaml title="router.yaml"
preview_persisted_queries:
persisted_queries:
enabled: true
```
#### `log_unknown`

Adding `log_unknown: true` to `preview_persisted_queries` configures the router to log any incoming operations not preregistered to the PQL.
Adding `log_unknown: true` to `persisted_queries` configures the router to log any incoming operations not preregistered to the PQL.

```yaml title="router.yaml"
preview_persisted_queries:
persisted_queries:
enabled: true
log_unknown: true
```
Expand All @@ -63,10 +61,10 @@ If used with the [`safelist`](#safelist) option, the router logs unregistered an

#### `safelist`

Adding `safelist: true` to `preview_persisted_queries` causes the router to reject any operations that haven't been preregistered to your PQL.
Adding `safelist: true` to `persisted_queries` causes the router to reject any operations that haven't been preregistered to your PQL.

```yaml title="router.yaml"
preview_persisted_queries:
persisted_queries:
enabled: true
safelist:
enabled: true
Expand All @@ -85,7 +83,7 @@ Adding `require_id: true` to the `safelist` option causes the router to reject a
- use a full operation string rather than the operation ID

```yaml title="router.yaml"
preview_persisted_queries:
persisted_queries:
enabled: true
safelist:
enabled: true
Expand Down
2 changes: 1 addition & 1 deletion examples/persisted-queries/additive_pq.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@
# 2) make requests against the router:
# curl --get http://localhost:4000 --header 'content-type: application/json' --data-urlencode 'extensions={"persistedQuery":{"sha256Hash":"hash-of-operation", "version": 1}}'

preview_persisted_queries:
persisted_queries:
enabled: true
Loading