diff --git a/.changeset/changesets/cheerily-mint-mouse.md b/.changeset/changesets/cheerily-mint-mouse.md new file mode 100644 index 0000000..ed8a82c --- /dev/null +++ b/.changeset/changesets/cheerily-mint-mouse.md @@ -0,0 +1,5 @@ +--- +lldap-operator-helm-chart: patch +lldap-operator-crds: patch +--- +Refine the error message shown when an LldapUser is rejected for missing passwordSecretRef diff --git a/.changeset/changesets/mawkishly-inspiring-thorntail.md b/.changeset/changesets/mawkishly-inspiring-thorntail.md new file mode 100644 index 0000000..0417732 --- /dev/null +++ b/.changeset/changesets/mawkishly-inspiring-thorntail.md @@ -0,0 +1,4 @@ +--- +lldap-operator-reconciler: minor +--- +Add validate_membership_references for checking LldapMembership user/group references against caller-supplied candidates diff --git a/Cargo.lock b/Cargo.lock index 7f2b44a..42550e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,15 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "aho-corasick" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" +dependencies = [ + "memchr", +] + [[package]] name = "anstream" version = "1.0.0" @@ -52,6 +61,23 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "antlr4rust" +version = "0.3.0-rc2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d240d49ee89063f90fa0cb18aead41a5893cd544a1785983dc3bf5c3d5faa58b" +dependencies = [ + "better_any", + "bit-set", + "byteorder", + "lazy_static", + "murmur3", + "once_cell", + "parking_lot", + "typed-arena", + "uuid", +] + [[package]] name = "anyhow" version = "1.0.102" @@ -70,24 +96,92 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "better_any" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4372b9543397a4b86050cc5e7ee36953edf4bac9518e8a774c2da694977fb6e4" + +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "2.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" +[[package]] +name = "bumpalo" +version = "3.20.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d20789868f4b01b2f2caec9f5c4e0213b41e3e5702a50157d699ae31ced2fcb" + +[[package]] +name = "byteorder" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" + [[package]] name = "bytes" version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" +[[package]] +name = "cel-interpreter" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a76c07820046cc8239526fceec6df147a979ae48644dbad274fc3ce38ab0973b" +dependencies = [ + "cel-parser", + "chrono", + "nom", + "paste", + "regex", + "serde", + "thiserror 1.0.69", +] + +[[package]] +name = "cel-parser" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "546fb134998490c5c47fc7a29c7535e725d2e403f172040e8f263d0b318bff5f" +dependencies = [ + "antlr4rust", + "lazy_static", +] + [[package]] name = "cfg-if" version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "chrono" +version = "0.4.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c673075a2e0e5f4a1dde27ce9dee1ea4558c7ffe648f576438a20ca1d2acc4b0" +dependencies = [ + "num-traits", + "serde", +] + [[package]] name = "clap" version = "4.6.1" @@ -219,6 +313,30 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "futures-core" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" + +[[package]] +name = "futures-task" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "037711b3d59c33004d3856fbdc83b99d4ff37a24768fa1be9ce3538a1cde4393" + +[[package]] +name = "futures-util" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6" +dependencies = [ + "futures-core", + "futures-task", + "pin-project-lite", + "slab", +] + [[package]] name = "getrandom" version = "0.4.2" @@ -323,6 +441,18 @@ dependencies = [ "syn", ] +[[package]] +name = "js-sys" +version = "0.3.98" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67df7112613f8bfd9150013a0314e196f4800d3201ae742489d999db2f979f08" +dependencies = [ + "cfg-if", + "futures-util", + "once_cell", + "wasm-bindgen", +] + [[package]] name = "k8s-openapi" version = "0.27.1" @@ -362,7 +492,7 @@ dependencies = [ "serde", "serde-value", "serde_json", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -379,6 +509,12 @@ dependencies = [ "syn", ] +[[package]] +name = "lazy_static" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" + [[package]] name = "leb128fmt" version = "0.1.0" @@ -402,7 +538,7 @@ name = "lldap-operator" version = "0.0.0" dependencies = [ "clap", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -413,6 +549,7 @@ version = "0.0.0" name = "lldap-operator-crds" version = "0.0.0" dependencies = [ + "cel-interpreter", "k8s-openapi", "kube", "schemars", @@ -420,12 +557,15 @@ dependencies = [ "serde_json", "serde_yaml_ng", "tempfile", - "thiserror", + "thiserror 2.0.18", ] [[package]] name = "lldap-operator-reconciler" version = "0.0.0" +dependencies = [ + "thiserror 2.0.18", +] [[package]] name = "lldap-operator-traits" @@ -434,6 +574,15 @@ dependencies = [ "tokio", ] +[[package]] +name = "lock_api" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "224399e74b87b5f3557511d98dff8b14089b3dadafcab6bb93eab67d3aace965" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.29" @@ -446,6 +595,31 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" +[[package]] +name = "minimal-lexical" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" + +[[package]] +name = "murmur3" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a198f9589efc03f544388dfc4a19fe8af4323662b62f598b8dcfdac62c14771c" +dependencies = [ + "byteorder", +] + +[[package]] +name = "nom" +version = "7.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a" +dependencies = [ + "memchr", + "minimal-lexical", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -476,6 +650,35 @@ dependencies = [ "num-traits", ] +[[package]] +name = "parking_lot" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-link", +] + +[[package]] +name = "paste" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" + [[package]] name = "percent-encoding" version = "2.3.2" @@ -537,6 +740,15 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" +[[package]] +name = "redox_syscall" +version = "0.5.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" +dependencies = [ + "bitflags", +] + [[package]] name = "ref-cast" version = "1.0.25" @@ -557,6 +769,35 @@ dependencies = [ "syn", ] +[[package]] +name = "regex" +version = "1.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e10754a14b9137dd7b1e3e5b0493cc9171fdd105e0ab477f51b72e7f3ac0e276" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" + [[package]] name = "rustc_version" version = "0.4.1" @@ -579,6 +820,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "rustversion" +version = "1.0.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" + [[package]] name = "ryu" version = "1.0.23" @@ -610,6 +857,12 @@ dependencies = [ "syn", ] +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "semver" version = "1.0.28" @@ -693,6 +946,18 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "slab" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" + +[[package]] +name = "smallvec" +version = "1.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" + [[package]] name = "strsim" version = "0.11.1" @@ -723,13 +988,33 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "thiserror" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" +dependencies = [ + "thiserror-impl 1.0.69", +] + [[package]] name = "thiserror" version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl", + "thiserror-impl 2.0.18", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" +dependencies = [ + "proc-macro2", + "quote", + "syn", ] [[package]] @@ -764,6 +1049,12 @@ dependencies = [ "syn", ] +[[package]] +name = "typed-arena" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6af6ae20167a9ece4bcb41af5b80f8a1f1df981f6391189ce00fd257af04126a" + [[package]] name = "unicode-ident" version = "1.0.24" @@ -788,6 +1079,16 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" +[[package]] +name = "uuid" +version = "1.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddd74a9687298c6858e9b88ec8935ec45d22e8fd5e6394fa1bd4e99a87789c76" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + [[package]] name = "wasip2" version = "1.0.1+wasi-0.2.4" @@ -806,6 +1107,51 @@ dependencies = [ "wit-bindgen 0.51.0", ] +[[package]] +name = "wasm-bindgen" +version = "0.2.121" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "49ace1d07c165b0864824eee619580c4689389afa9dc9ed3a4c75040d82e6790" +dependencies = [ + "cfg-if", + "once_cell", + "rustversion", + "wasm-bindgen-macro", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-macro" +version = "0.2.121" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e68e6f4afd367a562002c05637acb8578ff2dea1943df76afb9e83d177c8578" +dependencies = [ + "quote", + "wasm-bindgen-macro-support", +] + +[[package]] +name = "wasm-bindgen-macro-support" +version = "0.2.121" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d95a9ec35c64b2a7cb35d3fead40c4238d0940c86d107136999567a4703259f2" +dependencies = [ + "bumpalo", + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-shared" +version = "0.2.121" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4e0100b01e9f0d03189a92b96772a1fb998639d981193d7dbab487302513441" +dependencies = [ + "unicode-ident", +] + [[package]] name = "wasm-encoder" version = "0.244.0" diff --git a/charts/lldap-operator/crds/lldap-user.yaml b/charts/lldap-operator/crds/lldap-user.yaml index abcd475..fb1d72a 100644 --- a/charts/lldap-operator/crds/lldap-user.yaml +++ b/charts/lldap-operator/crds/lldap-user.yaml @@ -144,7 +144,7 @@ spec: - message: metadata.labels must include 'lldap-operator.lukidoescode.com/lldap-instance' with a non-empty value reason: FieldValueRequired rule: has(self.metadata.labels) && 'lldap-operator.lukidoescode.com/lldap-instance' in self.metadata.labels && size(self.metadata.labels['lldap-operator.lukidoescode.com/lldap-instance']) > 0 - - message: passwordSecretRef is required unless passwordPolicy is Ignore + - message: spec.passwordSecretRef is required when spec.passwordPolicy is 'Manage' or 'InitialOnly' (only 'Ignore' permits omitting it) reason: FieldValueRequired rule: self.spec.passwordPolicy == 'Ignore' || has(self.spec.passwordSecretRef) served: true diff --git a/crates/lldap-operator-crds/Cargo.toml b/crates/lldap-operator-crds/Cargo.toml index c823b79..952d9cc 100644 --- a/crates/lldap-operator-crds/Cargo.toml +++ b/crates/lldap-operator-crds/Cargo.toml @@ -26,4 +26,5 @@ serde_yaml_ng.workspace = true thiserror.workspace = true [dev-dependencies] +cel-interpreter = "0.10.0" tempfile = "3.27.0" diff --git a/crates/lldap-operator-crds/src/cel_test_support.rs b/crates/lldap-operator-crds/src/cel_test_support.rs new file mode 100644 index 0000000..dada4cd --- /dev/null +++ b/crates/lldap-operator-crds/src/cel_test_support.rs @@ -0,0 +1,56 @@ +use std::collections::HashMap; +use std::sync::Arc; + +use cel_interpreter::objects::{Map, Value}; +use cel_interpreter::{Context, Program}; + +pub(crate) fn eval_rule(rule: &str, self_json: &serde_json::Value) -> bool { + let program = Program::compile(rule).expect("CEL expression compiles"); + let mut context = Context::default(); + context.add_variable_from_value("self", json_to_cel(self_json)); + match program.execute(&context).expect("CEL program executes") { + Value::Bool(b) => b, + other => panic!("expected CEL rule to evaluate to Bool, got {other:?}"), + } +} + +pub(crate) fn rule_text_by_message<'a>(crd_json: &'a serde_json::Value, message: &str) -> &'a str { + let rules = + crd_json["spec"]["versions"][0]["schema"]["openAPIV3Schema"]["x-kubernetes-validations"] + .as_array() + .expect("x-kubernetes-validations is an array"); + rules + .iter() + .find(|r| r["message"].as_str() == Some(message)) + .and_then(|r| r["rule"].as_str()) + .unwrap_or_else(|| panic!("no CEL rule found with message {message:?}")) +} + +fn json_to_cel(value: &serde_json::Value) -> Value { + match value { + serde_json::Value::Null => Value::Null, + serde_json::Value::Bool(b) => Value::Bool(*b), + serde_json::Value::Number(n) => { + if let Some(i) = n.as_i64() { + Value::Int(i) + } else if let Some(u) = n.as_u64() { + Value::UInt(u) + } else if let Some(f) = n.as_f64() { + Value::Float(f) + } else { + Value::Null + } + } + serde_json::Value::String(s) => Value::String(Arc::new(s.clone())), + serde_json::Value::Array(arr) => { + Value::List(Arc::new(arr.iter().map(json_to_cel).collect())) + } + serde_json::Value::Object(obj) => { + let mut map: HashMap = HashMap::new(); + for (k, v) in obj { + map.insert(k.clone().into(), json_to_cel(v)); + } + Value::Map(Map { map: Arc::new(map) }) + } + } +} diff --git a/crates/lldap-operator-crds/src/lib.rs b/crates/lldap-operator-crds/src/lib.rs index 3eee488..749dc3e 100644 --- a/crates/lldap-operator-crds/src/lib.rs +++ b/crates/lldap-operator-crds/src/lib.rs @@ -1,4 +1,6 @@ mod attribute_schema; +#[cfg(test)] +mod cel_test_support; mod common; mod group; mod membership; diff --git a/crates/lldap-operator-crds/src/membership.rs b/crates/lldap-operator-crds/src/membership.rs index f89433d..4b6df09 100644 --- a/crates/lldap-operator-crds/src/membership.rs +++ b/crates/lldap-operator-crds/src/membership.rs @@ -124,4 +124,48 @@ mod tests { assert!(status.observed_generation.is_none()); assert!(status.conditions.is_none()); } + + const LABEL_MSG: &str = "metadata.labels must include 'lldap-operator.lukidoescode.com/lldap-instance' with a non-empty value"; + + fn membership_cr(labels: &serde_json::Value) -> serde_json::Value { + serde_json::json!({ + "apiVersion": "lldap-operator.lukidoescode.com/v1alpha1", + "kind": "LldapMembership", + "metadata": { "labels": labels }, + "spec": { "userName": "alice", "groupName": "Engineering" }, + }) + } + + #[test] + fn cel_instance_label_rule_passes_with_label() { + let crd_json = serde_json::to_value(LldapMembership::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, LABEL_MSG); + let cr = membership_cr( + &serde_json::json!({ "lldap-operator.lukidoescode.com/lldap-instance": "primary" }), + ); + assert!(crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_instance_label_rule_fails_without_labels_map() { + let crd_json = serde_json::to_value(LldapMembership::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, LABEL_MSG); + let cr = serde_json::json!({ + "apiVersion": "lldap-operator.lukidoescode.com/v1alpha1", + "kind": "LldapMembership", + "metadata": {}, + "spec": { "userName": "alice", "groupName": "Engineering" }, + }); + assert!(!crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_instance_label_rule_fails_with_empty_label_value() { + let crd_json = serde_json::to_value(LldapMembership::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, LABEL_MSG); + let cr = membership_cr( + &serde_json::json!({ "lldap-operator.lukidoescode.com/lldap-instance": "" }), + ); + assert!(!crate::cel_test_support::eval_rule(rule, &cr)); + } } diff --git a/crates/lldap-operator-crds/src/user.rs b/crates/lldap-operator-crds/src/user.rs index 4407073..1a91f17 100644 --- a/crates/lldap-operator-crds/src/user.rs +++ b/crates/lldap-operator-crds/src/user.rs @@ -32,7 +32,7 @@ pub enum PasswordPolicy { ).message("metadata.labels must include 'lldap-operator.lukidoescode.com/lldap-instance' with a non-empty value").reason(Reason::FieldValueRequired), validation = Rule::new( "self.spec.passwordPolicy == 'Ignore' || has(self.spec.passwordSecretRef)" - ).message("passwordSecretRef is required unless passwordPolicy is Ignore").reason(Reason::FieldValueRequired), + ).message("spec.passwordSecretRef is required when spec.passwordPolicy is 'Manage' or 'InitialOnly' (only 'Ignore' permits omitting it)").reason(Reason::FieldValueRequired), )] #[serde(rename_all = "camelCase")] pub struct LldapUserSpec { @@ -275,7 +275,7 @@ mod tests { #[test] fn crd_includes_password_secret_ref_validation() { let yaml = serde_yaml_ng::to_string(&LldapUser::crd()).expect("serialize"); - assert!(yaml.contains("passwordSecretRef is required unless passwordPolicy is Ignore")); + assert!(yaml.contains("spec.passwordSecretRef is required when spec.passwordPolicy is 'Manage' or 'InitialOnly' (only 'Ignore' permits omitting it)")); assert!(yaml.contains("self.spec.passwordPolicy == 'Ignore'")); } @@ -289,7 +289,7 @@ mod tests { .iter() .find(|r| { r["message"].as_str() - == Some("passwordSecretRef is required unless passwordPolicy is Ignore") + == Some("spec.passwordSecretRef is required when spec.passwordPolicy is 'Manage' or 'InitialOnly' (only 'Ignore' permits omitting it)") }) .expect("password secret ref CEL rule present"); assert_eq!( @@ -306,4 +306,147 @@ mod tests { assert!(status.password_hash.is_none()); assert!(status.conditions.is_none()); } + + const PASSWORD_MSG: &str = "spec.passwordSecretRef is required when spec.passwordPolicy is 'Manage' or 'InitialOnly' (only 'Ignore' permits omitting it)"; + const LABEL_MSG: &str = "metadata.labels must include 'lldap-operator.lukidoescode.com/lldap-instance' with a non-empty value"; + + fn user_cr(spec_extra: serde_json::Value, labels: &serde_json::Value) -> serde_json::Value { + let mut spec = serde_json::json!({ + "username": "alice", + "email": "alice@example.com", + }); + if let serde_json::Value::Object(extra) = spec_extra { + let spec_obj = spec.as_object_mut().expect("spec is object"); + for (k, v) in extra { + spec_obj.insert(k, v); + } + } + serde_json::json!({ + "apiVersion": "lldap-operator.lukidoescode.com/v1alpha1", + "kind": "LldapUser", + "metadata": { "labels": labels }, + "spec": spec, + }) + } + + #[test] + fn cel_password_secret_rule_passes_for_ignore_without_secret() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, PASSWORD_MSG); + let cr = user_cr( + serde_json::json!({ "passwordPolicy": "Ignore" }), + &serde_json::json!({}), + ); + assert!(crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_password_secret_rule_passes_for_ignore_with_secret() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, PASSWORD_MSG); + let cr = user_cr( + serde_json::json!({ + "passwordPolicy": "Ignore", + "passwordSecretRef": { "name": "alice-pw", "key": "password" }, + }), + &serde_json::json!({}), + ); + assert!(crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_password_secret_rule_fails_for_manage_without_secret() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, PASSWORD_MSG); + let cr = user_cr( + serde_json::json!({ "passwordPolicy": "Manage" }), + &serde_json::json!({}), + ); + assert!(!crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_password_secret_rule_passes_for_manage_with_secret() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, PASSWORD_MSG); + let cr = user_cr( + serde_json::json!({ + "passwordPolicy": "Manage", + "passwordSecretRef": { "name": "alice-pw", "key": "password" }, + }), + &serde_json::json!({}), + ); + assert!(crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_password_secret_rule_fails_for_initial_only_without_secret() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, PASSWORD_MSG); + let cr = user_cr( + serde_json::json!({ "passwordPolicy": "InitialOnly" }), + &serde_json::json!({}), + ); + assert!(!crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_password_secret_rule_passes_for_initial_only_with_secret() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, PASSWORD_MSG); + let cr = user_cr( + serde_json::json!({ + "passwordPolicy": "InitialOnly", + "passwordSecretRef": { "name": "alice-pw", "key": "password" }, + }), + &serde_json::json!({}), + ); + assert!(crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_instance_label_rule_passes_with_label() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, LABEL_MSG); + let cr = user_cr( + serde_json::json!({ "passwordPolicy": "Ignore" }), + &serde_json::json!({ "lldap-operator.lukidoescode.com/lldap-instance": "primary" }), + ); + assert!(crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_instance_label_rule_fails_without_labels_map() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, LABEL_MSG); + let cr = serde_json::json!({ + "apiVersion": "lldap-operator.lukidoescode.com/v1alpha1", + "kind": "LldapUser", + "metadata": {}, + "spec": { "username": "alice", "email": "alice@example.com", "passwordPolicy": "Ignore" }, + }); + assert!(!crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_instance_label_rule_fails_with_empty_label_value() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, LABEL_MSG); + let cr = user_cr( + serde_json::json!({ "passwordPolicy": "Ignore" }), + &serde_json::json!({ "lldap-operator.lukidoescode.com/lldap-instance": "" }), + ); + assert!(!crate::cel_test_support::eval_rule(rule, &cr)); + } + + #[test] + fn cel_instance_label_rule_fails_when_label_absent() { + let crd_json = serde_json::to_value(LldapUser::crd()).expect("serialize CRD"); + let rule = crate::cel_test_support::rule_text_by_message(&crd_json, LABEL_MSG); + let cr = user_cr( + serde_json::json!({ "passwordPolicy": "Ignore" }), + &serde_json::json!({ "other-label": "value" }), + ); + assert!(!crate::cel_test_support::eval_rule(rule, &cr)); + } } diff --git a/crates/lldap-operator-reconciler/Cargo.toml b/crates/lldap-operator-reconciler/Cargo.toml index d1521ba..8eb3633 100644 --- a/crates/lldap-operator-reconciler/Cargo.toml +++ b/crates/lldap-operator-reconciler/Cargo.toml @@ -13,3 +13,4 @@ description = "Reconciliation business logic for the lldap resources operator" workspace = true [dependencies] +thiserror.workspace = true diff --git a/crates/lldap-operator-reconciler/src/lib.rs b/crates/lldap-operator-reconciler/src/lib.rs index 8b13789..a4ad9df 100644 --- a/crates/lldap-operator-reconciler/src/lib.rs +++ b/crates/lldap-operator-reconciler/src/lib.rs @@ -1 +1,6 @@ +mod validation; +pub use validation::{ + GroupCandidate, MembershipReference, MembershipValidationError, UserCandidate, + validate_membership_references, +}; diff --git a/crates/lldap-operator-reconciler/src/validation.rs b/crates/lldap-operator-reconciler/src/validation.rs new file mode 100644 index 0000000..9ac2c87 --- /dev/null +++ b/crates/lldap-operator-reconciler/src/validation.rs @@ -0,0 +1,233 @@ +use thiserror::Error; + +#[derive(Debug, Error, PartialEq, Eq)] +pub enum MembershipValidationError { + #[error( + "referenced user '{user_name}' not found in namespace '{namespace}' for instance '{instance}'" + )] + UserNotFound { + user_name: String, + namespace: String, + instance: String, + }, + #[error( + "referenced group '{group_name}' not found in namespace '{namespace}' for instance '{instance}'" + )] + GroupNotFound { + group_name: String, + namespace: String, + instance: String, + }, +} + +pub struct MembershipReference<'a> { + pub namespace: &'a str, + pub instance: &'a str, + pub user_name: &'a str, + pub group_name: &'a str, +} + +pub struct UserCandidate<'a> { + pub namespace: &'a str, + pub instance: &'a str, + pub username: &'a str, +} + +pub struct GroupCandidate<'a> { + pub namespace: &'a str, + pub instance: &'a str, + pub display_name: &'a str, +} + +/// Checks that the membership's referenced user and group exist among the +/// candidates within the same namespace and instance. +/// +/// # Errors +/// +/// Returns [`MembershipValidationError::UserNotFound`] or +/// [`MembershipValidationError::GroupNotFound`] when no candidate matches. +pub fn validate_membership_references<'a, U, G>( + membership: &MembershipReference<'_>, + candidate_users: U, + candidate_groups: G, +) -> Result<(), MembershipValidationError> +where + U: IntoIterator>, + G: IntoIterator>, +{ + let user_found = candidate_users.into_iter().any(|user| { + user.namespace == membership.namespace + && user.instance == membership.instance + && user.username == membership.user_name + }); + if !user_found { + return Err(MembershipValidationError::UserNotFound { + user_name: membership.user_name.to_owned(), + namespace: membership.namespace.to_owned(), + instance: membership.instance.to_owned(), + }); + } + + let group_found = candidate_groups.into_iter().any(|group| { + group.namespace == membership.namespace + && group.instance == membership.instance + && group.display_name == membership.group_name + }); + if !group_found { + return Err(MembershipValidationError::GroupNotFound { + group_name: membership.group_name.to_owned(), + namespace: membership.namespace.to_owned(), + instance: membership.instance.to_owned(), + }); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn user<'a>(namespace: &'a str, instance: &'a str, username: &'a str) -> UserCandidate<'a> { + UserCandidate { + namespace, + instance, + username, + } + } + + fn group<'a>( + namespace: &'a str, + instance: &'a str, + display_name: &'a str, + ) -> GroupCandidate<'a> { + GroupCandidate { + namespace, + instance, + display_name, + } + } + + fn membership<'a>( + namespace: &'a str, + instance: &'a str, + user_name: &'a str, + group_name: &'a str, + ) -> MembershipReference<'a> { + MembershipReference { + namespace, + instance, + user_name, + group_name, + } + } + + #[test] + fn validate_membership_succeeds_when_user_and_group_match() { + let users = [user("ns", "primary", "alice")]; + let groups = [group("ns", "primary", "Engineering")]; + let m = membership("ns", "primary", "alice", "Engineering"); + assert_eq!(validate_membership_references(&m, &users, &groups), Ok(())); + } + + #[test] + fn validate_membership_fails_when_user_missing() { + let users: [UserCandidate<'_>; 0] = []; + let groups = [group("ns", "primary", "Engineering")]; + let m = membership("ns", "primary", "alice", "Engineering"); + assert_eq!( + validate_membership_references(&m, &users, &groups), + Err(MembershipValidationError::UserNotFound { + user_name: "alice".to_owned(), + namespace: "ns".to_owned(), + instance: "primary".to_owned(), + }) + ); + } + + #[test] + fn validate_membership_fails_when_group_missing() { + let users = [user("ns", "primary", "alice")]; + let groups: [GroupCandidate<'_>; 0] = []; + let m = membership("ns", "primary", "alice", "Engineering"); + assert_eq!( + validate_membership_references(&m, &users, &groups), + Err(MembershipValidationError::GroupNotFound { + group_name: "Engineering".to_owned(), + namespace: "ns".to_owned(), + instance: "primary".to_owned(), + }) + ); + } + + #[test] + fn validate_membership_fails_when_user_in_different_namespace() { + let users = [user("other-ns", "primary", "alice")]; + let groups = [group("ns", "primary", "Engineering")]; + let m = membership("ns", "primary", "alice", "Engineering"); + assert_eq!( + validate_membership_references(&m, &users, &groups), + Err(MembershipValidationError::UserNotFound { + user_name: "alice".to_owned(), + namespace: "ns".to_owned(), + instance: "primary".to_owned(), + }) + ); + } + + #[test] + fn validate_membership_fails_when_user_in_different_instance() { + let users = [user("ns", "secondary", "alice")]; + let groups = [group("ns", "primary", "Engineering")]; + let m = membership("ns", "primary", "alice", "Engineering"); + assert_eq!( + validate_membership_references(&m, &users, &groups), + Err(MembershipValidationError::UserNotFound { + user_name: "alice".to_owned(), + namespace: "ns".to_owned(), + instance: "primary".to_owned(), + }) + ); + } + + #[test] + fn validate_membership_fails_when_group_in_different_namespace() { + let users = [user("ns", "primary", "alice")]; + let groups = [group("other-ns", "primary", "Engineering")]; + let m = membership("ns", "primary", "alice", "Engineering"); + assert_eq!( + validate_membership_references(&m, &users, &groups), + Err(MembershipValidationError::GroupNotFound { + group_name: "Engineering".to_owned(), + namespace: "ns".to_owned(), + instance: "primary".to_owned(), + }) + ); + } + + #[test] + fn validate_membership_fails_when_group_in_different_instance() { + let users = [user("ns", "primary", "alice")]; + let groups = [group("ns", "secondary", "Engineering")]; + let m = membership("ns", "primary", "alice", "Engineering"); + assert_eq!( + validate_membership_references(&m, &users, &groups), + Err(MembershipValidationError::GroupNotFound { + group_name: "Engineering".to_owned(), + namespace: "ns".to_owned(), + instance: "primary".to_owned(), + }) + ); + } + + #[test] + fn validate_membership_accepts_iterator_inputs() { + let users = [user("ns", "primary", "alice")]; + let groups = [group("ns", "primary", "Engineering")]; + let m = membership("ns", "primary", "alice", "Engineering"); + assert_eq!( + validate_membership_references(&m, users.iter(), groups.iter()), + Ok(()) + ); + } +} diff --git a/docs/src/architecture.md b/docs/src/architecture.md index cd98ee1..af05c1c 100644 --- a/docs/src/architecture.md +++ b/docs/src/architecture.md @@ -1,18 +1,5 @@ # Architecture -This chapter is for contributors who want to understand how the operator is -structured internally. - -## Workspace Structure - -The operator is organized as a Cargo workspace with focused crates: - -| Crate | Purpose | -|-------|---------| -| `lldap-operator` | Operator binary (entry point, CLI, controller setup) | -| `lldap-operator-crds` | CRD type definitions (`CustomResource` derives) | -| `lldap-operator-traits` | Trait definitions for lldap operations | -| `lldap-operator-client` | GraphQL client for the lldap API | -| `lldap-operator-reconciler` | Reconciliation business logic | - - +This chapter will describe the architecture of how the LLDAP +operator fits into the eco system of a Kubernetes deployment +and how to employ it with Kyverno and OPA/Gatekeeper. diff --git a/docs/src/crds/lldap-membership.md b/docs/src/crds/lldap-membership.md index 673b094..651a3a1 100644 --- a/docs/src/crds/lldap-membership.md +++ b/docs/src/crds/lldap-membership.md @@ -22,6 +22,39 @@ The `userName` and `groupName` fields reference the lldap-side identifiers | `userName` | string | yes | lldap username (1–64 chars) | | `groupName` | string | yes | lldap group display name (1–128 chars) | +## Validation Rules + +### Admission-time validation (CEL) + +- **Instance label required** — rejected with: + + `metadata.labels must include 'lldap-operator.lukidoescode.com/lldap-instance' with a non-empty value` + + The label must be set to a non-empty string identifying which lldap + instance the membership belongs to. + +### Reconcile-time validation (reference checks) + +The reconciler verifies cross-resource references before applying a +membership: + +- The referenced `LldapUser` (matched by `spec.username` equal to this + resource's `spec.userName`) must exist in the same namespace as the + membership. +- The referenced `LldapGroup` (matched by `spec.displayName` equal to this + resource's `spec.groupName`) must exist in the same namespace as the + membership. +- All three resources (user, group, membership) must carry the same + `lldap-operator.lukidoescode.com/lldap-instance` label value. + +On failure the reconciler returns a hard error and the membership's +`Ready` condition is set to `False` with one of these reasons: +`UserNotFound`, `GroupNotFound`, `MissingNamespace`, `MissingInstanceLabel`. + +Note: the reconciler is not yet wired up. The validation function +(`validate_membership_references`) is implemented in `lldap-operator-reconciler` +and will be invoked once the reconciler is built. + ## Status | Field | Type | Description | diff --git a/docs/src/crds/lldap-user.md b/docs/src/crds/lldap-user.md index 30749c6..0d8eb8f 100644 --- a/docs/src/crds/lldap-user.md +++ b/docs/src/crds/lldap-user.md @@ -27,6 +27,26 @@ ignores resources without this label. | `passwordSecretRef.name` | string | yes (when set) | Name of the Secret | | `passwordSecretRef.key` | string | yes (when set) | Key within the Secret containing the password | +## Validation Rules + +The following rules are enforced at admission time by the Kubernetes API +server via CRD-embedded CEL expressions (requires Kubernetes 1.25+). + +- **Instance label required** — rejected with: + + `metadata.labels must include 'lldap-operator.lukidoescode.com/lldap-instance' with a non-empty value` + + The label must be set to a non-empty string identifying which lldap + instance owns this user. + +- **Password secret required for managed policies** — rejected with: + + `spec.passwordSecretRef is required when spec.passwordPolicy is 'Manage' or 'InitialOnly' (only 'Ignore' permits omitting it)` + + When `passwordPolicy` is `Manage` (the default) or `InitialOnly`, the + operator needs a Secret reference to source the password from. Only + `Ignore` permits omitting `passwordSecretRef`. + Group memberships are not declared on `LldapUser`. Use `LldapMembership` to attach users to groups.