kube-object-storage / lib-bucket-provisioner

Library for the dynamic provisioning of object store buckets to be used by object store providers.
Apache License 2.0
20 stars 22 forks source link

define Delete and Revoke wrt reclaimPolicy #64

Closed jeffvance closed 5 years ago

jeffvance commented 5 years ago

The current design is to call the Delete method only for new buckets (greenfield) with a reclaimPolicy of "Delete". All other cases call Revoke.

The problem with this is that provisioners cannot distinguish between Revoke called on a greenfield bkt's delete vs called for any brownfield bkt, regardless of its reclaimPolicy. And, perhaps, some provisioners will want to distinguish these two cases.

We could change the design such that Delete is always called when the reclaimPolicy == "Delete" and Revoke is always called when the reclaimPolicy == "Retain". And provisioners should delete the bkt when Delete is called, even if it was a brownfield. bkt. If this is not the desired behavior then the storage class's reclaimPolicy should not be "delete".

This approach keeps the library neutral on greenfield vs. brownfield and relies simply on the reclaim policy.

copejon commented 5 years ago

If this is not the desired behavior then the storage class's reclaimPolicy should not be "delete".

This feels a little to laissez faire WRT to data. In keeping with the policy of cleaning up only what we create, I don't think it should be possible for the library to delete buckets it didn't provision.

Rather, an OB field indicating that it was a provisioned bucket (e.g. BucketProvisionedBy: <provisioner>) is a necessary flag to key deletes off of.

copejon commented 5 years ago

Per our call - it's agreed that protecting static buckets from accidental deletion is the preferred course of action. This means that a field indicating whether it was a dynamically provisioned bucket or a static bucket will override the reclaim policy.

If the bucket is static, reclaim policy of Delete & Erase should be ignored and instead credentials should only be Revoked.

cc @jeffvance before I close, can you TAL to see if you agree?

jeffvance commented 5 years ago

Agree with concerns.