Skip to content

Commit

Permalink
refactor: use OTLP env variables only
Browse files Browse the repository at this point in the history
Signed-off-by: Fabrizio Sestito <[email protected]>
  • Loading branch information
fabriziosestito committed Dec 11, 2024
1 parent d7e156e commit f0879b1
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 139 deletions.
20 changes: 0 additions & 20 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,26 +194,6 @@ pub(crate) fn build_cli() -> Command {
.env("KUBEWARDEN_CONTINUE_ON_ERRORS")
.action(ArgAction::SetTrue)
.hide(true),

Arg::new("otlp-endpoint")
.long("otlp-endpoint")
.env("OTEL_EXPORTER_OTLP_ENDPOINT")
.help("The OTLP gRPC endpoint for exporting traces and metrics."),

Arg::new("otlp-certificate")
.long("otlp-certificate")
.env("OTEL_EXPORTER_OTLP_CERTIFICATE")
.help("Path to the trusted certificate in PEM format used for verifying the TLS credentials of the OTLP gRPC endpoint."),

Arg::new("otlp-client-certificate")
.long("otlp-client-certificate")
.env("OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE")
.help("Path to the certificate in PEM format to use in mTLS communication."),

Arg::new("otlp-client-key")
.long("otlp-client-key")
.env("OTEL_EXPORTER_OTLP_CLIENT_KEY")
.help("Path to the client private key in PEM format to use in mTLS communication."),
];

args.sort_by(|a, b| a.get_id().cmp(b.get_id()));
Expand Down
101 changes: 45 additions & 56 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ use serde::Deserialize;
use std::{
collections::{BTreeSet, HashMap},
env,
fs::File,
fs::{self, File},
net::SocketAddr,
path::{Path, PathBuf},
};
use tonic::transport::{Certificate, ClientTlsConfig, Identity};

pub static SERVICE_NAME: &str = "kubewarden-policy-server";
const DOCKER_CONFIG_ENV_VAR: &str = "DOCKER_CONFIG";
Expand All @@ -41,8 +42,6 @@ pub struct Config {
pub log_level: String,
pub log_fmt: String,
pub log_no_color: bool,
pub otlp_endpoint: Option<String>,
pub otlp_tls_config: OtlpTlsConfig,
pub daemon: bool,
pub enable_pprof: bool,
pub daemon_pid_file: String,
Expand All @@ -56,44 +55,6 @@ pub struct TlsConfig {
pub key_file: String,
}

#[derive(Clone, Default)]
pub struct OtlpTlsConfig {
pub ca_file: Option<PathBuf>,
pub cert_file: Option<PathBuf>,
pub key_file: Option<PathBuf>,
}

impl TryFrom<OtlpTlsConfig> for tonic::transport::ClientTlsConfig {
type Error = anyhow::Error;

fn try_from(value: OtlpTlsConfig) -> Result<tonic::transport::ClientTlsConfig, Self::Error> {
use std::fs;
use tonic::transport::{Certificate, ClientTlsConfig, Identity};

let mut tls = ClientTlsConfig::new();

if let Some(ca) = value.ca_file {
let ca_cert = fs::read(ca)?;
tls = tls.ca_certificate(Certificate::from_pem(ca_cert))
}

if let Some(cert) = value.cert_file {
let cert = fs::read(cert)?;

let key = value
.key_file
.map(fs::read)
.transpose()?
.unwrap_or_default();

let identity = Identity::from_pem(cert, key);
tls = tls.identity(identity);
}

Ok(tls)
}
}

impl Config {
pub fn from_args(matches: &ArgMatches) -> Result<Self> {
// init some variables based on the cli parameters
Expand Down Expand Up @@ -166,19 +127,6 @@ impl Config {
.expect("clap should have assigned a default value")
.to_owned();

let otlp_endpoint = matches.get_one::<String>("otlp-endpoint").cloned();
let otlp_ca_file = matches.get_one::<PathBuf>("otlp-certificate").cloned();
let otlp_cert_file = matches
.get_one::<PathBuf>("otlp-client-certificate")
.cloned();
let otlp_key_file = matches.get_one::<PathBuf>("otlp-client-key").cloned();

let otlp_tls_config = OtlpTlsConfig {
ca_file: otlp_ca_file,
cert_file: otlp_cert_file,
key_file: otlp_key_file,
};

let (cert_file, key_file) = tls_files(matches)?;
let tls_config = if cert_file.is_empty() {
None
Expand Down Expand Up @@ -214,8 +162,6 @@ impl Config {
log_level,
log_fmt,
log_no_color,
otlp_endpoint,
otlp_tls_config,
daemon,
daemon_pid_file,
daemon_stdout_file,
Expand Down Expand Up @@ -482,6 +428,49 @@ fn read_policies_file(path: &Path) -> Result<HashMap<String, PolicyOrPolicyGroup
Ok(ps)
}

/// Creates a `ClientTlsConfig` used by OTLP exporters based on the environment variables.
/// TODO: this function will be removed once this issue is resolved upstream:
/// https://github.com/open-telemetry/opentelemetry-rust/issues/984
pub fn build_client_tls_config_from_env(prefix: &str) -> Result<ClientTlsConfig> {
let mut client_tls_config = ClientTlsConfig::new();

let ca_env = format!("OTEL_EXPORTER_OTLP_{}CERTIFICATE", prefix);
let fallback_ca_env = "OTEL_EXPORTER_OTLP_CERTIFICATE";

let ca_file = env::var(&ca_env)
.or_else(|_| env::var(fallback_ca_env))
.ok();

if let Some(ca_path) = ca_file {
let ca_cert = std::fs::read(ca_path)?;
client_tls_config = client_tls_config.ca_certificate(Certificate::from_pem(ca_cert));
}

let client_cert_env = format!("OTEL_EXPORTER_OTLP_{}CLIENT_CERTIFICATE", prefix);
let fallback_client_cert_env = "OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE";

let client_cert_file = std::env::var(&client_cert_env)
.or_else(|_| std::env::var(fallback_client_cert_env))
.ok();

let client_key_env = format!("OTEL_EXPORTER_OTLP_{}CLIENT_KEY", prefix);
let fallback_client_key_env = "OTEL_EXPORTER_OTLP_CLIENT_KEY";

let client_key_file = std::env::var(&client_key_env)
.or_else(|_| std::env::var(fallback_client_key_env))
.ok();

if let (Some(cert_path), Some(key_path)) = (client_cert_file, client_key_file) {
let cert = fs::read(cert_path)?;
let key = fs::read(key_path)?;

let identity = Identity::from_pem(cert, key);
client_tls_config = client_tls_config.identity(identity);
}

Ok(client_tls_config)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
13 changes: 2 additions & 11 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,10 @@ async fn main() -> Result<()> {
let matches = cli::build_cli().get_matches();
let config = policy_server::config::Config::from_args(&matches)?;

setup_tracing(
&config.log_level,
&config.log_fmt,
config.log_no_color,
config.otlp_endpoint.as_deref(),
config.otlp_tls_config.clone(),
)?;
setup_tracing(&config.log_level, &config.log_fmt, config.log_no_color)?;

Check warning on line 24 in src/main.rs

View check run for this annotation

Codecov / codecov/patch

src/main.rs#L24

Added line #L24 was not covered by tests

if config.metrics_enabled {
setup_metrics(
config.otlp_endpoint.as_deref(),
config.otlp_tls_config.clone(),
)?;
setup_metrics()?;

Check warning on line 27 in src/main.rs

View check run for this annotation

Codecov / codecov/patch

src/main.rs#L27

Added line #L27 was not covered by tests
};

if config.daemon {
Expand Down
19 changes: 9 additions & 10 deletions src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,20 @@ pub use policy_evaluations_total::add_policy_evaluation;
mod policy_evaluations_latency;
pub use policy_evaluations_latency::record_policy_latency;

use crate::config::OtlpTlsConfig;
use crate::config::build_client_tls_config_from_env;

const METER_NAME: &str = "kubewarden";

pub fn setup_metrics(otlp_endpoint: Option<&str>, otlp_tls_config: OtlpTlsConfig) -> Result<()> {
let mut metric_exporter_builder = opentelemetry_otlp::MetricExporter::builder()
pub fn setup_metrics() -> Result<()> {
let metric_exporter = opentelemetry_otlp::MetricExporter::builder()
.with_tonic()
.with_tls_config(otlp_tls_config.try_into()?)
.with_export_config(ExportConfig::default());
if let Some(endpoint) = otlp_endpoint {
metric_exporter_builder = metric_exporter_builder.with_endpoint(endpoint);
}
let meter_reader = metric_exporter_builder.build()?;
.with_tls_config(build_client_tls_config_from_env("METRICS")?)
.with_export_config(ExportConfig::default())
.build()?;

let periodic_reader =
opentelemetry_sdk::metrics::PeriodicReader::builder(meter_reader, runtime::Tokio).build();
opentelemetry_sdk::metrics::PeriodicReader::builder(metric_exporter, runtime::Tokio)
.build();
let meter_provider = opentelemetry_sdk::metrics::SdkMeterProvider::builder()
.with_reader(periodic_reader)
.build();
Expand Down
25 changes: 6 additions & 19 deletions src/tracing.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
use std::convert::TryInto;

use anyhow::{anyhow, Result};
use opentelemetry::trace::TracerProvider;
use opentelemetry_otlp::{WithExportConfig, WithTonicConfig};
use opentelemetry_otlp::WithTonicConfig;

use tracing_subscriber::prelude::*;
use tracing_subscriber::{fmt, EnvFilter};

use crate::config::{self, OtlpTlsConfig};
use crate::config::{self, build_client_tls_config_from_env};

// Setup the tracing system. This MUST be done inside of a tokio Runtime
// because some collectors rely on it and would panic otherwise.
pub fn setup_tracing(
log_level: &str,
log_fmt: &str,
log_no_color: bool,
otlp_endpoint: Option<&str>,
otlp_tls_config: OtlpTlsConfig,
) -> Result<()> {
pub fn setup_tracing(log_level: &str, log_fmt: &str, log_no_color: bool) -> Result<()> {
// setup logging
let filter_layer = EnvFilter::new(log_level)
// some of our dependencies generate trace events too, but we don't care about them ->
Expand Down Expand Up @@ -50,15 +42,10 @@ pub fn setup_tracing(
// If no endpoint is provided, the default one is used.
// The default endpoint is "http://localhost:4317".
//
let mut otlp_exporter_builder = opentelemetry_otlp::SpanExporter::builder()
let otlp_exporter = opentelemetry_otlp::SpanExporter::builder()
.with_tonic()
.with_tls_config(otlp_tls_config.try_into()?);

if let Some(endpoint) = otlp_endpoint {
otlp_exporter_builder = otlp_exporter_builder.with_endpoint(endpoint);
}

let otlp_exporter = otlp_exporter_builder.build()?;
.with_tls_config(build_client_tls_config_from_env("OTLP")?)
.build()?;

let tracer_provider = opentelemetry_sdk::trace::TracerProvider::builder()
.with_resource(opentelemetry_sdk::Resource::new(vec![
Expand Down
2 changes: 0 additions & 2 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ pub(crate) fn default_test_config() -> Config {
log_level: "info".to_owned(),
log_fmt: "json".to_owned(),
log_no_color: false,
otlp_endpoint: None,
otlp_tls_config: Default::default(),
daemon: false,
daemon_pid_file: "policy_server.pid".to_owned(),
daemon_stdout_file: None,
Expand Down
38 changes: 17 additions & 21 deletions tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use policy_evaluator::{
admission_response::AdmissionResponseStatus,
policy_fetcher::verify::config::VerificationConfigV1,
};
use policy_server::config::OtlpTlsConfig;
use policy_server::{
api::admission_review::AdmissionReviewResponse,
config::{PolicyMode, PolicyOrPolicyGroup},
Expand Down Expand Up @@ -832,29 +831,26 @@ async fn test_otel() {
.await
.unwrap();

std::env::set_var("OTEL_EXPORTER_OTLP_ENDPOINT", "https://localhost:1337");
std::env::set_var(
"OTEL_EXPORTER_OTLP_CERTIFICATE",
server_ca_file.path().to_str().unwrap(),
);
std::env::set_var(
"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE",
client_cert_file.path().to_str().unwrap(),
);
std::env::set_var(
"OTEL_EXPORTER_OTLP_CLIENT_KEY",
client_key_file.path().to_str().unwrap(),
);

let mut config = default_test_config();
config.metrics_enabled = true;
config.log_fmt = "otlp".to_string();
config.otlp_endpoint = Some("https://localhost:1337".to_string());
config.otlp_tls_config = OtlpTlsConfig {
ca_file: Some(server_ca_file.path().to_owned()),
cert_file: Some(client_cert_file.path().to_owned()),
key_file: Some(client_key_file.path().to_owned()),
};

setup_metrics(
config.otlp_endpoint.as_deref(),
config.otlp_tls_config.clone(),
)
.unwrap();
setup_tracing(
&config.log_level,
&config.log_fmt,
config.log_no_color,
config.otlp_endpoint.as_deref(),
config.otlp_tls_config.clone(),
)
.unwrap();

setup_metrics().unwrap();
setup_tracing(&config.log_level, &config.log_fmt, config.log_no_color).unwrap();

let app = app(config).await;

Expand Down

0 comments on commit f0879b1

Please sign in to comment.