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

Start adding support for FabricClientSettings and FabricSecurityCredentials #139

Merged
merged 9 commits into from
Mar 10, 2025
Merged
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
68 changes: 58 additions & 10 deletions crates/libs/core/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------

use crate::Interface;
use crate::{
types::{FabricClientSettings, FabricSecurityCredentials},
Interface,
};
use connection::{ClientConnectionEventHandlerBridge, LambdaClientConnectionNotificationHandler};
use health_client::HealthClient;
use mssf_com::FabricClient::{
IFabricClientConnectionEventHandler, IFabricHealthClient4, IFabricPropertyManagementClient2,
IFabricQueryClient10, IFabricServiceManagementClient6, IFabricServiceNotificationEventHandler,
IFabricClientConnectionEventHandler, IFabricClientSettings2, IFabricHealthClient4,
IFabricPropertyManagementClient2, IFabricQueryClient10, IFabricServiceManagementClient6,
IFabricServiceNotificationEventHandler,
};
use notification::{
LambdaServiceNotificationHandler, ServiceNotificationEventHandler,
Expand All @@ -34,13 +38,22 @@ pub use notification::ServiceNotification;
#[cfg(test)]
mod tests;

#[non_exhaustive]
#[derive(Debug)]
pub enum FabricClientCreationError {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Impl std Error trait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do in follow up PR

InvalidFabricClientSettings(crate::Error),
InvalidFabricSecurityCredentials(crate::Error),
}

/// Creates FabricClient com object using SF com API.
fn create_local_client_internal<T: Interface>(
connection_strings: Option<&Vec<crate::WString>>,
service_notification_handler: Option<&IFabricServiceNotificationEventHandler>,
client_connection_handler: Option<&IFabricClientConnectionEventHandler>,
client_role: Option<ClientRole>,
) -> T {
client_settings: Option<FabricClientSettings>,
client_credentials: Option<FabricSecurityCredentials>,
) -> Result<T, FabricClientCreationError> {
let role = client_role.unwrap_or(ClientRole::Unknown);

// create raw conn str ptrs.
Expand All @@ -51,7 +64,7 @@ fn create_local_client_internal<T: Interface>(
.collect::<Vec<_>>()
});

match connection_strings_ptrs {
let client = match connection_strings_ptrs {
Some(addrs) => {
assert!(
role == ClientRole::Unknown,
Expand Down Expand Up @@ -80,7 +93,24 @@ fn create_local_client_internal<T: Interface>(
}
}
// if params are right, client should be created. There is no network call involved during obj creation.
.expect("failed to create fabric client")
.expect("failed to create fabric client");
if client_settings.is_some() || client_credentials.is_some() {
let setting_interface = client
.clone()
.cast::<IFabricClientSettings2>()
.expect("failed to cast fabric client to IFabricClientSettings2");
if let Some(desired_settings) = client_settings {
desired_settings
.apply(&setting_interface)
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO the combining of the setting value should be the responsibility of the user. U can provide api to load existing values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but I'm not doing the combining of the setting value in this PR at all - it just is applying the settings, if provided.
The "what does that mean, do we combine settings" isn't in this PR at all.

I could make two methods - with_client_settings_overrides and with_client_settings, if we want to expose the capability to not get defaults from the client. But they can also always fall back to the COM API for that sort of advanced scenario?

Or are you saying you'd rather client settings and credentials not be part of the builder, but instead be &mut self methods on FabricClient and the individual clients as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

with_client_settings.
FabricClientSettings should have fn to load existing settings.

.map_err(FabricClientCreationError::InvalidFabricClientSettings)?;
}
if let Some(desired_credentials) = client_credentials {
desired_credentials
.apply(&setting_interface)
.map_err(FabricClientCreationError::InvalidFabricSecurityCredentials)?;
}
};
Ok(client)
}

// Builder for FabricClient
Expand All @@ -89,6 +119,8 @@ pub struct FabricClientBuilder {
cc_handler: Option<LambdaClientConnectionNotificationHandler>,
client_role: ClientRole,
connection_strings: Option<Vec<crate::WString>>,
client_settings: Option<FabricClientSettings>,
client_credentials: Option<FabricSecurityCredentials>,
}

impl Default for FabricClientBuilder {
Expand All @@ -105,6 +137,8 @@ impl FabricClientBuilder {
cc_handler: None,
client_role: ClientRole::Unknown,
connection_strings: None,
client_settings: None,
client_credentials: None,
}
}

Expand Down Expand Up @@ -174,17 +208,29 @@ impl FabricClientBuilder {
self
}

/// Sets the client settings
pub fn with_client_settings(mut self, client_settings: FabricClientSettings) -> Self {
self.client_settings = Some(client_settings);
self
}

// Sets the client credentials
pub fn with_credentials(mut self, client_credentials: FabricSecurityCredentials) -> Self {
self.client_credentials = Some(client_credentials);
self
}

/// Build the fabricclient
/// Remarks: FabricClient connect to SF cluster when
/// the first API call is triggered. Build/create of the object does not
/// establish connection.
pub fn build(self) -> FabricClient {
let c = Self::build_interface(self);
FabricClient::from_com(c)
pub fn build(self) -> Result<FabricClient, FabricClientCreationError> {
let c = Self::build_interface(self)?;
Ok(FabricClient::from_com(c))
}

/// Build the specific com interface of the fabric client.
pub fn build_interface<T: Interface>(self) -> T {
pub fn build_interface<T: Interface>(self) -> Result<T, FabricClientCreationError> {
let cc_handler = self
.cc_handler
.map(ClientConnectionEventHandlerBridge::new_com);
Expand All @@ -193,6 +239,8 @@ impl FabricClientBuilder {
self.sn_handler.as_ref(),
cc_handler.as_ref(),
Some(self.client_role),
self.client_settings,
self.client_credentials,
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/core/src/client/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{

#[tokio::test]
async fn test_fabric_client() {
let c = FabricClient::builder().build();
let c = FabricClient::builder().build().unwrap();
let qc = c.get_query_manager();
let timeout = Duration::from_secs(1);
let paging_status;
Expand Down
6 changes: 4 additions & 2 deletions crates/libs/core/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ mod async_tests {
pub fn new() -> $name {
return $name {
com: paste::item! {
crate::client::FabricClientBuilder::new().build_interface::<mssf_com::FabricClient::[<I $name>]>()
crate::client::FabricClientBuilder::new().build_interface::<mssf_com::FabricClient::[<I $name>]>().unwrap()
},
};
}
Expand Down Expand Up @@ -242,7 +242,9 @@ mod async_tests {
impl FabricQueryClient {
pub fn new() -> FabricQueryClient {
FabricQueryClient {
com: crate::client::FabricClient::builder().build_interface::<IFabricQueryClient>(),
com: crate::client::FabricClient::builder()
.build_interface::<IFabricQueryClient>()
.expect("default settings and credentials must be valid"),
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/libs/core/src/types/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ mod metrics;
pub use metrics::*;
mod health;
pub use health::*;
mod settings;
pub use settings::*;

// FABRIC_SERVICE_NOTIFICATION_FILTER_FLAGS
bitflags::bitflags! {
Expand Down
31 changes: 31 additions & 0 deletions crates/libs/core/src/types/client/settings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------

use mssf_com::FabricClient::IFabricClientSettings2;

/// A idiomatic Rust version of FABRIC_CLIENT_SETTINGS
///
/// Note: we may choose to add additional optional fields in future without considering that a SemVer breaking change.
/// You should default fields you're not interested in like so:
/// ```
/// # use std::num::NonZeroU32;
/// # use mssf_core::types::FabricClientSettings;
/// let my_settings = FabricClientSettings {
/// // TODO: uncomment in next PR
/// // PartitionLocationCacheLimit: Some(NonZeroU32::new(1).expect("Non-zero value")),
/// // Any other hypothetical settings you're interested in here,
/// ..Default::default()
/// };
/// ```
#[derive(Default)]
pub struct FabricClientSettings {}

impl FabricClientSettings {
/// Note: only overrides non-default settings; leaves any settings set previously that don't explicitly have new values alone
pub(crate) fn apply(&self, _settings_interface: &IFabricClientSettings2) -> crate::Result<()> {
// Placeholder
Ok(())
}
}
2 changes: 2 additions & 0 deletions crates/libs/core/src/types/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
// This mod contains common types shared between FabricRuntime and FabricClient.
mod partition;
pub use partition::*;
mod security_credentials;
pub use security_credentials::*;
mod stateful;
pub use stateful::*;
mod metrics;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------
#![deny(unsafe_op_in_unsafe_fn)]
#![deny(clippy::undocumented_unsafe_blocks)]
// TODO: implement
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------
use mssf_com::FabricTypes::{
FABRIC_PROTECTION_LEVEL, FABRIC_PROTECTION_LEVEL_ENCRYPTANDSIGN, FABRIC_PROTECTION_LEVEL_NONE,
FABRIC_PROTECTION_LEVEL_SIGN,
};

/// The Fabric Protection Level
/// See https://learn.microsoft.com/en-us/dotnet/api/system.fabric.protectionlevel?view=azure-dotnet
#[non_exhaustive]
#[derive(Copy, Clone, PartialEq, Eq)]
pub enum FabricProtectionLevel {
None,
Sign,
EncryptAndSign,
}

#[derive(Debug)]
#[allow(dead_code, reason = "For error handling")]
pub struct FabricProtectionLevelUnknownValueError(pub FABRIC_PROTECTION_LEVEL);

impl TryFrom<FABRIC_PROTECTION_LEVEL> for FabricProtectionLevel {
type Error = FabricProtectionLevelUnknownValueError;

fn try_from(value: FABRIC_PROTECTION_LEVEL) -> Result<Self, Self::Error> {
match value {
FABRIC_PROTECTION_LEVEL_NONE => Ok(FabricProtectionLevel::None),
FABRIC_PROTECTION_LEVEL_SIGN => Ok(FabricProtectionLevel::Sign),
FABRIC_PROTECTION_LEVEL_ENCRYPTANDSIGN => Ok(FabricProtectionLevel::EncryptAndSign),
x => Err(FabricProtectionLevelUnknownValueError(x)),
}
}
}

impl From<FabricProtectionLevel> for FABRIC_PROTECTION_LEVEL {
fn from(value: FabricProtectionLevel) -> Self {
match value {
FabricProtectionLevel::None => FABRIC_PROTECTION_LEVEL_NONE,
FabricProtectionLevel::Sign => FABRIC_PROTECTION_LEVEL_SIGN,
FabricProtectionLevel::EncryptAndSign => FABRIC_PROTECTION_LEVEL_ENCRYPTANDSIGN,
}
}
}
27 changes: 27 additions & 0 deletions crates/libs/core/src/types/common/security_credentials/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------
#![deny(unsafe_op_in_unsafe_fn)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where are denies needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are just me tightening up warnings in new code. unsafe_op_in_unsafe_fn requires discrete unsafe blocks inside unsafe functions to better clarify where unsafeness is (believe this becomes on by default in edition 2024)
undocumented_unsafe_block makes me write // SAFETY comments

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see unsafe code in this mod in this PR. Assme it will be used in the next.

#![deny(clippy::undocumented_unsafe_blocks)]
use mssf_com::FabricClient::IFabricClientSettings2;

mod claims_credentials;
mod fabric_protection_level;
pub use fabric_protection_level::*;
mod windows_credentials;
mod x509_credentials;
pub use x509_credentials::*;

/// Idiomatic FABRIC_SECURITY_CREDENTIALS wrapper
/// Currently, just a placeholder
#[non_exhaustive]
pub enum FabricSecurityCredentials {}

impl FabricSecurityCredentials {
/// Note: only overrides non-default settings; leaves any settings set previously that don't explicitly have new values alone
pub(crate) fn apply(&self, _settings_interface: &IFabricClientSettings2) -> crate::Result<()> {
// Placeholder
Ok(())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------
#![deny(unsafe_op_in_unsafe_fn)]
#![deny(clippy::undocumented_unsafe_blocks)]
// TODO: implement
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------
#![deny(unsafe_op_in_unsafe_fn)]
#![deny(clippy::undocumented_unsafe_blocks)]

use mssf_com::FabricTypes::{
FABRIC_X509_FIND_TYPE, FABRIC_X509_FIND_TYPE_FINDBYEXTENSION,
FABRIC_X509_FIND_TYPE_FINDBYSUBJECTNAME, FABRIC_X509_FIND_TYPE_FINDBYTHUMBPRINT,
FABRIC_X509_STORE_LOCATION, FABRIC_X509_STORE_LOCATION_CURRENTUSER,
FABRIC_X509_STORE_LOCATION_INVALID, FABRIC_X509_STORE_LOCATION_LOCALMACHINE,
};
use windows_core::WString;

/// How to find the X509 certificate.
#[non_exhaustive]
pub enum FabricX509FindType {
FindByExtension { extension: WString },
FindBySubjectName { subject_name: WString },
FindByThumbprint { thumbprint: WString },
}

impl From<&FabricX509FindType> for FABRIC_X509_FIND_TYPE {
fn from(value: &FabricX509FindType) -> Self {
match value {
FabricX509FindType::FindByExtension { extension: _ } => {
FABRIC_X509_FIND_TYPE_FINDBYEXTENSION
}
FabricX509FindType::FindBySubjectName { subject_name: _ } => {
FABRIC_X509_FIND_TYPE_FINDBYSUBJECTNAME
}
FabricX509FindType::FindByThumbprint { thumbprint: _ } => {
FABRIC_X509_FIND_TYPE_FINDBYTHUMBPRINT
}
}
}
}

/// What store location the certificate will be found in
#[non_exhaustive]
#[derive(Copy, Clone)]
pub enum FabricX509StoreLocation {
CurrentUser,
LocalMachine,
}

#[non_exhaustive]
#[derive(Copy, Clone)]
pub enum FabricX509StoreLocationConversionError {
InvalidValue,
UnknownValue(FABRIC_X509_STORE_LOCATION),
}

impl TryFrom<FABRIC_X509_STORE_LOCATION> for FabricX509StoreLocation {
type Error = FabricX509StoreLocationConversionError;

fn try_from(value: FABRIC_X509_STORE_LOCATION) -> Result<Self, Self::Error> {
match value {
FABRIC_X509_STORE_LOCATION_CURRENTUSER => Ok(FabricX509StoreLocation::CurrentUser),
FABRIC_X509_STORE_LOCATION_LOCALMACHINE => Ok(FabricX509StoreLocation::LocalMachine),
FABRIC_X509_STORE_LOCATION_INVALID => {
Err(FabricX509StoreLocationConversionError::InvalidValue)
}
x => Err(FabricX509StoreLocationConversionError::UnknownValue(x)),
}
}
}

impl From<FabricX509StoreLocation> for FABRIC_X509_STORE_LOCATION {
fn from(value: FabricX509StoreLocation) -> Self {
match value {
FabricX509StoreLocation::CurrentUser => FABRIC_X509_STORE_LOCATION_CURRENTUSER,
FabricX509StoreLocation::LocalMachine => FABRIC_X509_STORE_LOCATION_LOCALMACHINE,
}
}
}
2 changes: 1 addition & 1 deletion crates/samples/echomain-stateful2/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ impl TestClient {
// Uses fabric client to perform various actions for this service.
#[tokio::test]
async fn test_partition_info() {
let fc = FabricClient::builder().build();
let fc = FabricClient::builder().build().unwrap();
let tc = TestClient::new(fc.clone());
let timeout = Duration::from_secs(1);

Expand Down
Loading
Loading