Allowed IPs take the full settings reload after a write
write_reload_target sent AllowedIp writes to the blocked-IP reload, but
that reload rebuilds only BlockedIps. Allowed IPs are parsed into the
core's security settings (Security::parse), which only a full reload
rebuilds, so an AllowedIp write reported x:settingsReload applied: true
while the change wasn't live until the next full reload.
AllowedIp now maps to the full reload, like the other settings objects;
BlockedIp keeps its targeted reload.
system::auto_reload::settings_reload_tests now creates an allowed IP
over JMAP and checks that is_ip_allowed sees it with no ReloadSettings,
and that destroying it takes it out again. On main it fails ("allowed
IP not in the running settings").
This commit is contained in:
Vendored
+8
-3
@@ -257,7 +257,8 @@ struct SettingsReloadState {
|
|||||||
/// The reload a write to `object` calls for: the object to reload, or None
|
/// The reload a write to `object` calls for: the object to reload, or None
|
||||||
/// when the running settings don't hold that object (accounts, domains and
|
/// when the running settings don't hold that object (accounts, domains and
|
||||||
/// other data read as needed, stores, which take a restart, and objects with
|
/// other data read as needed, stores, which take a restart, and objects with
|
||||||
/// reload actions of their own, such as applications).
|
/// reload actions of their own, such as applications). Blocked IPs have a
|
||||||
|
/// reload of their own; allowed IPs take the full one.
|
||||||
pub fn write_reload_target(object: ObjectType) -> Option<ObjectType> {
|
pub fn write_reload_target(object: ObjectType) -> Option<ObjectType> {
|
||||||
match object {
|
match object {
|
||||||
ObjectType::Certificate => Some(ObjectType::Certificate),
|
ObjectType::Certificate => Some(ObjectType::Certificate),
|
||||||
@@ -265,8 +266,12 @@ pub fn write_reload_target(object: ObjectType) -> Option<ObjectType> {
|
|||||||
| ObjectType::MemoryLookupKeyValue
|
| ObjectType::MemoryLookupKeyValue
|
||||||
| ObjectType::HttpLookup
|
| ObjectType::HttpLookup
|
||||||
| ObjectType::StoreLookup => Some(ObjectType::StoreLookup),
|
| ObjectType::StoreLookup => Some(ObjectType::StoreLookup),
|
||||||
ObjectType::BlockedIp | ObjectType::AllowedIp => Some(ObjectType::BlockedIp),
|
ObjectType::BlockedIp => Some(ObjectType::BlockedIp),
|
||||||
ObjectType::AcmeProvider
|
// Allowed IPs are part of the core's security settings
|
||||||
|
// (Security::parse), which only a full reload rebuilds; the blocked-IP
|
||||||
|
// reload doesn't touch them
|
||||||
|
ObjectType::AllowedIp
|
||||||
|
| ObjectType::AcmeProvider
|
||||||
| ObjectType::AddressBook
|
| ObjectType::AddressBook
|
||||||
| ObjectType::AiModel
|
| ObjectType::AiModel
|
||||||
| ObjectType::Asn
|
| ObjectType::Asn
|
||||||
|
|||||||
@@ -12,13 +12,16 @@ use crate::utils::{
|
|||||||
server::{TestServer, TestServerBuilder},
|
server::{TestServer, TestServerBuilder},
|
||||||
};
|
};
|
||||||
use common::BuildServer;
|
use common::BuildServer;
|
||||||
use registry::schema::{
|
use registry::{
|
||||||
enums::TracingLevel,
|
schema::{
|
||||||
prelude::ObjectType,
|
enums::TracingLevel,
|
||||||
structs::{
|
prelude::ObjectType,
|
||||||
CertificateManagement, DkimManagement, DnsManagement, Domain, Expression,
|
structs::{
|
||||||
MtaDeliverySchedule, MtaStageAuth, MtaVirtualQueue, Tracer, TracerStdout,
|
AllowedIp, CertificateManagement, DkimManagement, DnsManagement, Domain, Expression,
|
||||||
|
MtaDeliverySchedule, MtaStageAuth, MtaVirtualQueue, Tracer, TracerStdout,
|
||||||
|
},
|
||||||
},
|
},
|
||||||
|
types::ipmask::IpAddrOrMask,
|
||||||
};
|
};
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
@@ -152,6 +155,30 @@ async fn test_write_applies(test: &TestServer) {
|
|||||||
.await;
|
.await;
|
||||||
assert_applied(&response);
|
assert_applied(&response);
|
||||||
|
|
||||||
|
// An allowed IP is live as soon as it is saved, and gone once
|
||||||
|
// destroyed. It lives in the core's security settings, which the
|
||||||
|
// blocked-IP reload it used to get doesn't rebuild.
|
||||||
|
let ip: std::net::IpAddr = "198.51.100.7".parse().unwrap();
|
||||||
|
assert!(!is_allowed(test, ip));
|
||||||
|
let response = admin
|
||||||
|
.registry_create([AllowedIp {
|
||||||
|
address: IpAddrOrMask::from_ip(ip),
|
||||||
|
reason: Some("autoreload".into()),
|
||||||
|
..Default::default()
|
||||||
|
}])
|
||||||
|
.await;
|
||||||
|
assert_applied(&response);
|
||||||
|
assert!(
|
||||||
|
is_allowed(test, ip),
|
||||||
|
"allowed IP not in the running settings"
|
||||||
|
);
|
||||||
|
let allowed_id = response.created_id(0);
|
||||||
|
let response = admin
|
||||||
|
.registry_destroy(ObjectType::AllowedIp, [allowed_id])
|
||||||
|
.await;
|
||||||
|
assert_applied(&response);
|
||||||
|
assert!(!is_allowed(test, ip), "destroyed allowed IP still live");
|
||||||
|
|
||||||
// Data that isn't part of the running settings doesn't reload them
|
// Data that isn't part of the running settings doesn't reload them
|
||||||
let response = admin
|
let response = admin
|
||||||
.registry_create([Domain {
|
.registry_create([Domain {
|
||||||
@@ -187,3 +214,7 @@ fn has_schedule(test: &TestServer, name: &str) -> bool {
|
|||||||
.queue_strategy
|
.queue_strategy
|
||||||
.contains_key(name)
|
.contains_key(name)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn is_allowed(test: &TestServer, ip: std::net::IpAddr) -> bool {
|
||||||
|
test.server.inner.build_server().is_ip_allowed(ip)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user