server: rework the layout of HTTP APIs #55947
Labels
A-cc-enablement
Pertains to current CC production issues or short-term projects
A-observability-inf
A-server-architecture
Relates to the internal APIs and src org for server code
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-observability
Comments
|
Hi @knz, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate. While you're here, please consider adding an A- label to help keep our repository tidy. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
blathers-crl
bot
added
the
C-bug
knz
added
C-enhancement
|
@bdarnell this issue reflects what we talked about last week. @itsbilal as you have expressed interest in this, I'd ask you to review the issue, check whether the overall direction makes sense, and whether you have any improvements to the proposed plan. If it all makes sense, then I'd encourage you to file more fined grained issues that identify the individual steps we'd need to carry out, and for each of them describe the specific direct outcomes we're expecting (so that it's clear when we can close the issue). |
|
@taroface I encourage you too to look at the "summary" at the top of this issue, as it follows up on the initiatives in the Dev Interfaces team and suggests how we're going to move forward. If you have any suggestions about UX for end-users, we'll take them too. |
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jan 4, 2021
…oints This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements a few endpoints that are also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of cockroachdb#55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jan 15, 2021
…oints This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements a few endpoints that are also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of cockroachdb#55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jan 21, 2021
…oints This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements a few endpoints that are also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of cockroachdb#55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jan 25, 2021
…oints This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements a few endpoints that are also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of cockroachdb#55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jan 26, 2021
…point This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements the `/sessions/` endpoint that is also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of cockroachdb#55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jan 27, 2021
…point This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements the `/sessions/` endpoint that is also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of cockroachdb#55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jan 28, 2021
…point This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements the `/sessions/` endpoint that is also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of cockroachdb#55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Feb 1, 2021
…point This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements the `/sessions/` endpoint that is also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of cockroachdb#55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination.
craig bot
pushed a commit
that referenced
this issue
Feb 1, 2021
58436: server: add /api/v2/ tree with auth/pagination, port listSessions endpoint r=itsbilal a=itsbilal This change adds the skeleton for a new API tree that lives in `/api/v2/` in the http listener, and currently reimplements the `/sessions/` endpoint that is also implemented in `/_status/`. The new v2 API tree avoids the need to use GRPC Gateway, as well as cookie-based authentication which is less intuitive and idiomatic for REST APIs. Instead, for authentication, it uses a new session header that needs to be set on every request. As many RPC fan-out APIs use statusServer.iterateNodes, this change implements a pagination-aware method, paginatedIterateNodes, that works on a sorted set of node IDs and arranges results in such a way to be able to return the next `limit` results of an arbitary slice after the `next` cursor passed in. An example of how this works in practice is the new `/api/v2/sessions/` endpoint. A dependency on gorilla/mux is added to be able to pattern-match arguments in the URL. This was already an indirect dependency; now it's a direct dependency of cockroach. TODO that are likely to fall over into future PRs: - API Documentation, need to explore using swagger here. - Porting over remaining /_admin/ and /_status/ APIs, incl. SQL based ones Part of #55947. Release note (api change): Adds a new API tree, in /api/v2/*, currently undocumented, that avoids the use of and cookie-based authentication in favour of sessions in headers, and support for pagination. 58906: sql: improve interface for injecting descriptors into internal executor r=lucy-zhang a=lucy-zhang As part of validating new schema elements (unique indexes, constraints) in the schema changer, we inject in-memory descriptors to be used by the internal executor that are never written to disk. This is currently done by creating an entirely new dummy `descs.Collection` and adding the injected descriptors as uncommitted descriptors, and then setting this collection as the `tcModifier` on the internal executor. Then all subsequent queries using the internal executor, which each get their own child `connExecutor` and `descs.Collection`, will have their collection's uncommitted descriptors replaced by the ones in the dummy collection. This commit introduces the concept of a "synthetic descriptor" to refer to these injected descriptors, and slightly improves the interface in two ways: 1. Instead of creating a new `descs.Collection` to hold synthetic descriptors to copy, we now just use a slice of `catalog.Descriptor`s. 2. Synthetic descriptors are now made explicit in the `descs.Collection` and precede uncommitted descriptors when resolving immutable descriptors. Resolving these descriptors mutably is now illegal and will return an assertion error. This commit doesn't change the fact that the synthetic descriptors to be injected into each query/statement's child `descs.Collection` are set on the internal executor itself. This is still not threadsafe. It would make more sense for these descriptors to be scoped at the level of individual queries. Related to #34304. Release note: None 59258: changefeedccl: Freeze table name to the (optionally fully qualified) statement time name r=[miretskiy] a=HonoreDB Previously, Kafka topics and top-level keys were always derived from the table name in changefeeds. If the table name changed, the feed eventually failed, and if the table name was non-unique across databases, collisions were unavoidable. This PR adds a WITH full_table_name option to changefeeds, and honors it by serializing movr.public.drivers as the statement time name and relying on that. There are probably more things that need to change downstream. Release note (sql change): Added "WITH full_table_name" option to create a changefeed on "movr.public.drivers" instead of "drivers". 59281: sql: prevent DROP SCHEMA CASCADE from droping types with references r=ajwerner a=ajwerner Before this patch, a DROP SCHEMA CASCADE could cause database corruption by dropping types which were referenced by other tables. This would lead to bad errors like: ``` ERROR: object already exists: desc 687: type ID 685 in descriptor not found: descriptor not found SQLSTATE: 42710 ``` And doctor errors like: ``` Table 687: ParentID 50, ParentSchemaID 29, Name 't': type ID 685 in descriptor not found: descriptor not found ``` Fixes #59021. Release note (bug fix): Fixed a bug where `DROP SCHEMA ... CASCADE` could result in types which are referenced being dropped. 59591: sql: hook up multi-region DDL to new zone config attributes r=aayushshah15 a=aayushshah15 After the introduction of #57184, we can constrain voting replicas separately from non-voting replicas using the new attributes `num_voters` and `voter_constraints`. This commit connects our multi-region DDL statements to leverage these new attributes. Broadly speaking, - The existing `constraints` and `num_replicas` fields are set at the database level, to be inherited by table/partition level zone configs. - The new attributes: `num_voters` and `voter_constraints` (along with accompanying `lease_preferences` for these voters) are set at the table/partition level. This brings the implementation of these DDL statements inline with the functional specfication. Fixes #57663 Release note: None 59659: sql: fix bug preventing adding FKs referencing hidden columns r=lucy-zhang a=lucy-zhang The validation query for adding foreign keys had a pointless `SELECT *` on the referenced table that caused hidden columns to be omitted, so attempting to add foreign key constraints referencing hidden columns would fail. This PR fixes the query. Fixes #59582. Release note (bug fix): Fixed a bug preventing foreign key constraints referencing hidden columns (e.g., `rowid`) from being added. Co-authored-by: Bilal Akhtar <[email protected]> Co-authored-by: Lucy Zhang <[email protected]> Co-authored-by: Aaron Zinger <[email protected]> Co-authored-by: Andrew Werner <[email protected]> Co-authored-by: Aayush Shah <[email protected]>
craig bot
pushed a commit
that referenced
this issue
Feb 26, 2021
60744: sql: drop default value when its dependent sequence is dropped r=the-ericwang35 a=the-ericwang35 Fixes #51889 and #20965. Previously, we did not support `DROP SEQUENCE ... CASCADE`, and we simply disallowed dropping sequences that have usages by tables when a user tries to drop a sequence. However, we did not make this check when doing `DROP DATABASE/SCHEMA ... CASCADE`. As a result, if a sequence is used by a table in a different database/schema, we end up dropping a sequence that the table still relies on, resulting in a corrupted default expression. This patch addresses this issue by implementing `DROP SCHEMA ... CASCADE`, which solves the above issue. When we drop anything that contains/owns a sequence with `CASCADE`, we drop those sequences as if `DROP SCHEMA ... CASCADE` was called on them, which prevents the `DEFAULT` expressions from being corrupted, and also allows users to use `DROP SEQUENCE ... CASCADE. `DROP SEQUENCE ... CASCADE` behaves as in Postgres, wherein any default expressions that rely on the dropped sequences are also dropped. Release note (bug fix): Fixed a bug which could result in corrupted descriptors when dropping a database which contains a sequence that was referenced from another database. Release note (bug fix): Fixed a bug where DROP SEQUENCE CASCADE would not properly remove references to itself from table default expressions. Release note (sql change): Added support for dropping a table which owns a sequence. Release justification: low risk bug fix for existing functionality 60952: server: Migrate nodes, {hot-,}ranges, health endpoints to v2 API r=knz a=itsbilal This change adds API v2 compatible versions of the node list, range info per node, ranges info across nodes, hot ranges, and health endpoints. These endpoints all support API v2 header-based authentication, pagination (if applicable), and only return relevant information in the response payloads. One part of #55947. Release note (api change): Add these new HTTP API endpoints: - `/api/v2/nodes/`: Lists all nodes in the cluster - `/api/v2/nodes/<node_id>/ranges`: Lists all ranges on the specified node - `/api/v2/ranges/hot/`: Lists hot ranges in the cluster - `/api/v2/ranges/<range_id>/`: Describes range in more detail - `/api/v2/health/`: Returns an HTTP 200 response if node is healthy. Release justification: Adds more HTTP API endpoints in parallel that do not touch existing code. Co-authored-by: Eric Wang <[email protected]> Co-authored-by: Bilal Akhtar <[email protected]>
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Mar 24, 2021
This commit adds go-swagger annotations for API v2 endpoints, so that the swagger spec and by extension docs can be generated as part of `make generate`. Due to a limitation in go-swagger, the only map key type that's supported for swagger models is string, so some maps had to be modified to use that type instead. Part of cockroachdb#55947 , and the docs story around it. Release note (general change): Add docs for api v2 endpoints.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Mar 25, 2021
This commit adds go-swagger annotations for API v2 endpoints, so that the swagger spec and by extension docs can be generated as part of `make generate`. Due to a limitation in go-swagger, the only map key type that's supported for swagger models is string, so some maps had to be modified to use that type instead. Part of cockroachdb#55947 , and the docs story around it. Release note (general change): Add docs for api v2 endpoints.
craig bot
pushed a commit
that referenced
this issue
Mar 25, 2021
62506: sql: fix insert fast path when columns are not in order r=RaduBerinde a=RaduBerinde The insert fast path assumes that the input columns are in the right order. This is the case in practice in the common case when we provide values for all columns; but it is not the case when we provide a subset of columns. This results in the fast path not being used when it should. The fix is to permute the columns after we build the values matrix. Fixes #62270. Release note: None 62560: *: Build APIv2 spec using go-swagger, annotate endpoints r=knz a=itsbilal This change adds a dependency on `go-swagger/go-swagger`, and uses it as part of `make generate` to regenerate the swagger spec in `docs/generated/swagger/spec.json`, whenever one of the APIv2 files in `pkg/server/` are changed. The spec, once generated, can be served as docs using the command: bin/swagger serve -p 8080 ./docs/generated/swagger/spec.json Also adds related annotations to `pkg/server/api*.go` to populate said spec. Part of #55947 , and the docs story around it. Release note (general change): Add docs for api v2 endpoints. Release note (build change): Adds go-swagger dependency, updates Makefile to call it to rebuild spec in docs/generated/swagger/ which will eventually be used for API docs. 62613: backupccl: run FK schema migration during RESTORE AOST r=postamar a=postamar Part of the 19.x to 20.x upgrade included work on how schema metadata encoded foreign keys to eventually remove the need for them to be attached to indexes, and instead be stored on the table itself. This work included migrating existing schemas to reflect this reorganization and also included 'upgrading' schema metadata as it was RESTORE'd from a backup that may have backed up the old representation. However this on-the-fly upgrade was only applied to some of the copies of the schema stored in the backup, namely the snapshot of 'latest' table descriptors that is kept in backupManifest.Descriptors. A parallel optional structure called DescriptorChanges exists in so-called 'revision_history' backups that has not just the schema as it existed when backed up, but also prior version of it that were revised during the backup window. The schema migraiton upgraded the schema in 'Descriptors' but did not upgrade the prior copies of schema in 'DescriptorChanges'. This meant that if a RESTORE was performed with an AS OF SYSTEM TIME clause, it would search for the correct copy of the schema as of that time, then restore it, and thus would use a copy from DescriptorChanges that had not been migrated, instead of the copy from Descriptors, which the schema migration had processed. Release note (bug fix): Fix a bug where tables that were created by 19.x or older that included foreign key constraits and were backed up with 'revision_history' option would be malformed when restored by a 20.x cluster if that RESTORE used the `AS OF SYSTEM TIME` option. Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: Bilal Akhtar <[email protected]> Co-authored-by: David Taylor <[email protected]>
knz
added
A-server-architecture
|
We have marked this issue as stale because it has been inactive for |
knz
added
T-observability-inf
and removed
T-server-and-security
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This issue is an "epic" that describes what we are aiming for through the 21.1 cycle and beyond.
Summary: in v21.1 we are going to:
/_admin/v1, and/_statusURL prefixes for consumption by end-users. In particular/_statuswill be eventually removed e.g. in v21.2./api/v2URL prefix under which all public services will be available. From then on, all API additions for end-users will go under/apiand follow our general API versioning rules. It will be possible for multiple versions to co-exist side-by-side to offer forward and backward compatibility to clients./_adminwhich will not be versioned and will evolve in lock-step with the UI code, for simplicity by CockroachDB developers. There will be no guarantees offered to external users about the behavior of/_admin. We will also migrate anything under/_statusand/debugused by the admin UI to live under/_admin./apiand one under/_admin. Only the former will be public and documented./debugwill continue to be a special "anything goes" so that CockroachDB developers can quickly add or change troubleshooting endpoints there. However, any debug endpoint useful for the admin UI will also have an entry point under/_adminand any debug endpoint with value to end-users will need to be "promoted" under the/apitree./healthwill also continue to be special and will not change URLs nor behavior in this cycle. However, we're going to attempt to pull it out of mandatory TLS, so that non-TLS HTTP clients can check health too. (This is a separate project)Internal changes
We acknowledge that all our HTTP APIs today are also not used internally by RPC clients inside CockroachDB; and conversely all RPC services are otherwise not used by HTTP clients. Therefore:
This will greatly reduce the complexity of the server code, and also ensure that our RPC and HTTP authentication mechanisms are clearly separate.
Then at a later phase (see separate issue TBD) connect the SQL layer to the (new) HTTP methods via the new HTTP-over-SQL gateway (new project/component).
Possible strategy (needs to be discussed / refined further)
/api/v1tree with just a few of the APIs we already know are popular and need to be provided:/_admin(i.e. bring all uses of/_statusunder/_admin, and add URL aliases accordinglyRelated wiki pages:
Related github issues:
Jira issue: CRDB-3609
The text was updated successfully, but these errors were encountered: