Skip to content

Commit

Permalink
feat: Make ClientMetaInformation to avoid problems with ordering of a…
Browse files Browse the repository at this point in the history
…rguments
  • Loading branch information
chriswk committed Dec 20, 2024
1 parent b2c421e commit eec2fea
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 41 deletions.
31 changes: 24 additions & 7 deletions server/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use unleash_yggdrasil::EngineState;
use crate::cli::RedisMode;
use crate::feature_cache::FeatureCache;
use crate::http::feature_refresher::{FeatureRefreshConfig, FeatureRefresherMode};
use crate::http::unleash_client::new_reqwest_client;
use crate::http::unleash_client::{new_reqwest_client, ClientMetaInformation};
use crate::offline::offline_hotload::{load_bootstrap, load_offline_engine_cache};
use crate::persistence::file::FilePersister;
use crate::persistence::redis::RedisPersister;
Expand Down Expand Up @@ -218,7 +218,10 @@ async fn get_data_source(args: &EdgeArgs) -> Option<Arc<dyn EdgePersistence>> {
None
}

async fn build_edge(args: &EdgeArgs, app_name: &str, instance_id: &str) -> EdgeResult<EdgeInfo> {
async fn build_edge(
args: &EdgeArgs,
client_meta_information: ClientMetaInformation,
) -> EdgeResult<EdgeInfo> {
if !args.strict {
if !args.dynamic {
error!("You should explicitly opt into either strict or dynamic behavior. Edge has defaulted to dynamic to preserve legacy behavior, however we recommend using strict from now on. Not explicitly opting into a behavior will return an error on startup in a future release");
Expand All @@ -237,13 +240,12 @@ async fn build_edge(args: &EdgeArgs, app_name: &str, instance_id: &str) -> EdgeR
let persistence = get_data_source(args).await;

let http_client = new_reqwest_client(
instance_id.to_string().clone(),
args.skip_ssl_verification,
args.client_identity.clone(),
args.upstream_certificate_file.clone(),
Duration::seconds(args.upstream_request_timeout),
Duration::seconds(args.upstream_socket_timeout),
app_name.into(),
client_meta_information.clone(),
)?;

let unleash_client = Url::parse(&args.upstream_url.clone())
Expand All @@ -267,7 +269,7 @@ async fn build_edge(args: &EdgeArgs, app_name: &str, instance_id: &str) -> EdgeR
let feature_config = FeatureRefreshConfig::new(
Duration::seconds(args.features_refresh_interval_seconds as i64),
refresher_mode,
app_name.to_string(),
client_meta_information,
);
let feature_refresher = Arc::new(FeatureRefresher::new(
unleash_client,
Expand Down Expand Up @@ -316,7 +318,14 @@ pub async fn build_caches_and_refreshers(args: CliArgs) -> EdgeResult<EdgeInfo>
build_offline(offline_args).map(|cache| (cache, None, None, None))
}
EdgeMode::Edge(edge_args) => {
build_edge(&edge_args, &args.app_name, &args.instance_id).await
build_edge(
&edge_args,
ClientMetaInformation {
app_name: args.app_name,
instance_id: args.instance_id,
},
)
.await
}
_ => unreachable!(),
}
Expand All @@ -327,6 +336,7 @@ mod tests {
use crate::{
builder::{build_edge, build_offline},
cli::{EdgeArgs, OfflineArgs, TokenHeader},
http::unleash_client::ClientMetaInformation,
};

#[test]
Expand Down Expand Up @@ -377,7 +387,14 @@ mod tests {
streaming: false,
};

let result = build_edge(&args, "test-app", "test-instance-id").await;
let result = build_edge(
&args,
ClientMetaInformation {
app_name: "test-app".into(),
instance_id: "test-instance-id".into(),
},
)
.await;
assert!(result.is_err());
assert_eq!(
result.err().unwrap().to_string(),
Expand Down
4 changes: 2 additions & 2 deletions server/src/client_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ mod tests {

use crate::auth::token_validator::TokenValidator;
use crate::cli::{OfflineArgs, TokenHeader};
use crate::http::unleash_client::UnleashClient;
use crate::http::unleash_client::{UnleashClient, ClientMetaInformation};
use crate::middleware;
use crate::tests::{features_from_disk, upstream_server};
use actix_http::{Request, StatusCode};
Expand Down Expand Up @@ -1012,7 +1012,7 @@ mod tests {
persistence: None,
strict: false,
streaming: false,
app_name: "test-app".into(),
client_meta_information: ClientMetaInformation::test_config(),
});
let token_validator = Arc::new(TokenValidator {
unleash_client: unleash_client.clone(),
Expand Down
27 changes: 13 additions & 14 deletions server/src/http/feature_refresher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::{
types::{ClientFeaturesRequest, ClientFeaturesResponse, EdgeToken, TokenRefresh},
};

use super::unleash_client::UnleashClient;
use super::unleash_client::{ClientMetaInformation, UnleashClient};

fn frontend_token_is_covered_by_tokens(
frontend_token: &EdgeToken,
Expand All @@ -48,7 +48,7 @@ pub struct FeatureRefresher {
pub persistence: Option<Arc<dyn EdgePersistence>>,
pub strict: bool,
pub streaming: bool,
pub app_name: String,
pub client_meta_information: ClientMetaInformation,
}

impl Default for FeatureRefresher {
Expand All @@ -62,21 +62,21 @@ impl Default for FeatureRefresher {
persistence: None,
strict: true,
streaming: false,
app_name: "unleash_edge".into(),
client_meta_information: Default::default(),
}
}
}

fn client_application_from_token_and_name(
token: EdgeToken,
refresh_interval: i64,
app_name: &str,
client_meta_information: ClientMetaInformation,
) -> ClientApplication {
ClientApplication {
app_name: app_name.into(),
app_name: client_meta_information.app_name,
connect_via: None,
environment: token.environment,
instance_id: None,
instance_id: Some(client_meta_information.instance_id),
interval: refresh_interval as u32,
started: Utc::now(),
strategies: vec![],
Expand All @@ -99,19 +99,19 @@ pub enum FeatureRefresherMode {
pub struct FeatureRefreshConfig {
features_refresh_interval: chrono::Duration,
mode: FeatureRefresherMode,
app_name: String,
client_meta_information: ClientMetaInformation,
}

impl FeatureRefreshConfig {
pub fn new(
features_refresh_interval: chrono::Duration,
mode: FeatureRefresherMode,
app_name: String,
client_meta_information: ClientMetaInformation,
) -> Self {
Self {
features_refresh_interval,
mode,
app_name,
client_meta_information,
}
}
}
Expand All @@ -133,7 +133,7 @@ impl FeatureRefresher {
persistence,
strict: config.mode != FeatureRefresherMode::Dynamic,
streaming: config.mode == FeatureRefresherMode::Streaming,
app_name: config.app_name,
client_meta_information: config.client_meta_information,
}
}

Expand Down Expand Up @@ -254,7 +254,7 @@ impl FeatureRefresher {
client_application_from_token_and_name(
token.clone(),
self.refresh_interval.num_seconds(),
&self.app_name,
self.client_meta_information.clone(),
),
)
.await
Expand Down Expand Up @@ -558,7 +558,7 @@ mod tests {

use crate::feature_cache::{update_projects_from_feature_update, FeatureCache};
use crate::filters::{project_filter, FeatureFilterSet};
use crate::http::unleash_client::new_reqwest_client;
use crate::http::unleash_client::{new_reqwest_client, ClientMetaInformation};
use crate::tests::features_from_disk;
use crate::tokens::cache_key;
use crate::types::TokenValidationStatus::Validated;
Expand All @@ -581,13 +581,12 @@ mod tests {

fn create_test_client() -> UnleashClient {
let http_client = new_reqwest_client(
"unleash_edge".into(),
false,
None,
None,
Duration::seconds(5),
Duration::seconds(5),
"test-client".into(),
ClientMetaInformation::test_config(),
)
.expect("Failed to create client");

Expand Down
61 changes: 45 additions & 16 deletions server/src/http/unleash_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,30 @@ lazy_static! {
.unwrap();
}

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct ClientMetaInformation {
pub app_name: String,
pub instance_id: String,
}

impl Default for ClientMetaInformation {
fn default() -> Self {
Self {
app_name: "unleash-edge".into(),
instance_id: format!("unleash-edge@{}", ulid::Ulid::new().to_string()),
}
}
}

impl ClientMetaInformation {
pub fn test_config() -> Self {
Self {
app_name: "test-app-name".into(),
instance_id: "test-instance-id".into(),
}
}
}

#[derive(Clone, Debug, Default)]
pub struct UnleashClient {
pub urls: UnleashUrls,
Expand Down Expand Up @@ -130,13 +154,12 @@ fn build_identity(tls: Option<ClientIdentity>) -> EdgeResult<ClientBuilder> {
}

pub fn new_reqwest_client(
instance_id: String,
skip_ssl_verification: bool,
client_identity: Option<ClientIdentity>,
upstream_certificate_file: Option<PathBuf>,
connect_timeout: Duration,
socket_timeout: Duration,
app_name: String,
client_meta_information: ClientMetaInformation,
) -> EdgeResult<Client> {
build_identity(client_identity)
.and_then(|builder| {
Expand All @@ -149,12 +172,12 @@ pub fn new_reqwest_client(
let mut header_map = HeaderMap::new();
header_map.insert(
UNLEASH_APPNAME_HEADER,
header::HeaderValue::from_str(app_name.as_str())
header::HeaderValue::from_str(&client_meta_information.app_name)
.expect("Could not add app name as a header"),
);
header_map.insert(
UNLEASH_INSTANCE_ID_HEADER,
header::HeaderValue::from_bytes(instance_id.as_bytes()).unwrap(),
header::HeaderValue::from_str(&client_meta_information.instance_id).unwrap(),
);
header_map.insert(
UNLEASH_CLIENT_SPEC_HEADER,
Expand Down Expand Up @@ -195,13 +218,15 @@ impl UnleashClient {
Ok(Self {
urls: UnleashUrls::from_str(server_url)?,
backing_client: new_reqwest_client(
instance_id,
false,
None,
None,
Duration::seconds(5),
Duration::seconds(5),
"test-client".into(),
ClientMetaInformation {
instance_id,
app_name: "test-client".into(),
},
)
.unwrap(),
custom_headers: Default::default(),
Expand All @@ -211,18 +236,16 @@ impl UnleashClient {

#[cfg(test)]
pub fn new_insecure(server_url: &str) -> Result<Self, EdgeError> {
use ulid::Ulid;

Ok(Self {
urls: UnleashUrls::from_str(server_url)?,
backing_client: new_reqwest_client(
Ulid::new().to_string(),
true,
None,
None,
Duration::seconds(5),
Duration::seconds(5),
"test-client".into(),
ClientMetaInformation::test_config(),
)
.unwrap(),
custom_headers: Default::default(),
Expand Down Expand Up @@ -514,7 +537,7 @@ mod tests {
},
};

use super::{EdgeTokens, UnleashClient};
use super::{EdgeTokens, UnleashClient, ClientMetaInformation};

impl ClientFeaturesRequest {
pub(crate) fn new(api_key: String, etag: Option<String>) -> Self {
Expand Down Expand Up @@ -800,13 +823,15 @@ mod tests {
pkcs12_passphrase: Some(passphrase.into()),
};
let client = new_reqwest_client(
"test_pkcs12".into(),
false,
Some(identity),
None,
Duration::seconds(5),
Duration::seconds(5),
"test-client".into(),
ClientMetaInformation {
app_name: "test-client".into(),
instance_id: "test-pkcs12".into(),
},
);
assert!(client.is_ok());
}
Expand All @@ -822,13 +847,15 @@ mod tests {
pkcs12_passphrase: Some(passphrase.into()),
};
let client = new_reqwest_client(
"test_pkcs12".into(),
false,
Some(identity),
None,
Duration::seconds(5),
Duration::seconds(5),
"test-client".into(),
ClientMetaInformation {
app_name: "test-client".into(),
instance_id: "test-pkcs12".into(),
},
);
assert!(client.is_err());
}
Expand All @@ -844,13 +871,15 @@ mod tests {
pkcs12_passphrase: None,
};
let client = new_reqwest_client(
"test_pkcs8".into(),
false,
Some(identity),
None,
Duration::seconds(5),
Duration::seconds(5),
"test-client".into(),
ClientMetaInformation {
app_name: "test-client".into(),
instance_id: "test-pkcs8".into(),
},
);
assert!(client.is_ok());
}
Expand Down
3 changes: 1 addition & 2 deletions server/src/middleware/client_token_from_frontend_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,12 @@ mod tests {
.await;

let http_client = new_reqwest_client(
"unleash_edge".into(),
false,
None,
None,
Duration::seconds(5),
Duration::seconds(5),
"test-client".into(),
crate::http::unleash_client::ClientMetaInformation::test_config(),
)
.expect("Failed to create client");

Expand Down

0 comments on commit eec2fea

Please sign in to comment.