Skip to content

Commit

Permalink
Fix clippy warnings, other cleanups
Browse files Browse the repository at this point in the history
Signed-off-by: Santtu Lakkala <[email protected]>
  • Loading branch information
slakkala committed Sep 25, 2024
1 parent 1f23c6c commit 0ff57cd
Show file tree
Hide file tree
Showing 14 changed files with 123 additions and 112 deletions.
42 changes: 18 additions & 24 deletions client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,21 @@ use async_channel::Receiver;
use givc_common::pb;
pub use givc_common::query::{Event, QueryResult};
use givc_common::types::*;
use std::future::Future;
use std::pin::Pin;
use tokio_stream::StreamExt;
use tonic::transport::Channel;
use tracing::debug;

type Client = pb::admin_service_client::AdminServiceClient<Channel>;

#[derive(Debug)]
pub struct WatchResult {
pub initial: Vec<QueryResult>,
// Design defence: we use `async-channel` here, as it could be used with both
// tokio's and glib's eventloop, and recommended by gtk4-rs developers:
pub channel: Receiver<Event>,

task: tokio::task::JoinHandle<()>,
}

impl Drop for WatchResult {
fn drop(&mut self) {
self.task.abort()
}
pub task: Pin<Box<dyn Future<Output = ()>>>,
}

#[derive(Debug)]
Expand All @@ -39,20 +34,20 @@ impl AdminClient {

// New style api, not yet implemented, stub atm to make current code happy
// FIXME: Still doubt if constructor should be sync or async
pub fn new(addr: String, port: u16, tls_info: Option<(String, TlsConfig)>) -> Self {
let (name, tls) = match tls_info {
pub fn new(address: String, port: u16, tls_info: Option<(String, TlsConfig)>) -> Self {
let (tls_name, tls) = match tls_info {
Some((name, tls)) => (name, Some(tls)),
None => (String::from("bogus(no tls)"), None),
};
Self {
endpoint: EndpointConfig {
transport: TransportConfig {
address: addr,
port: port,
address,
port,
protocol: String::from("bogus"),
tls_name: name,
tls_name,
},
tls: tls,
tls,
},
}
}
Expand Down Expand Up @@ -92,8 +87,7 @@ impl AdminClient {
vm_name,
args,
};
let response = self.connect_to().await?.start_application(request).await?;
// Ok(response.into_inner().cmd_status)
let _response = self.connect_to().await?.start_application(request).await?;
Ok(())
}
pub async fn stop(&self, _app: String) -> anyhow::Result<()> {
Expand Down Expand Up @@ -164,6 +158,8 @@ impl AdminClient {
}

pub async fn watch(&self) -> anyhow::Result<WatchResult> {
use pb::admin::watch_item::Status;
use pb::admin::WatchItem;
let (tx, rx) = async_channel::bounded::<Event>(10);

let mut watch = self
Expand All @@ -174,15 +170,13 @@ impl AdminClient {
.into_inner();

let list = match watch.try_next().await? {
Some(first) => match first.status {
Some(pb::admin::watch_item::Status::Initial(init)) => QueryResult::parse_list(init.list)?,
Some(item) => bail!("Protocol error, first item in stream not pb::admin::watch_item::Status::Initial, {:?}", item),
None => bail!("Protocol error, initial item missing"),
},
Some(WatchItem { status: Some(Status::Initial(init)) }) => QueryResult::parse_list(init.list)?,
Some(WatchItem { status: Some(item) }) => bail!("Protocol error, first item in stream not pb::admin::watch_item::Status::Initial, {:?}", item),
Some(_) => bail!("Protocol error, initial item missing"),
None => bail!("Protocol error, status field missing"),
};

let task = tokio::task::spawn(async move {
let task = async move {
loop {
if let Ok(Some(event)) = watch.try_next().await {
let event = match Event::try_from(event) {
Expand All @@ -201,12 +195,12 @@ impl AdminClient {
break;
}
}
});
};

let result = WatchResult {
initial: list,
channel: rx,
task,
task: Box::pin(task),
};
Ok(result)
}
Expand Down
10 changes: 5 additions & 5 deletions client/src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ pub struct EndpointConfig {

impl TlsConfig {
pub fn client_config(&self) -> anyhow::Result<ClientTlsConfig> {
let pem = std::fs::read_to_string(self.ca_cert_file_path.as_os_str())?;
let pem = std::fs::read(&self.ca_cert_file_path)?;
let ca = Certificate::from_pem(pem);

let client_cert = std::fs::read(self.cert_file_path.as_os_str())?;
let client_key = std::fs::read(self.key_file_path.as_os_str())?;
let client_cert = std::fs::read(&self.cert_file_path)?;
let client_key = std::fs::read(&self.key_file_path)?;
let client_identity = Identity::from_pem(client_cert, client_key);
let tls_name = self
.tls_name
Expand All @@ -42,8 +42,8 @@ impl TlsConfig {
}

pub fn server_config(&self) -> anyhow::Result<ServerTlsConfig> {
let cert = std::fs::read(self.cert_file_path.as_os_str())?;
let key = std::fs::read(self.key_file_path.as_os_str())?;
let cert = std::fs::read(&self.cert_file_path)?;
let key = std::fs::read(&self.key_file_path)?;
let identity = Identity::from_pem(cert, key);
let config = ServerTlsConfig::new().identity(identity);
Ok(config)
Expand Down
26 changes: 13 additions & 13 deletions common/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ impl TryFrom<pb::QueryListItem> for QueryResult {
}
}

impl Into<pb::QueryListItem> for QueryResult {
fn into(self) -> pb::QueryListItem {
pb::QueryListItem {
name: self.name,
description: self.description,
vm_status: self.status.to_string(),
trust_level: self.trust_level.to_string(),
impl From<QueryResult> for pb::QueryListItem {
fn from(val: QueryResult) -> Self {
Self {
name: val.name,
description: val.description,
vm_status: val.status.to_string(),
trust_level: val.trust_level.to_string(),
}
}
}
Expand Down Expand Up @@ -117,12 +117,12 @@ impl TryFrom<pb::WatchItem> for Event {
}
}

impl Into<pb::WatchItem> for Event {
fn into(self) -> pb::WatchItem {
match self {
Event::UnitRegistered(value) => Self::watch_item(Status::Added(value.into())),
Event::UnitStatusChanged(value) => Self::watch_item(Status::Updated(value.into())),
Event::UnitShutdown(value) => Self::watch_item(Status::Removed(value.into())),
impl From<Event> for pb::WatchItem {
fn from(val: Event) -> Self {
match val {
Event::UnitRegistered(value) => Event::watch_item(Status::Added(value.into())),
Event::UnitStatusChanged(value) => Event::watch_item(Status::Updated(value.into())),
Event::UnitShutdown(value) => Event::watch_item(Status::Removed(value.into())),
}
}
}
44 changes: 22 additions & 22 deletions common/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// This module contain literal translations of types from internal/pkgs/types/types.go
// Some of them would be rewritten, replaced, or even removed
use crate::pb;
use anyhow::{anyhow, bail};
use anyhow::bail;
use std::convert::{Into, TryFrom};

#[derive(Debug, Copy, Clone, PartialEq)]
Expand Down Expand Up @@ -104,30 +104,30 @@ impl TryFrom<u32> for UnitType {
// so we can use just Into<u32> trait
// FIXME: Combination of `UnitType{ vm: Host, service: VM}` is ILLEGAL!!!
// Should we use TryInto, or fix type system?
impl Into<u32> for UnitType {
fn into(self) -> u32 {
impl From<UnitType> for u32 {
fn from(val: UnitType) -> Self {
use ServiceType::*;
use VmType::*;
match self.vm {
Host => match self.service {
match val.vm {
Host => match val.service {
Mgr => 0,
Svc => 1,
App => 2,
VM => 100500,
},
AdmVM => match self.service {
AdmVM => match val.service {
VM => 3,
Mgr => 4,
Svc => 5,
App => 6,
},
SysVM => match self.service {
SysVM => match val.service {
VM => 7,
Mgr => 8,
Svc => 9,
App => 10,
},
AppVM => match self.service {
AppVM => match val.service {
VM => 11,
Mgr => 12,
Svc => 13,
Expand Down Expand Up @@ -172,14 +172,14 @@ impl TryFrom<pb::UnitStatus> for UnitStatus {
}
}

impl Into<pb::UnitStatus> for UnitStatus {
fn into(self) -> pb::UnitStatus {
pb::UnitStatus {
name: self.name,
description: self.description,
load_state: self.load_state,
active_state: self.active_state,
path: self.path,
impl From<UnitStatus> for pb::UnitStatus {
fn from(val: UnitStatus) -> Self {
Self {
name: val.name,
description: val.description,
load_state: val.load_state,
active_state: val.active_state,
path: val.path,
}
}
}
Expand All @@ -206,13 +206,13 @@ impl TryFrom<pb::TransportConfig> for EndpointEntry {
}
}

impl Into<pb::TransportConfig> for EndpointEntry {
fn into(self) -> pb::TransportConfig {
impl From<EndpointEntry> for pb::TransportConfig {
fn from(val: EndpointEntry) -> Self {
pb::TransportConfig {
protocol: self.protocol,
address: self.address,
port: self.port.to_string(),
name: self.tls_name,
protocol: val.protocol,
address: val.address,
port: val.port.to_string(),
name: val.tls_name,
}
}
}
7 changes: 4 additions & 3 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,14 @@
./internal
];
};
in
rec {
givc-agent = pkgs.callPackage ./nixos/packages/givc-agent.nix { inherit src; };
givc-admin-rs = pkgs.callPackage ./nixos/packages/givc-admin-rs.nix {
inherit crane;
src = ./.;
};
in
{
inherit givc-admin-rs;
givc-agent = pkgs.callPackage ./nixos/packages/givc-agent.nix { inherit src; };
givc-cli = givc-admin-rs.cli;
};
};
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/givc-agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
func main() {

var err error
log.Infof("Executing %s", filepath.Base(os.Args[0]))
log.Infof("Running %s", filepath.Base(os.Args[0]))

name := os.Getenv("NAME")
if name == "" {
Expand Down
18 changes: 9 additions & 9 deletions src/admin/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Some of them would be rewritten, replaced, or even removed
use crate::pb;
use anyhow::anyhow;
use std::convert::{Into, TryFrom};
use std::convert::TryFrom;

use givc_common::query::*;
use givc_common::types::*;
Expand Down Expand Up @@ -83,25 +83,25 @@ impl TryFrom<pb::RegistryRequest> for RegistryEntry {
// Protocol very inconsistent here
Ok(Self {
name: req.name,
status: status,
watch: watch,
status,
watch,
r#type: ty,
placement: Placement::Endpoint(endpoint),
})
}
}

impl Into<QueryResult> for RegistryEntry {
fn into(self) -> QueryResult {
let status = if self.status.is_running() {
impl From<RegistryEntry> for QueryResult {
fn from(val: RegistryEntry) -> Self {
let status = if val.status.is_running() {
VMStatus::Running
} else {
VMStatus::PoweredOff
};
QueryResult {
name: self.name,
description: self.status.description,
status: status,
name: val.name,
description: val.status.description,
status,
trust_level: TrustLevel::default(),
}
}
Expand Down
18 changes: 14 additions & 4 deletions src/admin/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ pub struct Registry {
pubsub: broadcast::Sender<Event>,
}

impl Default for Registry {
fn default() -> Self {
Self::new()
}
}

impl Registry {
pub fn new() -> Self {
Self {
Expand All @@ -34,6 +40,7 @@ impl Registry {
info!("Replaced old entry {:#?}", old);
self.send_event(Event::UnitShutdown(old.into()))
};
info!("Sending event {event:?}");
self.send_event(event)
}

Expand All @@ -45,7 +52,10 @@ impl Registry {
self.send_event(Event::UnitShutdown(entry.into()));
Ok(())
}
None => bail!("Can't deregister entry {}, it not registered", name),
None => Err(anyhow!(
"Can't deregister entry {}, it not registered",
name
)),
}
}

Expand All @@ -64,7 +74,7 @@ impl Registry {
.filter(|x| x.starts_with(name))
.cloned()
.collect();
if list.len() == 0 {
if list.is_empty() {
bail!("No entries match string {}", name)
} else {
Ok(list)
Expand Down Expand Up @@ -115,7 +125,7 @@ impl Registry {
e.status = status;
self.send_event(Event::UnitStatusChanged(e.clone().into()))
})
.ok_or_else(|| anyhow!("Can't update state for {}, is not registered", name))
.ok_or_else(|| anyhow!("Can't update state for {name}, is not registered"))
}

// FIXME: Should we dump full contents here for `query`/`query_list` high-level API
Expand Down Expand Up @@ -156,7 +166,7 @@ mod tests {
r.register(bar);

assert!(r.contains(&foo_key));
assert!(r.contains(&"bar".to_string()));
assert!(r.contains("bar"));

let foo1 = r.by_name(&foo_key)?;
assert_eq!(foo1, foo);
Expand Down
Loading

0 comments on commit 0ff57cd

Please sign in to comment.