spring-projects / spring-vault

Provides familiar Spring abstractions for HashiCorp Vault
https://spring.io/projects/spring-vault
Apache License 2.0
284 stars 186 forks source link

Make the default LeaseStrategy configurable and default to retainOnIOException for more resilient applications #839

Closed bendem closed 1 year ago

bendem commented 1 year ago

I just learnt about #646 today. If I understand correctly, by default, if there is an network error during renewal, spring-vault will not re-attempt to renew whatever failed to renew.

There is no property to change the default LeaseStrategy in use that I could find.

https://github.com/spring-projects/spring-vault/blob/cdb687e4bab0aaa43b089f6fb158b1d46d6f369f/spring-vault-core/src/main/java/org/springframework/vault/core/lease/SecretLeaseContainer.java#L158

Can the default be changed to be more sensible (retry on IO error is pretty standard to have resilient software) and can we introduce a flag to chose between the 3 different behaviour without requiring code change?

mp911de commented 1 year ago

Have you seen SecretLeaseContainer.setLeaseStrategy(…)?

Generally, retrying at the lease container level isn't in particular a great idea as retries belong into an infrastructure abstraction such as the HTTP Client or Spring's ClientHttpRequest.

Also, Apache's HTTP Client has a facility to retry on I/O exceptions or status codes which gives much more control over retries than trying to rebuild something in Spring Vault that actually belongs somewhere else.

bendem commented 1 year ago

Have you seen SecretLeaseContainer.setLeaseStrategy(…)?

I have but we rely on autoconfig from spring boot, we don't actually have any code calling into vault (https://docs.spring.io/spring-cloud-vault/docs/current/reference/html/#vault-lease-renewal), so unless it's exposed as a property, we can't really use it.

I realise now that this might have been better filed against https://github.com/spring-cloud/spring-cloud-vault/ which contains spring-cloud-starter-vault-config.

mp911de commented 1 year ago

Alright, thank you. Indeed, Spring Cloud Vault is the place to introduce properties to pick up configuration details from config flags.