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

Add policy for file ops. #2997

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
3083aec
Add policy to exclude env vars.
meowjesty Dec 25, 2024
6182f1f
docs
meowjesty Dec 25, 2024
4aac118
changelog
meowjesty Dec 25, 2024
864de0b
fix tests
meowjesty Dec 25, 2024
eb375db
Make policy optional for backwards compat.
meowjesty Dec 26, 2024
14f79ff
typo
meowjesty Dec 26, 2024
391cb02
other typo
meowjesty Dec 26, 2024
49194e7
default serde and more explicit docs
meowjesty Dec 30, 2024
ba65c1c
fix mismatching policy
meowjesty Dec 31, 2024
dd4844e
make it look more like config
meowjesty Dec 31, 2024
5d13e17
make it pub
meowjesty Dec 31, 2024
5683027
fix test
meowjesty Dec 31, 2024
c3b4035
better docs
meowjesty Dec 31, 2024
cb96699
better docs due
meowjesty Dec 31, 2024
5a13f57
rustfmt
meowjesty Dec 31, 2024
a679987
Add policy for file ops.
meowjesty Dec 31, 2024
453413f
add handling to open local files when requested by the remote
meowjesty Jan 6, 2025
6ee04ba
no exclude
meowjesty Jan 6, 2025
bd9d11d
update protocol with open_local_version
meowjesty Jan 6, 2025
d2dd6d8
bump protocol
meowjesty Jan 6, 2025
355bcd5
merge main
meowjesty Jan 7, 2025
a4d212c
lint test
meowjesty Jan 7, 2025
749017e
docs
meowjesty Jan 7, 2025
79e307a
fix test
meowjesty Jan 7, 2025
9167373
change min protocol version
meowjesty Jan 7, 2025
b11eaea
e2e test for fspolicy
meowjesty Jan 8, 2025
9db44e7
Merge branch 'main' into meowchinist/mbe-606-add-mirrord-policy-to-de…
meowjesty Jan 8, 2025
53112c4
Ignore fs policy test/
meowjesty Jan 8, 2025
1f8fc85
namespaced test
meowjesty Jan 8, 2025
dfa2edd
hopefully fixed policy test
meowjesty Jan 9, 2025
9f5b738
the children get to live
meowjesty Jan 9, 2025
3f4a0a6
fix test policy name
meowjesty Jan 9, 2025
bf1073a
fix go fs test
meowjesty Jan 9, 2025
2ef52e6
remove read_write
meowjesty Jan 9, 2025
e942a0b
add newline to python test
meowjesty Jan 9, 2025
ed91355
changelog
meowjesty Jan 9, 2025
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
377 changes: 212 additions & 165 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions changelog.d/+104-policy-fs.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add policy to control file ops.
4 changes: 4 additions & 0 deletions mirrord/layer/src/detour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ pub(crate) enum Bypass {
/// Useful for operations that are version gated, and we want to bypass when the protocol
/// doesn't support them.
NotImplemented,

/// File `open` (any `open`-ish operation) was forced to be local, instead of remote, most
/// likely due to an operator fs policy.
OpenLocal,
}

impl Bypass {
Expand Down
1 change: 1 addition & 0 deletions mirrord/layer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ impl From<HookError> for i64 {
HookError::BincodeEncode(_) => libc::EINVAL,
HookError::ResponseError(response_fail) => match response_fail {
ResponseError::IdsExhausted(_) => libc::ENOMEM,
ResponseError::OpenLocal => libc::ENOENT,
ResponseError::NotFound(_) => libc::ENOENT,
ResponseError::NotDirectory(_) => libc::ENOTDIR,
ResponseError::NotFile(_) => libc::EISDIR,
Expand Down
7 changes: 6 additions & 1 deletion mirrord/layer/src/file/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,12 @@ pub(crate) fn open(path: Detour<PathBuf>, open_options: OpenOptionsInternal) ->

ensure_not_ignored!(path, open_options.is_write());

let OpenFileResponse { fd: remote_fd } = RemoteFile::remote_open(path.clone(), open_options)?;
let OpenFileResponse { fd: remote_fd } = RemoteFile::remote_open(path.clone(), open_options)
.or_else(|fail| match fail {
// The operator has a policy that matches this `path` as local-only.
HookError::ResponseError(ResponseError::OpenLocal) => Detour::Bypass(Bypass::OpenLocal),
other => Detour::Error(other),
})?;

// TODO: Need a way to say "open a directory", right now `is_dir` always returns false.
// This requires having a fake directory name (`/fake`, for example), instead of just converting
Expand Down
34 changes: 34 additions & 0 deletions mirrord/operator/src/crd/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ pub struct MirrordPolicySpec {
/// target.
#[serde(default)]
pub env: EnvPolicy,

/// Overrides fs ops behaviour, granting control over them to the operator policy, instead of
/// the user config.
#[serde(default)]
pub fs: FsPolicy,
}

/// Custom cluster-wide resource for policies that limit what mirrord features users can use.
Expand Down Expand Up @@ -90,6 +95,11 @@ pub struct MirrordClusterPolicySpec {
/// target.
#[serde(default)]
pub env: EnvPolicy,

/// Overrides fs ops behaviour, granting control over them to the operator policy, instead of
/// the user config.
#[serde(default)]
pub fs: FsPolicy,
}

/// Policy for controlling environment variables access from mirrord instances.
Expand All @@ -104,9 +114,33 @@ pub struct EnvPolicy {
/// Variable names can be matched using `*` and `?` where `?` matches exactly one occurrence of
/// any character and `*` matches arbitrary many (including zero) occurrences of any character,
/// e.g. `DATABASE_*` will match `DATABASE_URL` and `DATABASE_PORT`.
#[serde(default)]
pub exclude: HashSet<String>,
}

/// File operations policy that mimics the mirrord fs config.
///
/// 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.
#[derive(Clone, Default, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)]
#[serde(rename_all = "kebab-case")]
pub struct FsPolicy {
/// The file can only be opened in read-only mode, otherwise the operator returns an IO error.
#[serde(default)]
pub read_only: HashSet<String>,

/// The file cannot be opened in the remote target.
///
/// `open` calls that match this are forced to be opened in the local user's machine.
#[serde(default)]
pub local: HashSet<String>,
meowjesty marked this conversation as resolved.
Show resolved Hide resolved

/// Any file that matches this returns a file not found error from the operator.
#[serde(default)]
pub not_found: HashSet<String>,
}

#[test]
fn check_one_api_group() {
use kube::Resource;
Expand Down
2 changes: 1 addition & 1 deletion mirrord/protocol/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "mirrord-protocol"
version = "1.13.2"
version = "1.13.3"
authors.workspace = true
description.workspace = true
documentation.workspace = true
Expand Down
3 changes: 3 additions & 0 deletions mirrord/protocol/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ pub enum ResponseError {

#[error("Failed stripping path with `{0}`!")]
StripPrefix(String),

#[error("File has to be opened locally!")]
OpenLocal,
}

impl From<StripPrefixError> for ResponseError {
Expand Down
3 changes: 3 additions & 0 deletions mirrord/protocol/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ pub static READDIR_BATCH_VERSION: LazyLock<VersionReq> =
pub static MKDIR_VERSION: LazyLock<VersionReq> =
LazyLock::new(|| ">=1.13.0".parse().expect("Bad Identifier"));

pub static OPEN_LOCAL_VERSION: LazyLock<VersionReq> =
LazyLock::new(|| ">=1.13.3".parse().expect("Bad Identifier"));

/// Internal version of Metadata across operating system (macOS, Linux)
/// Only mutual attributes
#[derive(Encode, Decode, Debug, PartialEq, Clone, Copy, Eq, Default)]
Expand Down
15 changes: 11 additions & 4 deletions tests/go-e2e-dir/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,20 @@ func main() {
os.Exit(-1)
}
fmt.Printf("DirEntries: %s\n", dir)

// `os.ReadDir` does not include `.` and `..`.
if len(dir) != 2 {
if len(dir) < 2 {
os.Exit(-1)
}
// `os.ReadDir` sorts the result by file name.
if dir[0].Name() != "app.py" || dir[1].Name() != "test.txt" {
os.Exit(-1)

// Iterate over the files in this dir, exiting if it's not an expected file name.
for i := 0; i < len(dir); i++ {
dirName := dir[i].Name()

if dirName != "app.py" && dirName != "test.txt" && dirName != "file.local" && dirName != "file.not-found" && dirName != "file.read-only" && dirName != "file.read-write" {
os.Exit(-1)
}

}

err = os.Mkdir("/app/test_mkdir", 0755)
Expand Down
54 changes: 54 additions & 0 deletions tests/node-e2e/fspolicy/test_operator_fs_policy.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
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}`);
}
});

4 changes: 2 additions & 2 deletions tests/python-e2e/files_ro.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import uuid
import unittest

TEXT = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum."
TEXT = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.\n"


class FileOpsTest(unittest.TestCase):
Expand All @@ -22,4 +22,4 @@ def test_read_only(self):


if __name__ == "__main__":
unittest.main()
unittest.main()
2 changes: 1 addition & 1 deletion tests/python-e2e/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import uuid
import unittest

TEXT = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum."
TEXT = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.\n"


class FileOpsTest(unittest.TestCase):
Expand Down
9 changes: 9 additions & 0 deletions tests/src/operator/policies.rs
meowjesty marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use crate::utils::{
config_dir, kube_client, service, Application, KubeService, ResourceGuard, TestProcess,
};

mod fs;

/// Guard that deletes a mirrord policy when dropped.
struct PolicyGuard {
_inner: ResourceGuard,
Expand Down Expand Up @@ -128,6 +130,7 @@ fn block_steal_without_qualifiers() -> PolicyTestCase {
selector: None,
block: vec![BlockedFeature::Steal],
env: Default::default(),
fs: Default::default(),
},
),
service_b_can_steal: No,
Expand All @@ -147,6 +150,7 @@ fn block_steal_with_path_pattern() -> PolicyTestCase {
selector: None,
block: vec![BlockedFeature::Steal],
env: Default::default(),
fs: Default::default(),
},
),
service_b_can_steal: EvenWithoutFilter,
Expand All @@ -166,6 +170,7 @@ fn block_unfiltered_steal_with_path_pattern() -> PolicyTestCase {
selector: None,
block: vec![BlockedFeature::StealWithoutFilter],
env: Default::default(),
fs: Default::default(),
},
),
service_b_can_steal: EvenWithoutFilter,
Expand All @@ -185,6 +190,7 @@ fn block_unfiltered_steal_with_deployment_path_pattern() -> PolicyTestCase {
selector: None,
block: vec![BlockedFeature::StealWithoutFilter],
env: Default::default(),
fs: Default::default(),
},
),
service_a_can_steal: OnlyWithFilter,
Expand All @@ -210,6 +216,7 @@ fn block_steal_with_label_selector() -> PolicyTestCase {
}),
block: vec![BlockedFeature::Steal],
env: Default::default(),
fs: Default::default(),
},
),
service_b_can_steal: EvenWithoutFilter,
Expand All @@ -236,6 +243,7 @@ fn block_steal_with_unmatching_policy() -> PolicyTestCase {
}),
block: vec![BlockedFeature::Steal],
env: Default::default(),
fs: Default::default(),
},
),
service_b_can_steal: EvenWithoutFilter,
Expand Down Expand Up @@ -377,6 +385,7 @@ pub async fn create_cluster_policy_and_try_to_mirror(
selector: None,
block: vec![BlockedFeature::Mirror],
env: Default::default(),
fs: Default::default(),
},
),
)
Expand Down
87 changes: 87 additions & 0 deletions tests/src/operator/policies/fs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
use std::{collections::HashSet, time::Duration};

use mirrord_operator::crd::policy::{FsPolicy, MirrordPolicy, MirrordPolicySpec};
use rstest::{fixture, rstest};

use crate::{
operator::policies::PolicyGuard,
utils::{kube_client, service, Application, KubeService},
};

#[fixture]
async fn fs_service(#[future] kube_client: kube::Client) -> KubeService {
let namespace = format!("e2e-tests-fs-policies-{}", crate::utils::random_string());

service(
&namespace,
"NodePort",
"ghcr.io/metalbear-co/mirrord-pytest:latest",
"fs-policy-e2e-test-service",
false,
kube_client,
)
.await
}

#[rstest]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[timeout(Duration::from_secs(60))]
pub async fn create_cluster_fs_policy_and_try_file_operations(
#[future] service: KubeService,
#[future] kube_client: kube::Client,
) {
let kube_client = kube_client.await;
let service = service.await;

// Create policy, delete it when test exits.
let _policy_guard = PolicyGuard::namespaced(
kube_client,
&MirrordPolicy::new(
"e2e-test-fs-policy-with-path-pattern",
MirrordPolicySpec {
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()]),
},
},
),
&service.namespace,
)
.await;

let application = Application::NodeFsPolicy;
println!("Running mirrord {application:?} against {}", &service.name);

let mut test_process = application
.run(
&service.target,
Some(&service.namespace),
Some(vec!["--fs-mode=write"]),
None,
)
.await;

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;

test_process
.assert_stdout_contains("SUCCESS /app/file.read-only")
.await;
test_process
.assert_stdout_contains("SUCCESS /app/file.read-write")
.await;
}
Loading
Loading