From d01219d7ec92322cdd001f0962427cc77ea1437f Mon Sep 17 00:00:00 2001 From: Indy Jones Date: Sat, 23 May 2026 09:09:26 +0200 Subject: [PATCH 1/2] test(common): add proptest fuzz harness for JSON+proto round-trips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port a small-scale version of upstream apimachinery's `pkg/api/apitesting/roundtrip/roundtrip.go` (the randfill-driven codec roundtrip harness) to rusternetes. For each of six representative resources — Pod, Deployment, Service, ConfigMap, Secret, Event — generate 50 random instances per case and verify: 1. `decode(encode(decode(x)))` is stable as `serde_json::Value`. 2. The `k8s\0`-prefixed protobuf `Unknown` envelope round-trips and recovers the original `apiVersion` / `kind` plus the wrapped object. Strategies cap collection sizes at 2 and use a deterministic `make_object_meta` helper so values stay reproducible across runs. The Event strategy uses whole-second `metav1.Time` and exact- microsecond `metav1.MicroTime` values to dodge sub-microsecond truncation; that limitation is documented in the file header. Adds `proptest = "1"` to `crates/common` dev-dependencies. All 14 tests run in <1s on a developer laptop, well under the 30s budget. (cherry picked from commit 0d8f773128b7c21043b7c0f81da7c017e3315bf0) --- Cargo.lock | 62 ++ crates/common/Cargo.toml | 1 + .../tests/fuzz_roundtrip_jsonproto_test.rs | 685 ++++++++++++++++++ 3 files changed, 748 insertions(+) create mode 100644 crates/common/tests/fuzz_roundtrip_jsonproto_test.rs diff --git a/Cargo.lock b/Cargo.lock index 448bf7f2..0aa2b562 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3093,6 +3093,25 @@ dependencies = [ "url", ] +[[package]] +name = "proptest" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b45fcc2344c680f5025fe57779faef368840d0bd1f42f216291f0dc4ace4744" +dependencies = [ + "bit-set", + "bit-vec", + "bitflags", + "num-traits", + "rand 0.9.4", + "rand_chacha 0.9.0", + "rand_xorshift", + "regex-syntax", + "rusty-fork", + "tempfile", + "unarray", +] + [[package]] name = "prost" version = "0.13.5" @@ -3188,6 +3207,12 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "quick-error" +version = "1.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" + [[package]] name = "quinn" version = "0.11.9" @@ -3323,6 +3348,15 @@ dependencies = [ "getrandom 0.3.4", ] +[[package]] +name = "rand_xorshift" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "513962919efc330f829edb2535844d1b912b0fbe2ca165d613e4e8788bb05a5a" +dependencies = [ + "rand_core 0.9.5", +] + [[package]] name = "rayon" version = "1.12.0" @@ -3704,6 +3738,7 @@ dependencies = [ "opentelemetry-stdout", "opentelemetry_sdk 0.22.1", "prometheus", + "proptest", "prost 0.13.5", "prost-types", "rcgen", @@ -3966,6 +4001,18 @@ version = "1.0.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" +[[package]] +name = "rusty-fork" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc6bf79ff24e648f6da1f8d1f011e9cac26491b619e6b9280f2b47f1774e6ee2" +dependencies = [ + "fnv", + "quick-error", + "tempfile", + "wait-timeout", +] + [[package]] name = "ryu" version = "1.0.23" @@ -5224,6 +5271,12 @@ version = "1.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40ce102ab67701b8526c123c1bab5cbe42d7040ccfd0f64af1a385808d2f43de" +[[package]] +name = "unarray" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94" + [[package]] name = "unicase" version = "2.9.0" @@ -5364,6 +5417,15 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5c3082ca00d5a5ef149bb8b555a72ae84c9c59f7250f013ac822ac2e49b19c64" +[[package]] +name = "wait-timeout" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ac3b126d3914f9849036f826e054cbabdc8519970b8998ddaf3b5bd3c65f11" +dependencies = [ + "libc", +] + [[package]] name = "walkdir" version = "2.5.0" diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index 890da9ed..79b7f739 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -50,3 +50,4 @@ tracing-full = ["opentelemetry", "opentelemetry_sdk", "opentelemetry-stdout", "t [dev-dependencies] tempfile = "3.27.0" +proptest = "1" diff --git a/crates/common/tests/fuzz_roundtrip_jsonproto_test.rs b/crates/common/tests/fuzz_roundtrip_jsonproto_test.rs new file mode 100644 index 00000000..9a1c08fd --- /dev/null +++ b/crates/common/tests/fuzz_roundtrip_jsonproto_test.rs @@ -0,0 +1,685 @@ +//! Randomized JSON <-> protobuf round-trip harness. +//! +//! This is a small-scale port of upstream Kubernetes' +//! `staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go`, +//! which uses `sigs.k8s.io/randfill` to generate random instances of every +//! registered resource and then checks that: +//! +//! 1. `decode(encode(decode(x))) == decode(x)` as serde-projected JSON +//! (semantic equality — handles field ordering, default fields, etc.). +//! 2. The protobuf-Unknown-wrapped form survives a round-trip: encode to the +//! `k8s\x00` magic-prefixed `Unknown { apiVersion, kind, raw }` envelope, +//! decode it back, and verify the inner object matches the original. +//! +//! For each of six representative resources we run [`ProptestConfig::with_cases`] +//! at 50 iterations, with collection sizes capped at 3 elements to keep the +//! whole test target well under 30s on a developer laptop. +//! +//! ## Known limitations / fields we deliberately don't fuzz +//! +//! - **`metav1.Time` with sub-second precision.** The core/v1 path uses K8s' +//! no-fractional-seconds RFC3339 wire form, which truncates anything below +//! the second boundary. We only generate whole-second timestamps for any +//! field that flows through that codec. +//! - **`metav1.MicroTime`** preserves microseconds but not nanoseconds, so +//! strategies are restricted to `chrono` timestamps with zero nanos beyond +//! the microsecond boundary (`with_nanosecond(micros * 1000)`). +//! - **Generated names / random UIDs.** `ObjectMeta::new()` calls `Uuid::new_v4()` +//! and `Utc::now()` so we hand-build `ObjectMeta` literals rather than calling +//! the builders — otherwise the value would differ on every regenerate. +//! - **`Secret::stringData`.** Round-trips structurally but K8s normally +//! normalises it into `data` server-side; we don't exercise that path here. +//! - **Float / quantity edge cases.** Quantities are strings on the wire; we +//! keep them as canonical strings ("100m", "128Mi") rather than fuzzing the +//! parser, which already has dedicated coverage in `quantity_*_test.rs`. +//! +//! Reference: upstream `pkg/api/apitesting/roundtrip/roundtrip.go`, +//! `pkg/api/apitesting/fuzzer/`. Existing manual fixtures live in +//! `crates/common/tests/roundtrip_core_v1.rs`. + +use std::collections::HashMap; + +use proptest::collection::vec; +use proptest::option; +use proptest::prelude::*; + +use rusternetes_common::protobuf::{decode_protobuf, encode_protobuf}; +use rusternetes_common::resources::deployment::{DeploymentStrategy, RollingUpdateDeployment}; +use rusternetes_common::resources::{ + ConfigMap, Container, Deployment, DeploymentSpec, EnvVar, Event, EventSource, EventType, + ObjectReference, Pod, PodSpec, PodTemplateSpec, Secret, Service, ServiceExternalTrafficPolicy, + ServiceInternalTrafficPolicy, ServicePort, ServiceSpec, ServiceType, +}; +use rusternetes_common::types::{ + LabelSelector, LabelSelectorRequirement, ObjectMeta, ResourceRequirements, TypeMeta, +}; + +// ----------------------------------------------------------------------------- +// Common strategies +// ----------------------------------------------------------------------------- + +/// Generate a Kubernetes-valid DNS-1123 label (lowercase letters, digits, '-'). +fn k8s_name() -> impl Strategy { + // Note: regex strategy is greedy in proptest; cap at ~30 chars to keep + // generated payloads compact. Must start with a letter. + prop::string::string_regex("[a-z][a-z0-9-]{0,30}").unwrap() +} + +/// Generate a Kubernetes label / annotation value (relaxed printable ASCII, +/// no embedded quotes/newlines so serde_json::Value comparisons are stable). +fn k8s_label_value() -> impl Strategy { + prop::string::string_regex("[a-zA-Z0-9._-]{0,30}").unwrap() +} + +/// Generate a small `HashMap` (<=3 entries) for labels / data fields. +fn small_string_map() -> impl Strategy> { + prop::collection::hash_map(k8s_name(), k8s_label_value(), 0..3) +} + +/// Generate a non-empty small `HashMap` (1..=3 entries). +fn nonempty_string_map() -> impl Strategy> { + prop::collection::hash_map(k8s_name(), k8s_label_value(), 1..3) +} + +/// Build a deterministic `ObjectMeta` from a name + optional labels. +/// Avoids `ObjectMeta::new()` which calls `Uuid::new_v4()` + `Utc::now()`. +fn make_object_meta( + name: String, + namespace: Option, + labels: Option>, +) -> ObjectMeta { + ObjectMeta { + name, + generate_name: None, + namespace, + uid: String::new(), + generation: None, + resource_version: None, + managed_fields: None, + creation_timestamp: None, + deletion_timestamp: None, + deletion_grace_period_seconds: None, + labels, + annotations: None, + finalizers: None, + owner_references: None, + } +} + +/// Strategy for `ObjectMeta` with namespace. +fn object_meta_namespaced(kind: &'static str) -> impl Strategy { + // kind is unused but kept for readability of call sites. + let _ = kind; + ( + k8s_name(), + option::of(k8s_name()), + option::of(small_string_map()), + ) + .prop_map(|(name, ns, labels)| { + make_object_meta(name, ns.or(Some("default".into())), labels) + }) +} + +/// Round-trip assertion: serde JSON encode -> decode -> re-encode -> compare +/// via `serde_json::Value` (so default fields / ordering don't matter). +fn assert_json_value_roundtrip(value: &T) +where + T: serde::Serialize + serde::de::DeserializeOwned, +{ + let v1 = serde_json::to_value(value).expect("initial encode to Value"); + let decoded: T = serde_json::from_value(v1.clone()).expect("decode-from-Value (first round)"); + let v2 = serde_json::to_value(&decoded).expect("re-encode to Value"); + assert_eq!( + v1, v2, + "serde JSON projection drifted on re-encode:\nfirst: {v1}\nsecond: {v2}" + ); + + // Also check string-form decode->re-encode (closer to the wire path). + let s = serde_json::to_string(value).expect("encode to string"); + let decoded2: T = serde_json::from_str(&s).expect("decode from string"); + let v3 = serde_json::to_value(&decoded2).expect("re-encode to Value (round 2)"); + assert_eq!(v1, v3, "string-mode JSON round-trip drifted"); +} + +/// Round-trip assertion through the Unknown protobuf envelope. +fn assert_proto_unknown_roundtrip(value: &T, api_version: &str, kind: &str) +where + T: serde::Serialize + serde::de::DeserializeOwned, +{ + let bytes = encode_protobuf(value, api_version, kind).expect("encode_protobuf"); + let (decoded, tm) = decode_protobuf::(&bytes).expect("decode_protobuf"); + assert_eq!(tm.api_version, api_version); + assert_eq!(tm.kind, kind); + + // Compare original vs. decoded via serde_json::Value to dodge missing PartialEq. + let lhs = serde_json::to_value(value).expect("lhs to value"); + let rhs = serde_json::to_value(&decoded).expect("rhs to value"); + assert_eq!( + lhs, rhs, + "protobuf-Unknown round-trip drifted:\nlhs: {lhs}\nrhs: {rhs}" + ); +} + +// ----------------------------------------------------------------------------- +// Pod +// ----------------------------------------------------------------------------- + +fn env_var_strategy() -> impl Strategy { + (k8s_name(), option::of(k8s_label_value())).prop_map(|(name, value)| EnvVar { + name, + value, + value_from: None, + }) +} + +fn container_strategy() -> impl Strategy { + ( + k8s_name(), + k8s_name(), + option::of(vec(prop::string::string_regex("[a-z]{1,8}").unwrap(), 0..3)), + option::of(vec(env_var_strategy(), 0..3)), + option::of(prop::sample::select(vec![ + "Always".to_string(), + "IfNotPresent".to_string(), + "Never".to_string(), + ])), + ) + .prop_map(|(name, image, command, env, image_pull_policy)| Container { + name, + image, + command, + args: None, + working_dir: None, + ports: None, + env, + env_from: None, + resources: Some(ResourceRequirements { + limits: None, + requests: None, + claims: None, + }), + volume_mounts: None, + volume_devices: None, + image_pull_policy, + liveness_probe: None, + readiness_probe: None, + startup_probe: None, + security_context: None, + restart_policy: None, + resize_policy: None, + lifecycle: None, + termination_message_path: None, + termination_message_policy: None, + stdin: None, + stdin_once: None, + tty: None, + }) +} + +fn pod_strategy() -> impl Strategy { + ( + object_meta_namespaced("Pod"), + vec(container_strategy(), 1..3), + option::of(prop::sample::select(vec![ + "Always".to_string(), + "OnFailure".to_string(), + "Never".to_string(), + ])), + option::of(k8s_name()), + ) + .prop_map(|(metadata, containers, restart_policy, node_name)| { + let mut spec = PodSpec { + containers, + ..Default::default() + }; + spec.restart_policy = restart_policy; + spec.node_name = node_name; + Pod { + type_meta: TypeMeta { + api_version: "v1".into(), + kind: "Pod".into(), + }, + metadata, + spec: Some(spec), + status: None, + } + }) +} + +// ----------------------------------------------------------------------------- +// Deployment +// ----------------------------------------------------------------------------- + +fn label_selector_strategy() -> impl Strategy { + ( + option::of(nonempty_string_map()), + option::of(vec( + ( + k8s_name(), + prop::sample::select(vec![ + "In".to_string(), + "NotIn".to_string(), + "Exists".to_string(), + "DoesNotExist".to_string(), + ]), + option::of(vec(k8s_label_value(), 0..3)), + ) + .prop_map(|(key, operator, values)| LabelSelectorRequirement { + key, + operator, + values, + }), + 0..2, + )), + ) + .prop_map(|(match_labels, match_expressions)| LabelSelector { + match_labels, + match_expressions, + }) +} + +fn deployment_strategy() -> impl Strategy { + ( + object_meta_namespaced("Deployment"), + label_selector_strategy(), + vec(container_strategy(), 1..3), + option::of(0_i32..10), + option::of(prop::sample::select(vec![ + "Recreate".to_string(), + "RollingUpdate".to_string(), + ])), + ) + .prop_map( + |(metadata, selector, containers, replicas, strategy_type)| { + let template = PodTemplateSpec { + metadata: Some(make_object_meta( + "tmpl".into(), + Some("default".into()), + None, + )), + spec: PodSpec { + containers, + ..Default::default() + }, + }; + let strategy = strategy_type.map(|ty| DeploymentStrategy { + strategy_type: ty, + rolling_update: Some(RollingUpdateDeployment { + max_unavailable: None, + max_surge: None, + }), + }); + Deployment { + type_meta: TypeMeta { + api_version: "apps/v1".into(), + kind: "Deployment".into(), + }, + metadata, + spec: DeploymentSpec { + replicas, + selector, + template, + strategy, + min_ready_seconds: None, + revision_history_limit: None, + paused: None, + progress_deadline_seconds: None, + }, + status: None, + } + }, + ) +} + +// ----------------------------------------------------------------------------- +// Service +// ----------------------------------------------------------------------------- + +fn service_port_strategy() -> impl Strategy { + ( + option::of(k8s_name()), + 1_u16..65535, + option::of(prop::sample::select(vec![ + "TCP".to_string(), + "UDP".to_string(), + "SCTP".to_string(), + ])), + ) + .prop_map(|(name, port, protocol)| ServicePort { + name, + port, + target_port: None, + protocol, + node_port: None, + app_protocol: None, + }) +} + +fn service_type_strategy() -> impl Strategy> { + option::of(prop_oneof![ + Just(ServiceType::ClusterIP), + Just(ServiceType::NodePort), + Just(ServiceType::LoadBalancer), + ]) +} + +fn service_strategy() -> impl Strategy { + ( + object_meta_namespaced("Service"), + option::of(small_string_map()), + vec(service_port_strategy(), 1..3), + service_type_strategy(), + option::of(prop_oneof![ + Just(ServiceInternalTrafficPolicy::Cluster), + Just(ServiceInternalTrafficPolicy::Local), + ]), + option::of(prop_oneof![ + Just(ServiceExternalTrafficPolicy::Cluster), + Just(ServiceExternalTrafficPolicy::Local), + ]), + ) + .prop_map( + |( + metadata, + selector, + ports, + service_type, + internal_traffic_policy, + external_traffic_policy, + )| { + Service { + type_meta: TypeMeta { + api_version: "v1".into(), + kind: "Service".into(), + }, + metadata, + spec: ServiceSpec { + selector, + ports, + service_type, + cluster_ip: None, + external_ips: None, + session_affinity: None, + external_name: None, + cluster_ips: None, + ip_families: None, + ip_family_policy: None, + internal_traffic_policy, + external_traffic_policy, + health_check_node_port: None, + load_balancer_class: None, + load_balancer_ip: None, + load_balancer_source_ranges: None, + allocate_load_balancer_node_ports: None, + publish_not_ready_addresses: None, + session_affinity_config: None, + traffic_distribution: None, + }, + status: None, + } + }, + ) +} + +// ----------------------------------------------------------------------------- +// ConfigMap +// ----------------------------------------------------------------------------- + +fn binary_blob_strategy() -> impl Strategy> { + vec(any::(), 0..16) +} + +fn configmap_strategy() -> impl Strategy { + ( + object_meta_namespaced("ConfigMap"), + option::of(small_string_map()), + option::of(prop::collection::hash_map( + k8s_name(), + binary_blob_strategy(), + 0..2, + )), + option::of(any::()), + ) + .prop_map(|(metadata, data, binary_data, immutable)| ConfigMap { + type_meta: TypeMeta { + api_version: "v1".into(), + kind: "ConfigMap".into(), + }, + metadata, + data, + binary_data, + immutable, + }) +} + +// ----------------------------------------------------------------------------- +// Secret +// ----------------------------------------------------------------------------- + +fn secret_strategy() -> impl Strategy { + ( + object_meta_namespaced("Secret"), + option::of(prop::sample::select(vec![ + "Opaque".to_string(), + "kubernetes.io/tls".to_string(), + "kubernetes.io/service-account-token".to_string(), + ])), + option::of(prop::collection::hash_map( + k8s_name(), + binary_blob_strategy(), + 0..2, + )), + option::of(any::()), + ) + .prop_map(|(metadata, secret_type, data, immutable)| Secret { + type_meta: TypeMeta { + api_version: "v1".into(), + kind: "Secret".into(), + }, + metadata, + secret_type, + // Skip stringData fuzzing — it's a write-only field server-side and + // would normalise into `data` on persist, so a structural round-trip + // wouldn't be representative of real client/server traffic. + data, + string_data: None, + immutable, + }) +} + +// ----------------------------------------------------------------------------- +// Event +// ----------------------------------------------------------------------------- + +fn object_reference_strategy() -> impl Strategy { + ( + option::of(k8s_name()), + option::of(k8s_name()), + option::of(k8s_name()), + option::of(k8s_name()), + ) + .prop_map(|(kind, namespace, name, uid)| ObjectReference { + kind, + namespace, + name, + uid, + api_version: Some("v1".into()), + resource_version: None, + field_path: None, + }) +} + +/// Generate a `chrono::DateTime` truncated to whole seconds — the +/// metav1.Time codec used by the core/v1 Event path drops fractional seconds +/// on the wire, so anything finer would visibly drift on round-trip. +fn whole_second_timestamp() -> impl Strategy> { + (1_700_000_000_i64..1_900_000_000).prop_map(|secs| { + chrono::DateTime::::from_timestamp(secs, 0).expect("valid timestamp") + }) +} + +/// Generate a `chrono::DateTime` whose sub-second precision lands +/// exactly on a microsecond boundary — anything finer would drift through +/// MicroTime's `%.6f` formatter (see `micro_time` module in event.rs). +fn microsecond_timestamp() -> impl Strategy> { + (1_700_000_000_i64..1_900_000_000, 0_u32..1_000_000).prop_map(|(secs, micros)| { + chrono::DateTime::::from_timestamp(secs, micros * 1_000) + .expect("valid timestamp") + }) +} + +fn event_strategy() -> impl Strategy { + ( + object_meta_namespaced("Event"), + object_reference_strategy(), + k8s_label_value(), + k8s_label_value(), + prop_oneof![Just(EventType::Normal), Just(EventType::Warning)], + whole_second_timestamp(), + whole_second_timestamp(), + 0_i32..1000, + option::of(microsecond_timestamp()), + option::of(k8s_name()), + ) + .prop_map( + |( + metadata, + involved_object, + reason, + message, + event_type, + first_ts, + last_ts, + count, + event_time, + reporting_component, + )| Event { + api_version: "v1".into(), + kind: "Event".into(), + metadata, + involved_object, + reason, + message, + source: EventSource { + component: "rusternetes".into(), + host: None, + }, + event_type, + first_timestamp: Some(first_ts), + last_timestamp: Some(last_ts), + count, + action: None, + related: None, + series: None, + event_time, + reporting_component, + reporting_instance: None, + note: None, + regarding: None, + // The Event struct has a `#[serde(flatten)] extra` field that + // would round-trip *into* itself if non-empty (any unknown key + // would land here on decode, then re-emit at the top level on + // encode). Setting it to `None` keeps the JSON shape stable. + extra: None, + }, + ) +} + +// ----------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------- + +proptest! { + #![proptest_config(ProptestConfig::with_cases(50))] + + #[test] + fn fuzz_pod_json_roundtrip(pod in pod_strategy()) { + assert_json_value_roundtrip(&pod); + } + + #[test] + fn fuzz_pod_proto_unknown_roundtrip(pod in pod_strategy()) { + assert_proto_unknown_roundtrip(&pod, "v1", "Pod"); + } + + #[test] + fn fuzz_deployment_json_roundtrip(d in deployment_strategy()) { + assert_json_value_roundtrip(&d); + } + + #[test] + fn fuzz_deployment_proto_unknown_roundtrip(d in deployment_strategy()) { + assert_proto_unknown_roundtrip(&d, "apps/v1", "Deployment"); + } + + #[test] + fn fuzz_service_json_roundtrip(s in service_strategy()) { + assert_json_value_roundtrip(&s); + } + + #[test] + fn fuzz_service_proto_unknown_roundtrip(s in service_strategy()) { + assert_proto_unknown_roundtrip(&s, "v1", "Service"); + } + + #[test] + fn fuzz_configmap_json_roundtrip(c in configmap_strategy()) { + assert_json_value_roundtrip(&c); + } + + #[test] + fn fuzz_configmap_proto_unknown_roundtrip(c in configmap_strategy()) { + assert_proto_unknown_roundtrip(&c, "v1", "ConfigMap"); + } + + #[test] + fn fuzz_secret_json_roundtrip(s in secret_strategy()) { + assert_json_value_roundtrip(&s); + } + + #[test] + fn fuzz_secret_proto_unknown_roundtrip(s in secret_strategy()) { + assert_proto_unknown_roundtrip(&s, "v1", "Secret"); + } + + #[test] + fn fuzz_event_json_roundtrip(e in event_strategy()) { + assert_json_value_roundtrip(&e); + } + + #[test] + fn fuzz_event_proto_unknown_roundtrip(e in event_strategy()) { + assert_proto_unknown_roundtrip(&e, "v1", "Event"); + } +} + +// ----------------------------------------------------------------------------- +// Smoke tests for the harness itself. +// +// These are not fuzzed; they exist to give the file a non-property smoke check +// that the strategies build at least one valid instance and that the +// assertions catch a deliberately broken type. +// ----------------------------------------------------------------------------- + +#[test] +fn harness_pod_strategy_produces_valid_instance() { + use proptest::strategy::ValueTree; + let mut runner = proptest::test_runner::TestRunner::default(); + let pod = pod_strategy() + .new_tree(&mut runner) + .expect("strategy generates a value") + .current(); + // Sanity-check the produced Pod by running both round-trips on it. + assert_json_value_roundtrip(&pod); + assert_proto_unknown_roundtrip(&pod, "v1", "Pod"); +} + +#[test] +fn harness_object_meta_is_deterministic() { + // Regression: ObjectMeta::new() uses Uuid::new_v4() + Utc::now() which + // would make fuzzed instances non-reproducible. make_object_meta() is the + // canonical builder for this test file. + let a = make_object_meta("foo".into(), Some("default".into()), None); + let b = make_object_meta("foo".into(), Some("default".into()), None); + assert_eq!(a, b, "make_object_meta must be deterministic"); +} From bbc8c0f749dee15e0bd1ffabc974fbc8196422fb Mon Sep 17 00:00:00 2001 From: Indy Jones Date: Sun, 24 May 2026 14:18:25 +0200 Subject: [PATCH 2/2] fix(proto): nest TypeMeta in common Unknown per upstream wire format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runtime.Unknown` in k8s.io/apimachinery is: message Unknown { optional TypeMeta typeMeta = 1; // nested message optional bytes raw = 2; optional string contentEncoding = 3; optional string contentType = 4; } but the prost `Unknown` in `common::protobuf` flattens TypeMeta in at tags 1+2 (apiVersion, kind) and shifts raw/contentEncoding/contentType to 3-5. Its own encoder and decoder agree, so the round-trip tests pass — but any upstream client (kubectl, client-go) decoding the envelope parses the apiVersion string bytes as a nested TypeMeta message and hits `proto: illegal wireType 6` on byte 'v'=0x76. The api-server serving path dodges this today by hand-rolling the correct nested layout in `middleware.rs::wrap_json_in_protobuf_with_type_meta` instead of using this struct — its own comment notes the prost struct is wrong. That leaves two parallel encoders and a latent landmine: anything that routes serialization through `common::encode_protobuf` emits broken bytes. Align the prost struct with the real wire format (nested `UnknownTypeMeta` at field 1, raw at 2, contentEncoding 3, contentType 4), update encode/decode/extract_type_meta accordingly, and add a wire-shape test that fails fast if field 1 ever stops being an embedded message. Co-Authored-By: Claude Code --- crates/common/src/protobuf.rs | 129 +++++++++++++++++++++++++++------- 1 file changed, 104 insertions(+), 25 deletions(-) diff --git a/crates/common/src/protobuf.rs b/crates/common/src/protobuf.rs index e6bc94cc..05e507f9 100644 --- a/crates/common/src/protobuf.rs +++ b/crates/common/src/protobuf.rs @@ -17,29 +17,49 @@ pub const PROTOBUF_MAGIC: &[u8] = &[0x6b, 0x38, 0x73, 0x00]; pub const CONTENT_TYPE_PROTOBUF: &str = "application/vnd.kubernetes.protobuf"; pub const CONTENT_TYPE_PROTOBUF_STREAM: &str = "application/vnd.kubernetes.protobuf;stream=watch"; -/// Unknown wraps objects with type metadata for protobuf serialization -/// This mirrors k8s.io/apimachinery/pkg/runtime.Unknown +/// TypeMeta as a nested protobuf message. Sits inside `Unknown.typeMeta` +/// at field 1; itself has `apiVersion=1`, `kind=2`. Mirrors +/// `k8s.io/apimachinery/pkg/runtime/generated.proto::TypeMeta`. #[derive(Clone, PartialEq, Message)] -pub struct Unknown { - /// TypeMeta is embedded in line 1 - /// apiVersion field from TypeMeta +pub struct UnknownTypeMeta { #[prost(string, tag = "1")] pub api_version: String, - /// kind field from TypeMeta #[prost(string, tag = "2")] pub kind: String, +} + +/// Unknown wraps objects with type metadata for protobuf serialization. +/// +/// Mirrors `k8s.io/apimachinery/pkg/runtime.Unknown` exactly: +/// +/// ```text +/// message Unknown { +/// optional TypeMeta typeMeta = 1; +/// optional bytes raw = 2; +/// optional string contentEncoding = 3; +/// optional string contentType = 4; +/// } +/// ``` +/// +/// Before 2026-05-24 this struct flattened TypeMeta in at tags 1-2 and +/// shifted the other fields to 3-5. Encoder and our own decoder agreed, +/// so internal round-trips passed — but any upstream client (kubectl, +/// hydrophone, client-go) decoding the envelope hit +/// `proto: illegal wireType 6` reading the apiVersion bytes as a nested +/// TypeMeta message (run 26360429763, post-#766 Pod CREATE). +#[derive(Clone, PartialEq, Message)] +pub struct Unknown { + #[prost(message, optional, tag = "1")] + pub type_meta: Option, - /// Raw will hold the complete serialized object in protobuf format - #[prost(bytes, tag = "3")] + #[prost(bytes, tag = "2")] pub raw: Vec, - /// ContentEncoding is encoding used for the raw data (empty for protobuf) - #[prost(string, tag = "4")] + #[prost(string, tag = "3")] pub content_encoding: String, - /// ContentType specifies the media type of Raw. If empty, "application/vnd.kubernetes.protobuf" - #[prost(string, tag = "5")] + #[prost(string, tag = "4")] pub content_type: String, } @@ -83,8 +103,10 @@ pub fn encode_protobuf( // Create Unknown wrapper with type metadata let unknown = Unknown { - api_version: api_version.to_string(), - kind: kind.to_string(), + type_meta: Some(UnknownTypeMeta { + api_version: api_version.to_string(), + kind: kind.to_string(), + }), raw: json_bytes, content_encoding: String::new(), content_type: "application/json".to_string(), // Indicates raw contains JSON @@ -121,11 +143,19 @@ pub fn decode_protobuf Deserialize<'de>>(data: &[u8]) -> Result<(T, let unknown = Unknown::decode(&data[PROTOBUF_MAGIC.len()..]) .map_err(|e| format!("Failed to decode protobuf: {}", e))?; - // Extract type metadata - let type_meta = TypeMeta { - api_version: unknown.api_version.clone(), - kind: unknown.kind.clone(), - }; + // Extract type metadata from the nested message (defaults if absent — + // matches upstream Unknown decode which treats typeMeta as optional). + let type_meta = unknown + .type_meta + .as_ref() + .map(|tm| TypeMeta { + api_version: tm.api_version.clone(), + kind: tm.kind.clone(), + }) + .unwrap_or_else(|| TypeMeta { + api_version: String::new(), + kind: String::new(), + }); // Deserialize the object from raw bytes // In our implementation, raw contains JSON @@ -149,10 +179,16 @@ pub fn extract_type_meta(data: &[u8]) -> Result { let unknown = Unknown::decode(&data[PROTOBUF_MAGIC.len()..]) .map_err(|e| format!("Failed to decode protobuf: {}", e))?; - Ok(TypeMeta { - api_version: unknown.api_version, - kind: unknown.kind, - }) + Ok(unknown + .type_meta + .map(|tm| TypeMeta { + api_version: tm.api_version, + kind: tm.kind, + }) + .unwrap_or_else(|| TypeMeta { + api_version: String::new(), + kind: String::new(), + })) } #[cfg(test)] @@ -236,8 +272,10 @@ mod tests { fn test_unknown_message() { // Test direct Unknown encoding/decoding let unknown = Unknown { - api_version: "v1".to_string(), - kind: "Pod".to_string(), + type_meta: Some(UnknownTypeMeta { + api_version: "v1".to_string(), + kind: "Pod".to_string(), + }), raw: b"{\"test\": \"data\"}".to_vec(), content_encoding: String::new(), content_type: "application/json".to_string(), @@ -249,4 +287,45 @@ mod tests { let decoded = Unknown::decode(&buf[..]).expect("Failed to decode"); assert_eq!(decoded, unknown); } + + /// Wire-format compatibility: Unknown.typeMeta must be at field 1 as a + /// nested message (apiVersion=1, kind=2), Unknown.raw at field 2. This + /// is what every upstream client decoder expects — getting it wrong + /// produces `proto: illegal wireType 6` when the client tries to parse + /// the apiVersion string bytes as a nested TypeMeta. See module docs + /// on `Unknown`. + #[test] + fn test_unknown_wire_format_matches_upstream() { + let mut buf = Vec::new(); + let unknown = Unknown { + type_meta: Some(UnknownTypeMeta { + api_version: "v1".to_string(), + kind: "Pod".to_string(), + }), + raw: b"{}".to_vec(), + content_encoding: String::new(), + content_type: "application/json".to_string(), + }; + unknown.encode(&mut buf).expect("encode"); + + // First emitted bytes MUST be the field-1 tag for an embedded + // message: tag = (1 << 3) | 2 = 0x0A. (Old broken layout emitted + // 0x0A 0x02 'v' '1' as a *string* — same tag byte, but the inner + // payload is then parsed by clients as a TypeMeta proto message + // whose first byte 'v'=0x76 carries the illegal wireType 6.) + assert_eq!( + buf[0], 0x0A, + "Unknown.typeMeta must be embedded message at field 1; got tag byte 0x{:02x}", + buf[0], + ); + + // The embedded TypeMeta should contain field 1 (apiVersion="v1") + // then field 2 (kind="Pod"). Decode it back via the nested message + // type to be sure the wire shape round-trips. + let inner_len = buf[1] as usize; + let inner = &buf[2..2 + inner_len]; + let typed = UnknownTypeMeta::decode(inner).expect("typeMeta decode"); + assert_eq!(typed.api_version, "v1"); + assert_eq!(typed.kind, "Pod"); + } }