cli: allow for `cockroach demo` to start in secure mode by rohany · Pull Request #45727 · cockroachdb/cockroach

rohany · 2020-03-04T21:00:02Z

Release note (cli change): This PR adds support for cockroach demo
to start in secure mode using the flag --insecure=false.

cockroach-teamcity · 2020-03-04T21:00:11Z

This change is

rohany · 2020-03-04T21:00:49Z

cc @knz, this is a draft. I'm running into a problem where it seems to be working when I run on the CLI, but it fails during cluster setup in the Example_* tests. Does anything appear to standout as missing/wrong?

knz

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @rohany)

pkg/cli/cli_test.go, line 417 at r1 (raw file):

	setCLIDefaultsForTests()
	for _, cmd := range testData {
		c.RunWithArgs(cmd)

Look at the code for RunWithArgs. It tries to be to clever and synthetizes extra arguments --insecure / --certs-dir and injects them into your demo commands. Is that maybe what causes the failure?
If you can't figure it out, please ping me and we can sit together to troubleshoot.

rohany · 2020-03-05T17:25:50Z

Yuck -- after a while I was able to narrow this down to the dummy asset loader set by the security test init.

rohany · 2020-03-05T18:18:29Z

A downside of this is that you now need to login/create a user to access the admin UI

awoods187 · 2020-03-05T18:31:58Z

What is the motivation for this? Demo isn't meant for production applications and anything that puts friction into trial seems to be a mistake. I'd be fine for an option to start demo in secure to test that process out but why is it proposed as the default?

knz · 2020-03-05T18:35:12Z

What is the motivation for this?

The motivation for this comes from docs and IAM (talk to nate):

cockroach demo does not enable teaching about SQL privileges (all privilege checking is disabled in insecure mode)
cockroach demo does not enable teaching about roles (all role checking is disabled in insecure mode)
cockroach demo does not enable teaching about multiple users (disabled in insecure mode)

If we want to have a good alignment between our docs, which teach about security, and cockroach demo, which is used to try docs out, demo needs to work like the secure thing.

knz · 2020-03-05T18:35:51Z

A downside of this is that you now need to login/create a user to access the admin UI

No. Please have cockroach demo auto-populate a password for the root user so they can log in in the UI. Make the root user have password admin and also print a reminder on the terminal when demo starts.

jordanlewis · 2020-03-05T18:36:47Z

To Andy's point, I'm only okay with this if it doesn't add any more hoops for the user to jump through to get a regular demo session. Demo just has to be a one-click, simple way to get started w/ both CLI and admin ui, please!

rohany · 2020-03-05T18:39:23Z

Ok, let me add the default user/password.

@jordanlewis this change doesn't affect the UX at all -- ./cockroach demo just starts a secure cluster without anything extra to do.

rohany · 2020-03-05T19:14:02Z

Discussed this at standup today with @awoods187 and @jordanlewis and I think it would be better to have demo have an opt-in secure mode, rather than secure by default. One of the reasons is that it is harder to use the admin UI -- you have to click through the certificate warning page and login, vs the admin UI just opening.

knz · 2020-03-05T20:12:09Z

ok with making this opt-in in 20.1 because of the cert warning in UI

But we can solve this (after 20.1) too and make secure default. The solution is to have the UI served in a non-SSL conn even though the cluster is secure. This feature/option is already planned for 20.2.

Fixes cockroachdb#45607. Release note (cli change): This PR adds support for `cockroach demo` to start in secure mode using the flag `--insecure=false`.

rohany · 2020-03-05T20:31:35Z

Updated the pr to:

give the root user a password to login to the webui and notify the user about this
use the flag --insecure=false to start cockroach demo in secure mode (to be in parity with other commands that use the --insecure flag.

knz · 2020-03-05T20:32:55Z

i'll review tomorrow at the earliest

knz

well done

Reviewed 7 of 7 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained

rohany · 2020-03-06T15:01:02Z

bors r=knz

craig · 2020-03-06T15:29:56Z

Merge conflict (retrying...)

45704: colexec: add distinct mode to hashTable r=Azhng a=Azhng Previously hashTable will buffer all tuples before building `head` and `same` vector. Now hashTable supports distinct mode where it only buffers the distinct tuples from upstream operator. This removes the need of traversing through the `head` and `same` vectors. Instead, now user of the hashTable can directly build hashTable in distinct mode and copy the buffered tuples. Closes #44404 Release note: None 45727: cli: allow for `cockroach demo` to start in secure mode r=knz a=rohany Fixes #45607. Release note (cli change): This PR adds support for `cockroach demo` to start in secure mode using the flag `--insecure=false`. 45759: storage: Do not create nil data pointers in rocksdb slices r=miretskiy a=miretskiy Fixes #45524 Libroach's DBSlice data type gets converted to the rocksdb::Slice whenever we need to access underlying rocksdb (iterators, get/set, etc). However, internally, rocksdb::Slice asserts that the underlying data pointer may not be null in the rocksdb::Slice::compare as observed in #45524 This change modifies our rocksdb bridge code to never generate DBSlices with nullptr data. Release notes: None Co-authored-by: Azhng <archer.xn@gmail.com> Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu> Co-authored-by: Yevgeniy Miretskiy <yevgeniy@cockroachlabs.com>

craig · 2020-03-06T15:34:11Z

Canceled (will resume)

knz · 2020-03-06T15:34:36Z

bors r-

craig · 2020-03-06T15:34:39Z

Canceled

rohany requested a review from knz March 4, 2020 21:00

rohany requested a review from a team as a code owner March 4, 2020 21:00

knz reviewed Mar 5, 2020

View reviewed changes

rohany force-pushed the demo-secure branch 2 times, most recently from 3b61da8 to d801230 Compare March 5, 2020 17:25

rohany force-pushed the demo-secure branch from d801230 to 27f8bc5 Compare March 5, 2020 18:18

cli: allow for cockroach demo to start in secure mode

8275733

Fixes cockroachdb#45607. Release note (cli change): This PR adds support for `cockroach demo` to start in secure mode using the flag `--insecure=false`.

rohany force-pushed the demo-secure branch from 27f8bc5 to 8275733 Compare March 5, 2020 20:30

rohany changed the title ~~cli: make cockroach demo start in secure mode by default~~ cli: allow for cockroach demo to start in secure mode Mar 5, 2020

knz approved these changes Mar 6, 2020

View reviewed changes

knz merged commit 8e22e09 into cockroachdb:master Mar 6, 2020

knz mentioned this pull request Mar 7, 2020

cli: cockroach demo should start secure clusters by default #45606

Closed

jseldess mentioned this pull request Mar 19, 2020

cli: allow for cockroach demo to start in secure mode cockroachdb/docs#6930

Closed

Conversation

rohany commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Mar 4, 2020

Uh oh!

rohany commented Mar 4, 2020

Uh oh!

knz left a comment

Choose a reason for hiding this comment

Uh oh!

rohany commented Mar 5, 2020

Uh oh!

rohany commented Mar 5, 2020

Uh oh!

awoods187 commented Mar 5, 2020

Uh oh!

knz commented Mar 5, 2020

Uh oh!

knz commented Mar 5, 2020

Uh oh!

jordanlewis commented Mar 5, 2020

Uh oh!

rohany commented Mar 5, 2020

Uh oh!

rohany commented Mar 5, 2020

Uh oh!

knz commented Mar 5, 2020

Uh oh!

rohany commented Mar 5, 2020

Uh oh!

knz commented Mar 5, 2020

Uh oh!

knz left a comment

Choose a reason for hiding this comment

Uh oh!

rohany commented Mar 6, 2020

Uh oh!

craig bot commented Mar 6, 2020

Merge conflict (retrying...)

Uh oh!

craig bot commented Mar 6, 2020

Canceled (will resume)

Uh oh!

knz commented Mar 6, 2020

Uh oh!

craig bot commented Mar 6, 2020

Canceled

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rohany commented Mar 4, 2020 •

edited

Loading