Skip to content

Commit 8d250c8

Browse files
committed
change update_subnet to allow enabling SEV on an existing subnet
1 parent 8df3009 commit 8d250c8

File tree

1 file changed

+67
-32
lines changed

1 file changed

+67
-32
lines changed

rs/registry/canister/src/mutations/do_update_subnet.rs

Lines changed: 67 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -142,25 +142,32 @@ impl Registry {
142142
}
143143
}
144144

145-
/// Validates that the SEV (AMD Secure Encrypted Virtualization) feature is not changed on
146-
/// an existing subnet.
145+
/// Validates that the SEV (AMD Secure Encrypted Virtualization) feature is only
146+
/// enabled, but never disabled on an existing subnet.
147147
///
148-
/// Panics if the SEV feature is attempted to be changed.
148+
/// Panics when attempting to turn off SEV for an SEV-enabled subnet
149149
fn validate_update_sev_feature(&self, payload: &UpdateSubnetPayload) {
150150
let subnet_id = payload.subnet_id;
151151

152152
// Ensure the subnet record exists for this subnet ID.
153-
let _subnet_record = self.get_subnet_or_panic(subnet_id);
153+
let Some(subnet_features) = self.get_subnet_or_panic(subnet_id).features else {
154+
return;
155+
};
154156

155-
let Some(features) = payload.features else {
157+
let Some(update_features) = payload.features else {
156158
return;
157159
};
158160

159-
if let Some(sev_enabled) = features.sev_enabled {
160-
panic!(
161-
"{LOG_PREFIX}Proposal attempts to change sev_enabled for Subnet '{subnet_id}' to {sev_enabled}, \
162-
but sev_enabled can only be set during subnet creation.",
163-
);
161+
match (subnet_features.sev_enabled, update_features.sev_enabled) {
162+
(Some(true), Some(false)) => {
163+
panic!(
164+
"{LOG_PREFIX}Proposal attempts to disable SEV for Subnet '{subnet_id}', \
165+
but SEV cannot be turned off once enabled.",
166+
);
167+
}
168+
_ => {
169+
// All other cases are allowed, including when SEV is not being updated or when it is being enabled on an existing subnet.
170+
}
164171
}
165172
}
166173

@@ -995,40 +1002,68 @@ mod tests {
9951002
(registry, subnet_id)
9961003
}
9971004

998-
#[test]
999-
#[should_panic(expected = "Proposal attempts to change sev_enabled for Subnet \
1000-
'ge6io-epiam-aaaaa-aaaap-yai' to true, but sev_enabled can only be set during \
1001-
subnet creation.")]
1002-
fn test_sev_enabled_cannot_be_changed_to_true() {
1003-
let (mut registry, subnet_id) = make_registry_for_update_subnet_tests();
1004-
1005+
fn update_sev(subnet_id: SubnetId, enabled: bool) -> UpdateSubnetPayload {
10051006
let mut payload = make_empty_update_payload(subnet_id);
10061007
payload.features = Some(SubnetFeaturesPb {
10071008
canister_sandboxing: false,
10081009
http_requests: false,
1009-
sev_enabled: Some(true),
1010+
sev_enabled: Some(enabled),
10101011
});
1012+
payload
1013+
}
10111014

1012-
registry.do_update_subnet(payload);
1015+
#[test]
1016+
fn test_sev_enabled_can_be_enabled_if_disabled() {
1017+
let (mut registry, subnet_id) = make_registry_for_update_subnet_tests();
1018+
1019+
// transition from false -> true
1020+
registry.do_update_subnet(update_sev(subnet_id, false));
1021+
registry.do_update_subnet(update_sev(subnet_id, true));
1022+
1023+
let subnet_features = registry
1024+
.get_subnet_or_panic(subnet_id)
1025+
.features
1026+
.expect("failed to get subnet features");
1027+
1028+
assert_eq!(
1029+
subnet_features.sev_enabled,
1030+
Some(true),
1031+
"Expected SEV enabled to be Some(true), but was {:?}",
1032+
subnet_features.sev_enabled
1033+
);
10131034
}
10141035

10151036
#[test]
1016-
#[should_panic(expected = "Proposal attempts to change sev_enabled for Subnet \
1017-
'ge6io-epiam-aaaaa-aaaap-yai' to false, but sev_enabled can only be set during \
1018-
subnet creation.")]
1019-
fn test_sev_enabled_cannot_be_changed_to_false() {
1037+
fn test_sev_enabled_can_be_enabled_if_unset() {
10201038
let (mut registry, subnet_id) = make_registry_for_update_subnet_tests();
10211039

1022-
let mut payload = make_empty_update_payload(subnet_id);
1023-
payload.features = Some(SubnetFeaturesPb {
1024-
canister_sandboxing: false,
1025-
http_requests: false,
1026-
// The only difference compared to test_sev_enabled_cannot_be_changed_to_true
1027-
sev_enabled: Some(false),
1028-
});
1040+
// transition from unset (implicitly) -> true
1041+
registry.do_update_subnet(update_sev(subnet_id, true));
10291042

1030-
// Should panic because we are changing SEV-related subnet features.
1031-
registry.do_update_subnet(payload);
1043+
let subnet_features = registry
1044+
.get_subnet_or_panic(subnet_id)
1045+
.features
1046+
.expect("failed to get subnet features");
1047+
1048+
assert_eq!(
1049+
subnet_features.sev_enabled,
1050+
Some(true),
1051+
"Expected SEV enabled to be Some(true), but was {:?}",
1052+
subnet_features.sev_enabled
1053+
);
1054+
}
1055+
1056+
#[test]
1057+
#[should_panic(
1058+
expected = "Proposal attempts to disable SEV for Subnet 'ge6io-epiam-aaaaa-aaaap-yai', \
1059+
but SEV cannot be turned off once enabled."
1060+
)]
1061+
fn test_sev_enabled_cannot_be_disabled() {
1062+
let (mut registry, subnet_id) = make_registry_for_update_subnet_tests();
1063+
1064+
registry.do_update_subnet(update_sev(subnet_id, true));
1065+
// This should trigger the panic
1066+
registry.do_update_subnet(update_sev(subnet_id, false));
10321067
}
10331068

10341069
#[test]

0 commit comments

Comments
 (0)