From 7c945412fbfdb5857b13445383ff6477eb884e12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Smolarek?= <34063647+Razz4780@users.noreply.github.com> Date: Mon, 13 Jan 2025 11:08:48 +0100 Subject: [PATCH 1/2] Pin cargo chef (#3014) * Pinned cargo-chef to 0.1.68 * Changelog --- changelog.d/+pinned-cargo-chef.internal.md | 1 + mirrord/agent/Dockerfile | 3 ++- mirrord/cli/Dockerfile | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 changelog.d/+pinned-cargo-chef.internal.md diff --git a/changelog.d/+pinned-cargo-chef.internal.md b/changelog.d/+pinned-cargo-chef.internal.md new file mode 100644 index 00000000000..8b18e6071d2 --- /dev/null +++ b/changelog.d/+pinned-cargo-chef.internal.md @@ -0,0 +1 @@ +Pinned `cargo-chef` version to `0.1.68` in the dockerfiles. \ No newline at end of file diff --git a/mirrord/agent/Dockerfile b/mirrord/agent/Dockerfile index 83c97871cae..e9de8df84d6 100644 --- a/mirrord/agent/Dockerfile +++ b/mirrord/agent/Dockerfile @@ -8,7 +8,8 @@ RUN ./platform.sh # this takes around 1 minute since libgit2 is slow https://github.com/rust-lang/cargo/issues/9167 ENV CARGO_NET_GIT_FETCH_WITH_CLI=true -RUN cargo install cargo-chef +# cargo-chef 0.1.69 breaks the build +RUN cargo install cargo-chef@0.1.68 FROM chef AS planner diff --git a/mirrord/cli/Dockerfile b/mirrord/cli/Dockerfile index 81e37142055..144f4a0e162 100644 --- a/mirrord/cli/Dockerfile +++ b/mirrord/cli/Dockerfile @@ -8,7 +8,8 @@ RUN ./platform.sh # this takes around 1 minute since libgit2 is slow https://github.com/rust-lang/cargo/issues/9167 ENV CARGO_NET_GIT_FETCH_WITH_CLI=true -RUN cargo install cargo-chef +# cargo-chef 0.1.69 breaks the build +RUN cargo install cargo-chef@0.1.68 FROM chef AS planner From 6668e3ccac9236486634a161844cf486b46dba14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Smolarek?= <34063647+Razz4780@users.noreply.github.com> Date: Mon, 13 Jan 2025 15:48:20 +0100 Subject: [PATCH 2/2] Fixed fs policy E2E test (#3012) * Fixed case of FsPolicy fields, improved docs * Fixed FsPolicy test and improved test utils * Changelog * Fixed case of envpolicy field --- changelog.d/+fs-policy-test.internal.md | 1 + mirrord/operator/src/crd/policy.rs | 21 +- .../fspolicy/test_operator_fs_policy.mjs | 69 ++---- tests/src/operator/policies.rs | 22 +- tests/src/operator/policies/fs.rs | 43 ++-- tests/src/utils.rs | 213 +++++++++--------- 6 files changed, 172 insertions(+), 197 deletions(-) create mode 100644 changelog.d/+fs-policy-test.internal.md diff --git a/changelog.d/+fs-policy-test.internal.md b/changelog.d/+fs-policy-test.internal.md new file mode 100644 index 00000000000..2ebd288613f --- /dev/null +++ b/changelog.d/+fs-policy-test.internal.md @@ -0,0 +1 @@ +Fixed fs policy E2E test. diff --git a/mirrord/operator/src/crd/policy.rs b/mirrord/operator/src/crd/policy.rs index e236164da98..cf712606d3c 100644 --- a/mirrord/operator/src/crd/policy.rs +++ b/mirrord/operator/src/crd/policy.rs @@ -104,7 +104,7 @@ pub struct MirrordClusterPolicySpec { /// Policy for controlling environment variables access from mirrord instances. #[derive(Clone, Default, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] -#[serde(rename_all = "kebab-case")] +#[serde(rename_all = "camelCase")] pub struct EnvPolicy { /// List of environment variables that should be excluded when using mirrord. /// @@ -123,20 +123,29 @@ pub struct EnvPolicy { /// Allows the operator control over remote file ops behaviour, overriding what the user has set in /// their mirrord config file, if it matches something in one of the lists (regex sets) of this /// struct. +/// +/// If the file path matches regexes in multiple sets, priority is as follows: +/// 1. `local` +/// 2. `notFound` +/// 3. `readOnly` #[derive(Clone, Default, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] -#[serde(rename_all = "kebab-case")] +#[serde(rename_all = "camelCase")] pub struct FsPolicy { - /// The file can only be opened in read-only mode, otherwise the operator returns an IO error. + /// Files that cannot be opened for writing. + /// + /// Opening the file for writing is rejected with an IO error. #[serde(default)] pub read_only: HashSet, - /// The file cannot be opened in the remote target. + /// Files that cannot be opened at all. /// - /// `open` calls that match this are forced to be opened in the local user's machine. + /// Opening the file will be rejected and mirrord will open the file locally instead. #[serde(default)] pub local: HashSet, - /// Any file that matches this returns a file not found error from the operator. + /// Files that cannot be opened at all. + /// + /// Opening the file is rejected with an IO error. #[serde(default)] pub not_found: HashSet, } diff --git a/tests/node-e2e/fspolicy/test_operator_fs_policy.mjs b/tests/node-e2e/fspolicy/test_operator_fs_policy.mjs index 8e58bf52cec..af84f17ee50 100644 --- a/tests/node-e2e/fspolicy/test_operator_fs_policy.mjs +++ b/tests/node-e2e/fspolicy/test_operator_fs_policy.mjs @@ -1,54 +1,19 @@ import fs from 'fs'; -fs.open("/app/file.local", (fail, fd) => { - console.log(`open file.local ${fd}`); - if (fd) { - console.log(`SUCCESS /app/file.local ${fd}`); - } - - if (fail) { - console.error(`FAIL /app/file.local ${fail}`); - } -}); - -fs.open("/app/file.not-found", (fail, fd) => { - console.log(`open file.not-found ${fd}`); - if (fd) { - console.log(`SUCCESS /app/file.not-found ${fd}`); - } - - if (fail) { - console.error(`FAIL /app/file.not-found ${fail}`); - } -}); - -fs.open("/app/file.read-only", (fail, fd) => { - if (fd) { - console.log(`SUCCESS /app/file.read-only ${fd}`); - } - - if (fail) { - console.error(`FAIL /app/file.read-only ${fail}`); - } -}); - -fs.open("/app/file.read-only", "r+", (fail, fd) => { - if (fd) { - console.log(`SUCCESS r+ /app/file.read-only ${fd}`); - } - - if (fail) { - console.error(`FAIL r+ /app/file.read-only ${fail}`); - } -}); - -fs.open("/app/file.read-write", "r+", (fail, fd) => { - if (fd) { - console.log(`SUCCESS /app/file.read-write ${fd}`); - } - - if (fail) { - console.error(`FAIL /app/file.read-write ${fail}`); - } -}); - +function test_open(path, mode) { + fs.open(path, mode, (fail, fd) => { + if (fd) { + console.log(`SUCCESS ${mode} ${path} ${fd}`); + } + + if (fail) { + console.log(`FAIL ${mode} ${path} ${fail}`); + } + }); +} + +test_open("/app/file.local", "r"); +test_open("/app/file.not-found", "r"); +test_open("/app/file.read-only", "r"); +test_open("/app/file.read-only", "r+"); +test_open("/app/file.read-write", "r+"); diff --git a/tests/src/operator/policies.rs b/tests/src/operator/policies.rs index 3684a97fc20..2a0a8465bbe 100644 --- a/tests/src/operator/policies.rs +++ b/tests/src/operator/policies.rs @@ -33,28 +33,18 @@ impl PolicyGuard { ) -> Self { let policy_api: Api = Api::namespaced(kube_client.clone(), namespace); PolicyGuard { - _inner: ResourceGuard::create( - policy_api, - policy.metadata.name.clone().unwrap(), - policy, - true, - ) - .await - .expect("Could not create policy in E2E test."), + _inner: ResourceGuard::create(policy_api, policy, true) + .await + .expect("Could not create policy in E2E test."), } } pub async fn clusterwide(kube_client: kube::Client, policy: &MirrordClusterPolicy) -> Self { let policy_api: Api = Api::all(kube_client.clone()); PolicyGuard { - _inner: ResourceGuard::create( - policy_api, - policy.metadata.name.clone().unwrap(), - policy, - true, - ) - .await - .expect("Could not create policy in E2E test."), + _inner: ResourceGuard::create(policy_api, policy, true) + .await + .expect("Could not create policy in E2E test."), } } } diff --git a/tests/src/operator/policies/fs.rs b/tests/src/operator/policies/fs.rs index 3feae1d514c..79a1b7e7202 100644 --- a/tests/src/operator/policies/fs.rs +++ b/tests/src/operator/policies/fs.rs @@ -39,14 +39,14 @@ pub async fn create_namespaced_fs_policy_and_try_file_open( &MirrordPolicy::new( "e2e-test-fs-policy-with-path-pattern", MirrordPolicySpec { - target_path: Some("fs_policy_e2e-test-*".into()), + target_path: Some("*fs-policy-e2e-test-*".into()), selector: None, block: Default::default(), env: Default::default(), fs: FsPolicy { - read_only: HashSet::from_iter(vec!["file.read-only".to_string()]), - local: HashSet::from_iter(vec!["file.local".to_string()]), - not_found: HashSet::from_iter(vec!["file.not-found".to_string()]), + read_only: HashSet::from_iter(vec!["file\\.read-only".to_string()]), + local: HashSet::from_iter(vec!["file\\.local".to_string()]), + not_found: HashSet::from_iter(vec!["file\\.not-found".to_string()]), }, }, ), @@ -68,20 +68,25 @@ pub async fn create_namespaced_fs_policy_and_try_file_open( test_process.wait_assert_success().await; - test_process - .assert_stderr_contains("FAIL /app/file.local") - .await; - test_process - .assert_stderr_contains("FAIL /app/file.not-found") - .await; - test_process - .assert_stderr_contains("FAIL r+ /app/file.read-only") - .await; + let stdout = test_process.get_stdout().await; - test_process - .assert_stdout_contains("SUCCESS /app/file.read-only") - .await; - test_process - .assert_stdout_contains("SUCCESS /app/file.read-write") - .await; + let reading_local_failed = stdout.contains("FAIL r /app/file.local"); + let reading_not_found_failed = stdout.contains("FAIL r /app/file.not-found"); + let reading_read_only_succeeded = stdout.contains("SUCCESS r /app/file.read-only"); + let writing_read_only_failed = stdout.contains("FAIL r+ /app/file.read-only"); + let writing_read_write_succeeded = stdout.contains("SUCCESS r+ /app/file.read-write"); + + assert!( + reading_local_failed + && reading_not_found_failed + && reading_read_only_succeeded + && writing_read_only_failed + && writing_read_write_succeeded, + "some file operations did not finish as expected:\n + \treading_local_failed={reading_local_failed}\n + \treading_not_found_failed={reading_not_found_failed}\n + \treading_read_only_succeeded={reading_read_only_succeeded} \n + \twriting_read_only_failed={writing_read_only_failed}\n + \twriting_read_write_succeeded={writing_read_write_succeeded}", + ) } diff --git a/tests/src/utils.rs b/tests/src/utils.rs index 3b596efa357..bf8e0d1a79b 100644 --- a/tests/src/utils.rs +++ b/tests/src/utils.rs @@ -6,6 +6,7 @@ use std::{ fmt::Debug, net::IpAddr, ops::Not, + os::unix::process::ExitStatusExt, path::PathBuf, process::{ExitStatus, Stdio}, sync::{Arc, Once}, @@ -24,7 +25,7 @@ use kube::{ api::{DeleteParams, ListParams, PostParams, WatchParams}, core::WatchEvent, runtime::wait::{await_condition, conditions::is_pod_running}, - Api, Client, Config, Error, + Api, Client, Config, Error, Resource, }; use rand::{distributions::Alphanumeric, Rng}; use reqwest::{RequestBuilder, StatusCode}; @@ -155,68 +156,111 @@ impl TestProcess { pub async fn assert_log_level(&self, stderr: bool, level: &str) { if stderr { - assert!(!self.stderr_data.read().await.contains(level)); + assert!( + self.stderr_data.read().await.contains(level).not(), + "application stderr should not contain `{level}`" + ); } else { - assert!(!self.stdout_data.read().await.contains(level)); + assert!( + self.stdout_data.read().await.contains(level).not(), + "application stdout should not contain `{level}`" + ); } } pub async fn assert_python_fileops_stderr(&self) { - assert!(!self.stderr_data.read().await.contains("FAILED")); + assert!( + self.stderr_data.read().await.contains("FAILED").not(), + "application stderr should not contain `FAILED`" + ); } pub async fn wait_assert_success(&mut self) { let output = self.wait().await; - assert!(output.success()); + assert!( + output.success(), + "application unexpectedly failed: exit code {:?}, signal code {:?}", + output.code(), + output.signal(), + ); } pub async fn wait_assert_fail(&mut self) { let output = self.wait().await; - assert!(!output.success()); + assert!( + output.success().not(), + "application unexpectedly succeeded: exit code {:?}, signal code {:?}", + output.code(), + output.signal() + ); } pub async fn assert_stdout_contains(&self, string: &str) { - assert!(self.get_stdout().await.contains(string)); + assert!( + self.get_stdout().await.contains(string), + "application stdout should contain `{string}`", + ); } pub async fn assert_stdout_doesnt_contain(&self, string: &str) { - assert!(!self.get_stdout().await.contains(string)); + assert!( + self.get_stdout().await.contains(string).not(), + "application stdout should not contain `{string}`", + ); } pub async fn assert_stderr_contains(&self, string: &str) { - assert!(self.get_stderr().await.contains(string)); + assert!( + self.get_stderr().await.contains(string), + "application stderr should contain `{string}`", + ); } pub async fn assert_stderr_doesnt_contain(&self, string: &str) { - assert!(!self.get_stderr().await.contains(string)); + assert!( + self.get_stderr().await.contains(string).not(), + "application stderr should not contain `{string}`", + ); } pub async fn assert_no_error_in_stdout(&self) { - assert!(!self - .error_capture - .is_match(&self.stdout_data.read().await) - .unwrap()); + assert!( + self.error_capture + .is_match(&self.stdout_data.read().await) + .unwrap() + .not(), + "application stdout contains an error" + ); } pub async fn assert_no_error_in_stderr(&self) { - assert!(!self - .error_capture - .is_match(&self.stderr_data.read().await) - .unwrap()); + assert!( + self.error_capture + .is_match(&self.stderr_data.read().await) + .unwrap() + .not(), + "application stderr contains an error" + ); } pub async fn assert_no_warn_in_stdout(&self) { - assert!(!self - .warn_capture - .is_match(&self.stdout_data.read().await) - .unwrap()); + assert!( + self.warn_capture + .is_match(&self.stdout_data.read().await) + .unwrap() + .not(), + "application stdout contains a warning" + ); } pub async fn assert_no_warn_in_stderr(&self) { - assert!(!self - .warn_capture - .is_match(&self.stderr_data.read().await) - .unwrap()); + assert!( + self.warn_capture + .is_match(&self.stderr_data.read().await) + .unwrap() + .not(), + "application stderr contains a warning" + ); } pub async fn wait_for_line(&self, timeout: Duration, line: &str) { @@ -673,23 +717,27 @@ pub(crate) struct ResourceGuard { impl ResourceGuard { /// Create a kube resource and spawn a task to delete it when this guard is dropped. /// Return [`Error`] if creating the resource failed. - pub async fn create( + pub async fn create< + K: Resource + Debug + Clone + DeserializeOwned + Serialize + 'static, + >( api: Api, - name: String, data: &K, delete_on_fail: bool, ) -> Result { + let name = data.meta().name.clone().unwrap(); + println!("Creating {} `{name}`: {data:?}", K::kind(&())); api.create(&PostParams::default(), data).await?; + println!("Created {} `{name}`", K::kind(&())); let deleter = async move { - println!("Deleting resource `{name}`"); + println!("Deleting {} `{name}`", K::kind(&())); let delete_params = DeleteParams { grace_period_seconds: Some(0), ..Default::default() }; let res = api.delete(&name, &delete_params).await; if let Err(e) = res { - println!("Failed to delete resource `{name}`: {e:?}"); + println!("Failed to delete {} `{name}`: {e:?}", K::kind(&())); } }; @@ -1171,7 +1219,7 @@ async fn internal_service( }; println!( - "{} creating service {name:?} in namespace {namespace:?}", + "{} creating service {name} in namespace {namespace}", format_time() ); @@ -1189,7 +1237,6 @@ async fn internal_service( // Create namespace and wrap it in ResourceGuard if it does not yet exist. let namespace_guard = ResourceGuard::create( namespace_api.clone(), - namespace.to_string(), &namespace_resource, delete_after_fail, ) @@ -1201,14 +1248,9 @@ async fn internal_service( // `Deployment` let deployment = deployment_from_json(&name, image, env); - let pod_guard = ResourceGuard::create( - deployment_api.clone(), - name.to_string(), - &deployment, - delete_after_fail, - ) - .await - .unwrap(); + let pod_guard = ResourceGuard::create(deployment_api.clone(), &deployment, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&deployment_api, &name).await; // `Service` @@ -1216,14 +1258,9 @@ async fn internal_service( if ipv6_only { set_ipv6_only(&mut service); } - let service_guard = ResourceGuard::create( - service_api.clone(), - name.clone(), - &service, - delete_after_fail, - ) - .await - .unwrap(); + let service_guard = ResourceGuard::create(service_api.clone(), &service, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&service_api, "default").await; let pod_name = get_instance_name::(kube_client.clone(), &name, namespace) @@ -1237,8 +1274,8 @@ async fn internal_service( .unwrap(); println!( - "{:?} done creating service {name:?} in namespace {namespace:?}", - Utc::now() + "{} done creating service {name} in namespace {namespace}", + format_time(), ); KubeService { @@ -1303,7 +1340,6 @@ pub async fn service_for_mirrord_ls( // Create namespace and wrap it in ResourceGuard if it does not yet exist. let namespace_guard = ResourceGuard::create( namespace_api.clone(), - namespace.to_string(), &namespace_resource, delete_after_fail, ) @@ -1315,26 +1351,16 @@ pub async fn service_for_mirrord_ls( // `Deployment` let deployment = deployment_from_json(&name, image, default_env()); - let pod_guard = ResourceGuard::create( - deployment_api.clone(), - name.to_string(), - &deployment, - delete_after_fail, - ) - .await - .unwrap(); + let pod_guard = ResourceGuard::create(deployment_api.clone(), &deployment, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&deployment_api, &name).await; // `Service` let service = service_from_json(&name, service_type); - let service_guard = ResourceGuard::create( - service_api.clone(), - name.clone(), - &service, - delete_after_fail, - ) - .await - .unwrap(); + let service_guard = ResourceGuard::create(service_api.clone(), &service, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&service_api, "default").await; let pod_name = get_instance_name::(kube_client.clone(), &name, namespace) @@ -1425,7 +1451,6 @@ pub async fn service_for_mirrord_ls( // Create namespace and wrap it in ResourceGuard if it does not yet exist. let namespace_guard = ResourceGuard::create( namespace_api.clone(), - namespace.to_string(), &namespace_resource, delete_after_fail, ) @@ -1437,58 +1462,38 @@ pub async fn service_for_mirrord_ls( // `Deployment` let deployment = deployment_from_json(&name, image, default_env()); - let pod_guard = ResourceGuard::create( - deployment_api.clone(), - name.to_string(), - &deployment, - delete_after_fail, - ) - .await - .unwrap(); + let pod_guard = ResourceGuard::create(deployment_api.clone(), &deployment, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&deployment_api, &name).await; // `Service` let service = service_from_json(&name, service_type); - let service_guard = ResourceGuard::create( - service_api.clone(), - name.clone(), - &service, - delete_after_fail, - ) - .await - .unwrap(); + let service_guard = ResourceGuard::create(service_api.clone(), &service, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&service_api, "default").await; // `StatefulSet` let stateful_set = stateful_set_from_json(&name, image); - let stateful_set_guard = ResourceGuard::create( - stateful_set_api.clone(), - name.to_string(), - &stateful_set, - delete_after_fail, - ) - .await - .unwrap(); + let stateful_set_guard = + ResourceGuard::create(stateful_set_api.clone(), &stateful_set, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&stateful_set_api, &name).await; // `CronJob` let cron_job = cron_job_from_json(&name, image); - let cron_job_guard = ResourceGuard::create( - cron_job_api.clone(), - name.to_string(), - &cron_job, - delete_after_fail, - ) - .await - .unwrap(); + let cron_job_guard = ResourceGuard::create(cron_job_api.clone(), &cron_job, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&cron_job_api, &name).await; // `Job` let job = job_from_json(&name, image); - let job_guard = - ResourceGuard::create(job_api.clone(), name.to_string(), &job, delete_after_fail) - .await - .unwrap(); + let job_guard = ResourceGuard::create(job_api.clone(), &job, delete_after_fail) + .await + .unwrap(); watch_resource_exists(&job_api, &name).await; let pod_name = get_instance_name::(kube_client.clone(), &name, namespace)