Skip to content

Commit

Permalink
Merge branch 'rjb/no-readonly-ssh-access-for-api-bns' into 'master'
Browse files Browse the repository at this point in the history
fix(api-bns): readonly SSH access for API BNs

# BOUN-1125
# OR-326

This MR extends the SSH access manager such that it is also aware of the API boundary nodes. Before, the SSH access manager assumed that if the node is not part of a subnet, it is an unassigned node and therefore the SSH access policy of unassigned nodes was applied. Now, the SSH access manager checks if the node is part of a subnet, if not, it checks if it is an API boundary node and only then considers it to be an unassigned node. 

See merge request dfinity-lab/public/ic!19171
  • Loading branch information
r-birkner committed May 11, 2024
2 parents e6c4b48 + 4c2a05b commit 263055c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 9 deletions.
8 changes: 6 additions & 2 deletions rs/orchestrator/src/orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,12 @@ impl Orchestrator {
logger.clone(),
);

let ssh_access_manager =
SshAccessManager::new(Arc::clone(&registry), Arc::clone(&metrics), logger.clone());
let ssh_access_manager = SshAccessManager::new(
Arc::clone(&registry),
Arc::clone(&metrics),
node_id,
logger.clone(),
);

let subnet_id: Arc<RwLock<Option<SubnetId>>> = Default::default();

Expand Down
26 changes: 19 additions & 7 deletions rs/orchestrator/src/ssh_access_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
};
use ic_logger::{debug, warn, ReplicaLogger};
use ic_registry_client_helpers::unassigned_nodes::UnassignedNodeRegistry;
use ic_types::{RegistryVersion, SubnetId};
use ic_types::{NodeId, RegistryVersion, SubnetId};
use std::{
io::Write,
process::{Command, Stdio},
Expand All @@ -25,6 +25,7 @@ pub(crate) struct SshAccessParameters {
pub(crate) struct SshAccessManager {
registry: Arc<RegistryHelper>,
metrics: Arc<OrchestratorMetrics>,
node_id: NodeId,
logger: ReplicaLogger,
last_applied_parameters: Arc<RwLock<SshAccessParameters>>,
}
Expand All @@ -33,11 +34,13 @@ impl SshAccessManager {
pub(crate) fn new(
registry: Arc<RegistryHelper>,
metrics: Arc<OrchestratorMetrics>,
node_id: NodeId,
logger: ReplicaLogger,
) -> Self {
Self {
registry,
metrics,
node_id,
logger,
last_applied_parameters: Default::default(),
}
Expand Down Expand Up @@ -138,13 +141,22 @@ impl SshAccessManager {
match subnet_id {
None => match self
.registry
.registry_client
.get_unassigned_nodes_config(version)
.map_err(OrchestratorError::RegistryClientError)?
.get_api_boundary_node_record(self.node_id, version)
{
// Unassigned nodes do not need backup keys
Some(record) => Ok((record.ssh_readonly_access, vec![])),
None => Ok((vec![], vec![])),
// API boundary nodes (for now) do not have readonly or backup keys
Ok(_) => Ok((vec![], vec![])),
// If it is not an API boundary node, it is an unassigned node
Err(OrchestratorError::ApiBoundaryNodeMissingError(_, _)) => match self
.registry
.registry_client
.get_unassigned_nodes_config(version)
.map_err(OrchestratorError::RegistryClientError)?
{
// Unassigned nodes do not need backup keys
Some(record) => Ok((record.ssh_readonly_access, vec![])),
None => Ok((vec![], vec![])),
},
Err(err) => Err(err),
},
Some(subnet_id) => {
self.registry
Expand Down

0 comments on commit 263055c

Please sign in to comment.