Skip to content

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jgallagher committed Oct 17, 2023
1 parent 0a83237 commit 53e0654
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 19 deletions.
2 changes: 1 addition & 1 deletion nexus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl Server {
tls_config.map(dropshot::ConfigTls::Dynamic),
)
.map_err(|error| {
format!("initializing external server: {}", error)
format!("initializing external techport server: {}", error)
})?;
server_starter_external_techport.start()
};
Expand Down
6 changes: 5 additions & 1 deletion smf/wicketd/manifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@
for TLS
both of which are fine for our standard wicketd deployment, which only
listens on a `::1` (IPv6 localhost) address without TLS.
listens on a `::1` (IPv6 localhost) address without TLS. If any of the
above assumptions are violated, the `reload-config` endpoint we hit
here will return an error (either directly, if we attempt to change
`config/address`, or indirectly because we tried to post via http but
it expected https).
-->
<exec_method type='method' name='refresh'
exec='curl -X POST http://%{config/address}/reload-config'
Expand Down
8 changes: 1 addition & 7 deletions wicketd/src/bin/wicketd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,7 @@ async fn do_run() -> Result<(), CmdError> {
Some(addr) => Some(Ipv6Subnet::new(addr)),
None if read_smf_config => {
let smf_values = SmfConfigValues::read_current()
.map_err(|e| CmdError::Failure(e.to_string()))?
.ok_or_else(|| {
CmdError::Failure(
"--read-smf-config only available on illumos"
.to_string(),
)
})?;
.map_err(|e| CmdError::Failure(e.to_string()))?;
smf_values.rack_subnet
}
None => None,
Expand Down
1 change: 1 addition & 0 deletions wicketd/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use std::sync::OnceLock;

/// Shared state used by API handlers
pub struct ServerContext {
pub(crate) bind_address: SocketAddrV6,
pub mgs_handle: MgsHandle,
pub mgs_client: gateway_client::Client,
pub(crate) log: slog::Logger,
Expand Down
12 changes: 7 additions & 5 deletions wicketd/src/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1259,14 +1259,16 @@ async fn post_reload_config(
)
})?;

let Some(smf_values) = smf_values else {
let rqctx = rqctx.context();

// We do not allow a config reload to change our bound address; return an
// error if the caller is attempting to do so.
if rqctx.bind_address != smf_values.address {
return Err(HttpError::for_bad_request(
None,
"reloading config from SMF only available on illumos".to_string(),
"listening address cannot be reconfigured".to_string(),
));
};

let rqctx = rqctx.context();
}

if let Some(rack_subnet) = smf_values.rack_subnet {
let resolver = Resolver::new_from_subnet(
Expand Down
23 changes: 18 additions & 5 deletions wicketd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mod preflight_check;
mod rss_config;
mod update_tracker;

use anyhow::{anyhow, Result};
use anyhow::{anyhow, bail, Result};
use artifacts::{WicketdArtifactServer, WicketdArtifactStore};
use bootstrap_addrs::BootstrapPeers;
pub use config::Config;
Expand Down Expand Up @@ -63,17 +63,19 @@ pub struct Args {
}

pub struct SmfConfigValues {
pub address: SocketAddrV6,
pub rack_subnet: Option<Ipv6Subnet<AZ_PREFIX>>,
}

impl SmfConfigValues {
#[cfg(target_os = "illumos")]
pub fn read_current() -> Result<Option<Self>> {
pub fn read_current() -> Result<Self> {
use anyhow::Context;
use illumos_utils::scf::ScfHandle;

const CONFIG_PG: &str = "config";
const PROP_RACK_SUBNET: &str = "rack-subnet";
const PROP_ADDRESS: &str = "address";

let scf = ScfHandle::new()?;
let instance = scf.self_instance()?;
Expand All @@ -94,12 +96,22 @@ impl SmfConfigValues {
Some(Ipv6Subnet::new(addr))
};

Ok(Some(Self { rack_subnet }))
let address = {
let address = config.value_as_string(PROP_ADDRESS)?;
address.parse().with_context(|| {
format!(
"failed to parse {CONFIG_PG}/{PROP_ADDRESS} \
value {address:?} as a socket address"
)
})?
};

Ok(Some(Self { address, rack_subnet }))
}

#[cfg(not(target_os = "illumos"))]
pub fn read_current() -> Result<Option<Self>> {
Ok(None)
pub fn read_current() -> Result<Self> {
bail!("reading SMF config only available on illumos")
}
}

Expand Down Expand Up @@ -182,6 +194,7 @@ impl Server {
&dropshot_config,
http_entrypoints::api(),
ServerContext {
bind_address: args.address,
mgs_handle,
mgs_client,
log: log.clone(),
Expand Down

0 comments on commit 53e0654

Please sign in to comment.