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.

Sign up for GitHub

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

Jump to bottom

roachprod: Clusters should default to secure #38539

Closed
rmloveland opened this issue May 24, 2018 · 6 comments · Fixed by #123593
Closed

roachprod: Clusters should default to secure #38539

rmloveland opened this issue May 24, 2018 · 6 comments · Fixed by #123593
Labels
A-roachprod C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-testeng TestEng Team

Comments

@rmloveland
Copy link
Collaborator

rmloveland commented May 24, 2018 •
edited by cockroach-jira-scripts
Loading

Maybe it is insecure by default because it was faster to build it that way. (Totes understandable.)

However, if there are other reasons, e.g., default-secure clusters would break use cases and make things harder for internal folks, then that could be a great incentive to make it easier to do the right thing!

Suggestion: flip the default, and if internal users need an insecure cluster, they need to pass --insecure and we print the same warning we provide to our users. Delicious dog food!

(Filing because my experience working on adding secure options to the "Build an App" docs is teaching me that (at least from Java) doing it the "right way" is quite complex! But in the bad, uninteresting, fiddly and hard-to-get-right way, not in any of the good ways.)

Epic CRDB-10428

Jira issue: CRDB-5690
@petermattis
Copy link
Collaborator

@rmloveland Insecure was the default initially, and when support for --secure was added we never changed the default. If we change the default, we either need to fix roachtest to be able to work properly against secure clusters, or have it pass --insecure for the clusters it creates. The one annoyance I'm aware of with default-secure clusters is having to dismiss the browser warning about the cert when visiting the admin UI.

@kenliu kenliu changed the title Clusters should default to secure roachprod: Make --sequential the default in roachprodClusters should default to secure Jun 28, 2019
@kenliu kenliu transferred this issue from cockroachdb/roachprod Jun 28, 2019
@kenliu kenliu changed the title roachprod: Make --sequential the default in roachprodClusters should default to secure roachprod: Clusters should default to secure Jun 28, 2019
@awoods187 awoods187 added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Aug 30, 2019
@kenliu kenliu assigned kenliu and jlinder and unassigned kenliu Dec 13, 2019
jlinder added a commit to jlinder/cockroach that referenced this issue Jan 16, 2020
@jlinder
roachprod: default to secure clusters
094dbb6 
Before: roachprod defaulted to making insecure clusters and the useneeded
to use the --secure flag to create a secure cluster.

Why change? We tell our users that the best practice is to run in secure mode
but we don't dog food that configuration enough. Setting roachprod to secure
by default will require us to debug problems in secure mode, which means we'll
make it easer to run secure mode and we'll get better knowledge of how to run
in secure mode.

Now: clusters will be spun up in secure mode by default. To get an insecure
cluster, use the --insecure flag.

Fixes cockroachdb#38539.

Release note: None
@jlinder
Copy link
Collaborator

jlinder commented Jan 16, 2020

I'm putting this to the side for a bit to work on #33377. Here's status notes on progress:

  • Changes are on a branch:
    https://github.com/jlinder/cockroach/tree/jhl/roachprod-default-secure

  • Changes already made include:

    • switched roachprod to be secure by default
    • updated the majority of roachtest to default to using secure roachprod clusters
    • added --insecure flag to roachtest to use insecure roachprod clusters

  • There is still a bunch of testing that needs to be done for roachtest by
    running the different test suites and fixing the issues that arise. Possibly
    get all the tests to be run via teamcity on this branch. At least these tests
    have issues that need fixing:

    • kv/quiescence/nodes=3
    • kv/contention/nodes=4

@rafiss
Copy link
Collaborator

rafiss commented Mar 23, 2021 •
edited
Loading

Just reflagging this issue, as I think it's still very relevant. I'm curious if there are any plans to return to this. I am specifically thinking of nightly testing. It would go a long way towards dogfooding and making sure our tests reflect realistic usage.

Some related work: The SQL Experience team is working on a new roachtest in #62166 and in order for the test to really be valuable, we need a secure cluster. See the PR for a new --secure flag that is being added to workload

Also, I believe in the future cockroach --insecure will go away (#53404), but @knz or @aaron-crl can talk more about those plans.

cc @jlinder @kenliu

@knz
Copy link
Contributor

knz commented Mar 25, 2021

Yes so I would say this issue is relevant, and in an ideal world we would leverage the new cockroach connect command so that the roachtest code doesn't have to deal with cert creation and distribution itself. So I would say let's come back to this once the cockorach connect command gets out of alpha/beta testing.

@knz knz mentioned this issue Mar 25, 2021
Closed
25 tasks
@rafiss rafiss mentioned this issue Apr 8, 2021
Closed
rail pushed a commit to rail/cockroach that referenced this issue Apr 14, 2021
@jlinder @rail
roachprod: default to secure clusters
33c92ea 
Before: roachprod defaulted to making insecure clusters and the useneeded
to use the --secure flag to create a secure cluster.

Why change? We tell our users that the best practice is to run in secure mode
but we don't dog food that configuration enough. Setting roachprod to secure
by default will require us to debug problems in secure mode, which means we'll
make it easer to run secure mode and we'll get better knowledge of how to run
in secure mode.

Now: clusters will be spun up in secure mode by default. To get an insecure
cluster, use the --insecure flag.

Fixes cockroachdb#38539.

Release note: None
@rail rail mentioned this issue Apr 14, 2021
Closed
rail pushed a commit to rail/cockroach that referenced this issue Apr 14, 2021
@jlinder @rail
roachprod: default to secure clusters
fa0b1ba 
Before: roachprod defaulted to making insecure clusters and the useneeded
to use the --secure flag to create a secure cluster.

Why change? We tell our users that the best practice is to run in secure mode
but we don't dog food that configuration enough. Setting roachprod to secure
by default will require us to debug problems in secure mode, which means we'll
make it easer to run secure mode and we'll get better knowledge of how to run
in secure mode.

Now: clusters will be spun up in secure mode by default. To get an insecure
cluster, use the --insecure flag.

Fixes cockroachdb#38539.

Release note: None
rail pushed a commit to rail/cockroach that referenced this issue Apr 14, 2021
@jlinder @rail
roachprod: default to secure clusters
e410bd4 
Before: roachprod defaulted to making insecure clusters and the useneeded
to use the --secure flag to create a secure cluster.

Why change? We tell our users that the best practice is to run in secure mode
but we don't dog food that configuration enough. Setting roachprod to secure
by default will require us to debug problems in secure mode, which means we'll
make it easer to run secure mode and we'll get better knowledge of how to run
in secure mode.

Now: clusters will be spun up in secure mode by default. To get an insecure
cluster, use the --insecure flag.

Fixes cockroachdb#38539.

Release note: None
rail pushed a commit to rail/cockroach that referenced this issue Apr 15, 2021
@jlinder @rail
roachprod: default to secure clusters
3195270 
Before: roachprod defaulted to making insecure clusters and the useneeded
to use the --secure flag to create a secure cluster.

Why change? We tell our users that the best practice is to run in secure mode
but we don't dog food that configuration enough. Setting roachprod to secure
by default will require us to debug problems in secure mode, which means we'll
make it easer to run secure mode and we'll get better knowledge of how to run
in secure mode.

Now: clusters will be spun up in secure mode by default. To get an insecure
cluster, use the --insecure flag.

Fixes cockroachdb#38539.

Release note: None
@exalate-issue-sync exalate-issue-sync bot added T-testeng TestEng Team and removed T-dev-inf labels Mar 4, 2022
@srosenberg
Copy link
Member

Putting this back on TestEng's radar... While the bug in [1] didn't strictly require a "secure" cluster for reproducibility, it is suggestive of a blind spot; majority of roachtests still run with --insecure.

[1] #97178

@rafiss
Copy link
Collaborator

rafiss commented Mar 9, 2023

This is also somewhat related to #97535, which was another action item following the bug in #97178

@renatolabs renatolabs mentioned this issue May 3, 2024
Merged
renatolabs added a commit to renatolabs/cockroach that referenced this issue May 3, 2024
@renatolabs
roachprod: make clusters secure by default
afa2aa7 
Roachtest has been creating secure clusters in tests for a while
now. It makes sense for roachprod to use the same, better default when
setting up clusters.

If necessary, users can still create insecure clusters by passing the
`--insecure` flag to `roachprod start` and other commands.

Fixes: cockroachdb#38539

Release note: None
craig bot pushed a commit that referenced this issue May 6, 2024
@itsbilal @renatolabs @sudomateo
Merge #123564 #123593 #123603
27fc5d2 
123564: drtprod: move drt-chaos to local ssds r=dt a=itsbilal

Now that the local ssd bug is identified, we can move drt-chaos to local ssds as well and save some $.

Epic: none

Release note: None

123593: roachprod: make clusters secure by default r=srosenberg a=renatolabs

**roachprod: make clusters secure by default**
Roachtest has been creating secure clusters in tests for a while
now. It makes sense for roachprod to use the same, better default when
setting up clusters.

If necessary, users can still create insecure clusters by passing the
`--insecure` flag to `roachprod start` and other commands.

Fixes: #38539

Release note: None

**roachprod: use non-root user by default in roachprod sql**
This brings roachprod's and roachtest's defaults closer. Now that we
create secure roachprod clusters by default (which includes creating
an admin user), we are able to change the default authentication mode
used when starting a SQL shell.

Like with `pgurl`, the authentication mode can be changed with the
`--auth-mode` command line flag.


123603: roachprod: opt-in fluent-servers logging configuration r=sudomateo a=sudomateo

In #123227 we added the
ability for clusters created by `roachprod` to send their logs to an
external system using the `fluent-servers` attribute in the CockroachDB
logging configuration. However, the `fluent-servers` attribute was
enabled unconditionally which caused clusters to to log an error if the
requisite Fluent Bit server was not there to receive the logs.

```
E240503 20:21:50.574780 55 1@util/log/buffered_sink.go:353 ⋮ [-] 229  logging error from *log.fluentSink: dial tcp ‹127.0.0.1:5170›: connect: connection refused
```

This patch adds a `--enable-fluent-sink` option to the `roachprod start`
command to conditionally allow clusters to use the `fluent-servers`
logging configuration. It also updates `scripts/drtprod` to use this
option so they can continue to log to external systems.

Epic: none

Release note: none


Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com>
Co-authored-by: Renato Costa <renato@cockroachlabs.com>
Co-authored-by: Matthew Sanabria <24284972+sudomateo@users.noreply.github.com>
@craig craig bot closed this as completed in d53b0c4 May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-roachprod C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-testeng TestEng Team
Projects
Test Engineering
Status: Done
Milestone
No milestone
8 participants
@kenliu @jlinder @knz @rmloveland @rafiss @srosenberg @petermattis @awoods187