Skip to content

Add couchbase database plugin to spring-cloud-vault#408

Closed
fhitchen wants to merge 20 commits into
spring-cloud:masterfrom
fhitchen:master
Closed

Add couchbase database plugin to spring-cloud-vault#408
fhitchen wants to merge 20 commits into
spring-cloud:masterfrom
fhitchen:master

Conversation

@fhitchen
Copy link
Copy Markdown

@fhitchen fhitchen commented Jun 22, 2020

Hi,

I have added a couchbase database plugin, Hashicorp are reviewing it and will make it available at https://github.com/hashicorp/vault-plugin-database-couchbase when ready and I also added it to this package.

Note: this plugin is a database plugin, not a secrets plugin.

To run the two tests, you will need a 6.5.0 or newer Couchbase DB with an additional username "vault-static" added for the second static role test.

Use this command to start Couchbase container

docker run -d --name db -p 18091-18096:18091-18096 -p 11207:11207 -p 8091-8094:8091-8094 -p 11210:11210 fhitchen/vault-couchbase

This container already has the Administrator user configured and a test "bucket" added. The tests do not make use of the bucket.

The plugin is available from my own repo https://github.com/fhitche/vault-plugin-database-couchbase and needs to be build and registered in the sys/plugins/catalog/database/ before the tests will pass.

I had to disable the checkstyle plugin or I got these errors...

INFO] --- maven-checkstyle-plugin:3.1.0:check (checkstyle-validation) @ spring-cloud-vault-config ---
[INFO] Starting audit...
[ERROR] /home/fhitchen/git/spring-cloud-vault/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/util/Settings.java:31:1: Utility classes should not have a public or default constructor. [HideUtilityClassConstructor]
[ERROR] /home/fhitchen/git/spring-cloud-vault/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/util/CanConnect.java:29:1: Utility classes should not have a public or default constructor. [HideUtilityClassConstructor]
[ERROR] /home/fhitchen/git/spring-cloud-vault/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/util/TestRestTemplateFactory.java:40:1: Utility classes should not have a public or default constructor. [HideUtilityClassConstructor]
Audit done.

I could not regenerate the README.adoc as I was unable to locate ./docs/src/main/ruby/generate_readme.sh?

I hope you will be able to approve this pull request.

Copy link
Copy Markdown
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your pull request. This PR requires some cleanup and should not touch the buildfile at all. Please check out the comments.

Comment thread pom.xml Outdated
Comment thread spring-cloud-vault-config-databases/pom.xml
Comment thread src/test/bash/create_certificates.sh Outdated
@fhitchen fhitchen requested a review from mp911de June 24, 2020 14:33
@fhitchen
Copy link
Copy Markdown
Author

fhitchen commented Jul 13, 2020 via email

@mp911de
Copy link
Copy Markdown
Member

mp911de commented Jul 14, 2020

I was waiting for the signal from your side that you're done. It's not obvious from occasional commits flying into the PR.

@fhitchen
Copy link
Copy Markdown
Author

fhitchen commented Jul 14, 2020 via email

@fhitchen
Copy link
Copy Markdown
Author

fhitchen commented Jul 15, 2020 via email

@mp911de
Copy link
Copy Markdown
Member

mp911de commented Jul 16, 2020

Generally speaking, all @Bean methods should be defined in @Configuration classes. Check out the failure message: java.lang.IllegalStateException: Test classes cannot include @Bean methods

@fhitchen
Copy link
Copy Markdown
Author

fhitchen commented Jul 31, 2020

Hi, I still have not managed to get the tests to work with the @Autowired dependencies. A simple app does mind you so it is just my lack of knowledge about Spring Boot Annotations etc. that is a fault. I have been busy working on getting the Couchbase database plugin into the Hashicorp Git repo. It is finally there if you want to take a look at it here.

However I am truly stuck now as the build process is not working any more. Once Spencer Gibb has fixed it I will start working on it again.

@mp911de
Copy link
Copy Markdown
Member

mp911de commented Aug 11, 2020

Any update on this pull request?

@fhitchen
Copy link
Copy Markdown
Author

Hi, yes, I just got the maven build to work again (have been busy with Hashicorp getting the plugin into their Git repo) and will try and write a proper test as soon as i figure out the issue i am having with injected dependencies.

@fhitchen
Copy link
Copy Markdown
Author

Hi Mark,
Could you do another review now? I decided to stick with the native couchbase test as the Spring wrapping did not let me test that the credentials were correct as all that layer is abstracted away. You need to be able to call the WaitUntilReady method.

I am seeing this in the Couchbase surefire test logs and have no idea why it would be happening.

com.mongodb.MongoSocketOpenException: Exception opening socket
        at com.mongodb.internal.connection.SocketStream.open(SocketStream.java:70) ~[mongodb-driver-core-4.1.0.jar:na]
        at com.mongodb.internal.connection.InternalStreamConnection.open(InternalStreamConnection.java:143) ~[mongodb-driver-co>
        at com.mongodb.internal.connection.DefaultServerMonitor$ServerMonitorRunnable.lookupServerDescription(DefaultServerMoni>
        at com.mongodb.internal.connection.DefaultServerMonitor$ServerMonitorRunnable.run(DefaultServerMonitor.java:144) ~[mong>
        at java.base/java.lang.Thread.run(Thread.java:834) ~[na:na]
Caused by: java.net.ConnectException: Connection refused (Connection refused)

Copy link
Copy Markdown
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment about the dependencies. Please also squash your changes into a single commit.

Comment thread spring-cloud-vault-config-databases/pom.xml
@fhitchen
Copy link
Copy Markdown
Author

Hi,
A question before I do my first attempt at squashing. I have had to go back to the upstream/master branch a couple of times so I am a bit confused about which commits I should include in the squash.

Should I squash from f7573ac through 6f2bcab ?

* 6f2bcab (HEAD -> master, origin/master, origin/HEAD) fixed pom.xml
* 9e95a4a Switched to use Couchbase sandbox image.
*   50fc519 Merge commit '895df4456a6d61e3f2afb3679c787c5f29fc86b7' To fix broken build
|\  
| * 895df44 Uses new spring-cloud-starter-bootstrap.
| * 053c054 Ignores .sdkmanrc
| * d65368c Updates to use repo.spring.io/snapshot
| * d1ecef4 Going back to snapshots
| * b8a7610 Update SNAPSHOT to 3.0.0-M3
* | 27cde25 Fixed checkstyle import issues.
* | 1a35a2b Updated dynamic role test to use new role+group creation statement for the couchbase plugin.
* | 922b379 Fixed issues from the PR review.
* | 66bdef6 Fixed issues from the PR review.
* | 2a03cbc Fixed issues from the PR review.
* | 077e525 Fixed issues from the PR review.
* | c1d3edd Fixed issues from the PR review.
* | 7ed535e Added couchbase.
* | 6d587d1 Added note.
* | 511564c Added static role test for Couchbase.
* | 1c94f7f added documentation.
* | 4447b47 Working couchbase solution.
* | 5065b3c Added correct couchbase-database-plugin test file.
* |   7e72257 Merge remote-tracking branch 'upstream/master'
|\ \  
| |/  
| * ef59dcd Adds junit-vintage-engine as a test dependency
| * 5ba60c3 Changed packaging to jar
| * 970c648 Uploading sources for docs
| * bf890e0 Bumping versions
* | 454fbc7 Initial fumblings
* | 17d082b Worked.
* | f7573ac Start adding couchbase db to spring cloud vault
|/  
* 89dd6ab Going back to snapshots

@spencergibb
Copy link
Copy Markdown
Member

No need to squash, we can do it when it is merged

@fhitchen
Copy link
Copy Markdown
Author

Ok thanks. To test it against an official couchbase db, after you have installed the plugin, instructions here, spin up this docker container using the following command...

$ docker run -d --name db -p 18091-18096:18091-18096 -p 11207:11207 -p 8091-8094:8091-8094 -p 11210:11210 couchbase/server-sandbox:6.5.0

@fhitchen
Copy link
Copy Markdown
Author

Are you waiting on anything from me here?

@spencergibb
Copy link
Copy Markdown
Member

Mark is on leave right now

@fhitchen
Copy link
Copy Markdown
Author

fhitchen commented Aug 24, 2020 via email

@mp911de mp911de added this to the 3.0.0-M4 milestone Sep 21, 2020
@mp911de
Copy link
Copy Markdown
Member

mp911de commented Sep 21, 2020

I'm going to take this PR from here and merge it for 3.0-M4

@mp911de
Copy link
Copy Markdown
Member

mp911de commented Sep 21, 2020

This pull request is pretty much a mess. The Couchbase integration with Spring Boot never worked as the property names don't match and VaultConfigCouchbaseTests looks like an unfinished copy with references to Cassandra. It took quite some hours to get the tests working. Once this PR is merged, please take a look at the polishing commit.

mp911de pushed a commit that referenced this pull request Sep 21, 2020
@mp911de mp911de closed this in a60c519 Sep 21, 2020
@mp911de
Copy link
Copy Markdown
Member

mp911de commented Sep 21, 2020

That's merged and polished now.

spencergibb pushed a commit that referenced this pull request Sep 14, 2023
spencergibb pushed a commit that referenced this pull request Sep 14, 2023
Fix property names to spring.couchbase instead of spring.data.couchbase. Fix VaultConfigCouchbaseTests to make it work with Couchbase instead of using Cassandra.

Add CouchbaseSecretIntegrationTests to verify interaction through VaultConfigTemplate. Remove property overrides for username/password so that Vault PropertySources are used. Use unique role name to avoid clashes with other tests.

Enable VaultConfigCouchbaseDatabaseTests for ConfigData API by importing vault://. Update reference documentation.

Resolves gh-408.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants