Support key deletion in Data Protection

[amcasey]

Add the ability to delete keys to support #52916. The approach is more complicated than you'd expect to account for the fact that IXmlRepository offers only two (possibly slow) operations: enumerate and add - there's no random access.

Deleting keys remains discouraged.

Fixes #53880

[amcasey]

I had to move to per-target-platform API manifests because DIM isn't available on framework.

[amcasey]

I'll add tests once the API is reviewed, but the shape has changed several times already.

[amcasey]

Force push is a rebase - I'll address API Review feedback separately.

[amcasey]

I also prepared a version where the key manager gets to return an ordered list of IDeletableElements to delete, but I was unhappy with the fact that the implementation could return duplicate elements or elements that were not in the list it received. These are both solvable problems, but every repository implementation would need these safeguards, which I found less appealing than having every repository sort.

[dotnet-policy-service[bot]]

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

[amcasey]

Force push is a trivial rebase on main.

[amcasey]

TODO: Redis impl TODO: add tests

Edit: tests added, redis & EF impls deferred

[amcasey]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 3 pipeline(s).

[amcasey]

I'm definitely a little curious how my new tests could pass only on Windows. Investigating.

Edit: Opening a file with FileShare.None doesn't prevent its deletion on Linux (where there is also no registry 🤦).

[donnyv]

I found this while building a custom repository for Mongodb.

One idea that may be to late, since you look like you are pretty far along. You could simplify this by just adding a DeleteElement(string keyid) to IXmlRepository. Then let devs developing their own repositories to create the delete code. DeleteElement() method could be fired by Data Protection using a TTL. A TTL could be added to every key automatically when its created. Since the minimum expiration that can be set to a Key is 7 days. Data Protection could check the TTL every 7 days and fire deletes for all the expired keys. TTL date could be (expired date + time to live length) = TTL. Time to live length could be a setting with a default of (expired date * 2). I would make it so that devs would have to turn this feature on too.

[amcasey]

@donnyv The reason we didn't add DeleteElement(string keyid) is that random access is not presently part of the IXmlRepository contract and a number of existing implementations (e.g. file system) couldn't implement it efficiently.

Having said that, it was certainly our intention that IXmlRepository implementers would be able to provide their own implementations. Are you finding that not to be the case?

Unless we hear a lot of demand (feel free to open an issue to collect feedback), I don't think we're likely to add an automated TTL mechanism. Generally speaking, we recommend keys never be deleted, so we want doing so to be a very explicit action on the app author's part.