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.

Sign up for GitHub

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

RFC: connector deployment spec #94

Merged
merged 6 commits into from
Feb 13, 2024
Merged

RFC: connector deployment spec #94

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
49fa4c8
RFC: packaging spec
paf31 Jan 28, 2024
3ad485f
Extract deployment spec
paf31 Feb 6, 2024
278794c
Reserve OTEL_EXPORTER_*
paf31 Feb 6, 2024
d6156b3
respond to review requests
paf31 Feb 9, 2024
880051a
Use FromNamedImage
paf31 Feb 9, 2024
7bc0a11
Merge remote-tracking branch 'origin/main' into rfc/deployment-spec
paf31 Feb 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions rfcs/0000-deployment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# Connector Deployment

## Purpose

For execution of queries and mutations, connectors are specified by the [NDC specification](http://hasura.github.io/ndc-spec/). However, for the purpose of deployment and configuration, their behavior is unspecified, or informally specified.

This document exists to specify how connectors should be packaged in order to be accepted for inclusion in the Hasura Connector Hub. Any included connectors will be deployable via the CLI.

### Out of Scope

This RFC does not concern itself with the DX aspects of connector metadata authoring, development etc. in the CLI. As it relates to the connector hub, those aspects will be specified in a separate RFC.

## Related Changes

_This RFC does not specify the following planned changes:_

- Work on existing connectors has shown that we need more configuration structure than a flat file. Therefore we plan to change the configuration file to a configuration directory with a supporting set of secrets in environment variables.
- There will also be no more `HasuraHubConnector` in v3-engine metadata. Instead the engine will only see connector URLs, and the CLI will manage the instantiation and deployment of connectors, and the creation of those URLs.

## Proposal

- A Hasura Hub data connector will be provided as a Docker image.
- The connector can expect configuration files to be mounted at `/etc/connector` inside the Docker image on startup. If the `HASURA_CONFIGURATION_DIRECTORY` environment variable is set, it should overwrite this default value.
- The connector should not modify these files during execution, and can expect them not to be changed.
- The `/etc/connector/.hasura` subdirectory (or `{HASURA_CONFIGURATION_DIRECTORY}/.hasura` in the general case) is reserved for future use and should not be used for configuration. Any connectors which enumerate all subdirectories of `/etc/connector`, for any reason, should ignore this subdirectory if it exists.
- The connector executable should accept the following subcommands:
- `serve` should start a HTTP server on port `8080`, which is compatible with the NDC specification, with `/` as its base URL.
paf31 marked this conversation as resolved.
Show resolved Hide resolved
- For example, `http://connector:8080/query` should implement the query endpoint
- The default port can be overwritten using the `HASURA_CONNECTOR_PORT` environment variable.
paf31 marked this conversation as resolved.
Show resolved Hide resolved
- The image `ENTRYPOINT` should be set to the connector process, and the default `CMD` should be set to the `serve` command. This can mean setting it to the connector executable itself, or some intermediate executable/script that eventually provides its command line arguments to the connector executable.
- The connector can read environment variables on startup for configuration purposes
- The following environment variables are reserved, and should not be used for connector-specific configuration:
- `HASURA_*`
- `OTEL_EXPORTER_*`
- Connectors can use environment variables as part of their configuration. Configuration that varies between different environments or regions (like connection strings) should be configurable via environment variables.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a "namespace" for these? Perhaps CONNECTOR_*?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not currently, but maybe there should be. I was actually thinking that we could remove any reserved variables by using command line args, and leave all environment variables available for use. But OpenTelemetry stops us from doing that, since they require env vars.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can leave this for now and add it as a "best practice" if we feel the need.

- The connector should send any relevant trace spans in the OTLP format to the OTEL collector hosted at the URL provided by the `OTEL_EXPORTER_OTLP_ENDPOINT` environment variable.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be changed, per the spec the OTEL_EXPORTER_OTLP_ENDPOINT is a base url, and

For the per-signal variables (OTEL_EXPORTER_OTLP__ENDPOINT), the URL MUST be used as-is without any modification. The only exception is that if an URL contains no path part, the root path / MUST be used (see Example > 2).

If signals are sent that have no per-signal configuration from the previous point, OTEL_EXPORTER_OTLP_ENDPOINT is used as a base URL and the signals are sent to these paths relative to that:

Traces: v1/traces
Metrics: v1/metrics.
Logs: v1/logs.

So the URL for traces would be specified with either OTEL_EXPORTER_OTLP_TRACES_ENDPOINT or <OTEL_EXPORTER_OTLP_ENDPOINT>/v1/traces

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to the OTEL collector hosted at the URL provided by the OTEL_EXPORTER_OTLP_ENDPOINT

I think this is compatible with what the SDK does, and what the spec says? The OTEL collector root URL is provided in the OTEL_EXPORTER_OTLP_ENDPOINT variable. I will add a sentence clarifying this.

- Spans should indicate the service name provided by the `OTEL_SERVICE_NAME` environment variable.
- If the `HASURA_SERVICE_TOKEN_SECRET` environment variable is specified and non-empty, then the connector should implement bearer-token HTTP authorization using the provided static secret token.
- Information log messages should be logged in plain text to standard output.
- Error messages should be logged in plain text to standard error.
- On startup, in case of failure to start, the connector should flush any error messages to standard error, and exit with a non-zero exit code.
- The connector should respond to the following signals:
- `SIGTERM`/`SIGINT` - gracefully shutdown the server, and stop the connector process
- The connector should start as quickly as possible, without any build steps, by reading configuration from disk. Build steps should be performed in the construction of the Docker image, not on startup.
paf31 marked this conversation as resolved.
Show resolved Hide resolved
- To support these build steps, tooling should support building images from Dockerfiles. See "Deployment API" below.

### Open Questions

- Do we want to reserve environment variables `OTEL_*` for possible future use of the [OTLP exporter spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md)?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can avoid worrying about this and let connector devs suffer if they decide to intrude on another's namespace.

Fine with just pointing it out as a "probably don't do this".


## Deployment API

Docker images should be built in the same environment as which they are run, to avoid possible issues with differences in architecture, etc. Therefore, we need to specify _how to build_ images which meet the specification above.

The _connector build request_ data structure describes the ways in which we can build such an image unambiguously. There are two alternatives: from a named and versioned hub connector, or from a Dockerfile.

Here is a sketch of the data structure in Rust:

```rust
pub enum ConnectorBuildRequest {
FromHubConnector {
name: String,
version: Version, // sha hash
},
FromDockerfileAndBuildInputs {
tar: TarFile,
}
}
```

How this structure gets built by CLI (or its supporting web service) is out of scope. For example, we might fetch tar bundles from Git repos, or from the filesystem. Dockerfiles might be under the user's control, or not. But this structure is what is required to build images for deployment.

In the case of `FromHubConnector`, the expectation is that the connector build service maintains a list of prebuilt images, indexed by the names and versions of hub connectors.
paf31 marked this conversation as resolved.
Show resolved Hide resolved

Here, in the case of `FromHubConnector`, a full directory containing a `Dockerfile` and any supporting build inputs is provided as the bytes of a `.tar` file, but the exact protocol can be up to the service implementer.
Loading