dotnet-kube-client icon indicating copy to clipboard operation
dotnet-kube-client copied to clipboard

Initial port of KubeClient.Extensions.DataProtection

Open tintoy opened this issue 5 years ago • 5 comments

This code has been ported from https://github.com/rriverak/KubeClient.Extensions.DataProtection (thanks, @rriverak!).

Note that, at this, stage, it is a work in progress; we need to decide on strategies for:

  • [x] Concurrency
    • [x] Basic synchronisation of access to the SecretV1 instance used by the key store
      This is handled by the secret manager.
    • [x] What happens when underlying Secret is changed by one client, and another is in the middle of loading secrets?
      The store operates on a snapshot of the underlying Secret's data.
    • [ ] What happens when 2 clients attempt to create the secret simultaneously?
    • [x] Updating the Secret will result in the change-notification handler firing; this means the Secret is always re-loaded after being created. Is this OK, or would we want to suspend notifications while we know we are modifying the Secret?
  • [ ] Handling of common edge-cases / error conditions
    • [ ] Secret has been deleted
    • [ ] Secret has been deleted and then re-created
    • [ ] Secret does not exist and creation fails
    • [x] Secret values contain invalid data (e.g. invalid Base64-encoded data, or invalid XML).

Relates to tintoy/dotnet-kube-client#94.

tintoy avatar Sep 15 '19 00:09 tintoy

The subject of Concurrency is managed by DataProtection by default. We actually implement only a IXmlRepository. That's just the persistence logic.

Our PersistKeysToKubeSecret function only persists the XML Keys. (like Filesystem, Redis or AuzureBlob)

If you want to control the KeyManagement flow you need to the IKeyManager. That depends heavily on your Scenario.

services.AddDataProtection()
.PersistKeysToKubeSecret(...)
.DisableAutomaticKeyGeneration(); // ReadOnly Keyring

For example, by default a single keyring is generated for each content path. ms-doc To share a keyring between applications, SetApplicationName must be used.

services.AddDataProtection()
.PersistKeysToKubeSecret(...)
.SetApplicationName("shared app name"); // Shared Keyring

For your Querstions we need to look a bit deeper in the IXmlRepository usage.

Read

IReadOnlyCollection<XElement> GetAllElements()

The keymanager request only on startup the Keys with this Method. All Keys will stored in protected Memory by the DataProtection lib. This function is only recalled all 24h or on a Key-Invalidation. ms-doc

Insert / Update

void StoreElement(XElement element, string friendlyName)

By default the friendlyName is just the ID of the key. code-link If the value is not unique, we have completely different problems.

There is no way to delete keys because otherwise existing data will never be decrypted and migrated to a new key.

// Edit In other words, we do not need to worry about multiple instance scenarios. Just like RedisXmlRepository, AzureBlobXmlRepository or RegistryXmlRepository

In addition, the XML Entries are all immutable. ms-doc For example, a Key-Revoke only creates a new revocation XML File (other friendlyName) and never touch the real Key XML. ms-doc

rriverak avatar Sep 15 '19 11:09 rriverak

Thanks! This is exactly the detail I was going to go looking for :)

In that case I may skip locking and simply make a local copy of the reference to the class field (so future maintainers understand they’re working with an isolated copy but the original reference could be replaced at any time by the event handler.

What if the underlying Secret resource is deleted after the app is started? Should we attempt to re-create it if the patch fails because the resource no longer exists?

tintoy avatar Sep 15 '19 20:09 tintoy

Howdy, just curious if there are any plans to revisit this? I am in need of this exact functionality. Happy to help resolve merge conflicts or anything that might help get this across the finish line.

helmsb avatar Apr 01 '24 20:04 helmsb

Hi - I hadn’t looked at this one for quite some time, but I’ll take a look sometime this afternoon and figure out where it’s at 🙂

tintoy avatar Apr 01 '24 21:04 tintoy

Funnily enough, I've been deleting with another DataProtection provider at work so this reminder comes at the perfect time!

There are some other pending changes but they don't have to be included in the next release; I'll need to spend a little time in the next day or 2 adding some tests and documentation for this feature but after that I think it should be relatively quick and painless to get this merged and published in a new package 🙂

tintoy avatar Apr 03 '24 22:04 tintoy

Sorry for the delay; this is proving quite difficult to test without a live K8s instance (and I'm trying to avoid adding tests that can't be run during CI). Still working on it, but it's slow going 😂

tintoy avatar Apr 27 '24 04:04 tintoy

Ok, I think I have a working solution for testing; I'll tidy it up tomorrow morning and then I think we should be good to go 🙂

tintoy avatar Apr 28 '24 03:04 tintoy

Finally! Sorry it's taken so long 😄

tintoy avatar May 19 '24 00:05 tintoy