remove PREFERRED capabilities

Way back when I added support for core boot classic installs, we did not
have a way to indicate whether the installed model preferred to be
installed encrypted or unencrypted. Roll time on 9 months or so and our
API now supports a list of capabilites and we can define that we can
indicate which is preferred by putting it first in the list.

One part of the  implementation is a bit delicate really -- relying on
list.sort() being stable -- so maybe we should clean that up. But I
think the other changes make everything nicer.
This commit is contained in:
Michael Hudson-Doyle 2023-10-16 16:58:20 +13:00
parent 3452c5772e
commit 35e66a4805
5 changed files with 71 additions and 70 deletions

View File

@ -350,13 +350,12 @@ class GuidedCapability(enum.Enum):
CORE_BOOT_ENCRYPTED = enum.auto()
CORE_BOOT_UNENCRYPTED = enum.auto()
# These two are not valid as GuidedChoiceV2.capability:
CORE_BOOT_PREFER_ENCRYPTED = enum.auto()
CORE_BOOT_PREFER_UNENCRYPTED = enum.auto()
DD = enum.auto()
def __lt__(self, other) -> bool:
if self.is_core_boot() and other.is_core_boot():
return False
return self.value < other.value
def is_lvm(self) -> bool:
@ -366,8 +365,6 @@ class GuidedCapability(enum.Enum):
return self in [
GuidedCapability.CORE_BOOT_ENCRYPTED,
GuidedCapability.CORE_BOOT_UNENCRYPTED,
GuidedCapability.CORE_BOOT_PREFER_ENCRYPTED,
GuidedCapability.CORE_BOOT_PREFER_UNENCRYPTED,
]
def supports_manual_customization(self) -> bool:

View File

@ -394,11 +394,13 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
info.capability_info.allowed = [GuidedCapability.CORE_BOOT_ENCRYPTED]
elif se.storage_safety == StorageSafety.PREFER_ENCRYPTED:
info.capability_info.allowed = [
GuidedCapability.CORE_BOOT_PREFER_ENCRYPTED
GuidedCapability.CORE_BOOT_ENCRYPTED,
GuidedCapability.CORE_BOOT_UNENCRYPTED,
]
elif se.storage_safety == StorageSafety.PREFER_UNENCRYPTED:
info.capability_info.allowed = [
GuidedCapability.CORE_BOOT_PREFER_UNENCRYPTED
GuidedCapability.CORE_BOOT_UNENCRYPTED,
GuidedCapability.CORE_BOOT_ENCRYPTED,
]
return info
@ -651,26 +653,8 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
def set_info_for_capability(self, capability: GuidedCapability):
"""Given a request for a capability, select the variation to use."""
if capability == GuidedCapability.CORE_BOOT_ENCRYPTED:
# If the request is for encryption, a variation with any
# of these capabilities is OK:
caps = {
GuidedCapability.CORE_BOOT_ENCRYPTED,
GuidedCapability.CORE_BOOT_PREFER_ENCRYPTED,
GuidedCapability.CORE_BOOT_PREFER_UNENCRYPTED,
}
elif capability == GuidedCapability.CORE_BOOT_UNENCRYPTED:
# Similar if the request is for uncrypted
caps = {
GuidedCapability.CORE_BOOT_UNENCRYPTED,
GuidedCapability.CORE_BOOT_PREFER_ENCRYPTED,
GuidedCapability.CORE_BOOT_PREFER_UNENCRYPTED,
}
else:
# Otherwise, just look for what we were asked for.
caps = {capability}
for info in self._variation_info.values():
if caps & set(info.capability_info.allowed):
if capability in info.capability_info.allowed:
self._info = info
return
raise Exception("could not find variation for {}".format(capability))
@ -1313,12 +1297,12 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
if name == "hybrid":
# this check is conceptually unnecessary but results in a
# much cleaner error message...
core_boot_caps = set()
core_boot_caps = []
for variation in self._variation_info.values():
if not variation.is_valid():
continue
if variation.is_core_boot_classic():
core_boot_caps.update(variation.capability_info.allowed)
core_boot_caps.extend(variation.capability_info.allowed)
if not core_boot_caps:
raise Exception(
"can only use name: hybrid when installing core boot classic"
@ -1328,18 +1312,12 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
encrypted = layout.get("encrypted", None)
GC = GuidedCapability
if encrypted is None:
if (
GC.CORE_BOOT_ENCRYPTED in core_boot_caps
or GC.CORE_BOOT_PREFER_ENCRYPTED in core_boot_caps
):
capability = GC.CORE_BOOT_ENCRYPTED
else:
capability = GC.CORE_BOOT_UNENCRYPTED
capability = core_boot_caps[0]
elif encrypted:
capability = GC.CORE_BOOT_ENCRYPTED
else:
if (
core_boot_caps == {GuidedCapability.CORE_BOOT_ENCRYPTED}
core_boot_caps == [GuidedCapability.CORE_BOOT_ENCRYPTED]
and not encrypted
):
raise Exception("cannot install this model unencrypted")

View File

@ -696,7 +696,9 @@ class TestLayout(IsolatedAsyncioTestCase):
class TestGuidedV2(IsolatedAsyncioTestCase):
async def _setup(self, bootloader, ptable, fix_bios=True, **kw):
async def _setup(
self, bootloader, ptable, fix_bios=True, snapd_system_labels=(None,), **kw
):
self.app = make_app()
self.app.opts.bootloader = bootloader.value
self.fsc = FilesystemController(app=self.app)
@ -704,11 +706,16 @@ class TestGuidedV2(IsolatedAsyncioTestCase):
self.fsc.calculate_suggested_install_min.return_value = 10 << 30
self.fsc.model = self.model = make_model(bootloader)
self.fsc._examine_systems_task.start_sync()
self.app.snapdapi = snapdapi.make_api_client(AsyncSnapd(get_fake_connection()))
self.app.dr_cfg = DRConfig()
self.app.dr_cfg.systems_dir_exists = True
self.app.base_model.source.search_drivers = False
self.app.base_model.source.current.type = "fsimage"
self.app.base_model.source.current.variations = {
"default": CatalogEntryVariation(path="", size=1),
}
variations = self.app.base_model.source.current.variations = {}
for label in snapd_system_labels:
variations["var" + str(label)] = CatalogEntryVariation(
path="", size=1, snapd_system_label=label
)
self.app.controllers.Source.get_handler.return_value = TrivialSourceHandler("")
await self.fsc._examine_systems_task.wait()
self.disk = make_disk(self.model, ptable=ptable, **kw)
@ -719,7 +726,7 @@ class TestGuidedV2(IsolatedAsyncioTestCase):
"filesystem": self.fs_probe,
}
self.fsc._probe_task.task = mock.Mock()
self.fsc._examine_systems_task.task = mock.Mock()
# self.fsc._examine_systems_task.task = mock.Mock()
if bootloader == Bootloader.BIOS and ptable != "msdos" and fix_bios:
make_partition(
self.model,
@ -1153,6 +1160,29 @@ class TestGuidedV2(IsolatedAsyncioTestCase):
self.assertEqual(expected, resp.targets)
self.assertEqual(ProbeStatus.DONE, resp.status)
@parameterized.expand(
[
("mandatory", ["ENCRYPTED"]),
("prefer-encrypted", ["ENCRYPTED", "UNENCRYPTED"]),
("prefer-unencrypted", ["UNENCRYPTED", "ENCRYPTED"]),
("unavailable", ["UNENCRYPTED"]),
]
)
async def test_core_boot_and_classic(self, label, core_boot_cap_names):
expected_core_boot_caps = [
getattr(GuidedCapability, f"CORE_BOOT_{cap_name}")
for cap_name in core_boot_cap_names
]
await self._setup(
Bootloader.UEFI,
"gpt",
snapd_system_labels=[None, label],
)
resp = await self.fsc.v2_guided_GET()
[target, man] = resp.targets
got_core_boot_caps = [cap for cap in target.allowed if cap.is_core_boot()]
self.assertEqual(got_core_boot_caps, expected_core_boot_caps)
class TestManualBoot(IsolatedAsyncioTestCase):
def _setup(self, bootloader, ptable, **kw):

View File

@ -629,7 +629,7 @@ class TestCore(TestAPI):
await inst.post("/source", source_id="ubuntu-desktop")
resp = await inst.get("/storage/v2/guided", wait=True)
[reformat, manual] = resp["targets"]
self.assertIn("CORE_BOOT_PREFER_ENCRYPTED", reformat["allowed"])
self.assertIn("CORE_BOOT_ENCRYPTED", reformat["allowed"])
data = dict(target=reformat, capability="CORE_BOOT_ENCRYPTED")
await inst.post("/storage/v2/guided", data)
v2resp = await inst.get("/storage/v2")

View File

@ -134,16 +134,11 @@ class TPMChoice:
help: str
tpm_help_texts = {
"AVAILABLE_CAN_BE_DESELECTED": _(
"The entire disk will be encrypted and protected by the "
"TPM. If this option is deselected, the disk will be "
"unencrypted and without any protection."
),
"AVAILABLE_CANNOT_BE_DESELECTED": _(
tpm_help_texts_1 = {
GuidedCapability.CORE_BOOT_ENCRYPTED: _(
"The entire disk will be encrypted and protected by the TPM."
),
"UNAVAILABLE":
GuidedCapability.CORE_BOOT_UNENCRYPTED:
# for translators: 'reason' is the reason FDE is unavailable.
_(
"TPM backed full-disk encryption is not available "
@ -151,22 +146,11 @@ tpm_help_texts = {
),
}
choices = {
GuidedCapability.CORE_BOOT_ENCRYPTED: TPMChoice(
enabled=False,
default=True,
help=tpm_help_texts["AVAILABLE_CANNOT_BE_DESELECTED"],
),
GuidedCapability.CORE_BOOT_UNENCRYPTED: TPMChoice(
enabled=False, default=False, help=tpm_help_texts["UNAVAILABLE"]
),
GuidedCapability.CORE_BOOT_PREFER_ENCRYPTED: TPMChoice(
enabled=True, default=True, help=tpm_help_texts["AVAILABLE_CAN_BE_DESELECTED"]
),
GuidedCapability.CORE_BOOT_PREFER_UNENCRYPTED: TPMChoice(
enabled=True, default=False, help=tpm_help_texts["AVAILABLE_CAN_BE_DESELECTED"]
),
}
tpm_help_texts_can_be_deselected = _(
"The entire disk will be encrypted and protected by the "
"TPM. If this option is deselected, the disk will be "
"unencrypted and without any protection."
)
class GuidedChoiceForm(SubForm):
@ -223,16 +207,28 @@ class GuidedChoiceForm(SubForm):
self.use_lvm.enabled = GuidedCapability.LVM in val.allowed
core_boot_caps = [c for c in val.allowed if c.is_core_boot()]
if core_boot_caps:
assert len(val.allowed) == 1
cap = core_boot_caps[0]
assert len(val.allowed) == len(core_boot_caps)
reason = ""
for disallowed in val.disallowed:
if disallowed.capability == GuidedCapability.CORE_BOOT_ENCRYPTED:
reason = disallowed.message
self.tpm_choice = choices[cap]
cap0 = core_boot_caps[0]
if len(core_boot_caps) == 1:
self.tpm_choice = TPMChoice(
enabled=False,
default=cap0 == GuidedCapability.CORE_BOOT_ENCRYPTED,
help=tpm_help_texts_1[cap0],
)
else:
self.tpm_choice = TPMChoice(
enabled=True,
default=cap0 == GuidedCapability.CORE_BOOT_ENCRYPTED,
help=tpm_help_texts_can_be_deselected,
)
self.use_tpm.enabled = self.tpm_choice.enabled
self.use_tpm.value = self.tpm_choice.default
self.use_tpm.help = self.tpm_choice.help
self.use_tpm.help = self.tpm_choice.help.format(reason=reason)
else:
self.use_tpm.enabled = False