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

server,cli/demo: make the SQL and HTTP listeners configurable #64571

Closed
wants to merge 1 commit into from

Conversation

knz
Copy link
Contributor

@knz knz commented May 3, 2021

Fixes #64148.

Prior to this change, a demo instance would restrict
all its network listeners to 127.0.0.1.

Since this choice was initially made, we discovered the utility of
embedding a demo shell inside a Katakoda instance, where the
Katakoda infrastructure is able to provide a secure HTTPS proxy with a
public CA for a service running inside the sandbox.

Since that proxying requires the service to be exposed on the
sandbox's external address, we wanted the demo shell
to make its listeners configurable.

Release note (cli change): It is now possible to enable TLS for the
web UI console of cockroach demo with the new flag
--secure-http. This makes it possible to expose the HTTP interface
to a shared network.

Note however that this configuration still uses an ephemeral
self-signed CA generated by demo itself, and is thus insufficient
for use by end users web browsers directly: it should still be
combined with a HTTP proxy that exposes the service using a public CA.

This feature is not enabled by default.

Release note (cli change): The network address bound by cockroach demo simulated nodes is now configurable via the new command-line
flags --http-addr (for the HTTP interface) and --sql-addr (for the
TCP interface for SQL clients).

The default for these flags results in the same configuration as
in previous versions, that is, the demo nodes only listen
on 127.0.0.1.

Note that --http-addr cannot be used without passing --secure-http
explicitly (either via --secure-http=true or
--secure-http=false), so as to clearly signal intent.

Release note (cli change): The CommonName (CN) field of the TLS server
certificate generated for the HTTP port in cockroach demo (when
--secure-http is used) is now configurable via the new flag
--http-advertise-addr. This is also the address displayed in the web
UI URLs printed to the user via the \demo ls client-side command.

The default for this value is the same as --http-addr.

@knz knz requested a review from bdarnell May 3, 2021 14:57
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz
Copy link
Contributor Author

knz commented May 3, 2021

@jseldess @bdarnell before I proceed to create a thorough test suite for this change, please confirm that the proposed functionality actually helps with the use case.

Thanks

Prior to this change, a `demo` instance would restrict
all its network listeners to 127.0.0.1.

Since this choice was initially made, we discovered the utility of
embedding a `demo` shell inside a Katakoda instance, where the
Katakoda infrastructure is able to provide a secure HTTPS proxy with a
public CA for a service running inside the sandbox.

Since that proxying requires the service to be exposed on the
sandbox's external address, we wanted the `demo` shell
to make its listeners configurable.

Release note (cli change): It is now possible to enable TLS for the
web UI console of `cockroach demo` with the new flag
`--secure-http`. This makes it possible to expose the HTTP interface
to a shared network.

Note however that this configuration still uses an ephemeral
self-signed CA generated by `demo` itself, and is thus insufficient
for use by end users web browsers directly: it should still be
combined with a HTTP proxy that exposes the service using a public CA.

This feature is not enabled by default.

Release note (cli change): The network address bound by `cockroach
demo` simulated nodes is now configurable via the new command-line
flags `--http-addr` (for the HTTP interface) and `--sql-addr` (for the
TCP interface for SQL clients).

The default for these flags results in the same configuration as
in previous versions, that is, the `demo` nodes only listen
on 127.0.0.1.

Note that `--http-addr` cannot be used without passing `--secure-http`
explicitly (either via `--secure-http=true` or
`--secure-http=false`), so as to clearly signal intent.

Release note (cli change): The CommonName (CN) field of the TLS server
certificate generated for the HTTP port in `cockroach demo` (when
`--secure-http` is used) is now configurable via the new flag
`--http-advertise-addr`. This is also the address displayed in the web
UI URLs printed to the user via the `\demo ls` client-side command.

The default for this value is the same as `--http-addr`.
@knz knz force-pushed the 20210503-demo-addr branch from 4793fd6 to 9c9ee95 Compare May 3, 2021 15:07
Copy link
Contributor

@bdarnell bdarnell left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 7 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @knz)


pkg/cli/demo.go, line 200 at r1 (raw file):

	fs := flagSetForCmd(cmd)

	// Ensure that an explicit --http-addr is always combined with

I think this is unnecessarily strict - it's just demo mode; we don't need the user to explicitly opt in to an insecure mode in addition to setting the http-addr.


pkg/cli/cliflags/flags.go, line 1146 at r1 (raw file):

		Description: `The network address for demo nodes
to listen for connection requests from HTTP clients.
The port number must not be included.

These -addr flags should have the same relationship to their corresponding -port flags as the flags to start (i.e. the -port flags should be deprecated in favor of the combined -addr flags).


pkg/cli/cliflags/flags.go, line 1162 at r1 (raw file):

--secure-http.
The port number should be included; add :0 after the network
address to reuse the same HTTP port as each simulated node.

In the katacoda environment I'm not sure there's any relationship between the private and public port numbers. (and I'm not sure it supports multiple private ports at all - the one example I've seen just uses a single port and maps it to the public port 443). So there may be limitations on how well the "advertised" http addresses work even if we introduce this distinction.


pkg/cli/cliflags/flags.go, line 1168 at r1 (raw file):

	DemoSecureHTTP = FlagInfo{
		Name: "secure-http",

Do we need this for katacoda? I think in this situation it's fine with either HTTP or HTTPS for the backend port and we don't need an option to force HTTPS.

@jseldess
Copy link
Contributor

@knz, just want to mention that I'm having trouble getting the embedded multi-region tutorial working in other ways unrelated to the DB console, so this isn't a rush on my end. More broadly, it still would be nice to be able to access the DB Console from within an embedded Katacoda tutorial, so this work still seems valuable to me, but not time-sensitive.

@knz
Copy link
Contributor Author

knz commented May 14, 2021

Gotcha.

FWIW, ben's last code review makes the next implementation iteration difficult. It's actually non-trivial to achieve what ben suggests, so that will need more time.

@knz
Copy link
Contributor Author

knz commented Jan 10, 2023

we're making steps to make cockroach start-single-node more usable, which obviates the need for this PR.

@knz knz closed this Jan 10, 2023
@knz knz deleted the 20210503-demo-addr branch January 10, 2023 12:26
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.

cli/demo: allow Katakoda end-users to use the DB console for demo servers with global latencies
4 participants