redhat-cop / vault-config-operator

An operator to support Haschicorp Vault configuration workflows from within Kubernetes
Apache License 2.0
111 stars 49 forks source link

deleting RandomSecret resource not removing secret in vault #134

Closed kartikeyavashishth closed 1 year ago

kartikeyavashishth commented 1 year ago

secret created in vault is not cleaned up when the Randomsecret CR is deleted.

following is the randomsecret being used:

apiVersion: redhatcop.redhat.io/v1alpha1
kind: RandomSecret
metadata:
  name: test-random-secret
spec:
  authentication: 
    path: kubernetes
    role: test
    serviceAccount:
      name: test
  isKVSecretsEngineV2: true
  path: namespace/test-random-secret-a/
  secretKey: secret
  secretFormat:
    passwordPolicyName: 30-char

this can be replicated by creating (by adding YAML) and deleting Randomsecret on Openshift console or adding Randomsecret in helm chart.

erlisb commented 1 year ago

+1 We do have same issue in Openshift, not only for RandomSecrets but also for other VCO supported objects.


From: kartikeya vashishtha @.> Sent: Tuesday, March 21, 2023 3:14:43 PM To: redhat-cop/vault-config-operator @.> Cc: Subscribed @.***> Subject: [redhat-cop/vault-config-operator] deleting RandomSecret resource not removing secret in vault (Issue #134)

secret created in vault is not cleaned up when the Randomsecret CR is deleted.

following is the randomsecret being used:

apiVersion: redhatcop.redhat.io/v1alpha1 kind: RandomSecret metadata: name: test-random-secret spec: authentication: path: kubernetes role: test serviceAccount: name: test isKVSecretsEngineV2: true path: namespace/test-random-secret-a/ secretKey: secret secretFormat: passwordPolicyName: 30-char

this can be replicated by creating (by adding YAML) and deleting Randomsecret on Openshift console or adding Randomsecret in helm chart.

— Reply to this email directly, view it on GitHubhttps://github.com/redhat-cop/vault-config-operator/issues/134, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJSHCGW2RK7CVQMMWUNTGILW5GZVHANCNFSM6AAAAAAWCQDSU4. You are receiving this because you are subscribed to this thread.Message ID: @.***>

kartikeyavashishth commented 1 year ago

update: on further analysis we have noticed upon deleting Randomsecret CR the value of the secret is cleared from Vault, however the key that was created by CR is not removed.

trevorbox commented 1 year ago

If the RandomSecret is KV Secrets Engine v1, both the key and value of the secret is deleted. The path is not deleted. If the RandomSecret is KV Secrets Engine v2 only the value is deleted. The key and path remains. I'm curious, what level of "clean up" is expected and what problems emerge if things are not "cleaned up"? Scenarios would be helpful.

kartikeyavashishth commented 1 year ago

one of the current use-case we have is that we have Randomsecret CR as part of a helm-chart for DB instance. for any one/any pipeline using the the chart to create an instance, the password is created using RandomSecret.

each instance creates a Key and value, and as per current working "cleanup" in vault, value is being deleted and key remains, leading to list of keys in vault what would never be used again

davoustp commented 1 year ago

@trevorbox we do have a similar situation.

I find the default behavior safe and sound (this prevents to erroneously delete a critical secret, which can be recovered using kv-v2 version management).

But there are also automation-related use cases where being able to fully remove a kv-v2 secret (read: all its versions) when the CR is removed from k8s would be helpful.

So I would propose to be able to control the cleanup behavior using a new optional attribute into the RandomSecret resource, named clearAllVersionsOnDelete (default value: false), relevant only when isKVSecretsEngineV2 is set to true. When this new attribute is set to true, the controller would trigger a DELETE onto the metadata endpoint instead of the data endpoint.

I've got code and tests handy, so I can then submit a PR with all this for you to review - just let me know!

trevorbox commented 1 year ago

I believe the current behavior is the kvv2 randomsecret is permanently deleted. @davoustp perhaps your suggestion can be a separate feature enhancement? I believe this issue should be closed as of #174

davoustp commented 1 year ago

@trevorbox I agree, this is a connected but separate improvement. I'll drop a new issue and related PR as soon as I can.