smallstep / crypto

Crypto is a collection of packages used by Smallstep products
Apache License 2.0
71 stars 24 forks source link

Return a sentinel error when no storage is configured when required #561

Closed hslatman closed 1 month ago

hslatman commented 1 month ago

The goal of the blackhole storage is to be able to use an instance of tpm.TPM for operations that don't require an (metadata) storage. In some usages it could result in an error or even a panic, because it's not clear enough that some operations require an actual storage implementation for the operations to succeed.

This commit changes the behavior of the blackhole storage. It now returns a sentinel error for operations that require a storage to be successful. The sentinel error is handled in the tpm package wherever a storage operation takes place.

hslatman commented 1 month ago

@azazeal not green, but I blame wrong calculation of the actual diff 😛

hslatman commented 1 month ago

The reason I'd like the sentinel to be exposed from both packages, is so that a user of the tpm package doesn't need to import storage if they wanted to check for this specific case. Also, having to check the code paths to see which errors can be returned can be tedious, and is prone to missing cases. I deemed just documenting it insufficient.

I'll make the one in tpm an alias for the one from storage. That should result in the same behavior without having to put the switch/if in most places.

Same approach can probably work for the existing sentinel errors; will check those too, and follow up in a new PR for those.