migrate cors configuration from server to top level (#1586)

from: ``` server: cors: ``` to: ``` cors: ``` fixes: #1483
apollographql · Aug 26, 2022 · 1956b56 · 1956b56
1 parent f48ccc7
commit 1956b56
Show file tree

Hide file tree

Showing 14 changed files with 262 additions and 289 deletions.
diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md
@@ -27,6 +27,12 @@ By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/
 
 ## ❗ BREAKING ❗
 
+### Move cors configuration from `server` to top level ([PR #1586](https://github.com/apollographql/router/pull/1586))
+
+The cors configuration is now located at the top level of the configuration file.
+
+By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/1586
+
 ### Exit the router after logging panic details ([PR #1602](https://github.com/apollographql/router/pull/1602))
 
 If the router panics, it can leave the router in an unuseable state.

diff --git a/apollo-router/src/axum_http_server_factory.rs b/apollo-router/src/axum_http_server_factory.rs
@@ -87,14 +87,9 @@ pub(crate) fn make_axum_router<RF>(
 where
     RF: SupergraphServiceFactory,
 {
-    let cors = configuration
-        .server
-        .cors
-        .clone()
-        .into_layer()
-        .map_err(|e| {
-            ApolloRouterError::ServiceCreationError(format!("CORS configuration error: {e}").into())
-        })?;
+    let cors = configuration.cors.clone().into_layer().map_err(|e| {
+        ApolloRouterError::ServiceCreationError(format!("CORS configuration error: {e}").into())
+    })?;
     let graphql_endpoint = if configuration.server.endpoint.ends_with("/*") {
         // Needed for axum (check the axum docs for more information about wildcards https://docs.rs/axum/latest/axum/struct.Router.html#wildcards)
         format!("{}router_extra_path", configuration.server.endpoint)
@@ -1268,14 +1263,14 @@ mod tests {
                 ))
             });
         let conf = Configuration::builder()
+            .cors(
+                Cors::builder()
+                    .origins(vec!["http://studio".to_string()])
+                    .build(),
+            )
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(
-                        Cors::builder()
-                            .origins(vec!["http://studio".to_string()])
-                            .build(),
-                    )
                     .endpoint(String::from("/graphql"))
                     .build(),
             )
@@ -1337,14 +1332,14 @@ mod tests {
                 ))
             });
         let conf = Configuration::builder()
+            .cors(
+                Cors::builder()
+                    .origins(vec!["http://studio".to_string()])
+                    .build(),
+            )
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(
-                        Cors::builder()
-                            .origins(vec!["http://studio".to_string()])
-                            .build(),
-                    )
                     .endpoint(String::from("/:my_prefix/graphql"))
                     .build(),
             )
@@ -1406,14 +1401,14 @@ mod tests {
                 ))
             });
         let conf = Configuration::builder()
+            .cors(
+                Cors::builder()
+                    .origins(vec!["http://studio".to_string()])
+                    .build(),
+            )
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(
-                        Cors::builder()
-                            .origins(vec!["http://studio".to_string()])
-                            .build(),
-                    )
                     .endpoint(String::from("/graphql/*"))
                     .build(),
             )
@@ -1629,10 +1624,10 @@ mod tests {
     async fn cors_preflight() -> Result<(), ApolloRouterError> {
         let expectations = MockSupergraphService::new();
         let conf = Configuration::builder()
+            .cors(Cors::builder().build())
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(Cors::builder().build())
                     .endpoint(String::from("/graphql/*"))
                     .build(),
             )
@@ -1855,14 +1850,14 @@ Content-Type: application/json\r
     async fn it_doesnt_display_disabled_home_page() -> Result<(), ApolloRouterError> {
         let expectations = MockSupergraphService::new();
         let conf = Configuration::builder()
+            .cors(
+                Cors::builder()
+                    .origins(vec!["http://studio".to_string()])
+                    .build(),
+            )
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(
-                        Cors::builder()
-                            .origins(vec!["http://studio".to_string()])
-                            .build(),
-                    )
                     .landing_page(false)
                     .build(),
             )
@@ -1901,14 +1896,14 @@ Content-Type: application/json\r
         );
 
         let conf = Configuration::builder()
+            .cors(
+                Cors::builder()
+                    .origins(vec!["http://studio".to_string()])
+                    .build(),
+            )
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(
-                        Cors::builder()
-                            .origins(vec!["http://studio".to_string()])
-                            .build(),
-                    )
                     .build(),
             )
             .build();
@@ -2044,10 +2039,10 @@ Content-Type: application/json\r
     #[tokio::test]
     async fn cors_allow_any_origin() -> Result<(), ApolloRouterError> {
         let conf = Configuration::builder()
+            .cors(Cors::builder().allow_any_origin(true).build())
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(Cors::builder().allow_any_origin(true).build())
                     .build(),
             )
             .build();
@@ -2067,14 +2062,14 @@ Content-Type: application/json\r
         let valid_origin = "https://thisoriginisallowed.com";
 
         let conf = Configuration::builder()
+            .cors(
+                Cors::builder()
+                    .origins(vec![valid_origin.to_string()])
+                    .build(),
+            )
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(
-                        Cors::builder()
-                            .origins(vec![valid_origin.to_string()])
-                            .build(),
-                    )
                     .build(),
             )
             .build();
@@ -2097,15 +2092,15 @@ Content-Type: application/json\r
         let apollo_subdomains = "https://([a-z0-9]+[.])*apollographql[.]com";
 
         let conf = Configuration::builder()
+            .cors(
+                Cors::builder()
+                    .origins(vec!["https://anexactmatchorigin.com".to_string()])
+                    .match_origins(vec![apollo_subdomains.to_string()])
+                    .build(),
+            )
             .server(
                 crate::configuration::Server::builder()
                     .listen(SocketAddr::from_str("127.0.0.1:0").unwrap())
-                    .cors(
-                        Cors::builder()
-                            .origins(vec!["https://anexactmatchorigin.com".to_string()])
-                            .match_origins(vec![apollo_subdomains.to_string()])
-                            .build(),
-                    )
                     .build(),
             )
             .build();

diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs
@@ -68,6 +68,10 @@ pub struct Configuration {
     #[serde(default)]
     pub(crate) server: Server,
 
+    /// Cross origin request headers.
+    #[serde(default)]
+    pub(crate) cors: Cors,
+
     /// Plugin configuration
     #[serde(default)]
     plugins: UserPlugins,
@@ -90,11 +94,13 @@ impl Configuration {
     #[builder]
     pub(crate) fn new(
         server: Option<Server>,
+        cors: Option<Cors>,
         plugins: Map<String, Value>,
         apollo_plugins: Map<String, Value>,
     ) -> Self {
         Self {
             server: server.unwrap_or_default(),
+            cors: cors.unwrap_or_default(),
             plugins: UserPlugins {
                 plugins: Some(plugins),
             },
@@ -246,10 +252,6 @@ pub(crate) struct Server {
     #[serde(default = "default_listen")]
     pub(crate) listen: ListenAddr,
 
-    /// Cross origin request headers.
-    #[serde(default)]
-    pub(crate) cors: Cors,
-
     /// introspection queries
     /// enabled by default
     #[serde(default = "default_introspection")]
@@ -287,7 +289,6 @@ impl Server {
     #[allow(clippy::too_many_arguments)] // Used through a builder, not directly
     pub(crate) fn new(
         listen: Option<ListenAddr>,
-        cors: Option<Cors>,
         introspection: Option<bool>,
         landing_page: Option<bool>,
         endpoint: Option<String>,
@@ -297,7 +298,6 @@ impl Server {
     ) -> Self {
         Self {
             listen: listen.unwrap_or_else(default_listen),
-            cors: cors.unwrap_or_default(),
             introspection: introspection.unwrap_or_else(default_introspection),
             landing_page: landing_page.unwrap_or_else(default_landing_page),
             endpoint: endpoint.unwrap_or_else(default_endpoint),
@@ -1145,8 +1145,8 @@ server:
   # The socket address and port to listen on
   # Defaults to 127.0.0.1:4000
   listen: 127.0.0.1:4000
-  cors:
-    allow_headers: [ Content-Type, 5 ]
+cors:
+  allow_headers: [ Content-Type, 5 ]
         "#,
         )
         .expect_err("should have resulted in an error");
@@ -1161,10 +1161,10 @@ server:
   # The socket address and port to listen on
   # Defaults to 127.0.0.1:4000
   listen: 127.0.0.1:4000
-  cors:
-    allow_headers:
-      - Content-Type
-      - 5
+cors:
+  allow_headers:
+    - Content-Type
+    - 5
         "#,
         )
         .expect_err("should have resulted in an error");
@@ -1175,15 +1175,13 @@ server:
     fn it_does_not_allow_invalid_cors_headers() {
         let cfg = validate_configuration(
             r#"
-server:
-  cors:
-    allow_credentials: true
-    allow_headers: [ "*" ]
+cors:
+  allow_credentials: true
+  allow_headers: [ "*" ]
         "#,
         )
         .expect("should not have resulted in an error");
         let error = cfg
-            .server
             .cors
             .into_layer()
             .expect_err("should have resulted in an error");
@@ -1194,15 +1192,13 @@ server:
     fn it_does_not_allow_invalid_cors_methods() {
         let cfg = validate_configuration(
             r#"
-server:
-  cors:
-    allow_credentials: true
-    methods: [ GET, "*" ]
+cors:
+  allow_credentials: true
+  methods: [ GET, "*" ]
         "#,
         )
         .expect("should not have resulted in an error");
         let error = cfg
-            .server
             .cors
             .into_layer()
             .expect_err("should have resulted in an error");
@@ -1213,15 +1209,13 @@ server:
     fn it_does_not_allow_invalid_cors_origins() {
         let cfg = validate_configuration(
             r#"
-server:
-  cors:
-    allow_credentials: true
-    allow_any_origin: true
+cors:
+  allow_credentials: true
+  allow_any_origin: true
         "#,
         )
         .expect("should not have resulted in an error");
         let error = cfg
-            .server
             .cors
             .into_layer()
             .expect_err("should have resulted in an error");
@@ -1307,8 +1301,8 @@ server:
   # The socket address and port to listen on
   # Defaults to 127.0.0.1:4000
   listen: 127.0.0.1:4000
-  cors:
-    allow_headers: [ Content-Type, "${TEST_CONFIG_NUMERIC_ENV_UNIQUE}" ]
+cors:
+  allow_headers: [ Content-Type, "${TEST_CONFIG_NUMERIC_ENV_UNIQUE}" ]
         "#,
         )
         .expect_err("should have resulted in an error");
@@ -1325,10 +1319,10 @@ server:
   # The socket address and port to listen on
   # Defaults to 127.0.0.1:4000
   listen: 127.0.0.1:4000
-  cors:
-    allow_headers:
-      - Content-Type
-      - "${TEST_CONFIG_NUMERIC_ENV_UNIQUE:true}"
+cors:
+  allow_headers:
+    - Content-Type
+    - "${TEST_CONFIG_NUMERIC_ENV_UNIQUE:true}"
         "#,
         )
         .expect_err("should have resulted in an error");

diff --git a/...apollo_router__configuration__tests__line_precise_config_errors_with_inline_sequence.snap b/...apollo_router__configuration__tests__line_precise_config_errors_with_inline_sequence.snap
@@ -3,11 +3,11 @@ source: apollo-router/src/configuration/mod.rs
 expression: error.to_string()
 ---
 configuration had errors: 
-1. /server/cors/allow_headers/1
+1. /cors/allow_headers/1
 
   # The socket address and port to listen on
   # Defaults to 127.0.0.1:4000
   listen: 127.0.0.1:4000
-  cors:
-    allow_headers: [ Content-Type, 5 ]
-                                   ^----- 5 is not of type "string"
+cors:
+  allow_headers: [ Content-Type, 5 ]
+                                 ^----- 5 is not of type "string"
diff --git a/..._configuration__tests__line_precise_config_errors_with_inline_sequence_env_expansion.snap b/..._configuration__tests__line_precise_config_errors_with_inline_sequence_env_expansion.snap
@@ -3,11 +3,11 @@ source: apollo-router/src/configuration/mod.rs
 expression: error.to_string()
 ---
 configuration had errors: 
-1. /server/cors/allow_headers/1
+1. /cors/allow_headers/1
 
   # The socket address and port to listen on
   # Defaults to 127.0.0.1:4000
   listen: 127.0.0.1:4000
-  cors:
-    allow_headers: [ Content-Type, "${TEST_CONFIG_NUMERIC_ENV_UNIQUE}" ]
-                                   ^----- "${TEST_CONFIG_NUMERIC_ENV_UNIQUE}" is not of type "string"
+cors:
+  allow_headers: [ Content-Type, "${TEST_CONFIG_NUMERIC_ENV_UNIQUE}" ]
+                                 ^----- "${TEST_CONFIG_NUMERIC_ENV_UNIQUE}" is not of type "string"