Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable sdv.databroker.v1 by default #20

Merged
merged 1 commit into from
Apr 25, 2024
Merged

Disable sdv.databroker.v1 by default #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions databroker-cli/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ pub struct Cli {
ca_cert: Option<String>,

#[arg(value_enum)]
#[clap(long, short = 'p', value_enum, default_value = "sdv-databroker-v1")]
protocol: CliAPI,
#[clap(long, short = 'p', value_enum, default_value_t = Protocol::KuksaValV1)]
protocol: Protocol,

// Sub command
#[clap(subcommand)]
Expand All @@ -77,7 +77,7 @@ impl Cli {
self.server.clone()
}

pub fn get_protocol(&mut self) -> CliAPI {
pub fn get_protocol(&mut self) -> Protocol {
self.protocol
}
}
Expand All @@ -93,8 +93,10 @@ pub enum Commands {
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum)]
pub enum CliAPI {
pub enum Protocol {
#[clap(name = "kuksa.val.v1")]
KuksaValV1 = 1,
#[clap(name = "sdv.databroker.v1")]
SdvDatabrokerV1 = 2,
}

Expand Down
13 changes: 5 additions & 8 deletions databroker-cli/src/kuksa_cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ const TIMEOUT: Duration = Duration::from_millis(500);
const CLI_COMMANDS: &[(&str, &str, &str)] = &[
("connect", "[URI]", "Connect to server"),
("get", "<PATH> [[PATH] ...]", "Get signal value(s)"),
("set", "<PATH> <VALUE>", "Set actuator signal"),
("actuate", "<PATH> <VALUE>", "Set actuator signal"),
(
"subscribe",
"<QUERY>",
"Subscribe to signals with QUERY, if you use kuksa feature comma separated list",
),
("feed", "<PATH> <VALUE>", "Publish signal value"),
("publish", "<PATH> <VALUE>", "Publish signal value"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provide?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that crossed my mind as well. I'm not against it, but my idea is that publish is that act of publishing a single signal value, while "provide" might be better suited for describing the act of continuously providing a signal. "publish" would then be the logical counterpart to "subscribe".

I suppose we will have a discussion around this in the API design rounds. I just couldn't stand having feed still there 🥲

(
"metadata",
"[PATTERN]",
Expand Down Expand Up @@ -318,7 +318,7 @@ pub async fn kuksa_main(_cli: Cli) -> Result<(), Box<dyn std::error::Error>> {
)?,
}
}
"set" => {
"actuate" => {
interface.add_history_unique(line.clone());

let (path, value) = cli::split_first_word(args);
Expand Down Expand Up @@ -368,9 +368,6 @@ pub async fn kuksa_main(_cli: Cli) -> Result<(), Box<dyn std::error::Error>> {
cmd,
format!("{} is not an actuator.", path),
)?;
cli::print_info(
"If you want to provide the signal value, use `feed`.",
)?;
continue;
}

Expand Down Expand Up @@ -402,7 +399,7 @@ pub async fn kuksa_main(_cli: Cli) -> Result<(), Box<dyn std::error::Error>> {
}
}
}
"feed" => {
"publish" => {
interface.add_history_unique(line.clone());

let (path, value) = cli::split_first_word(args);
Expand Down Expand Up @@ -902,7 +899,7 @@ impl<Term: Terminal> Completer<Term> for CliCompleter {
Some(compls)
}
// Complete command parameters
Some("set") | Some("feed") => {
Some("actuate") | Some("publish") => {
if words.count() == 0 {
self.complete_entry_path(word)
} else {
Expand Down
8 changes: 4 additions & 4 deletions databroker-cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
********************************************************************************/

use clap::Parser;
use cli::CliAPI;
use cli::Protocol;

pub mod cli;
mod kuksa_cli;
Expand All @@ -21,19 +21,19 @@ mod sdv_cli;
#[tokio::main]
async fn main() {
let mut cli = cli::Cli::parse();
if cli.get_protocol() == CliAPI::SdvDatabrokerV1 {
if cli.get_protocol() == Protocol::SdvDatabrokerV1 {
let err = sdv_cli::sdv_main(cli.clone()).await;
match err {
Ok(_) => (),
Err(e) => eprintln!("Error: {}", e),
}
} else if cli.get_protocol() == CliAPI::KuksaValV1 {
} else if cli.get_protocol() == Protocol::KuksaValV1 {
let err = kuksa_cli::kuksa_main(cli.clone()).await;
match err {
Ok(_) => (),
Err(e) => eprintln!("Error: {}", e),
}
} else {
println!("Choose one protocol of either kuksa-val-v1 or sdv-databroker-v1")
println!("Choose one protocol of either kuksa.val.v1 or sdv.databroker.v1")
}
}
9 changes: 3 additions & 6 deletions databroker-cli/src/sdv_cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const CLI_COMMANDS: &[(&str, &str, &str)] = &[
"<QUERY>",
"Subscribe to signals with QUERY, if you use kuksa feature comma separated list",
),
("feed", "<PATH> <VALUE>", "Publish signal value"),
("publish", "<PATH> <VALUE>", "Publish signal value"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provide?

(
"metadata",
"[PATTERN]",
Expand Down Expand Up @@ -351,9 +351,6 @@ pub async fn sdv_main(_cli: Cli) -> Result<(), Box<dyn std::error::Error>> {
cmd,
format!("{} is not an actuator.", metadata.name),
)?;
cli::print_info(
"If you want to provide the signal value, use `feed`.",
)?;
continue;
}

Expand Down Expand Up @@ -404,7 +401,7 @@ pub async fn sdv_main(_cli: Cli) -> Result<(), Box<dyn std::error::Error>> {
}
}
}
"feed" => {
"publish" => {
interface.add_history_unique(line.clone());

let (path, value) = cli::split_first_word(args);
Expand Down Expand Up @@ -916,7 +913,7 @@ impl<Term: Terminal> Completer<Term> for CliCompleter {
Some(compls)
}
// Complete command parameters
Some("set") | Some("feed") => {
Some("set") | Some("publish") => {
if words.count() == 0 {
self.complete_entry_path(word)
} else {
Expand Down
45 changes: 33 additions & 12 deletions databroker/src/grpc/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ pub enum ServerTLS {
Enabled { tls_config: ServerTlsConfig },
}

#[derive(PartialEq)]
pub enum Api {
KuksaValV1,
SdvDatabrokerV1,
}

impl tonic::service::Interceptor for Authorization {
fn call(
&mut self,
Expand Down Expand Up @@ -93,6 +99,7 @@ pub async fn serve<F>(
addr: impl Into<std::net::SocketAddr>,
broker: broker::DataBroker,
#[cfg(feature = "tls")] server_tls: ServerTLS,
apis: &[Api],
authorization: Authorization,
signal: F,
) -> Result<(), Box<dyn std::error::Error>>
Expand Down Expand Up @@ -124,6 +131,7 @@ where
broker,
#[cfg(feature = "tls")]
server_tls,
apis,
authorization,
signal,
)
Expand All @@ -134,6 +142,7 @@ pub async fn serve_with_incoming_shutdown<F>(
listener: TcpListener,
broker: broker::DataBroker,
#[cfg(feature = "tls")] server_tls: ServerTLS,
apis: &[Api],
authorization: Authorization,
signal: F,
) -> Result<(), Box<dyn std::error::Error>>
Expand All @@ -146,15 +155,15 @@ where
}

let incoming = TcpListenerStream::new(listener);
let mut builder = Server::builder()
let mut server = Server::builder()
.http2_keepalive_interval(Some(Duration::from_secs(10)))
.http2_keepalive_timeout(Some(Duration::from_secs(20)));

#[cfg(feature = "tls")]
match server_tls {
ServerTLS::Enabled { tls_config } => {
info!("Using TLS");
builder = builder.tls_config(tls_config)?;
server = server.tls_config(tls_config)?;
}
ServerTLS::Disabled => {
info!("TLS is not enabled")
Expand All @@ -165,23 +174,35 @@ where
info!("Authorization is not enabled.");
}

builder
.add_service(
let kuksa_val_v1 = {
if apis.contains(&Api::KuksaValV1) {
Some(kuksa::val::v1::val_server::ValServer::with_interceptor(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

val_server can maybe cause confusion with old kuksa_val_server in c++?

Copy link
Contributor Author

@argerus argerus Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not changed as part of this commit. It is referencing generated code, which having a protobuf service definition like this:

service VAL {
  ...
}

will generate code <snake case service name>_server::<camel case service name>Server, i.e. val_server::ValServer.

broker.clone(),
authorization.clone(),
))
} else {
None
}
};

let mut router = server.add_optional_service(kuksa_val_v1);

if apis.contains(&Api::SdvDatabrokerV1) {
router = router.add_optional_service(Some(
sdv::databroker::v1::broker_server::BrokerServer::with_interceptor(
broker.clone(),
authorization.clone(),
),
)
.add_service(
));
router = router.add_optional_service(Some(
sdv::databroker::v1::collector_server::CollectorServer::with_interceptor(
broker.clone(),
authorization.clone(),
authorization,
),
)
.add_service(kuksa::val::v1::val_server::ValServer::with_interceptor(
broker.clone(),
authorization,
))
));
}

router
.serve_with_incoming_shutdown(incoming, shutdown(broker, signal))
.await?;

Expand Down
14 changes: 14 additions & 0 deletions databroker/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,13 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
.long("disable-authorization")
.help("Disable authorization")
.action(ArgAction::SetTrue),
)
.arg(
Arg::new("enable-databroker-v1")
.display_order(30)
.long("enable-databroker-v1")
.help("Enable sdv.databroker.v1 (GRPC) service")
.action(ArgAction::SetTrue),
);

#[cfg(feature = "tls")]
Expand Down Expand Up @@ -438,11 +445,18 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
}
}

let mut apis = vec![grpc::server::Api::KuksaValV1];

if args.get_flag("enable-databroker-v1") {
apis.push(grpc::server::Api::SdvDatabrokerV1);
}

grpc::server::serve(
addr,
broker,
#[cfg(feature = "tls")]
tls_config,
&apis,
authorization,
shutdown_handler(),
)
Expand Down
1 change: 1 addition & 0 deletions databroker/tests/world/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ impl DataBrokerWorld {
data_broker,
#[cfg(feature = "tls")]
CERTS.server_tls_config(),
&[grpc::server::Api::KuksaValV1],
_authorization,
poll_fn(|cx| {
let mut state = owned_state
Expand Down
2 changes: 1 addition & 1 deletion integration_test/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ VSS_DATA_DIR="$SCRIPT_DIR/../data"

echo "Starting databroker container (\"${DATABROKER_IMAGE}\") in insecure mode, requesting platform (\"${CONTAINER_PLATFORM}\")"
RUNNING_IMAGE=$(
docker run -d -v ${VSS_DATA_DIR}:/data -p 55555:55555 --rm --platform ${CONTAINER_PLATFORM} ${DATABROKER_IMAGE} --metadata data/vss-core/vss_release_4.0.json --insecure
docker run -d -v ${VSS_DATA_DIR}:/data -p 55555:55555 --rm --platform ${CONTAINER_PLATFORM} ${DATABROKER_IMAGE} --metadata data/vss-core/vss_release_4.0.json --insecure --enable-databroker-v1
)

python3 -m pytest -v "${SCRIPT_DIR}/test_databroker.py"
Expand Down
Loading