curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+461402@xxxxxxxxxxxxxxxxxx>]

Review: Approve

LGTM

Diff comments:

> diff --git a/curtin/block/zfs.py b/curtin/block/zfs.py
> index 01ba9b9..767ad30 100644
> --- a/curtin/block/zfs.py
> +++ b/curtin/block/zfs.py
> @@ -152,6 +256,11 @@ def zpool_create(poolname, vdevs, mountpoint=None, altroot=None,
>      if zfs_properties:
>          merge_config(zfs_cfg, zfs_properties)
>  
> +    encryption = ZPoolEncryption(poolname, encryption_style, keyfile)
> +    encryption.validate()
> +    if encryption.in_use():

I guess this check is actually not needed...

> +        merge_config(zfs_cfg, encryption.dataset_properties())
> +
>      options = _join_flags('-o', pool_cfg)
>      options.extend(_join_flags('-O', zfs_cfg))
>  
> diff --git a/curtin/util.py b/curtin/util.py
> index 53e2035..3249d1f 100644
> --- a/curtin/util.py
> +++ b/curtin/util.py
> @@ -1136,7 +1136,7 @@ def human2bytes(size):
>      if int(val) != val:
>          raise ValueError("'%s': resulted in non-integer (%s)" % (size_in, val))
>  
> -    return val
> +    return int(val)

Not necessarily for now but are there lots of calls in the curtin codebase that pass the return value of human2bytes to int()?

>  
>  
>  def bytes2human(size):
> diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
> index 2fc7c7d..f1c86ee 100644
> --- a/tests/integration/test_block_meta.py
> +++ b/tests/integration/test_block_meta.py
> @@ -1334,6 +1335,43 @@ table-length: 256'''.encode()
>              if key == "type":
>                  self.assertEqual("PLAIN", value.strip())
>  
> +    def test_zfs_luks_keystore(self):

I haven't run this!

> +        self.img = self.tmp_path('image.img')
> +        keyfile = self.tmp_path('zfs-luks-keystore-keyfile')
> +        with open(keyfile, "w") as fp:
> +            fp.write(self.random_string())
> +        config = StorageConfigBuilder(version=2)
> +        config.add_image(path=self.img, create=True, size='200M', ptable='gpt')
> +        p1 = config.add_part(number=1, offset=1 << 20, size=198 << 20)
> +        poolname = self.random_string()
> +        config._add(
> +            type='zpool',
> +            pool=poolname,
> +            vdevs=[p1["id"]],
> +            mountpoint="/",
> +            pool_properties=dict(ashift=12, autotrim="on", version=None),
> +            encryption_style="luks_keystore",
> +            keyfile=keyfile,
> +        )
> +        self.run_bm(config.render())
> +
> +        keystore_volume = f"/dev/zvol/{poolname}/keystore"
> +        dm_name = f"keystore-{poolname}"
> +        dmpath = f"/dev/mapper/{dm_name}"
> +        util.subp([
> +            "cryptsetup", "open", "--type", "luks", keystore_volume,
> +            dm_name, "--key-file", keyfile,
> +        ])
> +        mntdir = self.tmp_dir()
> +        try:
> +            with util.mount(dmpath, mntdir):
> +                system_key = Path(mntdir) / "system.key"
> +                st_mode = system_key.stat().st_mode
> +                self.assertEqual(0o400, stat.S_IMODE(st_mode))
> +                self.assertTrue(stat.S_ISREG(st_mode))
> +        finally:
> +            util.subp(["cryptsetup", "close", dmpath])
> +
>      @parameterized.expand(((1,), (2,)))
>      def test_msftres(self, sv):
>          self.img = self.tmp_path('image.img')


-- 
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/461402
Your team curtin developers is subscribed to branch curtin:master.