* fix: send flight keep-alives
This was tested using a local nginx proxy and a small modication to the
IOx source code: insert `make_stream_slow` into `GetStream::new` just
between `let query_results = ...` and
`let innr = FlightDataEncoderBuilder::new()...`.
Fixes https://github.com/influxdata/idpe/issues/17824 .
* fix: typo
* refactor: reduce keep-alive interval to 5s
Use a protobuf "Any" to wrap the "ReadInfo" message in a DoGet
ticket. This will make it easier to extend in the future different
ticket types, as appropriate. It also makes the comment speak the
truth.
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Historically the authz crate didn't distinguish between an invalid
token and a valid token without the required permissions. Recently
errors were added to distinguish these cases. This means that an
invalid token now returns an "Internal" error if supplied an invalid
token. Detect this case and return a "Permission Denied" error,
which is the error type that was previously returned in this case.
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
This commit fixes loads of crates (47!) had unused dependencies, or
mis-configured dependencies (test deps as normal deps).
I added the "unused_crate_dependencies" to all crates to help prevent
this mess from growing again!
https://doc.rust-lang.org/beta/nightly-rustc/rustc_lint_defs/builtin/static.UNUSED_CRATE_DEPENDENCIES.html
This has the minor downside of false-positives when specifying
dev-dependencies for test/bench binaries - these are files in /test or
/benches (not normal tests). This commit includes a workaround,
importing them in lib.rs (gated by a feature flag). I think the
trade-off of better dependency management is worth it!
Adds the standard lints to service_grpc_flight and fixes any lint
failures.
Note this doesn't include the normal "document things" lint, because
there's a load of missing docs
Introduce a new header called `iox-debug` which when set enables certain
debug features. The first one will be the `system.queries` table which
is a process-local, namespace-scoped query log. In most prod setups this
is only useful for debugging and will confuse the user a lot because
when multiple queries are deployed then the K8s routing decides which
pod/process the users hits. This leads to an inconsistent view. However
the log is still useful for debugging.
This also wires the "debug header set" flag through the Flight ticket,
because JDBC proved (integration tests FTW!) that headers are only
passed to `GetFlightInfo` but not to `DoGet` and the ticket must encode
all the relevant information.
Closes#7119.
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* chore: accept database in addition to namespace_name
* chore: accept bucket and bucket-name as well
* test: additional test for sql request namespace aliases
* refactor(authz): move extract_header_token into authz
Move the extract_header_token method into the authz package so that
it can be shared by the query path. The method is renamed to reflect
the fact that it can now also extract a token from gRPC metadata.
The extract_token function is now a little more generic to allow
it to be used with HTTP header values and gRPC metadata values.
* feat(service_grpc_flight): JDBC compatible Handshake
While testing some JDBC based clients we found that some, Tableau
in this case, cannot be configured with authoriztion tokens. In
these cases we need to be able to support username/password. The
approach taken is to ignore the username and make the token the
password. This is the same approach being taken throughout the
product.
To facilitate this the Flight RPC Handshake command has been extended
to look for Basic authorization credentials and respond with the
appropriate Bearer authorization header.
While adding end-to-end tests the subprocess commands were causing
a deadlock. These have been changed to using the tonic::process
module.
There are also some small changes to the JDBC test application where
the hardcoded values were clashing with the authorization parameters.
* fix: lint
* chore: apply suggestions from code review
Co-authored-by: Andrew Lamb <alamb@influxdata.com>
* chore: review suggestion
---------
Co-authored-by: Andrew Lamb <alamb@influxdata.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat: WIP support CommandGetXdbcTypeInfo metadata endpoint with tests
* chore: update test case and add jdbc test
* chore: uncomment jdbc getCoumns test
* chore: lint
---------
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* chore(flightsql): rename Namespace to Database in error message
* chore(flightsql): rename Namespace to Database in test error msg
---------
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat: support CommandGetCrossReference metadata endpoint with tests
* chore: create two tables in the test for GetCrossReference endpoint
---------
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* chore: support returning the database name if all the keys refer to the same database
* test: add test cases to check for same, different, and no database in request header
* chore: lint
* chore: more lint
* refactor: replace empty string with None for database_name
* refactor: simplify logic for NoFlightSQLDatabase error
---------
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat: support CommandGetImportedKeys metadata endpoint with tests
* chore: remove comments that is no longer valid
---------
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat: support `database`, `bucket`, and `bucket-name` as grpc header names
* chore: lint
* chore: update doc to accept `database`, `bucket`, and `bucket-name` as parameter names
* chore: update doc to only show `database` as the parameter name
* refactor: consolidate header names into a const vec and update comments on database
---------
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat: add CommandGetPrimaryKeys metadata endpoint and tests
* chore: update schema for the returned record batch
---------
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Part of the wider effort to consistently use tht term "database"
for the user-facing terminology, update the authorization system.
Whilst this system is technically user-facing, it is unlikely many
users will see it. It is however new enough that the change is
relatively little effort.
* feat(service_grpc_flight): optional query authorization
Add support for requiriing namespace-level authorization for
arrow flight based query requests. These are the flight SQL commands
as well as the IOx-specific SQL over flight and InfluxQL over flight
protocols.
Supports the optional configuration of an authorization sidecar,
in the same manner as is used in the router. If this is configured
then all arrow flight gRPC requests that are implemented will require
a valid authorization token to be supplied in the request. For a
multi-legged operation such as GetFlightInfo + DoGet required for
FlightSQL then a valid authorization is required for every request.
Ideally this support would be implemented using some sort of
interceptor, however the namespace isn't known until the request
processing has been started. The authorization check is performed
as soon as possible once the desired operation is known.
The legacy "storage" API has no authorization checks. Care should
be taken to ensure this API is never exposed to an untrusted network.
* chore(service_grpc_flight): review suggestions
Implement some suggestions from reviewers. The main change is adding
authorization checks to the handshake command.
* chore(service_grpc_flight): remove authorization of handshake
The Handshake call is used by existing clients to verify the
connection. These clients do not send a namespace header with the
request meaning there is nothing to authorize against. Remove this
authorization for now to avoid breaking existing clients.
* refactor: implement Authorizer trait on Option
Based on a suggestion from Dom implement the Authorizer trait on
Option<T: Authorizer> so that the call sites no longer need to check
if an authorizer is configured. This simplifies the code at the
call sites.
To maximise the utility the signature has changed so that a optional
token is now used. When no authorizer is configured this will not
be looked at. When a token is required a new error will be returned
if no token was supplied.
* fix: suggestions from clippy
* chore: Normalise name of Call expression to lowercase
Simplifies matching functions in planner, as they are guaranteed to be
lowercase.
This also ensures compatibility with InfluxQL when generating column
alias names, which are reflected in updated tests.
* chore: Ensure aggregate functions fail gracefully.
* feat: GROUP BY tag support
* feat: Ensure schema-level metadata is propagated
Requires: https://github.com/apache/arrow-rs/issues/3779
* chore: Add some tests to validate GROUP BY output
* chore: Add clarifying comment
* chore: Declare message in flight.proto
The metadata is public API, so best practice is to encode this in a way
that is most compatible for clients in other languages, and will also
document the history of schema changes.
Added tests to validate the metadata is encoded correctly.
* chore: Placate linters
* chore: Use correct column in test cases
* chore: Add `is_projected` to the TagKeyColumn message
`is_projected` is necessary to inform a client whether it should include
the tag key is used exclusively for the group key (false) or also
projected in the `SELECT` column list.
* refactor: Move constants to `schema` crate per PR feedback
* chore: rustfmt 🙄
* chore: Update docs for InfluxQlMetadata
Co-authored-by: Andrew Lamb <alamb@influxdata.com>
---------
Co-authored-by: Andrew Lamb <alamb@influxdata.com>