Merge remote-tracking branch 'origin/main' into problame/repartition-bail-on-concurrent-call

This commit is contained in:
Christian Schwarz
2024-02-21 18:05:28 +00:00
5 changed files with 31 additions and 16 deletions

View File

@@ -21,7 +21,7 @@ use crate::{
console,
error::{ReportableError, UserFacingError},
};
use std::io;
use std::{io, net::IpAddr};
use thiserror::Error;
/// Convenience wrapper for the authentication error.
@@ -62,10 +62,11 @@ pub enum AuthErrorImpl {
Io(#[from] io::Error),
#[error(
"This IP address is not allowed to connect to this endpoint. \
Please add it to the allowed list in the Neon console."
"This IP address {0} is not allowed to connect to this endpoint. \
Please add it to the allowed list in the Neon console. \
Make sure to check for IPv4 or IPv6 addresses."
)]
IpAddressNotAllowed,
IpAddressNotAllowed(IpAddr),
#[error("Too many connections to this endpoint. Please try again later.")]
TooManyConnections,
@@ -87,8 +88,8 @@ impl AuthError {
AuthErrorImpl::AuthFailed(user.into()).into()
}
pub fn ip_address_not_allowed() -> Self {
AuthErrorImpl::IpAddressNotAllowed.into()
pub fn ip_address_not_allowed(ip: IpAddr) -> Self {
AuthErrorImpl::IpAddressNotAllowed(ip).into()
}
pub fn too_many_connections() -> Self {
@@ -122,7 +123,7 @@ impl UserFacingError for AuthError {
MalformedPassword(_) => self.to_string(),
MissingEndpointName => self.to_string(),
Io(_) => "Internal error".to_string(),
IpAddressNotAllowed => self.to_string(),
IpAddressNotAllowed(_) => self.to_string(),
TooManyConnections => self.to_string(),
UserTimeout(_) => self.to_string(),
}
@@ -141,7 +142,7 @@ impl ReportableError for AuthError {
MalformedPassword(_) => crate::error::ErrorKind::User,
MissingEndpointName => crate::error::ErrorKind::User,
Io(_) => crate::error::ErrorKind::ClientDisconnect,
IpAddressNotAllowed => crate::error::ErrorKind::User,
IpAddressNotAllowed(_) => crate::error::ErrorKind::User,
TooManyConnections => crate::error::ErrorKind::RateLimit,
UserTimeout(_) => crate::error::ErrorKind::User,
}

View File

@@ -209,7 +209,7 @@ async fn auth_quirks(
// check allowed list
if !check_peer_addr_is_in_list(&ctx.peer_addr, &allowed_ips) {
return Err(auth::AuthError::ip_address_not_allowed());
return Err(auth::AuthError::ip_address_not_allowed(ctx.peer_addr));
}
let cached_secret = match maybe_secret {
Some(secret) => secret,

View File

@@ -32,7 +32,7 @@ impl PoolingBackend {
let backend = self.config.auth_backend.as_ref().map(|_| user_info.clone());
let (allowed_ips, maybe_secret) = backend.get_allowed_ips_and_secret(ctx).await?;
if !check_peer_addr_is_in_list(&ctx.peer_addr, &allowed_ips) {
return Err(AuthError::ip_address_not_allowed());
return Err(AuthError::ip_address_not_allowed(ctx.peer_addr));
}
let cached_secret = match maybe_secret {
Some(secret) => secret,

View File

@@ -24,7 +24,7 @@ async def test_proxy_psql_allowed_ips(static_proxy: NeonProxy, vanilla_pg: Vanil
with pytest.raises(psycopg2.Error) as exprinfo:
static_proxy.safe_psql(**kwargs)
text = str(exprinfo.value).strip()
assert "This IP address is not allowed to connect" in text
assert "not allowed to connect" in text
# no SNI, deprecated `options=project` syntax (before we had several endpoint in project)
check_cannot_connect(query="select 1", sslsni=0, options="project=private-project")

View File

@@ -1,3 +1,6 @@
import os
import pytest
from fixtures.log_helper import log
from fixtures.neon_fixtures import (
NeonEnvBuilder,
@@ -286,6 +289,12 @@ def test_sharding_split_smoke(
env.attachment_service.consistency_check()
@pytest.mark.skipif(
# The quantity of data isn't huge, but debug can be _very_ slow, and the things we're
# validating in this test don't benefit much from debug assertions.
os.getenv("BUILD_TYPE") == "debug",
reason="Avoid running bulkier ingest tests in debug mode",
)
def test_sharding_ingest(
neon_env_builder: NeonEnvBuilder,
):
@@ -319,10 +328,10 @@ def test_sharding_ingest(
workload = Workload(env, tenant_id, timeline_id)
workload.init()
workload.write_rows(512, upload=False)
workload.write_rows(512, upload=False)
workload.write_rows(512, upload=False)
workload.write_rows(512, upload=False)
workload.write_rows(4096, upload=False)
workload.write_rows(4096, upload=False)
workload.write_rows(4096, upload=False)
workload.write_rows(4096, upload=False)
workload.validate()
small_layer_count = 0
@@ -361,7 +370,12 @@ def test_sharding_ingest(
# - Because we roll layers on checkpoint_distance * shard_count, we expect to obey the target
# layer size on average, but it is still possible to write some tiny layers.
log.info(f"Totals: {small_layer_count} small layers, {ok_layer_count} ok layers")
assert float(small_layer_count) / float(ok_layer_count) < 0.25
if small_layer_count <= shard_count:
# If each shard has <= 1 small layer
pass
else:
# General case:
assert float(small_layer_count) / float(ok_layer_count) < 0.25
# Each shard may emit up to one huge layer, because initdb ingest doesn't respect checkpoint_distance.
assert huge_layer_count <= shard_count