Skip to content

Commit

Permalink
Introduce uid and gid to the CellService start method
Browse files Browse the repository at this point in the history
This allows the caller to override the uid and gid of the spawned
executable within the Cell.  The response now includes the uid and gid,
even if they've been inherited from the parent auraed.
  • Loading branch information
dmah42 committed Aug 16, 2024
1 parent e789441 commit d7e026f
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 33 deletions.
26 changes: 13 additions & 13 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -204,35 +204,35 @@ $(1)-lint: $(GEN_RS) $(GEN_TS)

.PHONY: $(1)-test
$(1)-test: $(GEN_RS) $(GEN_TS) auraed
$(cargo) test -p $(1)
$(cargo) test --locked -p $(1)

.PHONY: $(1)-test-all
$(1)-test-all: $(GEN_RS) $(GEN_TS) auraed
$(root_cargo) test -p $(1) -- --include-ignored
$(root_cargo) test --locked -p $(1) -- --include-ignored

.PHONY: $(1)-test-integration
$(1)-test-integration: $(GEN_RS) $(GEN_TS) auraed
$(root_cargo) test -p $(1) --test '*' -- --include-ignored
$(root_cargo) test --locked -p $(1) --test '*' -- --include-ignored

.PHONY: $(1)-test-watch
$(1)-test-watch: $(GEN_RS) $(GEN_TS) auraed # Use cargo-watch to continuously run a test (e.g. make $(1)-test-watch name=path::to::test)
$(root_cargo) watch -- $(cargo) test -p $(1) $(name) -- --include-ignored --nocapture
$(root_cargo) watch -- $(cargo) test --locked -p $(1) $(name) -- --include-ignored --nocapture

.PHONY: $(1)-build
$(1)-build: $(GEN_RS) $(GEN_TS)
$(cargo) build -p $(1)
$(cargo) build --locked -p $(1)

.PHONY: $(1)-build-release
$(1)-build-release: $(GEN_RS) $(GEN_TS)
$(cargo) build -p $(1) --release
$(cargo) build --locked -p $(1) --release

.PHONY: $(1)-debug
$(1)-debug: $(GEN_RS) $(GEN_TS) $(1)-lint
$(cargo) install --path ./$(1) --debug --force
$(cargo) install --locked --path ./$(1) --debug --force

.PHONY: $(1)-release
$(1)-release: $(GEN_RS) $(GEN_TS) $(1)-lint $(1)-test ## Lint, test, and install $(1)
$(cargo) install --path ./$(1) --force
$(cargo) install --locked --path ./$(1) --force
endef

$(foreach p,$(PROGS),$(eval $(call AURAE_template,$(p),$(if $(findstring auraed,$(p)),))))
Expand All @@ -257,19 +257,19 @@ endif

.PHONY: not-auraed-build
not-auraed-build: $(GEN_RS) $(GEN_TS)
$(cargo) build --workspace --exclude auraed
$(cargo) build --locked --workspace --exclude auraed

.PHONY: not-auraed-lint
not-auraed-lint: $(GEN_RS) $(GEN_TS)
$(cargo) clippy --all-features --workspace --exclude auraed -- -D clippy::all -D warnings

.PHONY: not-auraed-test
not-auraed-test: $(GEN_RS) $(GEN_TS)
$(cargo) test --workspace --exclude auraed
$(cargo) test --locked --workspace --exclude auraed

.PHONY: not-auraed-test-all
not-auraed-test-all: $(GEN_RS) $(GEN_TS)
$(cargo) test --workspace --exclude auraed -- --include-ignored
$(cargo) test --locked --workspace --exclude auraed -- --include-ignored

#------------------------------------------------------------------------------#

Expand All @@ -281,11 +281,11 @@ libs-lint: $(GEN_RS) $(GEN_TS)

.PHONY: libs-test
libs-test: $(GEN_RS) $(GEN_TS)
$(cargo) test --workspace --exclude auraed --exclude auraescript --exclude aer
$(cargo) test --locked --workspace --exclude auraed --exclude auraescript --exclude aer

.PHONY: libs-test-all
libs-test-all: $(GEN_RS) $(GEN_TS)
$(cargo) test --workspace --exclude auraed --exclude auraescript --exclude aer -- --include-ignored
$(cargo) test --locked --workspace --exclude auraed --exclude auraescript --exclude aer -- --include-ignored

.PHONY: ebpf
ebpf:
Expand Down
8 changes: 6 additions & 2 deletions api/v0/cells/cells.proto
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ message CellServiceFreeResponse {}
message CellServiceStartRequest {
optional string cell_name = 1;
Executable executable = 2;
optional uint32 uid = 3;
optional uint32 gid = 4;
}

// The response after starting an executable within a Cell.
Expand All @@ -130,8 +132,10 @@ message CellServiceStartResponse {
// in various libc libraries.
int32 pid = 1;

// int32 gid = 2; // TODO
// int32 uid = 3; // TODO
// Return uid and gid of the spawned child which should either match the
// `CellServiceStartRequest` or be inherited from the auraed process.
uint32 uid = 2;
uint32 gid = 3;
// string user = 4; // TODO
// string group = 5; // TODO
}
Expand Down
28 changes: 22 additions & 6 deletions auraed/src/cells/cell_service/cell_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use proto::{
},
observe::LogChannelType,
};
use std::os::unix::fs::MetadataExt;
use std::time::Duration;
use std::{process::ExitStatus, sync::Arc};
use tokio::sync::Mutex;
Expand Down Expand Up @@ -201,8 +202,12 @@ impl CellService {
&self,
request: ValidatedCellServiceStartRequest,
) -> std::result::Result<Response<CellServiceStartResponse>, Status> {
let ValidatedCellServiceStartRequest { cell_name, executable } =
request;
let ValidatedCellServiceStartRequest {
cell_name,
executable,
uid,
gid,
} = request;

assert!(cell_name.is_none());
info!("CellService: start() executable={:?}", executable);
Expand All @@ -211,7 +216,7 @@ impl CellService {

// Start the executable and handle any errors
let executable = executables
.start(executable)
.start(executable, uid, gid)
.map_err(CellsServiceError::ExecutablesError)?;

// Retrieve the process ID (PID) of the started executable
Expand Down Expand Up @@ -247,7 +252,14 @@ impl CellService {
warn!("failed to register stderr channel for pid {pid}: {e}");
}

Ok(Response::new(CellServiceStartResponse { pid }))
let (self_uid, self_gid) =
std::fs::metadata("/proc/self").map(|m| (m.uid(), m.gid()))?;

Ok(Response::new(CellServiceStartResponse {
pid,
uid: uid.unwrap_or(self_uid),
gid: gid.unwrap_or(self_gid),
}))
}

#[tracing::instrument(skip(self))]
Expand Down Expand Up @@ -646,7 +658,11 @@ mod tests {
// Create a validated cell for the allocate request
let cell = ValidatedCell {
name: CellName::from(cell_name),
cpu: Some(ValidatedCpuController { weight: None, max: None, period: None }),
cpu: Some(ValidatedCpuController {
weight: None,
max: None,
period: None,
}),
cpuset: Some(ValidatedCpusetController { cpus: None, mems: None }),
memory: Some(ValidatedMemoryController {
min: None,
Expand All @@ -660,4 +676,4 @@ mod tests {
// Return the validated allocate request
ValidatedCellServiceAllocateRequest { cell }
}
}
}
24 changes: 17 additions & 7 deletions auraed/src/cells/cell_service/executables/executable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use tracing::info_span;

// TODO: decide if we're going to use the description or not. Remove if not.
#[allow(dead_code)]

#[derive(Debug)]
pub struct Executable {
pub name: ExecutableName,
Expand Down Expand Up @@ -65,17 +64,27 @@ impl Executable {

/// Starts the underlying process.
/// Does nothing if [Executable] has previously been started.
pub fn start(&mut self) -> io::Result<()> {
pub fn start(
&mut self,
uid: Option<u32>,
gid: Option<u32>,
) -> io::Result<()> {
let ExecutableState::Init { command } = &mut self.state else {
return Ok(());
};

let mut child = command
let mut command = command
.kill_on_drop(true)
.current_dir("/")
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()?;
.stderr(Stdio::piped());
if uid.is_some() {
command = command.uid(uid.expect("uid"));
}
if gid.is_some() {
command = command.gid(gid.expect("gid"));
}
let mut child = command.spawn()?;

let log_channel = self.stdout.clone();
let stdout = child.stdout.take().expect("stdout");
Expand Down Expand Up @@ -146,10 +155,11 @@ impl Executable {

/// Returns the [Pid] while [Executable] is running, otherwise returns [None].
pub fn pid(&self) -> io::Result<Option<Pid>> {
let ExecutableState::Started { child: process, .. } = &self.state else {
let ExecutableState::Started { child: process, .. } = &self.state
else {
return Ok(None);
};

Ok(process.id().map(|id| Pid::from_raw(id as i32)))
}
}
}
4 changes: 3 additions & 1 deletion auraed/src/cells/cell_service/executables/executables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ impl Executables {
pub fn start<T: Into<ExecutableSpec>>(
&mut self,
executable_spec: T,
uid: Option<u32>,
gid: Option<u32>,
) -> Result<&Executable> {
let executable_spec = executable_spec.into();

Expand All @@ -46,7 +48,7 @@ impl Executables {

// start the exe before we add it to the cache, as otherwise a failure leads to the
// executable remaining in the cache and start cannot be called again.
executable.start().map_err(|e| {
executable.start(uid, gid).map_err(|e| {
ExecutablesError::FailedToStartExecutable {
executable_name: executable_name.clone(),
source: e,
Expand Down
2 changes: 1 addition & 1 deletion auraed/src/cells/cell_service/executables/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ pub struct ExecutableSpec {
pub name: ExecutableName,
pub description: String,
pub command: Command,
}
}
6 changes: 5 additions & 1 deletion auraed/src/cells/cell_service/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ pub struct ValidatedCellServiceStartRequest {
pub cell_name: Option<CellName>,
#[field_type(Option<Executable>)]
pub executable: ValidatedExecutable,
#[validate(none)]
pub uid: Option<u32>,
#[validate(none)]
pub gid: Option<u32>,
}

impl CellServiceStartRequestTypeValidator for CellServiceStartRequestValidator {
Expand Down Expand Up @@ -540,4 +544,4 @@ mod tests {
assert!(validated.is_ok());
assert_eq!(validated.unwrap(), OsString::from("command"));
}
}
}
23 changes: 21 additions & 2 deletions auraed/tests/common/cells.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,18 @@ impl ExecutableBuilder {
pub(crate) struct CellServiceStartRequestBuilder {
cell_name: Option<String>,
executable_builder: ExecutableBuilder,
uid: Option<u32>,
gid: Option<u32>,
}

impl CellServiceStartRequestBuilder {
pub fn new() -> Self {
Self { cell_name: None, executable_builder: ExecutableBuilder::new() }
Self {
cell_name: None,
executable_builder: ExecutableBuilder::new(),
uid: None,
gid: None,
}
}

pub fn cell_name(&mut self, cell_name: String) -> &mut Self {
Expand All @@ -132,11 +139,23 @@ impl CellServiceStartRequestBuilder {
self
}

pub fn uid(&mut self, uid: u32) -> &mut Self {
self.uid = Some(uid);
self
}

pub fn gid(&mut self, gid: u32) -> &mut Self {
self.gid = Some(gid);
self
}

pub fn build(&self) -> CellServiceStartRequest {
assert!(self.cell_name.is_some(), "cell_name needs to be set");
CellServiceStartRequest {
cell_name: self.cell_name.clone(),
executable: Some(self.executable_builder.build()),
uid: self.uid,
gid: self.gid,
}
}
}
}

0 comments on commit d7e026f

Please sign in to comment.