Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make CockroachDB test be able to run on secure clusters. #724

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DarrylWong
Copy link

The setup script for cockroachdb tests previously passed an --insecure flag. This change replaces that with a more generic ARGS flag that gives more flexibility. The test can still run on insecure clusters if --insecure is passed or on secure clusters if certs-dir is passed.

PavloTytarchuk
PavloTytarchuk previously approved these changes Feb 5, 2024
Copy link
Contributor

@PavloTytarchuk PavloTytarchuk left a comment

Choose a reason for hiding this comment

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

Approved.

@PavloTytarchuk PavloTytarchuk self-requested a review February 5, 2024 12:07
@PavloTytarchuk PavloTytarchuk dismissed their stale review February 5, 2024 12:09

CI/CD tests are failing with these changes.

@PavloTytarchuk
Copy link
Contributor

Hello @DarrylWong! I've opened PR with your changes in our repo, to check if CI/CD builds will pass with your changes. Unfortunately build fails with: ERROR: cannot establish secure connection to insecure server.

If I remove --insecure from command: start-single-node --insecure in our docker-config, build starts to fail with: ERROR: cannot load certificates.

The setup script for cockroachdb tests previously passed
an --insecure flag. This change replaces that with a more
generic ARGS flag that gives more flexibility. The test
can still run on insecure clusters if --insecure is passed
or on secure clusters if certs-dir is passed.
@DarrylWong
Copy link
Author

Hey Pavlo, thanks for taking a look at my PR!

I went with the easiest fix of passing the --insecure flag to the ./setup_db.sh:/setup_db.sh call in docker. This should retain the previous behavior of running the tests on insecure clusters for CI, but still be an improvement as anyone who wants to (namely cockroachdb's own tests) can run on secure mode.

Let me know if my reasoning makes sense! 😄

@liquibot liquibot removed the TypeBug label Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants