Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
- Use full match instead of `matches!`
- Use `server_context()` method
- Rename `ServerKind::for_internal()` constructor
  • Loading branch information
bnaecker committed May 14, 2024
1 parent c758366 commit 85d66c0
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 13 deletions.
7 changes: 5 additions & 2 deletions nexus/src/app/allow_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ impl super::Nexus {
// Note that we elide this check when handling a request proxied
// from `wicketd`. This is intentional and used as a safety
// mechanism in the even of lockout or other recovery scenarios.
let check_remote_addr = match server_kind {
ServerKind::External => true,
ServerKind::Techport | ServerKind::Internal => false,
};
let mut contains_remote = false;
for entry in list.iter() {
contains_remote |= entry.contains(remote_addr);
Expand All @@ -74,8 +78,7 @@ impl super::Nexus {
));
}
}
if !contains_remote && !matches!(server_kind, ServerKind::Techport)
{
if check_remote_addr && !contains_remote {
return Err(Error::invalid_request(
"The source IP allow list would prevent access \
from the current client! Ensure that the allowlist \
Expand Down
6 changes: 3 additions & 3 deletions nexus/src/app/sagas/disk_create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ pub(crate) mod test {
let log = &cptestctx.logctx.log;

let client = &cptestctx.external_client;
let nexus = &cptestctx.server.apictx.context.nexus;
let nexus = &cptestctx.server.server_context().nexus;
let project_id =
create_project(&client, PROJECT_NAME).await.identity.id;
let opctx = test_opctx(&cptestctx);
Expand All @@ -1111,7 +1111,7 @@ pub(crate) mod test {
}

async fn destroy_disk(cptestctx: &ControlPlaneTestContext) {
let nexus = &cptestctx.server.apictx.context.nexus;
let nexus = &cptestctx.server.server_context().nexus;
let opctx = test_opctx(&cptestctx);
let disk_selector = params::DiskSelector {
project: Some(
Expand All @@ -1134,7 +1134,7 @@ pub(crate) mod test {
let test = DiskTest::new(cptestctx).await;

let client = &cptestctx.external_client;
let nexus = &cptestctx.server.apictx.context.nexus;
let nexus = &cptestctx.server.server_context().nexus;
let project_id =
create_project(&client, PROJECT_NAME).await.identity.id;

Expand Down
8 changes: 4 additions & 4 deletions nexus/src/app/sagas/disk_delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,12 @@ pub(crate) mod test {
pub fn test_opctx(cptestctx: &ControlPlaneTestContext) -> OpContext {
OpContext::for_tests(
cptestctx.logctx.log.new(o!()),
cptestctx.server.apictx.context.nexus.datastore().clone(),
cptestctx.server.server_context().nexus.datastore().clone(),
)
}

async fn create_disk(cptestctx: &ControlPlaneTestContext) -> Disk {
let nexus = &cptestctx.server.apictx.context.nexus;
let nexus = &cptestctx.server.server_context().nexus;
let opctx = test_opctx(&cptestctx);

let project_selector = params::ProjectSelector {
Expand All @@ -232,7 +232,7 @@ pub(crate) mod test {
DiskTest::new(cptestctx).await;

let client = &cptestctx.external_client;
let nexus = &cptestctx.server.apictx.context.nexus;
let nexus = &cptestctx.server.server_context().nexus;
let project_id = create_project(client, PROJECT_NAME).await.identity.id;
let disk = create_disk(&cptestctx).await;

Expand All @@ -258,7 +258,7 @@ pub(crate) mod test {
let test = DiskTest::new(cptestctx).await;

let client = &cptestctx.external_client;
let nexus = &cptestctx.server.apictx.context.nexus;
let nexus = &cptestctx.server.server_context().nexus;
let project_id = create_project(client, PROJECT_NAME).await.identity.id;
let disk = create_disk(&cptestctx).await;

Expand Down
2 changes: 1 addition & 1 deletion nexus/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub struct ApiContext {
impl ApiContext {
/// Create a new context with a rack ID and logger. This creates the
/// underlying `Nexus` as well.
pub async fn new(
pub async fn for_internal(
rack_id: Uuid,
log: Logger,
config: &NexusConfig,
Expand Down
10 changes: 7 additions & 3 deletions nexus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,12 @@ impl InternalServer {

let ctxlog = log.new(o!("component" => "ServerContext"));

let context =
ApiContext::new(config.deployment.rack_id, ctxlog, &config).await?;
let context = ApiContext::for_internal(
config.deployment.rack_id,
ctxlog,
&config,
)
.await?;

// Launch the internal server.
let server_starter_internal = dropshot::HttpServerStarter::new(
Expand Down Expand Up @@ -228,7 +232,7 @@ impl Server {
/// immediately after calling `start()`, the program will block indefinitely
/// or until something else initiates a graceful shutdown.
pub(crate) async fn wait_for_finish(self) -> Result<(), String> {
self.apictx.context.nexus.wait_for_shutdown().await
self.server_context().nexus.wait_for_shutdown().await
}
}

Expand Down

0 comments on commit 85d66c0

Please sign in to comment.