feat: Add possibility to configure a subfolder inside an S3 bucket

[gbouv]

Description

This change adds the possibility to configure a subfolder inside an S3 bucket, to avoid having to spin up an entire bucket for hyperlane.

Drive-by changes

Nothing

Related issues

None, chatted about it on Slack with @nambrot

Backward compatibility

Yes

I made the field optional in the RawCheckpointSyncerConf so that current config are still valid and will continue to read/write at the root of the bucket. That being said, I'm new to this code so I hope I got it right

Testing

None

[nambrot]

Thanks for the PR @gbouv !

This is a great start, but I believe it's a bit more complicated than this:

• You need to modify the announcement that is generated of this, otherwise the validator will just announce without the folder

  hyperlane-monorepo/rust/hyperlane-base/src/types/s3_storage.rs

  Line 212 in 50f04db

  format!("s3://{}/{}", self.bucket, self.region.name())

• You need to modify the construction of the syncer based on the announcement

  hyperlane-monorepo/rust/hyperlane-base/src/types/checkpoint_syncer.rs

  Line 109 in 8c59839

  let [bucket, region]: [&str; 2] = suffix

And then obviously we need to make sure that previous announcements without a folder continue to work, might be a good test case for y'all

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (c402185) 64.50% compared to head (a83aa6e) 64.50%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2647   +/-   ##
=======================================
  Coverage   64.50%   64.50%           
=======================================
  Files          91       91           
  Lines        1386     1386           
  Branches      185      185           
=======================================
  Hits          894      894           
  Misses        485      485           
  Partials        7        7           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nambrot]

Actually nvm, i misinterpreted the code. I do think folder makes more sense as an option, but pending @tkporter or @mattiecnvr 's approval, lgtm

[tkporter]

Thanks for the PR, agree this is useful! I think it'd be nice to move to folder: Option<String>

[gbouv]

BTW @nambrot I tested this with Kurtosis and it seems to work for both cases where folder is specified or left empty

[gbouv]

@tkporter in my last commit I make folder and Option<String> so that you can see what it looks like. If you prefer it this way, I'll keep it, otherwise I'll revert it. Let me know what you think :)

[tkporter]

Lgtm, thank you!

[gbouv]

@tkporter @nambrot can you help merging this one? I am not authorized to press the merge button

[mattiekat]

Hi, I just saw this during a merge conflict. I have added this change to the schema in #2687 but this should not have been approved and represents an almost immediate divergence from the defined schema and parser support.