Add a database plugin to enable account management in the Couchbase NOSQL Database

[fhitchen]

Hi,
I have updated the plugin to support versions prior to the latest 6.5.0 version of Couchbase and incorporated suggestions from their engineering team regarding the use of the gocb SDK.

To do is to add support for provisioning accounts that use the group roles supported by Couchbase. I don't think this will be difficult, but haven't got around to it yet. **Note:**This has been done. Also need to add a few utilities to create test accounts and buckets in the off the shelf Couchbase Docker image rather than rely on the modified personal copy.

I was unable to think of a way to add a more sophisticated static role password rotation test, I hope you don't mind.

[tomhjp]

Hi @fhitchen, thanks for the fantastic contribution! Overall this looks really good, and it's great to see such good documentation and test coverage too.

I've left quite a few comments, but a lot are minor Go idioms/linting/naming etc. The main things I'd like to get sorted before landing this are:

• Context timeouts respected
• logrus -> hclog
• The docker image and its ports used in the tests
• And a few places where defer will improve the robustness

Please let me know if you need any further support or when you're ready for more reviews. Thanks!

[fhitchen]

Hi Tom, I'm done, ready for the next review. I have included tests for the current and the last two versions of couchbase. regards, Francis.

…

On Thu, Jul 23, 2020 at 7:03 AM Tom Proctor ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In couchbase_test.go <#1 (comment)> : > + return func() {}, os.Getenv("COUCHBASE_HOST"), 0 + } + + if containerInitialized == true { + return cleanup, "localhost", 0 + } + + pool, err := dockertest.NewPool("") + if err != nil { + t.Fatalf("Failed to connect to docker: %s", err) + } + + ro := &dockertest.RunOptions{ + Repository: "docker.io/fhitchen/vault-couchbase", + Tag: "latest", + ExposedPorts: []string{"8091", "8092", "8093", "8094", "11207", "11210", "18091", "18092", "18093", "18094"}, Yeah I think docker networking is a better idea, although stepping back, I wonder if maybe it's best left as-is after all. As long as this is the only test suite in the repo that runs the couchbase container (a reasonable constraint), the fact that the tests are running in a dedicated VM means this actually shouldn't be problematic with hardcoded ports. Given the difficulty and complexity of engineering around those fixed ports, it seems pragmatic to live with the constraint for now, and then reassess if it does cause issues down the line. WDYT? Apologies for the wild goose chase on this one - I've been learning on the go about CircleCI and couchbase, so didn't have the full picture before. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABML24H73ABNQAODMAHIFIDR5ARI5ANCNFSM4OA3HPJQ> .

[tomhjp]

Looking good! Thanks for addressing all the feedback. Not many comments left from me, mostly just some tidying up.

[tomhjp]

LGTM! Just two tiny things to tidy up.

There was a surprising flake in the CI here (hopefully you can see that?) but it hasn't happened again in 5 retries. Future PRs should have the CI run automatically now that the repo has been set up in our CI plan.

[fhitchen]

I think the flakiness is due to the hard coded timeout in the connection_producer Connection method. I should have replaced it with the Context based timeout. I pumped up the CPU load on my old Linux box and have managed to reproduce 2 failures as it waits for the cluster to be ready after connecting.

I should fork the hashicorp repo now to make this change?

Also, will someone update the main hashicorp documentation to add couchbase to the secret engines -> database documentation?

Yup, with the Connection method using a Context and the default timeout upped from 5 seconds to 20 seconds, the testing flakiness is resolved. I have run the test for several hours on one of the older database versions (they are much slower) and did not see a single failure.

[tomhjp]

I think the flakiness is due to the hard coded timeout in the connection_producer Connection method. I should have replaced it with the Context based timeout. I pumped up the CPU load on my old Linux box and have managed to reproduce 2 failures as it waits for the cluster to be ready after connecting.

Yup, with the Connection method using a Context and the default timeout upped from 5 seconds to 20 seconds, the testing flakiness is resolved. I have run the test for several hours on one of the older database versions (they are much slower) and did not see a single failure.

That's great! Thanks for looking into that.

I should fork the hashicorp repo now to make this change?

That would be great. Hopefully if you make a fresh fork and PR, the CI should work properly this time too as it has been set up prior to the PR creation.

Also, will someone update the main hashicorp documentation to add couchbase to the secret engines -> database documentation?

Yep, I've got a few follow up tasks to get it integrated and packaged with vault releases, one of which is documentation. You'll see a bit of activity from me soon on these pieces.