search

JetBrains / teamcity-hashicorp-vault-plugin

TeamCity plugin to support HashiCorp Vault
Apache License 2.0
28 stars 18 forks source link

Add Vault Namespace Support #31

Closed jeffwecan closed 4 years ago

jeffwecan commented 4 years ago

Resolves #20

~Creating a draft PR to add Vault Namespace support to this plugin. I'm still exploring some ways to better implement the feature but wanted to go ahead and open a PR to allow (potentially) soliciting feedback.~

https://github.com/JetBrains/teamcity-hashicorp-vault-plugin/issues/20#issuecomment-500859676 already does a great job of breaking down the goal of this PR with respect to Vault's enterprise namespace feature. As-is, this PR kludges in a duplicative createNamespaceInterceptor method that mirrors what was added in the upstream spring-vault library. I attempted to update to the latest spring-vault release for this codebase to avoid the duplication, but hit too many areas requiring updates that were beyond my skill set. In particular, the existing duplicated ClientHttpRequestFactoryFactory and LifecycleAwareSessionManager classes require substantial refactoring to operator correctly with the latest spring-vault release (2.2.1.RELEASE).

VladRassokhin commented 4 years ago

Looks good, thank you for contribution. Will review, test other functionality and merge hopefully next week. Since I don't have Vault Enterprise I'd blindly assume it works :)

I'm totally against updating spring-vault library, even more I'm thinking about getting rid of it since I've to reimplement a lot (duplicated classes) to just support features like retry, backoff, https certificates management, etc. Doubt library improved significantly. Also version 2.X requires some spring deps I'd like to keep away.

VladRassokhin commented 4 years ago

I've amended it a bit and manually merged into master. Thanks again for contribution!

jeffwecan commented 4 years ago

Since I don't have Vault Enterprise I'd blindly assume it works :)

For what it is worth, the enterprise binaries available on https://releases.hashicorp.com/vault/ start licensed with a expiry time 30 minutes from startup (so long enough for integration testing and the like). While I wish I could have sorted added some test coverage, I can at least individually attest to the functionality working when I tested it out locally ;)

Though if I had more Java knowledge / time, I would have really liked to include some additional test coverage as well. I suppose we can loop back on that point if there are any reports of unexpected behavior, etc...

jeffwecan commented 4 years ago

Oh and @VladRassokhin, when would y'all be able to publish a new version of the plugin with these additions? I'm assuming based on the latest update dates listed in the plugin directory a new release hasn't been made as of yet?

Thanks again!

VladRassokhin commented 4 years ago

Given that now there're enterprise binaries available, I'll try to set up tests using testcontainers (we already use them, just need docker container with Vault Enterprise).

After that I'll publish plugin to plugins.jetbrains.com. Meanwhile you could download latest build from CI

VladRassokhin commented 4 years ago

Added tests, published

jeffwecan commented 4 years ago

Fantastic, thanks so much @VladRassokhin!