search

hashicorp / vault-plugin-auth-kubernetes

Vault authentication plugin for Kubernetes Service Accounts
https://www.vaultproject.io/docs/auth/kubernetes.html
Mozilla Public License 2.0
206 stars 62 forks source link

CreateOperation should only be implemented alongside ExistenceCheck #176

Closed maxb closed 1 year ago

maxb commented 1 year ago

See hashicorp/vault#18492

benashz commented 1 year ago

I suppose an alternative approach would be to add the proper ExistenceCheck function, and preserve the backend's current behavior. I wonder if you had considered that approach?

maxb commented 1 year ago

This PR does preserve the backend's current behaviour. Applying it will not change the behaviour of the API that users experience.

It is not clear to me what you would consider to be a proper ExistenceCheck for a /config endpoint. I'd consider that to be no ExistenceCheck, because from the user perspective, they "create" the backend when they enable/mount it, and then they "update" the configuration of it - whilst the first ever write of a backend's config following mounting will be a create from the perspective of Vault's logical storage layer, I don't think users see it that way.

Vault is not very consistent in this though - I did a quick assessment of the capabilities supported by existing /config endpoints in Vault:

update only:

update and delete:

create and update:

create, update and delete:

benashz commented 1 year ago

This PR does preserve the backend's current behaviour. Applying it will not change the behaviour of the API that users experience.

I was more thinking of the behavior expected by the user, in the case of the create flow w.r.t ACL policies.

It is not clear to me what you would consider to be a proper ExistenceCheck for a /config endpoint. I'd consider that to be no ExistenceCheck, because from the user perspective, they "create" the backend when they enable/mount it, and then they "update" the configuration of it - whilst the first ever write of a backend's config following mounting will be a create from the perspective of Vault's logical storage layer, I don't think users see it that way.

No configuration exists in the storage is the case I was thinking of.

Vault is not very consistent in this though - I did a quick assessment of the capabilities supported by existing /config endpoints in Vault:

Definitely true. It would be ideal if there was one common model being followed, but that is not always possible.

maxb commented 1 year ago

I was more thinking of the behavior expected by the user, in the case of the create flow w.r.t ACL policies.

Yes, this is preserved unchanged.