BetterCloud / vault-java-driver

Zero-dependency Java client for HashiCorp's Vault
https://bettercloud.github.io/vault-java-driver/
335 stars 224 forks source link

X-Vault-Token should be optional #183

Closed jetersen closed 5 years ago

jetersen commented 5 years ago

Trying to add support for interacting with Vault agent: https://github.com/jenkinsci/hashicorp-vault-plugin/pull/36 Found out after much headache my issue was with vault-java-driver because it assumes vault-token is mandatory, which it is not in the case of the auto-auth use case for Vault agent. The null pointer results in 400 Bad Request.

image

Example of vault cli against the very same endpoint (ngrok used for inspection purpose :sweat:)

$ VAULT_ADDR=http://0c19b983.ngrok.io vault kv get kv-v1/admin
==== Data ====
Key     Value
---     -----
key1    123
key2    456

You can set up the vault agent to use auto-auth and handle the requesting the token. So the application does not need to be concerned about caching or reauthenticating against vault.

https://www.vaultproject.io/docs/agent/

steve-perkins commented 5 years ago

The associated pull request has been merged into master. However, I am seeing one broken integration test (AuthBackendDatabaseTest.testGetCredentials) when running the suite:

Aug 16, 2019 5:38:27 PM com.bettercloud.vault.Vault <init>
INFO: Constructing a Vault instance with no provided Engine version, defaulting to version 2.

Vault responded with HTTP status code: 500 {"errors":["1 error occurred:\n\t* failed to find entry for connection with name: \"postgres\"\n\n"]}

com.bettercloud.vault.VaultException: Vault responded with HTTP status code: 500 {"errors":["1 error occurred:\n\t* failed to find entry for connection with name: \"postgres\"\n\n"]}

    at com.bettercloud.vault.api.database.Database.creds(Database.java:350)
    at com.bettercloud.vault.api.AuthBackendDatabaseTests.testGetCredentials(AuthBackendDatabaseTests.java:95)
...

Based on the stacktrace above, I suspect this is related to PR #175 from @denglertai... which was merged into master shortly before this pull request.

Both the unit test suite and integration test suites passed successfully after #175 was merged, but it doesn't seem to be playing nice with #183 here. Could you guys (@casz and @denglertai) please try running the integration tests from master on your own machines, and see if you can reproduce the issue?

Aside from making sure that the test suites pass, I would like to give @bklawans a chance to respond to a question I've posed on PR #176. Either way, hopefully a new binary release can go out to Maven Central next week.

jetersen commented 5 years ago

I suspect it's a container issue? Spent a little time getting no where.

denglertai commented 5 years ago

I haven't had time to have a look at the issue yet and I won't, unfortunately, find time to have a look at it this week. But from the the error message itself it seems like that for some reasons the connection to the database has not been established. I'll try to have a look at the problem next week

jetersen commented 5 years ago

D0h 😆 I forgot about the fact that I decided to use withNetwork and withNetworkAliases which the DbContainer was not apart of.

I have some WIP that fixes code maturity and which is actually where I found the bug when rewriting the containers to extends GenericContainer<Class>

This should fix the integration tests all together

jetersen commented 5 years ago

I think the proper follow up is actually #186