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

Jump to bottom

GRPC Demo #798

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

GRPC Demo #798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 0 additions & 62 deletions .github/workflows/commit-schemas.yml
View file
Open in desktop

This file was deleted.

8 changes: 0 additions & 8 deletions .github/workflows/test-app.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,6 @@ jobs:
- name: Build image
run: |
make update
# TODO: Re-enable this when json schemas are ready to come back
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just removing some cruft, not relevant

# - name: Check schema
# run: |
# make schemas
# if ! git diff --quiet --ignore-submodules --cached; then
# echo "Error: Schema changes detected, run make schemas and commit those!"
# exit 1
# fi
- name: Typecheck with mypy
run: |
make mypy
Expand Down
16 changes: 16 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
FROM golang:1.20.0 as go-build
RUN mkdir go && cd go && export GOPATH=/go && \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tool that helps setup a mTLS key chain.

Copy link
Member

Choose a reason for hiding this comment

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

With mTLS requiring more infrastructure and cert management, we should also consider having a solution for bearer token based authentication with support for no-downtime token rotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But here's the thing -- token based authentication will, I feel, ultimately be even more infrastructure and management.
Once we start having multiple service to service sites, and once we may want things like ACLs (for instance, seer shouldn't have access to hybrid cloud endpoints), you need identity management, which token doesn't have baked in. We'd have to /build/ all that out, and we'd /still/ end up with infra tooling that allowed no downtime token rotations in the face of that identity management. Which we have to implement in each place.

With mTLS, we have one workflow, with tools that already exist, that is industry standard.

Copy link
Member

Choose a reason for hiding this comment

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

We can use JWTs to bake in identities and carry end-user context into downstream services. We could really get a lot of benefit with the added context in the multi-tenant environment and any time we cross some sort of domain trust boundary.

mTLS alone does not lend itself well to nuanced authorization decisions, at least in my opinion. A combination of mTLS for service-to-service authentication (PeerAuthentication and using JWT to carry end-user context (RequestAuthentication) is more robust than just one or the other. It's a fairly common pattern from my research. AuthorizationPolicies can be used to introspect the JWT's claims and make authorization decisions at the service mesh level.

In terms of added infrastructure, we'd be looking at some sort of trusted Secure Token Service (STS) to support the minting of JWTs. Tokens are issued through standard OAuth flows, and services would just implement JWT verification (if we decided to not just let the service mesh do this) and logic to pass it on where necessary.

Of course, implementing one thing at a time is probably best. 🙂

go install github.com/cloudflare/cfssl/cmd/...@latest

FROM nvidia/cuda:12.3.2-base-ubuntu22.04
COPY --from=go-build /go/bin/* /bin/

# Allow statements and log messages to immediately appear in the Cloud Run logs
ARG TEST
Expand Down Expand Up @@ -26,6 +31,8 @@ RUN ln -s /usr/bin/python /usr/local/bin/python && \
# Install libpq-dev for psycopg & git for 'sentry-sdk @ git://' in requirements.txt
RUN apt-get update && \
apt-get install -y supervisor \
curl \
protobuf-compiler \
libpq-dev \
git && \
rm -rf /var/lib/apt/lists/*
Expand All @@ -41,12 +48,21 @@ RUN chmod +x ./celeryworker.sh ./asyncworker.sh ./gunicorn.sh

# Install dependencies
RUN pip install --upgrade pip==24.0

COPY protobuf-adaptors/ protobuf-adaptors/
RUN pip wheel --default-timeout=120 ./protobuf-adaptors

COPY protos/ protos/
RUN pip install --default-timeout=120 --find-links=./ ./protos

RUN pip install -r requirements.txt

# Copy source code
COPY src/ src/
COPY pyproject.toml .

COPY certs/ certs/

# Copy the supervisord.conf file into the container
COPY supervisord.conf /etc/supervisord.conf

Expand Down
18 changes: 8 additions & 10 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ shell: .env # Opens a bash shell in the context of the project
update: .env # Updates the project's docker compose image.
docker compose build
docker compose run app flask db upgrade
docker compose run app bash -c 'cd certs && ./setup-certs.sh'

.PHONY: dev
dev: .env # Starts the webserver based on the current src on port 9091
Expand All @@ -40,16 +41,6 @@ test: # Executes all tests in the baked image file. Requires models/
mypy: # Runs mypy type checking
docker compose run app mypy

.PHONY: schemas
schemas: # Generates json files
#docker run --rm -v ./src/seer/schemas:/app/src/seer/schemas $(project_name):latest python src/seer/generate_schemas.py
docker compose run app python src/seer/generate_schemas.py
git clone --depth 1 https://github.com/getsentry/sentry-data-schemas.git $(tmpdir)
docker run --rm -t \
-v $(tmpdir):/sentry-data-schemas:ro \
-v $$(pwd)/src/:/src:ro \
tufin/oasdiff breaking /sentry-data-schemas/seer/seer_api.json /src/seer/schemas/seer_api.json

.PHONY: migration
migration: .env
docker compose run app flask db migrate -m 'Migration'
Expand All @@ -76,3 +67,10 @@ gocd: ## Build GoCD pipelines
# Convert JSON to yaml
cd ./gocd/generated-pipelines && find . -type f \( -name '*.yaml' \) -print0 | xargs -n 1 -0 yq -p json -o yaml -i
.PHONY: gocd


PROTOS_OUT=protos/build
PROTOS_SOURCE=protos/src
.PHONY: protos
protos: ## Regenerate protobuf python files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this workflow, all it takes is installing (pip install) to get both the protos and the generated files together, making it relatively simple to both deploy and use in development.

docker compose run app pip install --default-timeout=120 --find-links=./ ./protos
11 changes: 11 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# seer

## gRPC experiment
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Will remove, I don't intend to merge this into master. I have some ideas how we can break this out into a workable workflow and separate concerns.


This repo is currently in the process of experimenting with and migrating its endpoints to gRPC
services! This may include some new kinds of workflows and some dev ux bumps until issues are
sorted out.

However, we appreciate cooperation.

When building new endpoints, please use the new gRPC workflow and add new services & methods, then add
json compatible endpoint on top of that. Ideally all services would be supported via gRPC.

## Using Seer

### Autofix
Expand Down
19 changes: 19 additions & 0 deletions certs/ca/ca-csr.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"CN": "seer",
"hosts": [""],
"key": {
"algo": "rsa",
"size": 2048
},
"names": [
{
"O": "sentry",
"OU": "Sentry",
"ST": "California",
"C": "US"
}
],
"ca": {
"expiry": "876000h"
}
}
12 changes: 12 additions & 0 deletions certs/client/client-config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"signing": {
"default": {
"expiry": "876000h",
"usages": [
"signing",
"key encipherment",
"client auth"
]
}
}
}
14 changes: 14 additions & 0 deletions certs/client/client.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"CN": "consumer",
"hosts": ["consumer"],
"key": {
"algo": "rsa",
"size": 2048
},
"names": [
{
"ST": "California",
"C": "US"
}
]
}
12 changes: 12 additions & 0 deletions certs/server/server-config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"signing": {
"default": {
"expiry": "876000h",
"usages": [
"signing",
"key encipherment",
"server auth"
]
}
}
}
14 changes: 14 additions & 0 deletions certs/server/server.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"CN": "envoy",
"hosts": ["envoy"],
"key": {
"algo": "rsa",
"size": 2048
},
"names": [
{
"ST": "California",
"C": "US"
}
]
}
18 changes: 18 additions & 0 deletions certs/setup-certs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/usr/bin/env bash
set -xe

cd ca
cfssl genkey -initca ca-csr.json
cfssl genkey -initca ca-csr.json | cfssljson -bare ca
chmod a+r *.pem

cd ../server
cfssl gencert -ca=../ca/ca.pem -ca-key=../ca/ca-key.pem \
-config=./server-config.json server.json | cfssljson -bare server
chmod a+r *.pem

mkdir -p /app/certs/client
cd ../client
cfssl gencert -ca=../ca/ca.pem -ca-key=../ca/ca-key.pem \
-config=./client-config.json client.json | cfssljson -bare client
chmod a+r *.pem
13 changes: 13 additions & 0 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,31 @@ services:
POSTGRES_DB: seer
volumes:
- pgdata_test:/var/lib/postgresql/data
envoy:
image: envoyproxy/envoy:v1.30.0
restart: always
ports:
- "50051:50051"
volumes:
- ./envoy.yaml:/etc/envoy/envoy.yaml
- mtlstestcerts:/app/certs
app:
build:
context: .
working_dir: /app
volumes:
- ./src:/app/src
- ./protobuf-adaptors:/app/protobuf-adaptors
- ./protos:/app/protos
- ./tests:/app/tests
- ./models:/app/models
- ./data/chroma:/app/data/chroma
- ~/.config/gcloud:/root/.config/gcloud
- mtlstestcerts:/app/certs
depends_on:
- rabbitmq
- db
- envoy
env_file:
- .env
environment:
Expand All @@ -58,3 +70,4 @@ services:
volumes:
pgdata:
pgdata_test:
mtlstestcerts:
70 changes: 70 additions & 0 deletions envoy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
static_resources:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using envoy here because I know it has decent GKE support (https://cloud.google.com/kubernetes-engine/docs/tutorials/exposing-grpc-services-on-gke-using-envoy-proxy)

But to be honest, it wouldn't have to be GKE if we didn't want it to. Here, it handles the mTLS and the http 2 -> 1.1 proxying, but I'm confident both are also configurable in k8s with many other filters. That said, envoy is pretty nice and contains a lot of nice advantages, including decent XDS configuration. For service to service at scale, I feel inclined to bring this up.

Copy link
Member

Choose a reason for hiding this comment

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

We have envoy in a few places in our infrastructure already. Using envoy for grpc service discovery and mtls termination should be ok. If we can avoid downgrading to HTTP1.1 I think it would be worth the extra effort.

listeners:
- name: grpc
address:
socket_address:
address: 0.0.0.0
port_value: 50051
filter_chains:
- filters:
- name: envoy.filters.network.http_connection_manager
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
forward_client_cert_details: APPEND_FORWARD
set_current_client_cert_details:
subject: true
dns: true
access_log:
- name: envoy.access_loggers.stdout
typed_config:
"@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StdoutAccessLog
stat_prefix: ingress_http
route_config:
name: local_route
virtual_hosts:
- name: local_service
domains: ["*"]
routes:
- match:
prefix: "/"
route:
cluster: app
http_filters:
- name: envoy.filters.http.grpc_http1_reverse_bridge
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.http.grpc_http1_reverse_bridge.v3.FilterConfig
content_type: application/grpc
withhold_grpc_frames: true
- name: envoy.filters.http.router
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
transport_socket:
name: envoy.transport_sockets.tls
typed_config:
"@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext
require_client_certificate: true
common_tls_context:
alpn_protocols: ["h2"]
validation_context:
# match_subject_alt_names: # Only accept client certificates with this hostname in the SAN
# - exact: "app-internal-api"
trusted_ca:
filename: /app/certs/ca/ca.pem
tls_certificates:
- certificate_chain:
filename: /app/certs/server/server.pem
private_key:
filename: /app/certs/server/server-key.pem
clusters:
- name: app
type: STRICT_DNS
lb_policy: ROUND_ROBIN
load_assignment:
cluster_name: app
endpoints:
- lb_endpoints:
- endpoint:
address:
socket_address:
address: app
port_value: 9091
Empty file added protobuf-adaptors/__init__.py
View file
Open in desktop
Empty file.
3 changes: 3 additions & 0 deletions protobuf-adaptors/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
types-protobuf==4.25.0.20240417
protobuf==4.25.3
mypy-protobuf==3.6.0
Loading