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.

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

Support for containerized plugins #959

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ members = [
"plugins/typo",
"test-plugins/dummy_rand_data",
"test-plugins/dummy_sha256",
"test-plugins/activity-container",
"xtask",
]

Expand Down
17 changes: 13 additions & 4 deletions sdk/rust/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<P: Plugin> PluginServer<P> {
/// Run the plugin server on the provided port.
pub async fn listen(self, port: u16) -> Result<()> {
let service = PluginServiceServer::new(self);
let host = format!("127.0.0.1:{}", port).parse().unwrap();
let host = format!("0.0.0.0:{}", port).parse().unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this change is valid when running outside of a container; did you try it? If it's not, we may need to expose an SDK setting for what host to listen on, perhaps something like:

enum Host {
    // 127.0.0.1
    Loopback,
    // 0.0.0.0.0
    Any,
    // Any other IP address.
    Other(Ipv4Addr),
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried running normally with the 0.0.0.0 address and it was successful, but I can try to rework this so it isn't at 0.0.0.0 by default

Copy link
Collaborator

Choose a reason for hiding this comment

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

The question is whether we should be concerned about listening on all network interfaces and potentially enabling a remote process to connect to the plugin first and surreptitiously pose as hipcheck core.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, 0.0.0.0 inside a container is less concerning because what really matters on the host is the port bindings that the container runtime does. But for plugins running outside of the container we really ought to use the loopback address.


Server::builder()
.add_service(service)
Expand Down Expand Up @@ -168,16 +168,25 @@ impl<P: Plugin> PluginService for PluginServer<P> {
) -> QueryResult<Resp<Self::InitiateQueryProtocolStream>> {
let rx = req.into_inner();
// TODO: - make channel size configurable
let (tx, out_rx) = mpsc::channel::<QueryResult<InitiateQueryProtocolResp>>(10);
let (tx, out_rx) = mpsc::channel::<QueryResult<InitiateQueryProtocolResp>>(100);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the increase to the channel size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I ran it with a channel size of 10, I would sometimes get an error that the channel closed abruptly. I increased it by a factor of 10 but if there's another recommended size I should change it to, let me know


let cloned_plugin = self.plugin.clone();

let tx_clone = tx.clone();
tokio::spawn(async move {
let mut channel = HcSessionSocket::new(tx, rx);
if let Err(e) = channel.run(cloned_plugin).await {
panic!("Error: {e}");
eprintln!("Channel error: {e}");
if !tx_clone.is_closed() {
if let Err(send_err) = tx_clone
.send(Err(tonic::Status::internal(format!("Session error: {e}"))))
.await
{
eprintln!("Failed to send error through channel: {send_err}");
}
}
}
});

Ok(Resp::new(RecvStream::new(out_rx)))
}
}
12 changes: 7 additions & 5 deletions site/content/docs/guide/making-plugins/release.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,13 @@ system path, followed by some number of arguments. For example, if your plugin
is an executable file, the entrypoint string may be as simple as
`"<PLUGIN_FILE_NAME> [ARGS}"`, as above. If your plugin were a Python script,
the entrypoint string may be `"python3 <PLUGIN_PY_FILE>" [ARGS]`. If your plugin
code is represented by a container image, you may use `"podman <IMAGE_FILE>
[ARGS]"` or `"docker <IMAGE_FILE> [ARGS]"`. Whatever it is, at runtime Hipcheck
will append ` --port <PORT>` to this string to tell the plugin which port to
listen on, so you will need to ensure that the behavior of your entrypoint
string can handle this addition.
code is represented by a container image, your entrypoint can be a shell script
that will load your image file and format your run command to include port
mapping as `docker run -p "$PORT":50051 <IMAGE_FILE> [ARGS]` or
`podman run -p "$PORT":50051 <IMAGE_FILE> [ARGS]`. Whatever it is, at runtime
Hipcheck will append ` --port <PORT>` to the entrypoint to tell the plugin
which port to listen on, so you will need to ensure that the behavior of your
entrypoint string can handle this addition.

You may have as many or as few entries in the `entrypoint` section of the plugin
manifest. If you are doing a [local deployment](#local-deployment), you may
Expand Down
23 changes: 23 additions & 0 deletions test-plugins/activity-container/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
[package]
name = "activity-container"
version = "0.4.0"
license = "Apache-2.0"
edition = "2021"
publish = false

[dependencies]
clap = { version = "4.5.27", features = ["derive"] }
hipcheck-sdk = { path = "../../sdk/rust", features = [
"macros",
] }
jiff = { version = "0.1.16", features = ["serde"] }
log = "0.4.22"
schemars = { version = "0.8.21", features = ["url"] }
serde = { version = "1.0.215", features = ["derive", "rc"] }
serde_json = "1.0.134"
tokio = { version = "1.43.0", features = ["rt"] }

[dev-dependencies]
hipcheck-sdk = { path = "../../sdk/rust", features = [
"mock_engine",
] }
13 changes: 13 additions & 0 deletions test-plugins/activity-container/Containerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# SPDX-License-Identifier: Apache-2.0

FROM debian:bookworm-slim

WORKDIR /app

COPY ../../target/debug/activity /app/activity

RUN chmod +x /app/activity

EXPOSE 50051

ENTRYPOINT ["/app/activity", "--port", "50051"]
32 changes: 32 additions & 0 deletions test-plugins/activity-container/activity-container-deploy.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/bin/bash

# Default values
IMAGE_TAR="./test-plugins/activity-container/activity-image.tar"
IMAGE_NAME="activity-image"
PORT=8888

while [[ $# -gt 0 ]]; do
if [[ "$1" == "--port" && -n "$2" && "$2" =~ ^[0-9]+$ ]]; then
PORT="$2"
shift 2
else
echo "Unknown or invalid argument: $1"
exit 1
fi
done

if [[ ! -f "$IMAGE_TAR" ]]; then
echo "Error: Image tar file '$IMAGE_TAR' not found!"
exit 1
fi


# Check if the image is already loaded
if ! docker images | grep -q "$IMAGE_NAME"; then
echo "Image '$IMAGE_NAME' not found. Loading the image..."
docker load -i "$IMAGE_TAR" > /dev/null 2>&1
fi
# Otherwise, the image is already loaded

# Format the run statement for container port mapping
docker run --init -p "$PORT":50051 activity-image
15 changes: 15 additions & 0 deletions test-plugins/activity-container/local-plugin.kdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
publisher "mitre"
name "activity-container"
version "0.0.0"
license "Apache-2.0"

entrypoint {
on arch="aarch64-apple-darwin" "activity-container-deploy.sh"
on arch="x86_64-apple-darwin" "activity-container-deploy.sh"
on arch="x86_64-unknown-linux-gnu" "activity-container-deploy.sh"
on arch="x86_64-pc-windows-msvc" "activity-container-deploy.sh"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is Windows able to run this shell script? You may need to also have a PowerShell script for Windows specifically.

}

dependencies {
plugin "mitre/git" version="0.0.0" manifest="./plugins/git/local-plugin.kdl"
}
156 changes: 156 additions & 0 deletions test-plugins/activity-container/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
// SPDX-License-Identifier: Apache-2.0

//! Plugin for querying how long it has been since a commit was last made to a repo

use clap::Parser;
use hipcheck_sdk::{prelude::*, types::Target};
use jiff::Timestamp;
use serde::Deserialize;
use std::{result::Result as StdResult, sync::OnceLock};

#[derive(Deserialize)]
struct Config {
weeks: Option<u16>,
}

static CONFIG: OnceLock<Config> = OnceLock::new();

/// Returns the span of time since the most recent commit to a Git repo as `jiff:Span` displayed as a String
/// (Which means that anything expecting a `Span` must parse the output of this query appropriately)
#[query(default)]
async fn activity(engine: &mut PluginEngine, target: Target) -> Result<String> {
log::debug!("running activity query");

let repo = target.local;

// Get today's date
let today = Timestamp::now();

// Get the date of the most recent commit.
let value = engine
.query("mitre/git/last_commit_date", repo)
.await
.map_err(|e| {
log::error!("failed to get last commit date for activity metric: {}", e);
Error::UnspecifiedQueryState
})?;

let Value::String(date_string) = value else {
return Err(Error::UnexpectedPluginQueryInputFormat);
};
let last_commit_date: Timestamp = date_string.parse().map_err(|e| {
log::error!("{}", e);
Error::UnspecifiedQueryState
})?;

// Get the time between the most recent commit and today.
let time_since_last_commit = today.since(last_commit_date).map_err(|e| {
log::error!("{}", e);
Error::UnspecifiedQueryState
})?;

Ok(time_since_last_commit.to_string())
}

#[derive(Clone, Debug)]
struct ActivityPlugin;

impl Plugin for ActivityPlugin {
const PUBLISHER: &'static str = "mitre";

const NAME: &'static str = "activity";

fn set_config(&self, config: Value) -> StdResult<(), ConfigError> {
let conf =
serde_json::from_value::<Config>(config).map_err(|e| ConfigError::Unspecified {
message: e.to_string(),
})?;
CONFIG.set(conf).map_err(|_e| ConfigError::Unspecified {
message: "config was already set".to_owned(),
})
}

fn default_policy_expr(&self) -> Result<String> {
let Some(conf) = CONFIG.get() else {
log::error!("tried to access config before set by Hipcheck core!");
return Err(Error::UnspecifiedQueryState);
};

Ok(format!("(lte $ P{}w)", conf.weeks.unwrap_or(71)))
}

fn explain_default_query(&self) -> Result<Option<String>> {
Ok(Some(
"span of time that has elapsed since last activity in repo".to_string(),
))
}

queries! {}
}

#[derive(Parser, Debug)]
struct Args {
#[arg(long)]
p: u16,
}

#[tokio::main(flavor = "current_thread")]
async fn main() -> Result<()> {
let args = Args::try_parse().unwrap();
log::info!("Activity container plugin is registering {:?}", args);
PluginServer::register(ActivityPlugin {})
.listen(args.p)
.await
}

#[cfg(test)]
mod test {
use super::*;

use hipcheck_sdk::types::LocalGitRepo;
use jiff::{Span, SpanRound, Unit};
use std::result::Result as StdResult;

fn repo() -> LocalGitRepo {
LocalGitRepo {
path: "/home/users/me/.cache/hipcheck/clones/github/expressjs/express/".to_string(),
git_ref: "main".to_string(),
}
}

fn mock_responses() -> StdResult<MockResponses, Error> {
let repo = repo();
let output = "2024-06-19T19:22:45Z".to_string();

// when calling into query, the input repo gets passed to `last_commit_date`, lets assume it returns the datetime `output`
let mut mock_responses = MockResponses::new();
mock_responses.insert("mitre/git/last_commit_date", repo, Ok(output))?;
Ok(mock_responses)
}

#[tokio::test]
async fn test_activity() {
let repo = repo();
let target = Target {
specifier: "express".to_string(),
local: repo,
remote: None,
package: None,
};

let mut engine = PluginEngine::mock(mock_responses().unwrap());
let output = activity(&mut engine, target).await.unwrap();
let span: Span = output.parse().unwrap();
let result = span.round(SpanRound::new().smallest(Unit::Day)).unwrap();

let today = Timestamp::now();
let last_commit: Timestamp = "2024-06-19T19:22:45Z".parse().unwrap();
let expected = today
.since(last_commit)
.unwrap()
.round(SpanRound::new().smallest(Unit::Day))
.unwrap();

assert_eq!(result, expected);
}
}
Loading