Mbe 776 decouple credentialstore from kubers and add more complex

changelog.d/3164.internal.md

@@ -0,0 +1,2 @@
+decoupled Client from kube.rs in CredentialsStore and Credentials and 
+added unit test leveraging on achieved decoupling 

mirrord/auth/Cargo.toml

@@ -48,6 +48,7 @@ thiserror = { workspace = true, optional = true }
 x509-certificate = "0.24"
 reqwest = { workspace = true, features=["json", "rustls-tls-native-roots"], default-features = false, optional = true }
 tracing = { workspace = true, optional = true }
+futures = "0.3.31"
 
 [dev-dependencies]
 bcder = "0.7"

mirrord/auth/src/cluster_api.rs

@@ -0,0 +1,108 @@
+use std::{fmt::Debug, future::Future};
+
+use futures::{future, FutureExt};
+use kube::{api::PostParams, Api, Client, Resource};
+use serde::Deserialize;
+#[cfg(feature = "client")]
+use x509_certificate::{
+    rfc2986, InMemorySigningKeyPair, X509CertificateBuilder, X509CertificateError,
+};
+
+use crate::{certificate::Certificate, error::CredentialStoreError, key_pair::KeyPair};
+
+pub trait AuthClient<R> {
+    fn obtain_certificate(
+        &self,
+        key_pair: &KeyPair,
+        common_name: &str,
+    ) -> impl Future<Output = Result<Certificate, CredentialStoreError>> + Send;
+}
+
+impl<R> AuthClient<R> for Client
+where
+    R: for<'de> Deserialize<'de> + Resource + Clone + Debug,
+    <R as Resource>::DynamicType: Default,
+{
+    fn obtain_certificate(
+        &self,
+        key_pair: &KeyPair,
+        common_name: &str,
+    ) -> impl Future<Output = Result<Certificate, CredentialStoreError>> + Send {
+        let certificate_request = match certificate_request(common_name, key_pair)
+            .and_then(|req| req.encode_pem().map_err(X509CertificateError::from))
+        {
+            Ok(req) => req,
+            Err(e) => return future::err(e.into()).boxed(),
+        };
+
+        async move {
+            let api: Api<R> = Api::all(self.clone());
+
+            api.create_subresource(
+                "certificate",
+                "operator",
+                &PostParams::default(),
+                certificate_request.into(),
+            )
+            .await
+            .map_err(CredentialStoreError::from)
+        }
+        .boxed()
+    }
+}
+
+fn certificate_request(
+    common_name: &str,
+    key_pair: &InMemorySigningKeyPair,
+) -> Result<rfc2986::CertificationRequest, X509CertificateError> {
+    let mut builder = X509CertificateBuilder::default();
+
+    let _ = builder
+        .subject()
+        .append_common_name_utf8_string(common_name);
+
+    builder.create_certificate_signing_request(key_pair)
+}
+
+#[cfg(test)]
+pub mod client_mock {
+    use std::fmt::Debug;
+
+    use kube::Resource;
+    use serde::Deserialize;
+
+    use crate::{
+        certificate::Certificate, error::CredentialStoreError, key_pair::KeyPair, AuthClient,
+    };
+
+    #[derive(Clone)]
+    pub struct ClientMock {
+        pub return_error: bool,
+    }
+
+    impl<R> AuthClient<R> for ClientMock
+    where
+        R: for<'de> Deserialize<'de> + Resource + Clone + Debug,
+        <R as Resource>::DynamicType: Default,
+    {
+        async fn obtain_certificate(
+            &self,
+            _key_pair: &KeyPair,
+            _common_name: &str,
+        ) -> Result<Certificate, CredentialStoreError> {
+            if self.return_error {
+                Err(CredentialStoreError::FileAccess(std::io::Error::new(
+                    std::io::ErrorKind::Other,
+                    "Mock Error",
+                )))
+            } else {
+                Ok(certificate_mock())
+            }
+        }
+    }
+
+    pub fn certificate_mock() -> Certificate {
+        const SERIALIZED: &str = r#""-----BEGIN CERTIFICATE-----\r\nMIICGTCCAcmgAwIBAgIBATAHBgMrZXAFADBwMUIwQAYDVQQDDDlUaGUgTWljaGHF\r\ngiBTbW9sYXJlayBPcmdhbml6YXRpb25gcyBUZWFtcyBMaWNlbnNlIChUcmlhbCkx\r\nKjAoBgNVBAoMIVRoZSBNaWNoYcWCIFNtb2xhcmVrIE9yZ2FuaXphdGlvbjAeFw0y\r\nNDAyMDgxNTUwNDFaFw0yNDEyMjQwMDAwMDBaMBsxGTAXBgNVBAMMEHJheno0Nzgw\r\nLW1hY2hpbmUwLDAHBgMrZW4FAAMhAAfxTouyk5L5lB3eFwC5Rg9iI4KmQaFpnGVM\r\n2sYpv9HOo4HYMIHVMIHSBhcvbWV0YWxiZWFyL2xpY2Vuc2UvaW5mbwEB/wSBs3si\r\ndHlwZSI6InRlYW1zIiwibWF4X3NlYXRzIjpudWxsLCJzdWJzY3JpcHRpb25faWQi\r\nOiJmMWIxZDI2ZS02NGQzLTQ4YjYtYjVkMi05MzAxMzAwNWE3MmUiLCJvcmdhbml6\r\nYXRpb25faWQiOiIzNTdhZmE4MS0yN2QxLTQ3YjEtYTFiYS1hYzM1ZjlhM2MyNjMi\r\nLCJ0cmlhbCI6dHJ1ZSwidmVyc2lvbiI6IjMuNzMuMCJ9MAcGAytlcAUAA0EAJbbo\r\nu42KnHJBbPMYspMdv9ZdTQMixJgQUheNEs/o4+XfwgYOaRjCVQTzYs1m9f720WQ9\r\n4J04GdQvcu7B/oTgDQ==\r\n-----END CERTIFICATE-----\r\n""#;
+        serde_yaml::from_str(SERIALIZED).unwrap()
+    }
+}

mirrord/auth/src/credential_store.rs

@@ -6,7 +6,7 @@
 };
 
 use fs4::tokio::AsyncFileExt;
-use kube::{Client, Resource};
+use kube::Resource;
 use serde::{Deserialize, Serialize};
 use tokio::{
     fs,
@@ -15,8 +15,8 @@
 use whoami::fallible;
 
 use crate::{
-    certificate::Certificate, credentials::Credentials, error::CredentialStoreError,
-    key_pair::KeyPair,
+    certificate::Certificate, cluster_api::AuthClient, credentials::Credentials,
+    error::CredentialStoreError, key_pair::KeyPair,
 };
 
 /// "~/.mirrord"
@@ -115,13 +115,14 @@
     ///
     /// Also, subscription id is accepted as an [`Option`] to make the CLI backwards compatible.
     #[tracing::instrument(level = "trace", skip(self, client))]
-    pub async fn get_or_init<R>(
+    pub async fn get_or_init<R, C>(
         &mut self,
-        client: &Client,
+        client: &C,
         operator_fingerprint: String,
         operator_subscription_id: Option<String>,
     ) -> Result<&mut Credentials, CredentialStoreError>
     where
+        C: AuthClient<R> + Clone,
         R: Resource + Clone + Debug,
         R: for<'de> Deserialize<'de>,
         R::DynamicType: Default,
@@ -133,7 +134,7 @@
                     .and_then(|id| self.signing_keys.get(id))
                     .cloned();
 
-                let credentials = Credentials::init::<R>(
+                let credentials = Credentials::init::<R, C>(
                     client.clone(),
                     &Self::certificate_common_name(),
                     key_pair,
@@ -146,7 +147,7 @@
 
                 if !credentials.is_valid() {
                     credentials
-                        .refresh::<R>(client.clone(), &Self::certificate_common_name())
+                        .refresh::<R, C>(client.clone(), &Self::certificate_common_name())
                         .await?;
                 }
 
@@ -163,6 +164,60 @@
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use k8s_openapi::kube_aggregator::pkg::apis::apiregistration::v1::APIService;
+
+    use crate::{
+        cluster_api::client_mock::{certificate_mock, ClientMock},
+        credential_store::CredentialStore,
+    };
+
+    #[tokio::test]
+    async fn get_or_init() {
+        let operator_fingerprint = "operator_fingerprint".to_string();
+        let mut client = ClientMock {
+            return_error: false,
+        };
+
+        let mut credentials_store = CredentialStore::default();
+        // test when vacant
+        credentials_store
+            .get_or_init::<APIService, ClientMock>(&client, operator_fingerprint.clone(), None)
+            .await
+            .unwrap();
+        //check if saved cert is the same as the mock cert used
+        let saved_cert = credentials_store
+            .credentials
+            .get(&operator_fingerprint)
+            .unwrap()
+            .as_ref()
+            .clone();
+        let saved_cert = serde_yaml::to_string(&saved_cert).unwrap();
+        let mock_cert = serde_yaml::to_string(&certificate_mock()).unwrap();
+        assert_eq!(mock_cert, saved_cert);
+        // test when entry exists
+        credentials_store
+            .get_or_init::<APIService, ClientMock>(&client, operator_fingerprint.clone(), None)
+            .await
+            .unwrap();
+        client.return_error = true;
+        // test when entry exists and the client fails
+        credentials_store
+            .get_or_init::<APIService, ClientMock>(&client, operator_fingerprint.clone(), None)
+            .await
+            .unwrap_err();
+
+        credentials_store = CredentialStore::default();
+        client.return_error = true;
+        // test when vacant and the client fails
+        credentials_store
+            .get_or_init::<APIService, ClientMock>(&client, operator_fingerprint, None)
+            .await
+            .unwrap_err();
+    }
+}
+
 /// Exposes methods to safely access [`CredentialStore`] stored in a file.
 pub struct CredentialStoreSync {
     store_file: fs::File,
@@ -190,18 +245,19 @@
 
     /// Try and get/create a specific client certificate.
     /// The exclusive file lock is already acquired.
-    async fn access_credential<R, C, V>(
+    async fn access_credential<R, F, C, V>(
         &mut self,
-        client: &Client,
+        client: &C,
         operator_fingerprint: String,
         operator_subscription_id: Option<String>,
-        callback: C,
+        callback: F,
     ) -> Result<V, CredentialStoreError>
     where
         R: Resource + Clone + Debug,
         R: for<'de> Deserialize<'de>,
         R::DynamicType: Default,
-        C: FnOnce(&mut Credentials) -> V,
+        C: AuthClient<R> + Clone,
+        F: FnOnce(&mut Credentials) -> V,
     {
         let mut store = CredentialStore::load(&mut self.store_file)
             .await
@@ -210,7 +266,7 @@
 
         let value = callback(
             store
-                .get_or_init::<R>(client, operator_fingerprint, operator_subscription_id)
+                .get_or_init::<R, C>(client, operator_fingerprint, operator_subscription_id)
                 .await?,
         );
 
@@ -230,13 +286,14 @@
     }
 
     /// Get or create specific client certificate with an exclusive lock on the file.
-    pub async fn get_client_certificate<R>(
+    pub async fn get_client_certificate<R, C>(
         &mut self,
-        client: &Client,
+        client: &C,
         operator_fingerprint: String,
         operator_subscription_id: Option<String>,
     ) -> Result<Certificate, CredentialStoreError>
     where
+        C: AuthClient<R> + Clone,
         R: Resource + Clone + Debug,
         R: for<'de> Deserialize<'de>,
         R::DynamicType: Default,
@@ -246,7 +303,7 @@
             .map_err(CredentialStoreError::Lockfile)?;
 
         let result = self
-            .access_credential::<R, _, Certificate>(
+            .access_credential::<R, _, C, Certificate>(
                 client,
                 operator_fingerprint,
                 operator_subscription_id,