KBS: Add a Delete method to /resource/{repository}/{type}/{tag}

[davidhadas]

KBS API supports the following resource manipulation:

 /resource/{repository}/{type}/{tag}:
    get:
      operationId: getResource
      summary: Get a secret resource from the Key Broker Service.
       ... 
    post:
      operationId: registerSecretResource
      summary: Register a secret resource into the Key Broker Service.
      ...

This API is missing the option to delete resources that were added.

It is suggested that we add:

delete:
      operationId: deleteSecretResource
      summary: Delete a secret resource from the Key Broker Service.
      parameters:
        - name: repository
          in: path
          description: A parent path of resource, can be empty to use the default repository.
          schema:
            type: string
          required: false
        - name: type
          in: path
          description: Resource type name
          schema:
            type: string
          required: true
        - name: tag
          in: path
          description: Resource instance tag
          schema:
            type: string
          required: true

Use Case:
Under Peer-Pods Secure Comms, Keys are added to the KBS for each Peer Pod created.
It is required to delete such keys once a Peer Pod is deleted.
Without delete support, KBS will explode over time as keys will continue to be added and never deleted.

[davidhadas]

CC: @bpradipt, @fitzthum, @portersrc

[Xynnn007]

comment: we should ensure that the delete operation is performed by the admin, which is a different persona of the one performing GET.

Also, I am not sure if anybody is deploying KBS resources as configmap, thus they are not deletable. cc @mkulke

[davidhadas]

@Xynnn007, an entity having privileges to add a resource should also have the privilege to delete it.

It seems that there are two types of approaches that are now being served:

• Clients authenticated using an attestation token can read keys
• Clients authenticated using a key pair can add keys (I assume also read keys) and here it is suggested that these clients would also be able to remove keys. << I think this is the closest thing we have now to "Admins"

Keys are now stored in localFS, it is suggested here that the key files would be deleted when delete is performed.

[mkulke]

If there's an endpoint that creates resources then I agree, it makes sense to add one that is able to delete resources for consistency's sake (and probably also PUT /resource to cover the whole CRUD) machinery.

But yeah, if the secrets are backed by some external CSI store (which I'd recommend, atm), it's likely that the resources will be read-only to KBS and those write calls will fail.

Maybe we should be able to disable the resource management endpoints via cli option to avoid confusion.

[larrydewey]

I agree that if we are going to add in administrative permissions, we should definitely make it a complete story. That being said, I don't necessarily agree that we should be doing any hard-deletes of any data, though. Compliance laws definitely differ by country, so we should be careful about how we roll this out. Perhaps have it be configurable, or perform a soft-delete and require manual pruning of the records?

[fitzthum]

I am in favor of adding a delete endpoint. This will probably mean adding to our resource backend trait. Also, we might want to disable this in certain cases or even not implement it for certain backends to help with the state issues mentioned above.