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

add support for setting vary header in plugins #1660

Merged
merged 6 commits into from
Aug 31, 2022
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: 11 additions & 4 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/

## ❗ BREAKING ❗

### Preserve Plugin response Vary headers([PR #1660](https://github.com/apollographql/router/issues/1297))

It is now possible to set a `Vary` header in a client response from a plugin.

Note: This is a breaking change because the prior behaviour provided three default Vary headers and we've had to drop those to enable this change. If, after all plugin processing, there is no Vary header, the router will add one with a value of "origin".

By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/1660

### Fix the supported defer specification version to 20220824 ([PR #1652](https://github.com/apollographql/router/issues/1652))

Since the router will ship before the `@defer` specification is done, we
Expand All @@ -42,13 +50,12 @@ By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/p
## 🚀 Features
## 🐛 Fixes

### Update our helm documentation to illustrate how to use our registry ([PR #1649](https://github.com/apollographql/router/issues/1649))
### Update our helm documentation to illustrate how to use our registry ([#1643](https://github.com/apollographql/router/issues/1643))

The helm chart never used to have a registry, so our docs were really just placeholders. I've updated them to reflect the fact that we now store the chart in our OCI registry.

By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/1649


### Update router-bridge to `query-planner` v2.1.0 ([PR #1650](https://github.com/apollographql/router/pull/1650))

The 2.1.0 release of the query planner comes with fixes to fragment interpretation and reduced memory usage.
Expand All @@ -57,7 +64,7 @@ By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/p

## 🛠 Maintenance

### Remove cache layer ([PR #1647](https://github.com/apollographql/router/issues/1647))
### Remove cache layer ([PR #1647](https://github.com/apollographql/router/pull/1647))

We removed ServiceBuilderExt::cache in 0.16.0. That was the only consumer of
the cache layer. This completes the removal by deleting the cache layer.
Expand All @@ -66,7 +73,7 @@ By [@garypen](https://github.com/garypen) in https://github.com/apollographql/ro

### Refactor `SupergraphService` ([PR #1615](https://github.com/apollographql/router/issues/1615))

The `SupergraphService` code became too complex, so much that `rsutfmt` could not modify it anymore.
The `SupergraphService` code became too complex, so much that `rustfmt` could not modify it anymore.
This breaks up the code in more manageable functions.

By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1615
Expand Down
46 changes: 46 additions & 0 deletions apollo-router/src/axum_http_server_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use futures::stream::BoxStream;
use futures::StreamExt;
use http::header::CONTENT_ENCODING;
use http::header::CONTENT_TYPE;
use http::header::VARY;
use http::HeaderValue;
use http::Request;
use http::Uri;
Expand Down Expand Up @@ -483,6 +484,13 @@ async fn health_check() -> impl IntoResponse {
Json(json!({ "status": "pass" }))
}

// Process the headers to make sure that `VARY` is set correctly
fn process_vary_header(headers: &mut HeaderMap<HeaderValue>) {
if headers.get(VARY).is_none() {
// We don't have a VARY header, add one with value "origin"
headers.insert(VARY, HeaderValue::from_static("origin"));
}
}
async fn run_graphql_request<RS>(
service: RS,
http_request: Request<graphql::Request>,
Expand Down Expand Up @@ -518,6 +526,8 @@ where
Ok(response) => {
let (mut parts, mut stream) = response.into_parts();

process_vary_header(&mut parts.headers);

match stream.next().await {
None => {
tracing::error!("router service is not available to process request",);
Expand Down Expand Up @@ -2295,4 +2305,40 @@ Content-Type: application/json\r

server.shutdown().await
}

// Test Vary processing

#[test]
fn it_adds_default_with_value_origin_if_no_vary_header() {
let mut default_headers = HeaderMap::new();
process_vary_header(&mut default_headers);
let vary_opt = default_headers.get(VARY);
assert!(vary_opt.is_some());
let vary = vary_opt.expect("has a value");
assert_eq!(vary, "origin");
}

#[test]
fn it_leaves_vary_alone_if_set() {
let mut default_headers = HeaderMap::new();
default_headers.insert(VARY, HeaderValue::from_static("*"));
process_vary_header(&mut default_headers);
let vary_opt = default_headers.get(VARY);
assert!(vary_opt.is_some());
let vary = vary_opt.expect("has a value");
assert_eq!(vary, "*");
}

#[test]
fn it_leaves_varys_alone_if_there_are_more_than_one() {
let mut default_headers = HeaderMap::new();
default_headers.insert(VARY, HeaderValue::from_static("one"));
default_headers.append(VARY, HeaderValue::from_static("two"));
process_vary_header(&mut default_headers);
let vary = default_headers.get_all(VARY);
assert_eq!(vary.iter().count(), 2);
for value in vary {
assert!(value == "one" || value == "two");
}
Comment on lines +2312 to +2342
Copy link
Contributor

Choose a reason for hiding this comment

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

<3

}
}
1 change: 1 addition & 0 deletions apollo-router/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ impl Cors {
}))
};
let cors = CorsLayer::new()
.vary([])
.allow_credentials(self.allow_credentials)
.allow_headers(allow_headers)
.expose_headers(cors::ExposeHeaders::list(
Expand Down
4 changes: 4 additions & 0 deletions docs/source/configuration/cors.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,7 @@ cors:
# browser in response to a cross-origin request.
expose_headers: []
```

## Response `Vary` header

A plugin may set a response `Vary` header. If, after all plugins are processed, there is no response `Vary` header, then the router will add one with a value of "origin".