Goal

Last time I attempted to change things so that all kinds of chunks can be stored in the chunk store. Status quo is that only data chunks can be, and credential chunks can't. Client chunks get encoded as data chunks, so they also can be.

I tried to use a trait for chunks, but this failed, because I wanted the trait to provide serialization and deserialization, and that brought in a lifetime constraint that I could resolve.

Today the goal remains the same: store any kind of chunk in chunk store. I'll try another approach: instead of traits, add an enum for chunks. This should work for Obnam, because I don't need to store arbitrary chunks, only the one supported by Obnam.

Plan

  • Rename Chunk to DataChunk.
  • Introduce a new type enum Chunk with variants for data chunks, client chunks, and credential chunks.
  • Change Store to use the new chunk type.

Notes

  • Create new branch, store-chunk-try2.
  • This starts well: tests don't pass.
---- credential::test::roundtrip_sop_method stdout ----

thread 'credential::test::roundtrip_sop_method' panicked at src/credential.rs:130:46:
called `Result::unwrap()` on an `Err` value: SopEncrypt(Encrypt(CommandFailed { program_name: "rsop", exit_code: 61, output: Output { status: ExitStatus(unix_wait_status(15616)), stdout: "", stderr: "           Failed to open file \"/tmp/.tmpPvjUOF/0.cert\"\n  because: Input file does not exist\n" } }))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- credential::test::roundtrip_sop_credential stdout ----

thread 'credential::test::roundtrip_sop_credential' panicked at src/credential.rs:139:51:
called `Result::unwrap()` on an `Err` value: SopEncrypt(Encrypt(CommandFailed { program_name: "rsop", exit_code: 61, output: Output { status: ExitStatus(unix_wait_status(15616)), stdout: "", stderr: "           Failed to open file \"/tmp/.tmplJkOCH/0.cert\"\n  because: Input file does not exist\n" } }))
  • However, when I started to debug this, the failure went away. Flaky test. Ugh. Opened an issue ticket for that.
  • Renamed obnam::chunk::Chunmk to DataChunk. Tests pass.
  • What should the interface for enum Chunk be? DataChunk has:
    pub fn fake(plaintext: &Plaintext, metadata: Metadata) -> Result<Self, ChunkError> {...}
    pub fn encrypt(
        engine: &Engine,
        plaintext: &Plaintext,
        metadata: Metadata,
    ) -> Result<Self, ChunkError> {...}
    pub fn metadata(&self) -> &Metadata {...}
    pub fn serialize(&self) -> Result<Vec<u8>, ChunkError> {...}
    pub fn parse(bytes: &[u8]) -> Result<Self, ChunkError> {...}
    pub fn decrypt(&self, engine: &Engine) -> Result<Plaintext, ChunkError> {...}
  • DataChunk::fake is just for testing. Or so I thought. It's used by the obnam store add subcommand. That's at least marginally useful. Therefore I'll want to rename fake to something more constructive. It'll be another variant fr Chunk.
  • DataChunk::encrypt and decrypt are specific for data chunks. The engine in specifically for AEAD, only used for data and client chunks. This makes it trickier to lift to the Chunk API, which needs to handle credentials as well.
  • DataChunk::metatdata returns chunk metadata. Every chunk needs that, so Chunk::metadata needs to exist.
  • DataChunk:: serialize and parse are generic, and need to be part of Chunk interface.
  • I'll first make a version of Chunk that only has a variant for data chunks, and implements the interface sketched above. Then I'll change Store to use that. Then I'll re-evaluate.1
  • Parsing isn't a method. It needs to know the type of chunk we want to have. I'm going to punt on that, for now.
  • To create a Chunk from DataCunk, I'll implement the From trait.
  • OK, that works. I have a Chunk type and Store uses it. Encrypting a data chunk and turning that into a generic chunk is a little awkward, as it's a two step process. I'll ignore that for now.
  • I'll add a variant to get rid of DataChunk::fake first. The core of this is to have a way to represent an already encrypted blob as a chunk, so I'll call the new variant Blob.
  • However, this brings up the issue for magic cookies. Currently data chunks serialize themselves and prepend a cookie. Should blobs be assumed to contain a cookie or not? Should Chunk handle serialization itself or hand it off to another type?
  • As a first step, I'll assume the blob contains any magic cookie it needs, and I'll put the blob first in the variant.
  • Added Chunk::Blob, with a constructor. Then added a From instead of constructor, for consistency.
  • Back to magic cookies. I think it'd be more systematic for Chunk to handle them and serialization and parsing in general. So I'll refactor to have that.
  • Actually, the only cookie we currently use is added by the AEAD cipher engine. There's an unused on in the src/chunk.rs module. I think I want only Chunk to add cookies so that they're conveniently in one place.
  • Oh, I don't actually need to know the type of chunk being de-serialized. I'm de-serialize to the enum Chunk, and serde handles the variants just fine. The caller can then match the returned value and handle any variants it needs to.
  • OK, that worked.
  • Store::get_chunk still always returns a DataChunk on success. Need to change that.
  • Done.
  • While doing that, realized that it'd be useful to have per-variant methods for Store: like Store::get_data_chunk and Store::get_credential_chunk, but I'll add those later.
  • More urgent is to add a chunk variant for client chunks.
  • Added a variant Client that contains a DataChunk, as that's what's been done so far.
  • This opens an ambiguity for From<DataChunk> that I'll need to do something about. Either I need to not re-use DataChunk, or drop the From, or do something clever.
  • I can drop the From by adding constructors to Chunk. I'll do that at least for now.
  • Done. This is clearer than the From approach, because it's not ambiguous of what variant is being created.
  • Next up, a credential chunk variant.
  • Since I don't yet actually store any credential in the store, adding the credential variant was easy.
  • Rebased the branch history to be clearer, or at least less meandering.
  • Passes CI.
  • Merged.

Summary

The enum approach turned out to be a lot easier than the trait approach. It was a fair bit of work, but all quite straightforward. I now can store credential chunks in the chunk store, and can get back to implementing OpenPGP software key credentials next time.

Comments?

If you have feedback on this development session, please use the following fediverse thread: https://toot.liw.fi/@liw/114963626193500263.