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

cmd: consistent flags for http/grpc servers #254

Merged
merged 5 commits into from
Nov 4, 2021

Conversation

jzelinskie
Copy link
Member

@jzelinskie jzelinskie commented Nov 3, 2021

This commit also:

  • renames internal-grpc flags to dispatch-redispatch
  • fixes logging occurring during the version command
  • adds a missing error to a log in serve-devtools
  • adds missing flags to various servers
  • refactors gateway & dashboard to return http.Handler

@jzelinskie jzelinskie added area/CLI Affects the command line priority/3 low This would be nice to have kind/breaking change Breaks existing usage kind/tech debt Addresses legacy code/decisions area/api devtools Affects the devtools API labels Nov 3, 2021
@github-actions github-actions bot added the area/dependencies Affects dependencies label Nov 3, 2021
This commit also:
- renames internal-grpc flags to dispatch-redispatch
- fixes logging occurring during the version command
- adds a missing error to a log in `serve-devtools`
- adds missing flags to various servers
go.mod Outdated Show resolved Hide resolved
@ecordell
Copy link
Contributor

ecordell commented Nov 3, 2021

Changing the flags broke the e2e tests:

Error: unknown flag: --internal-grpc-addr

@github-actions github-actions bot added the area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) label Nov 3, 2021
@ecordell
Copy link
Contributor

ecordell commented Nov 3, 2021

panic: failed to find cobra flag: dispatch-redispatch-max-conn-age

goroutine 1 [running]:
github.com/jzelinskie/cobrautil.MustGetDuration(0x1374b17, {0xc0004d0e80, 0x20})
	/home/runner/go/pkg/mod/github.com/jzelinskie/cobrautil@v0.0.6-0.20211103005615-2d3ced57292a/must.go:72 +0x79
github.com/jzelinskie/cobrautil.GrpcServerFromFlags(0xc000123488, {0x1374b17, 0x0}, {0xc0007dfaa0, 0x2, 0x2})
	/home/runner/go/pkg/mod/github.com/jzelinskie/cobrautil@v0.0.6-0.20211103005615-2d3ced57292a/cobrautil.go:227 +0xc5
main.serveRun(0xc00044d180, {0xc00022a240, 0x0, 0xc})
	/home/runner/work/spicedb/spicedb/cmd/spicedb/serve.go:266 +0x1a45
github.com/spf13/cobra.(*Command).execute(0xc00044d180, {0xc00022a180, 0xc, 0xc})
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.2.1/command.go:860 +0x5f8
github.com/spf13/cobra.(*Command).ExecuteC(0xc00044cc80)
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.2.1/command.go:974 +0x3bc
github.com/spf13/cobra.(*Command).Execute(...)
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.2.1/command.go:902
main.main()
	/home/runner/work/spicedb/spicedb/cmd/spicedb/main.go:38 +0x65
exit status 2

@jzelinskie jzelinskie force-pushed the bump-cobrautil branch 2 times, most recently from 23262d1 to 22a2538 Compare November 3, 2021 22:12
@github-actions github-actions bot added the area/docs Affects docs or metadata (e.g. README) label Nov 3, 2021
@jzelinskie
Copy link
Member Author

New flags:

Usage:
  spicedb serve [flags]

Examples:
	No TLS and in-memory:
		spicedb serve --grpc-preshared-key "somerandomkeyhere"

	TLS and a real datastore:
		spicedb serve --grpc-preshared-key "realkeyhere" --grpc-tls-cert-path path/to/tls/cert --grpc-tls-key-path path/to/tls/key \
			--http-tls-cert-path path/to/tls/cert --http-tls-key-path path/to/tls/key \
			--datastore-engine postgres --datastore-conn-uri "postgres-connection-string-here"


Flags:
      --dashboard-addr string                                   address to listen on to serve dashboard (default ":8080")
      --dashboard-enabled                                       enable dashboard http server (default true)
      --dashboard-tls-cert-path string                          local path to the TLS certificate used to serve dashboard
      --dashboard-tls-key-path string                           local path to the TLS key used to serve dashboard
      --datastore-bootstrap-files strings                       bootstrap data yaml files to load
      --datastore-bootstrap-overwrite                           overwrite any existing data with bootstrap data
      --datastore-conn-healthcheck-interval duration            time between a remote datastore's connection pool health checks (default 30s)
      --datastore-conn-max-idletime duration                    maximum amount of time a connection can idle in a remote datastore's connection pool (default 30m0s)
      --datastore-conn-max-lifetime duration                    maximum amount of time a connection can live in a remote datastore's connection pool (default 30m0s)
      --datastore-conn-max-open int                             number of concurrent connections open in a remote datastore's connection pool (default 20)
      --datastore-conn-min-open int                             number of minimum concurrent connections open in a remote datastore's connection pool (default 10)
      --datastore-conn-uri string                               connection string used by remote datastores (e.g. "postgres://postgres:password@localhost:5432/spicedb")
      --datastore-engine string                                 type of datastore to initialize ("memory", "postgres", "cockroachdb") (default "memory")
      --datastore-gc-interval duration                          amount of time between passes of garbage collection (postgres driver only) (default 3m0s)
      --datastore-gc-max-operation-time duration                maximum amount of time a garbage collection pass can operate before timing out (postgres driver only) (default 1m0s)
      --datastore-gc-window duration                            amount of time before revisions are garbage collected (default 24h0m0s)
      --datastore-max-tx-retries int                            number of times a retriable transaction should be retried (cockroach driver only) (default 50)
      --datastore-query-split-size string                       estimated number of bytes at which a query is split when using a remote datastore (default "8MiB")
      --datastore-readonly                                      set the service to read-only mode
      --datastore-request-hedging                               enable request hedging (default true)
      --datastore-request-hedging-initial-slow-value duration   initial value to use for slow datastore requests, before statistics have been collected (default 10ms)
      --datastore-request-hedging-max-requests uint             maximum number of historical requests to consider (default 1000000)
      --datastore-request-hedging-quantile float                quantile of historical datastore request time over which a request will be considered slow (default 0.95)
      --datastore-revision-fuzzing-duration duration            amount of time to advertize stale revisions (default 5s)
      --datastore-tx-overlap-key string                         static key to touch when writing to ensure transactions overlap (only used if --datastore-tx-overlap-strategy=static is set; cockroach driver only) (default "key")
      --datastore-tx-overlap-strategy string                    strategy to generate transaction overlap keys ("prefix", "static", "insecure") (cockroach driver only) (default "static")
      --disable-v1-schema-api                                   disables the V1 schema API
      --dispatch-cluster-addr string                            address to listen on to serve dispatch (default ":50053")
      --dispatch-cluster-dns-name string                        DNS SRV record name to resolve for cluster dispatch
      --dispatch-cluster-enabled                                enable dispatch gRPC server
      --dispatch-cluster-max-conn-age duration                  how long a connection serving dispatch should be able to live (default 30s)
      --dispatch-cluster-service-name string                    DNS SRV record service name to resolve for cluster dispatch (default "grpc")
      --dispatch-cluster-tls-cert-path string                   local path to the TLS certificate used to serve dispatch
      --dispatch-cluster-tls-key-path string                    local path to the TLS key used to serve dispatch
      --dispatch-max-depth uint32                               maximum recursion depth for nested calls (default 50)
      --dispatch-peer-resolver-addr string                      address used to connect to the peer endpoint resolver
      --dispatch-peer-resolver-preshared-key string             preshared key used to authenticate with the peer endpoint resolver
      --dispatch-peer-resolver-tls-cert-path string             local path to the TLS certificate for the peer endpoint resolver
      --grpc-addr string                                        address to listen on to serve gRPC (default ":50051")
      --grpc-enabled                                            enable gRPC gRPC server (default true)
      --grpc-max-conn-age duration                              how long a connection serving gRPC should be able to live (default 30s)
      --grpc-preshared-key string                               preshared key to require for authenticated requests
      --grpc-shutdown-grace-period duration                     amount of time after receiving sigint to continue serving
      --grpc-tls-cert-path string                               local path to the TLS certificate used to serve gRPC
      --grpc-tls-key-path string                                local path to the TLS key used to serve gRPC
  -h, --help                                                    help for serve
      --http-addr string                                        address to listen on to serve http (default ":8443")
      --http-enabled                                            enable http http server
      --http-tls-cert-path string                               local path to the TLS certificate used to serve http
      --http-tls-key-path string                                local path to the TLS key used to serve http
      --metrics-addr string                                     address to listen on to serve metrics (default ":9090")
      --metrics-enabled                                         enable metrics http server (default true)
      --metrics-tls-cert-path string                            local path to the TLS certificate used to serve metrics
      --metrics-tls-key-path string                             local path to the TLS key used to serve metrics
      --ns-cache-expiration duration                            amount of time a namespace entry should remain cached (default 1m0s)
      --schema-prefixes-required                                require prefixes on all object definitions in schemas

Global Flags:
      --log-format string                 format of logs ("auto", "human", "json") (default "auto")
      --log-level string                  verbosity of logging ("trace", "debug", "info", "warn", "error") (default "info")
      --otel-jaeger-endpoint string       jaeger collector endpoint (default "http://jaeger:14268/api/traces")
      --otel-jaeger-service-name string   jaeger service name for trace data (default "spicedb")
      --otel-provider string              opentelemetry provider for tracing ("none", "jaeger") (default "none")

Copy link
Contributor

@ecordell ecordell left a comment

Choose a reason for hiding this comment

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

LGTM

at some point in the future we should probably customize the help text to group the flags better, it's getting pretty long.

@jzelinskie jzelinskie merged commit b8694a8 into authzed:main Nov 4, 2021
@jzelinskie jzelinskie deleted the bump-cobrautil branch November 4, 2021 21:48
@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/api devtools Affects the devtools API area/CLI Affects the command line area/dependencies Affects dependencies area/docs Affects docs or metadata (e.g. README) area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) kind/breaking change Breaks existing usage kind/tech debt Addresses legacy code/decisions priority/3 low This would be nice to have
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants