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

fix(pd): re-add cors headers #3870

Merged
merged 1 commit into from
Feb 23, 2024
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
5 changes: 4 additions & 1 deletion .github/workflows/smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ jobs:
- name: Display comet logs
if: always()
run: cat deployments/logs/comet.log
- name: Display pd logs
- name: Display pd runtime logs
if: always()
run: cat deployments/logs/pd.log
- name: Display pd test logs
if: always()
run: cat deployments/logs/pd-tests.log
- name: Display pclientd logs
if: always()
run: cat deployments/logs/pclientd.log
Expand Down
16 changes: 7 additions & 9 deletions crates/bin/pd/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,6 @@ async fn main() -> anyhow::Result<()> {
use penumbra_shielded_pool::component::rpc::Server as ShieldedPoolServer;
use penumbra_stake::component::rpc::Server as StakeServer;

// Set rather permissive CORS headers for pd's gRPC: the service
// should be accessible from arbitrary web contexts, such as localhost,
// or any FQDN that wants to reference its data.
let cors_layer = CorsLayer::permissive();

let mut grpc_server = Server::builder()
.trace_fn(|req| match remote_addr(req) {
Some(remote_addr) => {
Expand All @@ -185,9 +180,6 @@ async fn main() -> anyhow::Result<()> {
// typically uses HTTP/2, which requires HTTPS. Accepting HTTP/2
// allows local applications such as web browsers to talk to pd.
.accept_http1(true)
// Add permissive CORS headers, so pd's gRPC services are accessible
// from arbitrary web contexts, including from localhost.
.layer(cors_layer)
// As part of #2932, we are disabling all timeouts until we circle back to our
// performance story.
// Sets a timeout for all gRPC requests, but note that in the case of streaming
Expand Down Expand Up @@ -242,7 +234,13 @@ async fn main() -> anyhow::Result<()> {
//
// TODO(kate): this is where we may attach additional routes upon this router in the
// future. see #3646 for more information.
let router = grpc_server.into_router();
let router = grpc_server
.into_router()
// Set rather permissive CORS headers for pd's gRPC: the service
// should be accessible from arbitrary web contexts, such as localhost,
// or any FQDN that wants to reference its data.
.layer(CorsLayer::permissive());

let make_svc = router.into_make_service();

// Now start the GRPC server, initializing an ACME client to use as a certificate
Expand Down
23 changes: 23 additions & 0 deletions crates/bin/pd/tests/network_integration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//! Basic integration testing for pd.
//!
//! Validates behavior of the pd binary itself, such as serving specific HTTP
//! headers in all contexts. Does NOT evaluate application logic; see the
//! integration tests for pcli/pclientd for that.

#[ignore]
#[tokio::test]
/// Confirm that permissive CORS headers are returned in HTTP responses
/// by pd. We want these headers to be served directly by pd, so that
/// an intermediate reverse-proxy is not required.
async fn check_cors_headers() -> anyhow::Result<()> {
let client = reqwest::Client::new();
let pd_url =
std::env::var("PENUMBRA_NODE_PD_URL").unwrap_or("http://localhost:8080".to_string());
let r = client.get(pd_url).send().await?;
assert_eq!(r.headers().get("access-control-allow-origin").unwrap(), "*");
assert_eq!(
r.headers().get("access-control-expose-headers").unwrap(),
"*"
);
Ok(())
}
3 changes: 3 additions & 0 deletions deployments/scripts/smoke-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ trap 'kill -9 "$cometbft_pid" "$pd_pid"' EXIT
echo "Waiting $TESTNET_BOOTTIME seconds for network to boot..."
sleep "$TESTNET_BOOTTIME"

echo "Running pd integration tests against running pd binary"
cargo test --release --package pd -- --ignored --test-threads 1 --nocapture | tee "${SMOKE_LOG_DIR}/pd-tests.log"

echo "Running pclientd integration tests against network"
PENUMBRA_NODE_PD_URL="http://127.0.0.1:8080" \
PCLI_UNLEASH_DANGER="yes" \
Expand Down
Loading