feat(modules.cockroachdb) Adds cockroachdb module #2131

rcrowe · 2024-01-21T18:52:51Z

What does this PR do?

Adds CockroachDB as a module

TLS support via cockroachdb.WithTLS & the helper cockroachdb.NewTLSConfig
COCKROACH_DATABASE support via cockroachdb.WithDatabase
COCKROACH_USER support via cockroachdb.WithUser
COCKROACH_PASSWORD support via cockroachdb.WithPassword

Why is it important?

Previously an example only, a module lets us develop it over time to include features like TLS.

netlify · 2024-01-21T18:52:56Z

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	`9153188`
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/65b67b2e983bdd0008a5557b
😎 Deploy Preview	https://deploy-preview-2131--testcontainers-go.netlify.app/modules/cockroachdb
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mdelapenya

LGTM, thanks for this new module! I only left a comment regarding the WithImageTag option. Once discussed, I think this PR is ready to be merged, thanks!

docs/modules/cockroachdb.md

mdelapenya · 2024-01-22T10:26:47Z

@rcrowe the errors in the rootless-mode pipeline are not related to this PR but to an automatic bump in the Github worker, which is installing Docker v25.0.0 since a few days ago. I'm talking to the Moby team to check if that's a regression or the expected behaviour, because the failures are consistent:

One of the failures is related to an error message that seems to have changed, when build target is not found, from failed to reach build target target-foo in Dockerfile to target stage 'target-foo' could not be found #46754
- I think we can simply check for the error and avoid asserting for a specific text in the error message
Second error is related to the number of network aliases for the the bridge network, which in rootless mode seems to have changed from 0 to 1: Expected number of aliases for 'bridge' network 0. Got 1.
- I'm not sure about this one, as in non-rootless mode, the same test passes

rcrowe · 2024-01-22T13:41:51Z

Thanks @mdelapenya

Gives me some time to finish TLS support & push that up as a separate PR, unless you want me to include it here?

modules/cockroachdb/cockroachdb.go

rcrowe · 2024-01-23T10:09:45Z

@mdelapenya I'm assuming docker rootless failing here because it's flaky, notice on main that it passed, rather than anything in my changeset?

Have addressed the feedback of COCKROACH_DATABASE, COCKROACH_USER & COCKROACH_PASSWORD, along with TLS support. The PR grew a little because of that, so apologies for the larger diff.

mdelapenya

Thanks for your hard work adding this new module. I left a few comments that are non-blocking, so I think we can merge the module once discussed.

In any case, great job!

modules/cockroachdb/cockroachdb.go

docs/modules/cockroachdb.md

modules/cockroachdb/cockroachdb.go

rcrowe · 2024-01-25T21:53:49Z

@mdelapenya 🤞🏻 this is in a shape to get merged & iterate once it's in the hands of people 🤞🏻

Thanks for your feedback 🙇🏻

mdelapenya

LGTM, thank you for your contribution!

I added a few minor comments, once they are addressed, we are good to go! 🚀

modules/cockroachdb/cockroachdb.go

Cockroach wasn't being consistent with log messages to use that as a reliable mechanism across the different authentication methods. This commit makes an actual SQL query to validate CRDB is correctly running. So we can make use of sql.DB driver and `wait.ForSQL` we've had to register a connection URL that includes TLS config, as there's no way to do that through standard connection strings.

Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

docs/modules/cockroachdb.md

Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

mdelapenya

LGTM, thanks for this contribution and for your patience while reviewing the code.

Merging it now

* main: chore(deps): bump github.com/docker/compose/v2 in /modules/compose (testcontainers#2162) feat(modules.cockroachdb) Adds cockroachdb module (testcontainers#2131)

* main: (74 commits) chore(deps): bump github.com/docker/compose/v2 in /modules/compose (testcontainers#2162) feat(modules.cockroachdb) Adds cockroachdb module (testcontainers#2131) chore(deps): bump golang.org/x/crypto in /modules/minio (testcontainers#2161) chore(deps): bump golang.org/x/crypto in /modules/openldap (testcontainers#2165) chore(deps): bump github.com/google/uuid from 1.5.0 to 1.6.0 (testcontainers#2169) chore(deps): bump google.golang.org/api from 0.156.0 to 0.159.0, google.golang.org/grpc from 1.60.1 to 1.61.0, cloud.google.com/go/pubsub from 1.33.0 to 1.35.0 in /modules/gcloud (testcontainers#2168) chore(deps): bump github.com/hashicorp/consul/api in /examples/consul (testcontainers#2152) chore(deps): bump github.com/couchbase/gocb/v2 in /modules/couchbase (testcontainers#2145) chore(deps): bump k8s.io/api, k8s.io/apimachinery and k8s.io/client-go from 0.29.0 to 0.29.1 in /modules/k3s (testcontainers#2167) chore: do not compile modules on macos workers on GH (testcontainers#2164) Openldap module support (testcontainers#2117) Adding inbucket module (testcontainers#2142) testifylint: enable compares rule (testcontainers#2143) Bump containerd version to v1.7.12 (testcontainers#2137) feat: Add Minio module (testcontainers#2132) Adding LogConsumers start as part of the ContainerRequest (testcontainers#2073) chore: bring back assertion for network aliases for bridge in rootless mode (testcontainers#2141) chore(deps): bump github.com/docker/compose/v2 from 2.23.3 to 2.24.0 in /modules/compose (testcontainers#2096) chore(deps): bump github.com/dvsekhvalnov/jose2go in /modules/pulsar (testcontainers#2136) fix: skip-host-cache option removed in latest MySQL 8.3.0 version (testcontainers#2130) ...

* main: (33 commits) feat (postgres): support for creating and restoring Snapshots (testcontainers#2199) fix: apply volume options only to volumes (testcontainers#2201) redpanda/test: add admin client call (testcontainers#2200) chore(deps): bump cloud.google.com/go/spanner from 1.55.0 to 1.56.0 in /modules/gcloud, cloud.google.com/go/pubsub from 1.35.0 to 1.36.1 in /modules/gcloud, cloud.google.com/go/bigquery from 1.57.1 to 1.58.0 in /modules/gcloud (testcontainers#2197) chore(deps): bump github.com/docker/docker from 25.0.1+incompatible to 25.0.2+incompatible (testcontainers#2196) fix: go doc reference broken image (testcontainers#2195) Add Support for WASM Transforms to Redpanda Module (testcontainers#2170) feat(modules.clickhouse): Add zookeeper for clickhouse clusterization (testcontainers#1995) redpanda: allow using SASL and TLS together (testcontainers#2140) chore: do not panic in testable examples (testcontainers#2193) fix: all mounts should contain the testcontainers labels (testcontainers#2191) [redpanda] sasl test for wrong mechanism (testcontainers#2048) fix: deprecate BindMounts correctly (testcontainers#2190) chore(docker_mounts): stop doing misleading logging (testcontainers#2178) fix: Add HTTPStrategy WithForcedIPv4LocalHost To Fix Docker Port Map (testcontainers#1775) chore(deps): bump github.com/docker/compose/v2 in /modules/compose (testcontainers#2162) feat(modules.cockroachdb) Adds cockroachdb module (testcontainers#2131) chore(deps): bump golang.org/x/crypto in /modules/minio (testcontainers#2161) chore(deps): bump golang.org/x/crypto in /modules/openldap (testcontainers#2165) chore(deps): bump github.com/google/uuid from 1.5.0 to 1.6.0 (testcontainers#2169) ...

rcrowe requested a review from a team as a code owner January 21, 2024 18:52

mdelapenya reviewed Jan 22, 2024

View reviewed changes

docs/modules/cockroachdb.md Outdated Show resolved Hide resolved

mdelapenya self-assigned this Jan 22, 2024

mdelapenya added the enhancement New feature or request label Jan 22, 2024

rcrowe requested a review from mdelapenya January 22, 2024 08:18

eddumelendez reviewed Jan 22, 2024

View reviewed changes

modules/cockroachdb/cockroachdb.go Show resolved Hide resolved

rcrowe mentioned this pull request Jan 22, 2024

feat(modules.cockroachdb) TLS support #2133

Closed

eddumelendez reviewed Jan 22, 2024

View reviewed changes

modules/cockroachdb/cockroachdb.go Outdated Show resolved Hide resolved

mdelapenya mentioned this pull request Jan 22, 2024

Add Must methods for other database modules #2134

Closed

rcrowe force-pushed the module-cockroachdb branch 2 times, most recently from 0a6b8ca to 9e4fa57 Compare January 22, 2024 21:39

rcrowe commented Jan 23, 2024

View reviewed changes

modules/cockroachdb/cockroachdb.go Show resolved Hide resolved

rcrowe requested a review from eddumelendez January 23, 2024 11:48

mdelapenya reviewed Jan 24, 2024

View reviewed changes

modules/cockroachdb/cockroachdb.go Show resolved Hide resolved

docs/modules/cockroachdb.md Outdated Show resolved Hide resolved

modules/cockroachdb/cockroachdb.go Show resolved Hide resolved

rcrowe requested a review from mdelapenya January 25, 2024 21:53

mdelapenya reviewed Jan 26, 2024

View reviewed changes

modules/cockroachdb/cockroachdb.go Show resolved Hide resolved

modules/cockroachdb/cockroachdb.go Outdated Show resolved Hide resolved

rcrowe added 9 commits January 27, 2024 10:56

modulegen cockroachdb init

05c8486

start insecure crdb node

ca4e0e4

Method & options to docs

346ae4d

Use more standard naming for connection

708a055

Remove examples/cockroachdb

e530b3f

Cover changing default database

d6c9206

Drop ImageTag option to simplify

d39cf36

Set COCKROACH_DATABASE

0c40f9b

Reduce API surface area

fb5fa24

rcrowe and others added 9 commits January 27, 2024 11:01

Supply user & password

815a357

Docs for password authn

0605e48

Move to test-suite ahead of TLS

e0c40c3

TLS support

72d0662

Warn cant currently change user with TLS

da44f09

Update docs/modules/cockroachdb.md

da0d2c3

Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

Use lifecycle to add TLS files

9ee540b

Bump pgx/v5 deps

ebf8373

rcrowe force-pushed the module-cockroachdb branch from f8e20cd to ebf8373 Compare January 27, 2024 11:03

rcrowe added 2 commits January 27, 2024 11:10

Use clearer name for connection TLSConfig

f03967e

Note about the possibility of a panic when starting

94def50

rcrowe requested a review from mdelapenya January 27, 2024 11:19

mdelapenya reviewed Jan 28, 2024

View reviewed changes

docs/modules/cockroachdb.md Outdated Show resolved Hide resolved

Update docs/modules/cockroachdb.md

9153188

Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

rcrowe requested a review from mdelapenya January 28, 2024 16:05

mdelapenya approved these changes Jan 29, 2024

View reviewed changes

mdelapenya merged commit 7151628 into testcontainers:main Jan 29, 2024
80 checks passed

rcrowe deleted the module-cockroachdb branch January 29, 2024 08:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(modules.cockroachdb) Adds cockroachdb module #2131

feat(modules.cockroachdb) Adds cockroachdb module #2131

rcrowe commented Jan 21, 2024 •

edited

netlify bot commented Jan 21, 2024 •

edited

mdelapenya left a comment

mdelapenya commented Jan 22, 2024

rcrowe commented Jan 22, 2024 •

edited

rcrowe commented Jan 23, 2024 •

edited

mdelapenya left a comment

rcrowe commented Jan 25, 2024 •

edited

mdelapenya left a comment

mdelapenya left a comment

feat(modules.cockroachdb) Adds cockroachdb module #2131

feat(modules.cockroachdb) Adds cockroachdb module #2131

Conversation

rcrowe commented Jan 21, 2024 • edited

What does this PR do?

Why is it important?

netlify bot commented Jan 21, 2024 • edited

✅ Deploy Preview for testcontainers-go ready!

mdelapenya left a comment

Choose a reason for hiding this comment

mdelapenya commented Jan 22, 2024

rcrowe commented Jan 22, 2024 • edited

rcrowe commented Jan 23, 2024 • edited

mdelapenya left a comment

Choose a reason for hiding this comment

rcrowe commented Jan 25, 2024 • edited

mdelapenya left a comment

Choose a reason for hiding this comment

mdelapenya left a comment

Choose a reason for hiding this comment

rcrowe commented Jan 21, 2024 •

edited

netlify bot commented Jan 21, 2024 •

edited

rcrowe commented Jan 22, 2024 •

edited

rcrowe commented Jan 23, 2024 •

edited

rcrowe commented Jan 25, 2024 •

edited