Skip to content

Commit 4bed959

Browse files
committed
tests: add unit tests for server-side apply
Signed-off-by: Chirag Rao <crao@redhat.com> Assisted-by: Cursor
1 parent be643ea commit 4bed959

6 files changed

Lines changed: 606 additions & 50 deletions

File tree

operator/src/attestation_key_register.rs

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,7 @@ pub async fn launch_secret_ak_controller(client: Client) {
388388
#[cfg(test)]
389389
mod tests {
390390
use super::*;
391+
use http::{Method, Request};
391392
use trusted_cluster_operator_test_utils::mock_client::*;
392393

393394
#[tokio::test]
@@ -419,4 +420,243 @@ mod tests {
419420
|client| create_attestation_key_register_service(client, Default::default(), Some(80));
420421
test_create_error(clos).await;
421422
}
423+
424+
fn dummy_ak() -> AttestationKey {
425+
AttestationKey {
426+
metadata: ObjectMeta {
427+
name: Some("ak-test".to_string()),
428+
uid: Some("ak-uid".to_string()),
429+
generation: Some(1),
430+
..Default::default()
431+
},
432+
spec: trusted_cluster_operator_lib::AttestationKeySpec {
433+
public_key: "test-key".to_string(),
434+
uuid: Some("machine-uuid".to_string()),
435+
},
436+
status: None,
437+
}
438+
}
439+
440+
fn dummy_machine() -> Machine {
441+
Machine {
442+
metadata: ObjectMeta {
443+
name: Some("machine-test".to_string()),
444+
uid: Some("machine-uid".to_string()),
445+
..Default::default()
446+
},
447+
spec: trusted_cluster_operator_lib::MachineSpec {
448+
id: "machine-uuid".to_string(),
449+
},
450+
status: None,
451+
}
452+
}
453+
454+
#[tokio::test]
455+
async fn test_approve_ak_full_flow() {
456+
// approve_ak with no prior status, no Machine owner, and secret not existing:
457+
// 1. PATCH /status (SSA status update). AttestationKey is approved.
458+
// 2. PATCH owner transfer (Merge). Ownership transfered to Machine.
459+
// 3. GET secret (check existence)
460+
// 4. PATCH secret (apply_resource! SSA create)
461+
let clos = async |req: Request<_>, ctr| match (ctr, req.method()) {
462+
(0, &Method::PATCH) => {
463+
assert!(req.uri().path().contains("/status"));
464+
Ok(serde_json::to_string(&dummy_ak()).unwrap())
465+
}
466+
(1, &Method::PATCH) => {
467+
assert!(!req.uri().path().contains("/status"));
468+
Ok(serde_json::to_string(&dummy_ak()).unwrap())
469+
}
470+
(2, &Method::GET) => Err(http::StatusCode::NOT_FOUND),
471+
(3, &Method::PATCH) => {
472+
let body = get_body_string(req).await;
473+
let v: serde_json::Value = serde_json::from_str(&body).unwrap();
474+
let owners = v["metadata"]["ownerReferences"]
475+
.as_array()
476+
.expect("Secret must have ownerReferences");
477+
assert_eq!(
478+
owners[0]["kind"], "AttestationKey",
479+
"Secret must be owned by AttestationKey"
480+
);
481+
assert_eq!(
482+
owners[0]["controller"], true,
483+
"AttestationKey must be controller of Secret"
484+
);
485+
assert_eq!(
486+
owners[0]["uid"], "ak-uid",
487+
"Secret owner UID must match AK UID"
488+
);
489+
490+
// Asserting finalizers
491+
let finalizers = v["metadata"]["finalizers"]
492+
.as_array()
493+
.expect("Secret must have finalizers");
494+
assert!(
495+
finalizers.iter().any(|f| f
496+
.as_str()
497+
.unwrap()
498+
.contains("attestationkey-secret-finalizer")),
499+
"Secret must have the attestation key secret finalizer"
500+
);
501+
let data = v["data"].as_object().expect("Secret must have data");
502+
assert!(
503+
data.contains_key("public_key"),
504+
"Secret data must contain public_key"
505+
);
506+
Ok(serde_json::to_string(&Secret::default()).unwrap())
507+
}
508+
509+
_ => panic!("unexpected API interaction: {req:?}, counter {ctr}"),
510+
};
511+
count_check!(4, clos, |client| {
512+
let ak = dummy_ak();
513+
let machine = dummy_machine();
514+
assert!(approve_ak(&ak, &machine, client).await.is_ok());
515+
});
516+
}
517+
518+
#[tokio::test]
519+
async fn test_approve_ak_already_approved_and_owned() {
520+
// Building a pre-populated AttestationKey with the approved condition. This would prevent upsert from changing the status field, preventing the initial status PATCH.
521+
// Further-more, we set owner of AttestationKey to the Machine, so that no owner transfer PATCH is needed.
522+
// Further, get secret returns valid secret, so no secret PATCH is needed.
523+
// Only 1 GET call to fetch secret is needed.
524+
let mut ak = dummy_ak();
525+
let approve_reason =
526+
trusted_cluster_operator_lib::conditions::ATTESTATION_KEY_MACHINE_APPROVE;
527+
let existing_condition = crate::conditions::attestation_key_approved_condition(
528+
approve_reason,
529+
ak.metadata.generation,
530+
&ak.status,
531+
);
532+
ak.status = Some(AttestationKeyStatus {
533+
conditions: Some(vec![existing_condition]),
534+
});
535+
ak.metadata.owner_references = Some(vec![OwnerReference {
536+
kind: "Machine".to_string(),
537+
name: "machine-test".to_string(),
538+
uid: "machine-uid".to_string(),
539+
api_version: "trusted-execution-clusters.io/v1alpha1".to_string(),
540+
controller: Some(true),
541+
block_owner_deletion: Some(true),
542+
}]);
543+
544+
// No status or owner PATCH needed; only GET secret (exists)
545+
let clos = async |req: Request<_>, ctr| match (ctr, req.method()) {
546+
(0, &Method::GET) => Ok(serde_json::to_string(&Secret::default()).unwrap()),
547+
_ => panic!("unexpected API interaction: {req:?}, counter {ctr}"),
548+
};
549+
count_check!(1, clos, |client| {
550+
let machine = dummy_machine();
551+
assert!(approve_ak(&ak, &machine, client).await.is_ok());
552+
});
553+
}
554+
555+
#[tokio::test]
556+
async fn test_approve_ak_status_update_error() {
557+
let clos = async |req: Request<_>, ctr| match (ctr, req.method()) {
558+
(0, &Method::PATCH) => Err(http::StatusCode::INTERNAL_SERVER_ERROR),
559+
_ => panic!("unexpected API interaction: {req:?}, counter {ctr}"),
560+
};
561+
count_check!(1, clos, |client| {
562+
let ak = dummy_ak();
563+
let machine = dummy_machine();
564+
assert!(approve_ak(&ak, &machine, client).await.is_err());
565+
});
566+
}
567+
568+
#[tokio::test]
569+
async fn test_approve_ak_status_patch_contains_approved_condition() {
570+
use kube::client::Body;
571+
let clos = async |req: Request<Body>, ctr| match (ctr, req.method()) {
572+
// Validates whether attestation key is immediately approved as per TOFU (Trust on first use) principles.
573+
// Also makes sure that the approved condition is set to True, and the reason is MachineCreated.
574+
(0, &Method::PATCH) => {
575+
assert!(
576+
req.uri().path().contains("/status"),
577+
"First PATCH must target /status subresource"
578+
);
579+
let body = get_body_string(req).await;
580+
let v: serde_json::Value = serde_json::from_str(&body).unwrap();
581+
let conditions = v["status"]["conditions"]
582+
.as_array()
583+
.expect("Status body must contain conditions array");
584+
let approved_type =
585+
trusted_cluster_operator_lib::conditions::ATTESTATION_KEY_APPROVED_CONDITION;
586+
let approved = conditions.iter().find(|c| c["type"] == approved_type);
587+
assert!(
588+
approved.is_some(),
589+
"Must contain the '{approved_type}' condition"
590+
);
591+
let cond = approved.unwrap();
592+
assert_eq!(cond["status"], "True", "Approved condition must be True");
593+
assert_eq!(
594+
cond["reason"], ATTESTATION_KEY_MACHINE_APPROVE,
595+
"Reason must be MachineCreated"
596+
);
597+
Ok(serde_json::to_string(&dummy_ak()).unwrap())
598+
}
599+
(1, &Method::PATCH) => Ok(serde_json::to_string(&dummy_ak()).unwrap()),
600+
(2, &Method::GET) => Err(http::StatusCode::NOT_FOUND),
601+
(3, &Method::PATCH) => Ok(serde_json::to_string(&Secret::default()).unwrap()),
602+
_ => panic!("unexpected API interaction: {req:?}, counter {ctr}"),
603+
};
604+
count_check!(4, clos, |client| {
605+
let ak = dummy_ak();
606+
let machine = dummy_machine();
607+
assert!(approve_ak(&ak, &machine, client).await.is_ok());
608+
});
609+
}
610+
611+
// Makes sure that the owner transfer patch uses merge patch, and not SSA patch.
612+
// SSA can't transfer ownership, and would cause issues where we might not cleanly remove the TEC owner reference.
613+
#[tokio::test]
614+
async fn test_approve_ak_owner_transfer_uses_merge_patch() {
615+
use kube::client::Body;
616+
let clos = async |req: Request<Body>, ctr| match (ctr, req.method()) {
617+
(0, &Method::PATCH) => Ok(serde_json::to_string(&dummy_ak()).unwrap()),
618+
(1, &Method::PATCH) => {
619+
assert!(
620+
!req.uri().path().contains("/status"),
621+
"Owner transfer must not target /status"
622+
);
623+
let query = req.uri().query().unwrap_or("");
624+
assert!(
625+
!query.contains("fieldManager"),
626+
"Merge patch must NOT use a field manager (not SSA): {query}"
627+
);
628+
let content_type = req
629+
.headers()
630+
.get("content-type")
631+
.and_then(|v| v.to_str().ok())
632+
.unwrap_or("");
633+
assert!(
634+
content_type.contains("merge-patch"),
635+
"Owner transfer must use Merge patch, got content-type: {content_type}"
636+
);
637+
let body = get_body_string(req).await;
638+
let v: serde_json::Value = serde_json::from_str(&body).unwrap();
639+
let owners = v["metadata"]["ownerReferences"]
640+
.as_array()
641+
.expect("Merge patch must set ownerReferences");
642+
assert_eq!(
643+
owners.len(),
644+
1,
645+
"Must replace entire ownerReferences (not append)"
646+
);
647+
assert_eq!(owners[0]["kind"], "Machine", "New owner must be Machine");
648+
assert_eq!(owners[0]["name"], "machine-test");
649+
assert_eq!(owners[0]["controller"], true, "Machine must be controller");
650+
Ok(serde_json::to_string(&dummy_ak()).unwrap())
651+
}
652+
(2, &Method::GET) => Err(http::StatusCode::NOT_FOUND),
653+
(3, &Method::PATCH) => Ok(serde_json::to_string(&Secret::default()).unwrap()),
654+
_ => panic!("unexpected API interaction: {req:?}, counter {ctr}"),
655+
};
656+
count_check!(4, clos, |client| {
657+
let ak = dummy_ak();
658+
let machine = dummy_machine();
659+
assert!(approve_ak(&ak, &machine, client).await.is_ok());
660+
});
661+
}
422662
}

operator/src/main.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,7 @@ mod tests {
505505
};
506506

507507
let clos = async |req: Request<Body>, _ctr| {
508+
let is_status_patch = req.uri().path().ends_with("/status");
508509
match *req.method() {
509510
Method::GET => {
510511
let object_list = ObjectList::<TrustedExecutionCluster> {
@@ -514,7 +515,11 @@ mod tests {
514515
};
515516
Ok(serde_json::to_string(&object_list).unwrap())
516517
}
517-
Method::POST => Ok(serde_json::to_string(&dummy_cluster()).unwrap()),
518+
// Patches which update the status field, like installing secrets, config maps etc.
519+
Method::PATCH if !is_status_patch => {
520+
Ok(serde_json::to_string(&dummy_cluster()).unwrap())
521+
}
522+
// Patches which update the status field, to check whether foreign conditions are present and not overwritten.
518523
Method::PATCH => {
519524
let body = req.into_body().collect_bytes().await.unwrap().to_vec();
520525
let body = String::from_utf8_lossy(&body);

0 commit comments

Comments
 (0)