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.

Sign up for GitHub

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

Test and document the ordering of plugins #3321

Merged
merged 3 commits into from
Jun 30, 2023
Merged

Test and document the ordering of plugins #3321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79e06cd
Add test for plugin ordering
SimonSapin Jun 26, 2023
e0326d0
Enforce ordering of Apollo plugins
SimonSapin Jun 26, 2023
102b6c0
Document the ordering of Rust plugins
SimonSapin Jun 26, 2023
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
27 changes: 2 additions & 25 deletions apollo-router/src/configuration/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ pub struct Configuration {

/// Plugin configuration
#[serde(default)]
plugins: UserPlugins,
pub(crate) plugins: UserPlugins,

/// Built-in plugin configuration. Built in plugins are pushed to the top level of config.
#[serde(default)]
Expand Down Expand Up @@ -210,7 +210,7 @@ impl<'de> serde::Deserialize<'de> for Configuration {
}
}

const APOLLO_PLUGIN_PREFIX: &str = "apollo.";
pub(crate) const APOLLO_PLUGIN_PREFIX: &str = "apollo.";

fn default_graphql_listen() -> ListenAddr {
SocketAddr::from_str("127.0.0.1:4000").unwrap().into()
Expand Down Expand Up @@ -278,29 +278,6 @@ impl Configuration {

conf.validate()
}

pub(crate) fn plugins(&self) -> Vec<(String, Value)> {
let mut plugins = vec![];

// Add all the apollo plugins
for (plugin, config) in &self.apollo_plugins.plugins {
let plugin_full_name = format!("{APOLLO_PLUGIN_PREFIX}{plugin}");
tracing::debug!(
"adding plugin {} with user provided configuration",
plugin_full_name.as_str()
);
plugins.push((plugin_full_name, config.clone()));
}

// Add all the user plugins
if let Some(config_map) = self.plugins.plugins.as_ref() {
for (plugin, config) in config_map {
plugins.push((plugin.clone(), config.clone()));
}
}

plugins
}
}

impl Default for Configuration {
Expand Down
52 changes: 26 additions & 26 deletions ...r/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1116,6 +1116,32 @@ expression: "&schema"
"type": "boolean"
}
}
},
"test.always_fails_to_start": {
"description": "Configuration for the test plugin",
"type": "object",
"required": [
"name"
],
"properties": {
"name": {
"description": "The name of the test",
"type": "string"
}
}
},
"test.always_starts_and_stops": {
"description": "Configuration for the test plugin",
"type": "object",
"required": [
"name"
],
"properties": {
"name": {
"description": "The name of the test",
"type": "string"
}
}
}
},
"additionalProperties": false
Expand Down Expand Up @@ -4775,32 +4801,6 @@ expression: "&schema"
},
"additionalProperties": false
},
"test.always_fails_to_start": {
"description": "Configuration for the test plugin",
"type": "object",
"required": [
"name"
],
"properties": {
"name": {
"description": "The name of the test",
"type": "string"
}
}
},
"test.always_starts_and_stops": {
"description": "Configuration for the test plugin",
"type": "object",
"required": [
"name"
],
"properties": {
"name": {
"description": "The name of the test",
"type": "string"
}
}
},
"tls": {
"description": "TLS related configuration options.",
"default": {
Expand Down
207 changes: 105 additions & 102 deletions apollo-router/src/router_factory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use std::collections::HashMap;
use std::io;
use std::sync::Arc;

use axum::response::IntoResponse;
use http::StatusCode;
use multimap::MultiMap;
use once_cell::sync::Lazy;
use rustls::RootCertStore;
use serde_json::Map;
use serde_json::Value;
Expand All @@ -18,6 +18,7 @@ use tower_service::Service;
use crate::configuration::Configuration;
use crate::configuration::ConfigurationError;
use crate::configuration::TlsSubgraph;
use crate::configuration::APOLLO_PLUGIN_PREFIX;
use crate::plugin::DynPlugin;
use crate::plugin::Handler;
use crate::plugin::PluginFactory;
Expand Down Expand Up @@ -423,107 +424,113 @@ pub(crate) async fn create_plugins(
schema: &Schema,
extra_plugins: Option<Vec<(String, Box<dyn DynPlugin>)>>,
) -> Result<Vec<(String, Box<dyn DynPlugin>)>, BoxError> {
// List of mandatory plugins. Ordering is important!!
let mandatory_plugins = vec![
"apollo.include_subgraph_errors",
"apollo.csrf",
"apollo.headers",
"apollo.telemetry",
];

let mut apollo_plugins_config = configuration.apollo_plugins.clone().plugins;
let user_plugins_config = configuration.plugins.clone().plugins.unwrap_or_default();
let extra = extra_plugins.unwrap_or_default();
let plugin_registry = &*crate::plugin::PLUGINS;
let mut apollo_plugin_factories: HashMap<&str, &PluginFactory> = plugin_registry
.iter()
.filter(|factory| factory.name.starts_with(APOLLO_PLUGIN_PREFIX))
.map(|factory| (factory.name.as_str(), &**factory))
.collect();
let mut errors = Vec::new();
let plugin_registry: Vec<&'static Lazy<PluginFactory>> = crate::plugin::plugins().collect();
let mut plugin_instances = Vec::new();
let extra = extra_plugins.unwrap_or_default();
let notify = configuration.notify.clone();

for (name, mut configuration) in configuration.plugins().into_iter() {
if extra.iter().any(|(n, _)| *n == name) {
// An instance of this plugin was already added through TestHarness::extra_plugin
continue;
}
// Use fonction-like macros to avoid borrow conflicts of captures
macro_rules! add_plugin {
($name: expr, $factory: expr, $plugin_config: expr) => {{
match $factory
.create_instance(
&$plugin_config,
schema.as_string().clone(),
configuration.notify.clone(),
)
.await
{
Ok(plugin) => plugin_instances.push(($name, plugin)),
Err(err) => errors.push(ConfigurationError::PluginConfiguration {
plugin: $name,
error: err.to_string(),
}),
}
}};
}

match plugin_registry.iter().find(|factory| factory.name == name) {
Some(factory) => {
tracing::debug!(
"creating plugin: '{}' with configuration:\n{:#}",
name,
configuration
);
macro_rules! add_apollo_plugin {
($name: literal, $opt_plugin_config: expr) => {{
let name = format!("{}{}", APOLLO_PLUGIN_PREFIX, $name);
let factory = apollo_plugin_factories
.remove(name.as_str())
.unwrap_or_else(|| panic!("Apollo plugin not registered: {name}"));
if let Some(mut plugin_config) = $opt_plugin_config {
if name == "apollo.telemetry" {
inject_schema_id(schema, &mut configuration);
}
match factory
.create_instance(&configuration, schema.as_string().clone(), notify.clone())
.await
{
Ok(plugin) => {
plugin_instances.push((name, plugin));
}
Err(err) => errors.push(ConfigurationError::PluginConfiguration {
plugin: name,
error: err.to_string(),
}),
// The apollo.telemetry" plugin isn't happy with empty config, so we
// give it some. If any of the other mandatory plugins need special
// treatment, then we'll have to perform it here.
// This is *required* by the telemetry module or it will fail...
inject_schema_id(schema, &mut plugin_config);
}
add_plugin!(name, factory, plugin_config);
}
None => errors.push(ConfigurationError::PluginUnknown(name)),
}
}};
}
plugin_instances.extend(extra);

// At this point we've processed all of the plugins that were provided in configuration.
// We now need to do process our list of mandatory plugins:
// - If a mandatory plugin is already in the list, then it must be re-located
// to its mandatory location
// - If it is missing, it must be added at its mandatory location

for (desired_position, name) in mandatory_plugins.iter().enumerate() {
let position_maybe = plugin_instances.iter().position(|(x, _)| x == name);
match position_maybe {
Some(actual_position) => {
// Found it, re-locate if required.
if actual_position != desired_position {
let temp = plugin_instances.remove(actual_position);
plugin_instances.insert(desired_position, temp);
}
}
None => {
// Didn't find it, insert
match plugin_registry
.iter()
.find(|factory| factory.name == **name)
{
// Create an instance
Some(factory) => {
// Create default (empty) config
let mut config = Value::Object(Map::new());
// The apollo.telemetry" plugin isn't happy with empty config, so we
// give it some. If any of the other mandatory plugins need special
// treatment, then we'll have to perform it here.
// This is *required* by the telemetry module or it will fail...
if *name == "apollo.telemetry" {
inject_schema_id(schema, &mut config);
}
match factory
.create_instance(&config, schema.as_string().clone(), notify.clone())
.await
{
Ok(plugin) => {
plugin_instances.insert(
desired_position.min(plugin_instances.len()),
(name.to_string(), plugin),
);
}
Err(err) => errors.push(ConfigurationError::PluginConfiguration {
plugin: name.to_string(),
error: err.to_string(),
}),
}
}
None => errors.push(ConfigurationError::PluginUnknown(name.to_string())),

macro_rules! add_mandatory_apollo_plugin {
($name: literal) => {
add_apollo_plugin!(
$name,
Some(
apollo_plugins_config
.remove($name)
.unwrap_or(Value::Object(Map::new()))
)
);
};
}

macro_rules! add_optional_apollo_plugin {
($name: literal) => {
add_apollo_plugin!($name, apollo_plugins_config.remove($name));
};
}

macro_rules! add_user_plugins {
() => {
for (name, plugin_config) in user_plugins_config {
if let Some(factory) = plugin_registry.iter().find(|factory| factory.name == name) {
add_plugin!(name, factory, plugin_config);
} else {
errors.push(ConfigurationError::PluginUnknown(name))
}
}
}
plugin_instances.extend(extra);
};
}

add_mandatory_apollo_plugin!("include_subgraph_errors");
add_mandatory_apollo_plugin!("csrf");
add_mandatory_apollo_plugin!("headers");
add_mandatory_apollo_plugin!("telemetry");
add_optional_apollo_plugin!("traffic_shaping");
add_optional_apollo_plugin!("forbid_mutations");
add_optional_apollo_plugin!("subscription");
add_optional_apollo_plugin!("override_subgraph_url");
add_optional_apollo_plugin!("authorization");
add_optional_apollo_plugin!("authentication");

// This relative ordering is documented in `docs/source/customizations/native.mdx`:
add_optional_apollo_plugin!("rhai");
add_optional_apollo_plugin!("coprocessor");
add_user_plugins!();

// Macros above remove from `apollo_plugin_factories`, so anything left at the end
// indicates a missing macro call.
let unused_apollo_plugin_names = apollo_plugin_factories.keys().copied().collect::<Vec<_>>();
if !unused_apollo_plugin_names.is_empty() {
panic!(
"Apollo plugins without their ordering specified in `fn create_plugins`: {}",
unused_apollo_plugin_names.join(", ")
)
}

let plugin_details = plugin_instances
Expand Down Expand Up @@ -625,7 +632,7 @@ mod test {
}

register_plugin!(
"apollo.test",
"test",
"always_starts_and_stops",
AlwaysStartsAndStopsPlugin
);
Expand All @@ -645,11 +652,7 @@ mod test {
}
}

register_plugin!(
"apollo.test",
"always_fails_to_start",
AlwaysFailsToStartPlugin
);
register_plugin!("test", "always_fails_to_start", AlwaysFailsToStartPlugin);

#[tokio::test]
async fn test_yaml_no_extras() {
Expand All @@ -663,7 +666,7 @@ mod test {
let config: Configuration = serde_yaml::from_str(
r#"
plugins:
apollo.test.always_starts_and_stops:
test.always_starts_and_stops:
name: albert
"#,
)
Expand All @@ -677,7 +680,7 @@ mod test {
let config: Configuration = serde_yaml::from_str(
r#"
plugins:
apollo.test.always_fails_to_start:
test.always_fails_to_start:
name: albert
"#,
)
Expand All @@ -691,9 +694,9 @@ mod test {
let config: Configuration = serde_yaml::from_str(
r#"
plugins:
apollo.test.always_starts_and_stops:
test.always_starts_and_stops:
name: albert
apollo.test.always_fails_to_start:
test.always_fails_to_start:
name: albert
"#,
)
Expand Down
Loading